Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion strr-api/pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tool.poetry]
name = "strr-api"
version = "0.3.11"
version = "0.3.12"
description = ""
authors = ["thorwolpert <thor@wolpert.ca>"]
license = "BSD 3-Clause"
Expand Down
2 changes: 1 addition & 1 deletion strr-api/src/strr_api/models/dataclass.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,4 @@ class RegistrationSearch:
noc_statuses: List[str] | None = None
is_set_aside: bool | None = None
local_gov: str | None = None
renewals_only: bool | None = None
review_renew: bool | None = None
77 changes: 42 additions & 35 deletions strr-api/src/strr_api/models/rental.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,27 @@ class RegistrationType(BaseEnum):
def search_registrations(cls, filter_criteria: RegistrationSearch):
"""Returns the registrations matching the search criteria."""
query = cls.query
query = cls._apply_base_search_filters(query, filter_criteria)
if filter_criteria.requirements:
query = cls._filter_by_registration_requirement(filter_criteria.requirements, query)
if filter_criteria.local_gov:
query = query.join(RentalProperty).filter(
RentalProperty.jurisdiction.ilike(f"%{filter_criteria.local_gov}%")
)
sub_status_conditions = cls._collect_sub_status_conditions(filter_criteria)
if sub_status_conditions:
query = query.filter(db.or_(*sub_status_conditions))
sort_column = getattr(Registration, filter_criteria.sort_by, Registration.id)
if filter_criteria.sort_order and filter_criteria.sort_order.lower() == "asc":
query = query.order_by(sort_column.asc())
else:
query = query.order_by(sort_column.desc())
paginated_result = query.paginate(per_page=filter_criteria.limit, page=filter_criteria.page)
return paginated_result

@classmethod
def _apply_base_search_filters(cls, query, filter_criteria: RegistrationSearch):
"""Apply common registration filters before specialized filters."""
if filter_criteria.account_id:
query = query.filter(Registration.sbc_account_id == filter_criteria.account_id)
if filter_criteria.search_text:
Expand All @@ -139,27 +160,21 @@ def search_registrations(cls, filter_criteria: RegistrationSearch):
query = query.join(User, Registration.reviewer_id == User.id).filter(
User.username.ilike(f"%{filter_criteria.assignee}%")
)
if filter_criteria.requirements:
query = cls._filter_by_registration_requirement(filter_criteria.requirements, query)
return query

@classmethod
def _collect_sub_status_conditions(cls, filter_criteria: RegistrationSearch):
"""Collect OR conditions for registration sub-status filters."""
sub_status_conditions = []
if filter_criteria.approval_methods:
query = cls._filter_by_approval_method(filter_criteria.approval_methods, query)
sub_status_conditions.append(cls._approval_method_condition(filter_criteria.approval_methods))
if filter_criteria.noc_statuses:
query = query.filter(Registration.noc_status.in_(filter_criteria.noc_statuses))
sub_status_conditions.append(Registration.noc_status.in_(filter_criteria.noc_statuses))
if filter_criteria.is_set_aside is True:
query = query.filter(Registration.is_set_aside == True) # noqa: E712
if filter_criteria.local_gov:
query = query.join(RentalProperty).filter(
RentalProperty.jurisdiction.ilike(f"%{filter_criteria.local_gov}%")
)
if filter_criteria.renewals_only is True:
query = cls._filter_by_renewals_only(query)
sort_column = getattr(Registration, filter_criteria.sort_by, Registration.id)
if filter_criteria.sort_order and filter_criteria.sort_order.lower() == "asc":
query = query.order_by(sort_column.asc())
else:
query = query.order_by(sort_column.desc())
paginated_result = query.paginate(per_page=filter_criteria.limit, page=filter_criteria.page)
return paginated_result
sub_status_conditions.append(Registration.is_set_aside == True) # noqa: E712
if filter_criteria.review_renew is True:
sub_status_conditions.append(cls._review_renew_condition())
return sub_status_conditions

@classmethod
def _host_condition_bl_or_pr(cls, application_model):
Expand Down Expand Up @@ -436,15 +451,8 @@ def _filter_by_registration_requirement(cls, requirement: list[str], query):
return query

