From 5d5fe2510932f63674e8b97a725665f2eeaf1147 Mon Sep 17 00:00:00 2001 From: Tyler Zale Date: Fri, 19 Jul 2024 11:55:08 -0700 Subject: [PATCH] registration search and sort improvements --- .../versions/20240719_2b707ec995f7_.py | 46 +++++++++++++++++ strr-api/src/strr_api/enums/enum.py | 12 ++--- strr-api/src/strr_api/exceptions/responses.py | 6 +-- strr-api/src/strr_api/models/certificate.py | 3 ++ strr-api/src/strr_api/models/rental.py | 1 + .../src/strr_api/resources/registrations.py | 38 +++++++++++--- .../responses/RegistrationResponse.py | 9 ++++ .../strr_api/services/registration_service.py | 43 +++++++++++++--- .../unit/resources/test_registrations.py | 3 +- strr-api/tests/unit/utils/mocks.py | 50 +++++++++++++++++++ 10 files changed, 188 insertions(+), 23 deletions(-) create mode 100644 strr-api/migrations/versions/20240719_2b707ec995f7_.py diff --git a/strr-api/migrations/versions/20240719_2b707ec995f7_.py b/strr-api/migrations/versions/20240719_2b707ec995f7_.py new file mode 100644 index 00000000..2c39842d --- /dev/null +++ b/strr-api/migrations/versions/20240719_2b707ec995f7_.py @@ -0,0 +1,46 @@ +"""Add search and sort indices + +Revision ID: 2b707ec995f7 +Revises: 269514c7d861 +Create Date: 2024-07-19 11:03:26.021314 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '2b707ec995f7' +down_revision = '269514c7d861' +branch_labels = None +depends_on = None + + +def upgrade(): + with op.batch_alter_table('addresses', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_addresses_city'), ['city'], unique=False) + batch_op.create_index(batch_op.f('ix_addresses_street_address'), ['street_address'], unique=False) + + with op.batch_alter_table('registrations', schema=None) as batch_op: + batch_op.create_index(batch_op.f('ix_registrations_status'), ['status'], unique=False) + batch_op.create_index(batch_op.f('ix_registrations_submission_date'), ['submission_date'], unique=False) + + op.execute( + """ + CREATE INDEX ix_contacts_fullname ON contacts ( + (firstname || ' ' || lastname) + ); + """ + ) + + +def downgrade(): + with op.batch_alter_table('addresses', schema=None) as batch_op: + op.drop_index('ix_addresses_street_address') + op.drop_index('ix_addresses_city') + + with op.batch_alter_table('registrations', schema=None) as batch_op: + op.drop_index('ix_registrations_submission_date') + op.drop_index('ix_registrations_status') + + op.execute('DROP INDEX ix_contacts_fullname;') diff --git a/strr-api/src/strr_api/enums/enum.py b/strr-api/src/strr_api/enums/enum.py index d3be0f56..5c52f5dc 100644 --- a/strr-api/src/strr_api/enums/enum.py +++ b/strr-api/src/strr_api/enums/enum.py @@ -83,12 +83,12 @@ class RegistrationSortBy(Enum): """STRR Registration Sort By Columns.""" ID = 0 - USER_ID = 1 - SBC_ACCOUNT_ID = 2 - RENTAL_PROPERTY_ID = 3 - SUBMISSION_DATE = 4 - UPDATED_DATE = 5 - STATUS = 6 + REGISTRATION_NUMBER = 1 + LOCATION = 2 + ADDRESS = 3 + NAME = 4 + STATUS = 5 + SUBMISSION_DATE = 6 class PropertyType(Enum): diff --git a/strr-api/src/strr_api/exceptions/responses.py b/strr-api/src/strr_api/exceptions/responses.py index 820ec986..838dc8ca 100644 --- a/strr-api/src/strr_api/exceptions/responses.py +++ b/strr-api/src/strr_api/exceptions/responses.py @@ -40,7 +40,7 @@ def error_response( - message: str = "Bad request", http_status: HTTPStatus = HTTPStatus.BAD_REQUEST, errors: list[dict[str, str]] = None + http_status: HTTPStatus = HTTPStatus.BAD_REQUEST, message: str = "Bad request", errors: list[dict[str, str]] = None ): """Build generic request response with errors.""" return jsonify({"message": message, "details": errors or []}), http_status @@ -51,6 +51,6 @@ def exception_response(exception: BaseExceptionE): current_app.logger.error(repr(exception)) if isinstance(exception, ExternalServiceException): return error_response( - exception.message, exception.status_code if exception.status_code > 500 else HTTPStatus.BAD_GATEWAY + (exception.status_code if exception.status_code > 500 else HTTPStatus.BAD_GATEWAY), exception.message ) - return error_response(exception.message, exception.status_code) + return error_response(exception.status_code, exception.message) diff --git a/strr-api/src/strr_api/models/certificate.py b/strr-api/src/strr_api/models/certificate.py index 4a92e527..c111e63b 100644 --- a/strr-api/src/strr_api/models/certificate.py +++ b/strr-api/src/strr_api/models/certificate.py @@ -3,6 +3,7 @@ """ from __future__ import annotations +from sqlalchemy.orm import relationship from sqlalchemy.sql import text from .db import db @@ -19,3 +20,5 @@ class Certificate(db.Model): creation_date = db.Column(db.DateTime, nullable=False, server_default=text("(NOW())")) expiry_date = db.Column(db.DateTime, nullable=False, server_default=text("(NOW() + interval '1 year')")) certificate = db.Column(db.LargeBinary, nullable=False) + + registration = relationship("Registration", back_populates="certificates") diff --git a/strr-api/src/strr_api/models/rental.py b/strr-api/src/strr_api/models/rental.py index 8a702a8c..dd24b287 100644 --- a/strr-api/src/strr_api/models/rental.py +++ b/strr-api/src/strr_api/models/rental.py @@ -101,6 +101,7 @@ class Registration(db.Model): rental_property = relationship("RentalProperty", back_populates="registrations") eligibility = relationship("Eligibility", back_populates="registrations", uselist=False) invoices = relationship("Invoice", back_populates="registration") + certificates = relationship("Certificate", back_populates="registration") def save(self): """Store the Registration into the local cache.""" diff --git a/strr-api/src/strr_api/resources/registrations.py b/strr-api/src/strr_api/resources/registrations.py index 48bdcd6d..3cfd7d27 100644 --- a/strr-api/src/strr_api/resources/registrations.py +++ b/strr-api/src/strr_api/resources/registrations.py @@ -87,19 +87,26 @@ def get_registrations(): - in: header name: Account-Id type: integer - description: Optionally filters results based on SBC Account ID + description: Optionally filters results based on SBC Account ID. - in: query name: filter_by_status - enum: [PENDING,APPROVED,UNDER_REVIEW,MORE_INFO_NEEDED,PROVISIONAL,DENIED] - description: Filters affect pagination count returned + enum: [PENDING,APPROVED,ISSUED,UNDER_REVIEW,MORE_INFO_NEEDED,PROVISIONAL,DENIED] + description: Filters affect pagination count returned. + - in: query + name: search + type: string + minLength: 3 + description: > + Search for wildcard term: %% in Registration#, Location, Address, and Applicant Name. + Affects pagination count returned. Minimum length of 3 characters. - in: query name: sort_by - enum: [ID,USER_ID,SBC_ACCOUNT_ID,RENTAL_PROPERTY_ID,SUBMISSION_DATE,UPDATED_DATE,STATUS] - description: Filters affect pagination count returned + enum: [REGISTRATION_NUMBER,LOCATION,ADDRESS,NAME,STATUS,SUBMISSION_DATE] + description: Filters affect pagination count returned. - in: query name: sort_desc type: boolean - description: false or omitted for ascending, true for descending order + description: false or omitted for ascending, true for descending order. - in: query name: offset type: integer @@ -117,6 +124,11 @@ def get_registrations(): account_id = request.headers.get("Account-Id") filter_by_status: RegistrationStatus = None status_value = request.args.get("filter_by_status", None) + search = request.args.get("search", None) + + if search and len(search) < 3: + return error_response(HTTPStatus.BAD_REQUEST, "Search term must be at least 3 characters long.") + try: if status_value is not None: filter_by_status = RegistrationStatus[status_value.upper()] @@ -136,7 +148,7 @@ def get_registrations(): limit: int = request.args.get("limit", 100) registrations, count = RegistrationService.list_registrations( - g.jwt_oidc_token_info, account_id, filter_by_status, sort_by_column, sort_desc, offset, limit + g.jwt_oidc_token_info, account_id, search, filter_by_status, sort_by_column, sort_desc, offset, limit ) pagination = Pagination(count=count, results=[Registration.from_db(registration) for registration in registrations]) @@ -789,6 +801,12 @@ def approve_registration(registration_id): if not registration: return error_response(HTTPStatus.NOT_FOUND, "Registration not found") + if registration.status == RegistrationStatus.APPROVED: + return error_response(HTTPStatus.BAD_REQUEST, "Registration has already been approved.") + + if registration.status == RegistrationStatus.ISSUED: + return error_response(HTTPStatus.BAD_REQUEST, "Registration has already been issued a certificate.") + ApprovalService.process_manual_approval(registration) return jsonify(Registration.from_db(registration).model_dump(mode="json")), HTTPStatus.OK except AuthException as auth_exception: @@ -831,6 +849,12 @@ def issue_registration_certificate(registration_id): if not registration: return error_response(HTTPStatus.NOT_FOUND, "Registration not found") + if registration.status == RegistrationStatus.ISSUED: + return error_response(HTTPStatus.BAD_REQUEST, "Registration has already been issued a certificate.") + + if registration.status != RegistrationStatus.APPROVED: + return error_response(HTTPStatus.BAD_REQUEST, "Registration must be approved before issuing a certificate.") + ApprovalService.generate_registration_certificate(registration) return jsonify(Registration.from_db(registration).model_dump(mode="json")), HTTPStatus.OK except AuthException as auth_exception: diff --git a/strr-api/src/strr_api/responses/RegistrationResponse.py b/strr-api/src/strr_api/responses/RegistrationResponse.py index 55dc144f..0835d1fa 100644 --- a/strr-api/src/strr_api/responses/RegistrationResponse.py +++ b/strr-api/src/strr_api/responses/RegistrationResponse.py @@ -114,6 +114,7 @@ class Registration(BaseModel): submissionDate: datetime updatedDate: datetime status: str + registration_number: Optional[str] = None primaryContact: Contact secondaryContact: Optional[Contact] = None unitAddress: UnitAddress @@ -125,6 +126,13 @@ class Registration(BaseModel): @classmethod def from_db(cls, source: models.Registration): """Return a Registration object from a database model.""" + latest_certificate = None + for certificate in source.certificates: + if latest_certificate is None or certificate.creation_date > latest_certificate.creation_date: + latest_certificate = certificate + + registration_number = latest_certificate.registration_number if latest_certificate else None + return cls( id=source.id, user_id=source.user_id, @@ -132,6 +140,7 @@ def from_db(cls, source: models.Registration): submissionDate=source.submission_date, updatedDate=source.updated_date, status=source.status.name, + registration_number=registration_number, primaryContact=Contact( name=ContactName( firstName=source.rental_property.property_manager.primary_contact.firstname, diff --git a/strr-api/src/strr_api/services/registration_service.py b/strr-api/src/strr_api/services/registration_service.py index 3d7489a7..31cf450f 100644 --- a/strr-api/src/strr_api/services/registration_service.py +++ b/strr-api/src/strr_api/services/registration_service.py @@ -159,6 +159,7 @@ def list_registrations( cls, jwt_oidc_token_info, account_id: int = None, + search: str = None, filter_by_status: RegistrationStatus = None, sort_by: RegistrationSortBy = RegistrationSortBy.ID, sort_desc: bool = False, @@ -169,7 +170,37 @@ def list_registrations( user = models.User.get_or_create_user_by_jwt(jwt_oidc_token_info) if not user: return [], 0 - query = models.Registration.query + query = ( + models.Registration.query.join( + models.RentalProperty, models.Registration.rental_property_id == models.RentalProperty.id + ) + .join(models.Address, models.RentalProperty.address_id == models.Address.id) + .join(models.PropertyManager, models.RentalProperty.property_manager_id == models.PropertyManager.id) + .join(models.Contact, models.PropertyManager.primary_contact_id == models.Contact.id) + ) + + certificates_subquery = db.session.query( + models.Certificate, + func.row_number() + .over(partition_by=models.Certificate.registration_id, order_by=models.Certificate.creation_date.desc()) + .label("rank"), + ).subquery() + + query = query.join( + certificates_subquery, + (models.Registration.id == certificates_subquery.c.registration_id) & (certificates_subquery.c.rank == 1), + isouter=True, + ) + + if search and len(search) >= 3: + query = query.filter( + func.concat(models.Contact.firstname, " ", models.Contact.lastname).ilike(f"%{search}%") + | models.Address.city.ilike(f"%{search}%") + | models.Address.street_address.ilike(f"%{search}%") + | models.Address.postal_code.ilike(f"%{search}%") + | certificates_subquery.c.registration_number.ilike(f"%{search}%") + ) + if not user.is_examiner(): query = query.filter(models.Registration.user_id == user.id) if account_id: @@ -180,12 +211,12 @@ def list_registrations( count = query.count() sort_column = { RegistrationSortBy.ID: models.Registration.id, - RegistrationSortBy.USER_ID: models.Registration.user_id, - RegistrationSortBy.SBC_ACCOUNT_ID: models.Registration.sbc_account_id, - RegistrationSortBy.RENTAL_PROPERTY_ID: models.Registration.rental_property_id, - RegistrationSortBy.SUBMISSION_DATE: models.Registration.submission_date, - RegistrationSortBy.UPDATED_DATE: models.Registration.updated_date, + RegistrationSortBy.REGISTRATION_NUMBER: certificates_subquery.c.registration_number, + RegistrationSortBy.LOCATION: models.Address.city, + RegistrationSortBy.ADDRESS: models.Address.street_address, + RegistrationSortBy.NAME: func.concat(models.Contact.firstname, " ", models.Contact.lastname), RegistrationSortBy.STATUS: models.Registration.status, + RegistrationSortBy.SUBMISSION_DATE: models.Registration.submission_date, } query = query.order_by(sort_column[sort_by].desc() if sort_desc else sort_column[sort_by].asc()) return query.offset(offset).limit(limit).all(), count diff --git a/strr-api/tests/unit/resources/test_registrations.py b/strr-api/tests/unit/resources/test_registrations.py index 6c674cac..d153b211 100644 --- a/strr-api/tests/unit/resources/test_registrations.py +++ b/strr-api/tests/unit/resources/test_registrations.py @@ -11,6 +11,7 @@ fake_get_token_auth_header, fake_invoice, fake_registration, + fake_registration_pending, fake_user_from_token, no_op, throw_external_service_exception, @@ -310,7 +311,7 @@ def test_get_registration_ltsa_403(client): assert rv.status_code == HTTPStatus.FORBIDDEN -@patch("strr_api.services.registration_service.RegistrationService.get_registration", new=fake_registration) +@patch("strr_api.services.registration_service.RegistrationService.get_registration", new=fake_registration_pending) @patch("strr_api.models.rental.Registration.save", new=no_op) @patch("strr_api.models.user.User.find_by_jwt_token", new=fake_examiner_from_token) @patch("flask_jwt_oidc.JwtManager.get_token_auth_header", new=fake_get_token_auth_header) diff --git a/strr-api/tests/unit/utils/mocks.py b/strr-api/tests/unit/utils/mocks.py index 90f22a38..2ec01c80 100644 --- a/strr-api/tests/unit/utils/mocks.py +++ b/strr-api/tests/unit/utils/mocks.py @@ -56,6 +56,56 @@ def fake_user_from_db(*args, **kwargs): ) +def fake_registration_pending(*args, **kwargs): + return Registration( + id=1, + user_id=1, + sbc_account_id=1000, + status=RegistrationStatus.PENDING, + submission_date="2021-01-01T00:00:00Z", + updated_date="2021-01-01T00:00:00Z", + eligibility=Eligibility( + id=1, + registration_id=1, + is_principal_residence=True, + agreed_to_rental_act=True, + agreed_to_submit=True, + ), + rental_property=RentalProperty( + id=1, + property_type=PropertyType.PRIMARY, + ownership_type=OwnershipType.OWN, + address=Address( + id=1, + street_address="123 Fake St", + country="CA", + city="Victoria", + province="BC", + postal_code="V8V 8V8", + ), + property_manager=PropertyManager( + id=1, + primary_contact=Contact( + id=1, + firstname="First", + lastname="Last", + email="first.last@bc.gov.ca", + phone_number="123-456-7890", + date_of_birth="1970-01-01", + address=Address( + id=1, + street_address="123 Fake St", + country="CA", + city="Victoria", + province="BC", + postal_code="V8V 8V8", + ), + ), + ), + ), + ) + + def fake_registration(*args, **kwargs): return Registration( id=1,