Skip to content

Commit

Permalink
registration search and sort improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
turb0c0w committed Jul 22, 2024
1 parent b3e4a07 commit 5d5fe25
Show file tree
Hide file tree
Showing 10 changed files with 188 additions and 23 deletions.
46 changes: 46 additions & 0 deletions strr-api/migrations/versions/20240719_2b707ec995f7_.py
Original file line number Diff line number Diff line change
@@ -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;')
12 changes: 6 additions & 6 deletions strr-api/src/strr_api/enums/enum.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
6 changes: 3 additions & 3 deletions strr-api/src/strr_api/exceptions/responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
3 changes: 3 additions & 0 deletions strr-api/src/strr_api/models/certificate.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"""
from __future__ import annotations

from sqlalchemy.orm import relationship
from sqlalchemy.sql import text

from .db import db
Expand All @@ -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")
1 change: 1 addition & 0 deletions strr-api/src/strr_api/models/rental.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
38 changes: 31 additions & 7 deletions strr-api/src/strr_api/resources/registrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: %<search-text>% 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
Expand All @@ -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()]
Expand All @@ -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])
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions strr-api/src/strr_api/responses/RegistrationResponse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -125,13 +126,21 @@ 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,
sbc_account_id=source.sbc_account_id,
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,
Expand Down
43 changes: 37 additions & 6 deletions strr-api/src/strr_api/services/registration_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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:
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion strr-api/tests/unit/resources/test_registrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
50 changes: 50 additions & 0 deletions strr-api/tests/unit/utils/mocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 5d5fe25

Please sign in to comment.