@classmethod
def _filter_by_approval_method(cls, approval_methods: list[str], query):
"""Filter registrations by application approval method.

Only considers the most recent application (index 0 when sorted by
application_date desc) for each registration. Returns registrations
where that most recent application's status is in the given approval methods.
"""
if not approval_methods:
return query
def _approval_method_condition(cls, approval_methods: list[str]):
"""Build SQL condition for filtering by latest application status."""
# pylint: disable=import-outside-toplevel
from sqlalchemy import select

Expand All @@ -458,29 +466,28 @@ def _filter_by_approval_method(cls, approval_methods: list[str], query):
.limit(1)
.scalar_subquery()
)
return query.filter(latest_app_status_subq.in_(approval_methods))
return latest_app_status_subq.in_(approval_methods)

@classmethod
def _filter_by_renewals_only(cls, query):
"""Filter to registrations that have at least one renewal application in an approved/provisional status."""
def _review_renew_condition(cls):
"""Build SQL condition for registrations requiring renewal review."""
# pylint: disable=import-outside-toplevel
from strr_api.enums.enum import ApplicationType
from strr_api.models.application import Application

renewal_approved_statuses = [
Application.Status.FULL_REVIEW_APPROVED.value,
renewal_not_fully_approved_statuses = [
Application.Status.FULL_REVIEW.value,
Application.Status.PROVISIONALLY_APPROVED.value,
Application.Status.PROVISIONAL_REVIEW.value,
Application.Status.AUTO_APPROVED.value,
]
renewal_exists = db.exists().where(
db.and_(
Application.registration_id == Registration.id,
Application.type == ApplicationType.RENEWAL.value,
Application.status.in_(renewal_approved_statuses),
Application.status.in_(renewal_not_fully_approved_statuses),
)
)
return query.filter(renewal_exists)
return renewal_exists


class RentalProperty(Versioned, BaseModel):
Expand Down
10 changes: 5 additions & 5 deletions strr-api/src/strr_api/resources/registrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1038,9 +1038,9 @@ def search_registrations():
type: string
description: Local government (jurisdiction) filter
- in: query
name: renewalsOnly
name: reviewRenew
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed the query param to reviewReview to match the sub-status in the UI.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, was wondering from above.

type: boolean
description: When true, only return registrations that have a renewal application in an approved/provisional status (FULL_REVIEW_APPROVED, PROVISIONALLY_APPROVED, PROVISIONAL_REVIEW, AUTO_APPROVED).
description: When true, only return registrations that have a renewal application that is not fully approved (FULL_REVIEW, PROVISIONAL_REVIEW, PROVISIONALLY_APPROVED).
responses:
200:
description:
Expand All @@ -1065,8 +1065,8 @@ def search_registrations():
is_set_aside_param = request.args.get("isSetAside", None)
is_set_aside = is_set_aside_param.lower() == "true" if is_set_aside_param else None
local_gov = request.args.get("localGov", None) or None
renewals_only_param = request.args.get("renewalsOnly", None)
renewals_only = renewals_only_param.lower() == "true" if renewals_only_param else None
review_renew_param = request.args.get("reviewRenew", None)
review_renew = review_renew_param.lower() == "true" if review_renew_param else None
if sort_by not in VALID_REGISTRATION_SORT_FIELDS:
sort_by = "id"
if sort_order not in ["asc", "desc"]:
Expand All @@ -1089,7 +1089,7 @@ def search_registrations():
noc_statuses=noc_statuses,
is_set_aside=is_set_aside,
local_gov=local_gov,
renewals_only=renewals_only,
review_renew=review_renew,
)

