From 5c11a04d13bed1e7018b6020500bf300ed1dd9ca Mon Sep 17 00:00:00 2001 From: Doug Lovett Date: Mon, 26 Aug 2024 10:12:01 -0700 Subject: [PATCH] MHR bug fixes and updates. (#2009) * MHR bug fixes and updates. Signed-off-by: Doug Lovett * MHR bug fixes and updates. Signed-off-by: Doug Lovett * MHR bug fixes and updates. Signed-off-by: Doug Lovett * Fix search by serial number multiple matches same MHR. Signed-off-by: Doug Lovett --------- Signed-off-by: Doug Lovett --- .../postgres_views/mhr_account_reg_vw.sql | 2 +- .../mhr_search_mhr_number_vw.sql | 1 + .../postgres_views/mhr_search_serial_vw.sql | 1 + mhr_api/src/mhr_api/models/address.py | 11 +++--- .../src/mhr_api/models/mhr_registration.py | 4 +- mhr_api/src/mhr_api/models/search_request.py | 3 +- mhr_api/src/mhr_api/models/search_utils.py | 2 +- .../mhr_api/resources/v1/search_results.py | 13 +++++++ mhr_api/src/mhr_api/resources/v1/transfers.py | 1 - mhr_api/src/mhr_api/utils/validator_utils.py | 5 ++- mhr_api/src/mhr_api/version.py | 2 +- mhr_api/tests/unit/models/test_address.py | 37 +++++++++++++++++++ .../tests/unit/utils/test_admin_validator.py | 18 ++++++--- .../tests/unit/utils/test_permit_validator.py | 11 +++--- 14 files changed, 86 insertions(+), 25 deletions(-) diff --git a/mhr_api/src/database/postgres_views/mhr_account_reg_vw.sql b/mhr_api/src/database/postgres_views/mhr_account_reg_vw.sql index 3485094ca..14d192fe6 100644 --- a/mhr_api/src/database/postgres_views/mhr_account_reg_vw.sql +++ b/mhr_api/src/database/postgres_views/mhr_account_reg_vw.sql @@ -63,7 +63,7 @@ SELECT r.mhr_number, r.status_type, r.registration_ts, FETCH FIRST 1 ROWS ONLY) AS frozen_doc_type, r.account_id, dt.document_type_desc, - (SELECT CASE WHEN r.registration_type != 'MHREG' THEN '' + (SELECT CASE WHEN r.registration_type NOT IN ('MHREG', 'MHREG_CONVERSION') THEN '' ELSE (SELECT lcv.registration_type FROM mhr_lien_check_vw lcv WHERE lcv.mhr_number = r.mhr_number diff --git a/mhr_api/src/database/postgres_views/mhr_search_mhr_number_vw.sql b/mhr_api/src/database/postgres_views/mhr_search_mhr_number_vw.sql index 3c2e002ad..582983840 100644 --- a/mhr_api/src/database/postgres_views/mhr_search_mhr_number_vw.sql +++ b/mhr_api/src/database/postgres_views/mhr_search_mhr_number_vw.sql @@ -18,6 +18,7 @@ SELECT r.mhr_number, r.status_type, r.registration_ts, a.city, WHERE ro.mhr_number = r.mhr_number AND ro.id = og.registration_id AND og.registration_id = p.registration_id + AND og.status_type IN ('ACTIVE', 'EXEMPT') ORDER BY p.id DESC FETCH FIRST 1 ROWS ONLY) AS owner_info FROM mhr_registrations r, diff --git a/mhr_api/src/database/postgres_views/mhr_search_serial_vw.sql b/mhr_api/src/database/postgres_views/mhr_search_serial_vw.sql index 19b0ec280..121933669 100644 --- a/mhr_api/src/database/postgres_views/mhr_search_serial_vw.sql +++ b/mhr_api/src/database/postgres_views/mhr_search_serial_vw.sql @@ -12,6 +12,7 @@ SELECT r.mhr_number, r.status_type, r.registration_ts, a.city, WHERE ro.mhr_number = r.mhr_number AND ro.id = og.registration_id AND og.registration_id = p.registration_id + AND og.status_type IN ('ACTIVE', 'EXEMPT') ORDER BY p.id DESC FETCH FIRST 1 ROWS ONLY) AS owner_info, s.id AS section_id diff --git a/mhr_api/src/mhr_api/models/address.py b/mhr_api/src/mhr_api/models/address.py index 51e24164f..7f8534163 100644 --- a/mhr_api/src/mhr_api/models/address.py +++ b/mhr_api/src/mhr_api/models/address.py @@ -97,13 +97,14 @@ def create_from_dict(new_info: dict): def create_from_json(json_data): """Create an address object from a json Address schema object: map json to db.""" address = Address() - # API requests everything but streetAdditional is mandatory. + # API requests everything but streetAdditional and region are mandatory. address.street = json_data['street'].strip().upper() - if 'streetAdditional' in json_data: + if json_data.get('streetAdditional'): address.street_additional = json_data['streetAdditional'].strip().upper() address.city = json_data['city'].strip().upper() - address.region = json_data['region'].strip().upper() + if json_data.get('region'): + address.region = json_data['region'].strip().upper() address.country = json_data['country'].strip().upper() - address.postal_code = json_data['postalCode'].strip().upper() - + if json_data.get('postalCode'): + address.postal_code = json_data['postalCode'].strip().upper() return address diff --git a/mhr_api/src/mhr_api/models/mhr_registration.py b/mhr_api/src/mhr_api/models/mhr_registration.py index b15bca146..37db495e9 100755 --- a/mhr_api/src/mhr_api/models/mhr_registration.py +++ b/mhr_api/src/mhr_api/models/mhr_registration.py @@ -713,7 +713,9 @@ def is_exre_transfer(base_reg, json_data: dict) -> bool: if reg.owner_groups: group = reg.owner_groups[0] current_app.logger.info(f'Checking group {group.status_type} {group.group_id} {group.tenancy_type}') - if reg.owner_groups and reg.documents[0].document_type == MhrDocumentTypes.EXRE and \ + if reg.owner_groups and reg.documents[0].document_type in (MhrDocumentTypes.EXRE, MhrDocumentTypes.PUBA, + MhrDocumentTypes.REGC_CLIENT, + MhrDocumentTypes.REGC_STAFF) and \ len(reg.owner_groups) == 1 and reg.owner_groups[0].status_type == MhrOwnerStatusTypes.ACTIVE: if reg.owner_groups[0].group_id == group_id and reg.owner_groups[0].tenancy_type == ten_type: current_app.logger.info(f'Found matching group {group_id} and tenatncy type {ten_type}') diff --git a/mhr_api/src/mhr_api/models/search_request.py b/mhr_api/src/mhr_api/models/search_request.py index ffe6fcb63..57dfccc4a 100755 --- a/mhr_api/src/mhr_api/models/search_request.py +++ b/mhr_api/src/mhr_api/models/search_request.py @@ -345,8 +345,9 @@ def update_result_matches(cls, results, result, search_type: str) -> bool: updated = True elif search_type == cls.SearchTypes.OWNER_NAME and existing.get('ownerName') == result.get('ownerName'): updated = True - elif search_type == cls.SearchTypes.SERIAL_NUM and existing.get('serialNumber') == result.get('serialNumber'): + elif search_type == cls.SearchTypes.SERIAL_NUM: updated = True + existing['serialNumber'] = existing['serialNumber'] + ', ' + result.get('serialNumber') if updated: if result.get('activeCount') == 1: existing['activeCount'] = existing['activeCount'] + 1 diff --git a/mhr_api/src/mhr_api/models/search_utils.py b/mhr_api/src/mhr_api/models/search_utils.py index 5bb1ebead..ccca5744a 100644 --- a/mhr_api/src/mhr_api/models/search_utils.py +++ b/mhr_api/src/mhr_api/models/search_utils.py @@ -253,7 +253,7 @@ def build_search_result_serial(row): 'make': str(row[6]).strip() if row[6] is not None else '', 'model': str(row[7]).strip() if row[7] is not None else '' }, - 'activeCount': 0, + 'activeCount': 1, 'exemptCount': 0, 'historicalCount': 0, 'mhId': int(row[8]) diff --git a/mhr_api/src/mhr_api/resources/v1/search_results.py b/mhr_api/src/mhr_api/resources/v1/search_results.py index f0f4089a4..bc0077ba2 100644 --- a/mhr_api/src/mhr_api/resources/v1/search_results.py +++ b/mhr_api/src/mhr_api/resources/v1/search_results.py @@ -95,6 +95,7 @@ def post_search_results(search_id: str): # pylint: disable=too-many-branches, t request_json = search_request.updated_selection or [] else: request_json = request.get_json(silent=True) + request_json = remove_serial_numbers(request_json) # Validate schema. valid_format, errors = schema_utils.validate(request_json, 'searchSummary', 'mhr') if not valid_format: @@ -403,3 +404,15 @@ def report_error(code: str, search_id: str, status_code, message: str = None): # Track event here. EventTracking.create(search_id, EventTracking.EventTrackingTypes.SEARCH_REPORT, status_code, message) return resource_utils.error_response(status_code, error) + + +def remove_serial_numbers(request_json: dict) -> dict: + """If searching by serial number remove selection serialNumber to avoid schema validation length error.""" + if not request_json: + return request_json + for result in request_json: + if result.get('serialNumber'): + serial: str = result.get('serialNumber') + if serial.find(',') > 0: + del result['serialNumber'] + return request_json diff --git a/mhr_api/src/mhr_api/resources/v1/transfers.py b/mhr_api/src/mhr_api/resources/v1/transfers.py index d32c6d816..d311e10fe 100644 --- a/mhr_api/src/mhr_api/resources/v1/transfers.py +++ b/mhr_api/src/mhr_api/resources/v1/transfers.py @@ -58,7 +58,6 @@ def post_transfers(mhr_number: str): # pylint: disable=too-many-return-statemen if is_bcol_help(account_id, jwt): return resource_utils.helpdesk_unauthorized_error_response('transfer of ownership') request_json = request.get_json(silent=True) - current_app.logger.info(request_json) group: str = get_group(jwt) if not model_reg_utils.is_transfer_due_to_death(request_json.get('registrationType')) and \ not authorized_role(jwt, TRANSFER_SALE_BENEFICIARY) and group != DEALERSHIP_GROUP: diff --git a/mhr_api/src/mhr_api/utils/validator_utils.py b/mhr_api/src/mhr_api/utils/validator_utils.py index e8c164797..6378d8c77 100644 --- a/mhr_api/src/mhr_api/utils/validator_utils.py +++ b/mhr_api/src/mhr_api/utils/validator_utils.py @@ -757,8 +757,7 @@ def validate_description_different(current_description: dict, new_description: d def validate_location(location): # pylint: disable=too-many-branches """Verify the combination of location values is valid.""" error_msg = '' - # No point validating if no no required locationType. - if not location or not location.get('locationType'): + if not location or not location.get('locationType'): # No point validating if no no required locationType. return error_msg loc_type = location['locationType'] if loc_type == MhrLocationTypes.RESERVE: @@ -829,6 +828,8 @@ def has_location_ltsa_details(location) -> bool: def validate_tax_certificate(request_location: dict, current_location: dict, staff: bool) -> str: """Validate transport permit business rules specific to a tax certificate.""" error_msg = '' + if staff: # Skip for staff. + return error_msg if request_location and request_location.get('taxExpiryDate'): tax_ts = model_utils.ts_from_iso_format(request_location.get('taxExpiryDate')) current_ts = model_utils.now_ts() diff --git a/mhr_api/src/mhr_api/version.py b/mhr_api/src/mhr_api/version.py index 61cd3bf28..70bf1455c 100644 --- a/mhr_api/src/mhr_api/version.py +++ b/mhr_api/src/mhr_api/version.py @@ -22,4 +22,4 @@ Development release segment: .devN """ -__version__ = '1.8.20' # pylint: disable=invalid-name +__version__ = '1.8.21' # pylint: disable=invalid-name diff --git a/mhr_api/tests/unit/models/test_address.py b/mhr_api/tests/unit/models/test_address.py index 7c44aac6b..1f2c0981e 100644 --- a/mhr_api/tests/unit/models/test_address.py +++ b/mhr_api/tests/unit/models/test_address.py @@ -29,6 +29,13 @@ (False, False, False, True, False), (False, False, False, False, True) ] +# testdata pattern is ({additional}, {hasRegion}, {hasPostalCode}) +TEST_CREATE_DATA = [ + (True, True, True), + (False, True, False), + (False, False, True), + (False, False, False) +] def test_find_by_id(session): @@ -121,3 +128,33 @@ def test_address_legacy_json(session, street, city, region, country, postal_code assert address_json['postalCode'] else: assert 'postalCode' not in address_json + + +@pytest.mark.parametrize('additional,region,postal_code', TEST_CREATE_DATA) +def test_create_from_json(session, additional, region, postal_code): + """Assert that the create from json with no address region or postal code works correctly.""" + address_json = { + 'street': 'street', + 'city': 'city', + 'country': 'BE' + } + if additional: + address_json['streetAdditional'] = 'extra' + if region: + address_json['region'] = '??' + if postal_code: + address_json['postalCode'] = 'XXXXX' + + address: Address = Address.create_from_json(address_json) + if additional: + assert address.street_additional + else: + assert not address.street_additional + if region: + assert address.region + else: + assert not address.region + if postal_code: + assert address.postal_code + else: + assert not address.postal_code diff --git a/mhr_api/tests/unit/utils/test_admin_validator.py b/mhr_api/tests/unit/utils/test_admin_validator.py index cca056b61..154e348b4 100644 --- a/mhr_api/tests/unit/utils/test_admin_validator.py +++ b/mhr_api/tests/unit/utils/test_admin_validator.py @@ -154,6 +154,10 @@ } LOCATION_TAX_INVALID = { 'locationType': 'STRATA', + 'lot': '3', + 'plan': '3A', + 'landDistrict': 'Caribou', + 'parcel': 'A (69860M)', 'address': { 'street': '7612 LUDLOM RD.', 'city': 'DEKA LAKE', @@ -167,6 +171,10 @@ } LOCATION_TAX_MISSING = { 'locationType': 'STRATA', + 'lot': '3', + 'plan': '3A', + 'landDistrict': 'Caribou', + 'parcel': 'A (69860M)', 'address': { 'street': '7612 LUDLOM RD.', 'city': 'DEKA LAKE', @@ -442,11 +450,9 @@ ('Valid location no tax cert', True, '000900', LOCATION_PARK, None), ('Valid existing active PERMIT', True, '000931', LOCATION_VALID, None), ('Invalid MH_PARK no name', False, '000900', LOCATION_PARK_NO_NAME, validator_utils.LOCATION_PARK_NAME_REQUIRED), - ('Invalid location RESERVE no tax cert', False, '000919', LOCATION_RESERVE, - validator_utils.LOCATION_TAX_CERT_REQUIRED), - ('Invalid location tax cert date', False, '000900', LOCATION_TAX_INVALID, - validator_utils.LOCATION_TAX_DATE_INVALID), - ('Missing location tax cert', False, '000919', LOCATION_TAX_MISSING, validator_utils.LOCATION_TAX_CERT_REQUIRED), + ('Staff invalid location RESERVE no tax cert', True, '000919', LOCATION_RESERVE, None), + ('Staff invalid location tax cert date', True, '000900', LOCATION_TAX_INVALID, None), + ('Staff missing location tax cert', True, '000919', LOCATION_TAX_MISSING, None), ('Invalid identical location', False, '000931', LOCATION_000931, validator_utils.LOCATION_INVALID_IDENTICAL) ] # testdata pattern is ({description}, {valid}, {mhr_num}, {note}, {doc_type}, {message content}) @@ -625,7 +631,7 @@ def test_validate_stat(session, desc, valid, mhr_num, location, message_content) reg_json = registration.new_registration_json if reg_json['location'].get('taxExpiryDate'): json_data['location']['taxExpiryDate'] = reg_json['location'].get('taxExpiryDate') - error_msg = validator.validate_admin_reg(registration, json_data) + error_msg = validator.validate_admin_reg(registration, json_data, True) # current_app.logger.debug(error_msg) if valid: assert error_msg == '' diff --git a/mhr_api/tests/unit/utils/test_permit_validator.py b/mhr_api/tests/unit/utils/test_permit_validator.py index b15c5b3f0..b5cd8bb67 100644 --- a/mhr_api/tests/unit/utils/test_permit_validator.py +++ b/mhr_api/tests/unit/utils/test_permit_validator.py @@ -306,14 +306,13 @@ ('Non utf-8 exception plan', None, None, None, INVALID_TEXT_CHARSET, None, None), ('Non utf-8 band name', None, None, None, None, INVALID_TEXT_CHARSET, None) ] -# testdata pattern is ({description}, {valid}, {has_tax_cert}, {valid_date}, {current_loc}, {new_loc}, {message content}) +# testdata pattern is ({description}, {valid}, {has_tax_cert}, {valid_date}, {current_loc}, {new_loc}, {staff}, {message content}) TEST_TAX_CERT_DATA = [ ('Valid same park', True, False, False, LOCATION_PARK, LOCATION_PARK, False, None), ('Valid current outside bc', True, False, False, LOCATION_RESERVE, LOCATION_TAX_MISSING, False, None), ('Invalid no tax cert', False, False, False, LOCATION_RESERVE, LOCATION_RESERVE, False, validator_utils.LOCATION_TAX_CERT_REQUIRED), - ('Invalid tax date', False, True, False, LOCATION_PARK, LOCATION_TAX_INVALID, True, - validator_utils.LOCATION_TAX_DATE_INVALID), + ('Staff invalid tax date', True, True, False, LOCATION_PARK, LOCATION_TAX_INVALID, True, None), ('Invalid tax date QS', False, True, False, LOCATION_PARK, LOCATION_OTHER, False, validator_utils.LOCATION_TAX_DATE_INVALID_QS) ] @@ -423,7 +422,7 @@ ('Valid staff future year', True, True, 365, None, '000900', 'PS12345', STAFF_ROLE), ('Invalid non-staff future year', False, False, 365, validator_utils.LOCATION_TAX_DATE_INVALID_QS, '000900', 'PS12345', REQUEST_TRANSPORT_PERMIT), - ('Invalid staff past', False, True, -1, validator_utils.LOCATION_TAX_DATE_INVALID, '000900', 'PS12345', STAFF_ROLE), + ('Invalid staff past', True, True, -1, None, '000900', 'PS12345', STAFF_ROLE), ('Invalid non-staff past', False, False, -1, validator_utils.LOCATION_TAX_DATE_INVALID, '000900', 'PS12345', REQUEST_TRANSPORT_PERMIT) ] @@ -613,8 +612,8 @@ def test_validate_permit_extra(session, desc, valid, mhr_num, location, message_ @pytest.mark.parametrize('desc,valid,has_tax_cert,valid_date,current_loc,new_loc,staff,message_content', TEST_TAX_CERT_DATA) -def test_validate_tax_certificat(session, desc, valid, has_tax_cert, valid_date, current_loc, new_loc, staff, - message_content): +def test_validate_tax_certificate(session, desc, valid, has_tax_cert, valid_date, current_loc, new_loc, staff, + message_content): """Assert that location tax certificate validation works as expected.""" new_location = copy.deepcopy(new_loc) current_location = copy.deepcopy(current_loc)