registration_list = RegistrationService.search_registrations(filter_criteria=filter_criteria)
Expand Down
40 changes: 19 additions & 21 deletions strr-api/tests/unit/resources/test_registrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -2576,10 +2576,10 @@ def test_search_registrations_approval_method_uses_most_recent_application_only(


@patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE)
def test_search_registrations_renewals_only_includes_registration_with_approved_renewal(
def test_search_registrations_review_renew_includes_registration_with_renewal_not_fully_approved(
mock_create_invoice, session, client, jwt
):
"""Test that renewalsOnly=true returns registrations that have a renewal in an approved/provisional status."""
"""Test that reviewRenew=true returns registrations that have a renewal that is not fully approved."""
from nanoid import generate

with open(CREATE_HOST_REGISTRATION_REQUEST) as f:
Expand All @@ -2606,33 +2606,33 @@ def test_search_registrations_renewals_only_includes_registration_with_approved_
registration_number = response_json.get("header").get("registrationNumber")
registration = Registration.query.filter_by(registration_number=registration_number).one_or_none()

# Add a renewal application in an approved status
# Add a renewal application that is not fully approved (FULL_REVIEW, PROVISIONAL_REVIEW, PROVISIONALLY_APPROVED)
session.refresh(application)
renewal_app = Application(
application_json=application.application_json,
application_number=generate(alphabet="0123456789", size=14),
type=ApplicationType.RENEWAL.value,
registration_type=application.registration_type,
status=Application.Status.FULL_REVIEW_APPROVED.value,
status=Application.Status.FULL_REVIEW.value,
registration_id=registration.id,
application_date=application.application_date + timedelta(seconds=1),
)
session.add(renewal_app)
session.commit()

# renewalsOnly=true should return this registration (has approved renewal)
# reviewRenew=true should return this registration (has renewal that is not fully approved)
rv = client.get(
f"/registrations/search?renewalsOnly=true&recordNumber={registration_number}&status={RegistrationStatus.ACTIVE.value}",
f"/registrations/search?reviewRenew=true&recordNumber={registration_number}&status={RegistrationStatus.ACTIVE.value}",
headers=staff_headers,
)
assert rv.status_code == HTTPStatus.OK
registrations = rv.json
assert len(registrations.get("registrations")) == 1
assert registrations.get("registrations")[0].get("registrationNumber") == registration_number

# renewalsOnly=false should still return it (no filter)
# reviewRenew=false should still return it (no filter)
rv = client.get(
f"/registrations/search?renewalsOnly=false&recordNumber={registration_number}",
f"/registrations/search?reviewRenew=false&recordNumber={registration_number}",
headers=staff_headers,
)
assert rv.status_code == HTTPStatus.OK
Expand All @@ -2642,10 +2642,10 @@ def test_search_registrations_renewals_only_includes_registration_with_approved_


@patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE)
def test_search_registrations_renewals_only_excludes_registration_without_renewal(
def test_search_registrations_review_renew_excludes_registration_without_renewal(
mock_create_invoice, session, client, jwt
):
"""Test that renewalsOnly=true excludes registrations that have no renewal application."""
"""Test that reviewRenew=true excludes registrations that have no renewal application."""
with open(CREATE_HOST_REGISTRATION_REQUEST) as f:
json_data = json.load(f)
headers = create_header(jwt, [PUBLIC_USER], "Account-Id")
Expand All @@ -2669,16 +2669,16 @@ def test_search_registrations_renewals_only_excludes_registration_without_renewa
response_json = rv.json
registration_number = response_json.get("header").get("registrationNumber")

# renewalsOnly=true should NOT return this registration (no renewal)
# reviewRenew=true should NOT return this registration (no renewal)
rv = client.get(
f"/registrations/search?renewalsOnly=true&recordNumber={registration_number}&status={RegistrationStatus.ACTIVE.value}",
f"/registrations/search?reviewRenew=true&recordNumber={registration_number}&status={RegistrationStatus.ACTIVE.value}",
headers=staff_headers,
)
assert rv.status_code == HTTPStatus.OK
registrations = rv.json
assert len(registrations.get("registrations")) == 0

# Without renewalsOnly param, registration is returned
# Without reviewRenew param, registration is returned
rv = client.get(
f"/registrations/search?recordNumber={registration_number}",
headers=staff_headers,
Expand All @@ -2690,10 +2690,8 @@ def test_search_registrations_renewals_only_excludes_registration_without_renewa


@patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE)
def test_search_registrations_renewals_only_excludes_renewal_in_non_approved_status(
mock_create_invoice, session, client, jwt
):
"""Test that renewalsOnly=true excludes registrations whose only renewal is not in an approved status."""
def test_search_registrations_review_renew_excludes_renewal_fully_approved(mock_create_invoice, session, client, jwt):
"""Test that reviewRenew=true excludes registrations whose only renewal is fully approved (e.g. FULL_REVIEW_APPROVED)."""
from nanoid import generate

with open(CREATE_HOST_REGISTRATION_REQUEST) as f:
Expand All @@ -2720,23 +2718,23 @@ def test_search_registrations_renewals_only_excludes_renewal_in_non_approved_sta
registration_number = response_json.get("header").get("registrationNumber")
registration = Registration.query.filter_by(registration_number=registration_number).one_or_none()

# Add a renewal application in DECLINED status (not in approved/provisional set)
# Add a renewal application that is fully approved (FULL_REVIEW_APPROVED)
session.refresh(application)
renewal_app = Application(
application_json=application.application_json,
application_number=generate(alphabet="0123456789", size=14),
type=ApplicationType.RENEWAL.value,
registration_type=application.registration_type,
status=Application.Status.DECLINED.value,
status=Application.Status.FULL_REVIEW_APPROVED.value,
registration_id=registration.id,
application_date=application.application_date + timedelta(seconds=1),
)
session.add(renewal_app)
session.commit()

# renewalsOnly=true should NOT return this registration (renewal is DECLINED)
# reviewRenew=true should NOT return this registration (renewal is fully approved)
rv = client.get(
f"/registrations/search?renewalsOnly=true&recordNumber={registration_number}&status={RegistrationStatus.ACTIVE.value}",
f"/registrations/search?reviewRenew=true&recordNumber={registration_number}&status={RegistrationStatus.ACTIVE.value}",
headers=staff_headers,
)
assert rv.status_code == HTTPStatus.OK
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const emptyFilters = () => ({
applicantName: '',
propertyAddress: '',
status: [],
subStatus: [],
localGov: '',
adjudicator: '',
submissionDate: { start: null, end: null },
Expand Down Expand Up @@ -55,6 +56,7 @@ function getStateFromStore (exStore: ReturnType<typeof useExaminerStore>) {
applicantName: filters.applicantName,
propertyAddress: filters.propertyAddress,
status: [...filters.status],
subStatus: [...(filters.subStatus || [])],
localGov: filters.localGov,
adjudicator: filters.adjudicator,
submissionDate: { start: filters.submissionDate?.start ?? null, end: filters.submissionDate?.end ?? null },
Expand Down Expand Up @@ -94,8 +96,10 @@ function applyStateToStore (
!isApplicationTab &&
(!state.filters.status || state.filters.status.length === 0)
) {
(exStore.tableFilters.status as any[]).splice(
0, exStore.tableFilters.status.length, ...exStore.registrationsOnlyStatuses)
const statusFilters = exStore.tableFilters.status as any[]
const subStatusFilters = exStore.tableFilters.subStatus as any[]
statusFilters.splice(0, statusFilters.length, ...exStore.registrationsOnlyStatuses)
subStatusFilters.splice(0, subStatusFilters.length, ...exStore.registrationsOnlySubStatuses)
}
nextTick(() => {
exStore.tablePage = state.page
Expand Down Expand Up @@ -126,8 +130,8 @@ export function useExaminerDashboardPersistence (
) {
const { isSplitDashboardTableEnabled } = useExaminerFeatureFlags()
// Capture "had saved state" before useSessionStorage runs, so we don't treat first visit as returning
const hadSavedAppState = hasSavedAppState()
const hadSavedRegState = hasSavedRegState()
const hadSavedAppStateOnLoad = hasSavedAppState()
const hadSavedRegStateOnLoad = hasSavedRegState()

const appState = useSessionStorage(APP_KEY, defaultState())
const regState = useSessionStorage(REG_KEY, defaultState())
Expand All @@ -141,7 +145,7 @@ export function useExaminerDashboardPersistence (
if (isSplitDashboardTableEnabled.value) {
const state = mergeSavedStateWithDefaults(currentState())
const isApp = isApplicationTab.value
applyStateToStore(exStore, state, isApp, isApp && !hadSavedAppState, !isApp && !hadSavedRegState)
applyStateToStore(exStore, state, isApp, isApp && !hadSavedAppStateOnLoad, !isApp && !hadSavedRegStateOnLoad)
}

// When the user switches tab: persist tab, save current table to storage, load the other table's state into the store
Expand Down Expand Up @@ -179,5 +183,8 @@ export function useExaminerDashboardPersistence (
}
})

return { hasSavedAppState, hasSavedRegState }
return {
hasSavedAppState: () => hadSavedAppStateOnLoad,
hasSavedRegState: () => hadSavedRegStateOnLoad
}
}
Loading
Loading