From 6aab7ab6631079285106116c66fca98aa15c5ba9 Mon Sep 17 00:00:00 2001 From: Karim El Jazzar Date: Tue, 3 Feb 2026 14:43:10 -0800 Subject: [PATCH 1/2] feat: pass document dates in applications and registrations endpoint responses --- strr-api/pyproject.toml | 2 +- strr-api/src/strr_api/models/application.py | 17 +++ .../src/strr_api/resources/application.py | 11 +- .../strr_api/responses/DocumentResponse.py | 4 +- .../responses/RegistrationSerializer.py | 4 +- .../strr_api/services/application_service.py | 19 ++++ .../strr_api/services/gcp_storage_service.py | 17 +++ .../test_registration_applications.py | 107 ++++++++++++++++++ .../unit/resources/test_registrations.py | 94 ++++++++++++++- .../unit/services/test_gcp_storage_service.py | 77 +++++++++++++ 10 files changed, 346 insertions(+), 6 deletions(-) create mode 100644 strr-api/tests/unit/services/test_gcp_storage_service.py diff --git a/strr-api/pyproject.toml b/strr-api/pyproject.toml index 2e287180c..5c57ae700 100644 --- a/strr-api/pyproject.toml +++ b/strr-api/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "strr-api" -version = "0.1.16" +version = "0.1.17" description = "" authors = ["thorwolpert "] license = "BSD 3-Clause" diff --git a/strr-api/src/strr_api/models/application.py b/strr-api/src/strr_api/models/application.py index 6e02a60ac..9df359598 100644 --- a/strr-api/src/strr_api/models/application.py +++ b/strr-api/src/strr_api/models/application.py @@ -599,4 +599,21 @@ def to_dict(application: Application) -> dict: application_dict["header"]["nocStartDate"] = application.noc.start_date.strftime("%Y-%m-%d") application_dict["header"]["nocEndDate"] = application.noc.end_date.strftime("%Y-%m-%d") + # Set addedOn for registration documents (application-stage docs live only in application_json, not in DB) + registration_docs = application_dict.get("registration", {}).get("documents") + if registration_docs: + # First: use stored addedOn or uploadDate so application-stage docs show a date when we have one + for doc_item in registration_docs: + doc_item["addedOn"] = doc_item.get("addedOn") or doc_item.get("uploadDate") + # Then: overwrite with DB (created/added_on) when we have a live registration + if application.registration and application.registration.documents: + file_key_to_added_on = {} + for doc in application.registration.documents: + added_on_value = doc.added_on if doc.added_on is not None else getattr(doc, "created", None) + if added_on_value is not None: + file_key_to_added_on[doc.path] = added_on_value.isoformat() + for doc_item in registration_docs: + file_key = doc_item.get("fileKey") + doc_item["addedOn"] = file_key_to_added_on.get(file_key) if file_key else doc_item.get("addedOn") + return application_dict diff --git a/strr-api/src/strr_api/resources/application.py b/strr-api/src/strr_api/resources/application.py index bac185122..b676f067b 100644 --- a/strr-api/src/strr_api/resources/application.py +++ b/strr-api/src/strr_api/resources/application.py @@ -39,6 +39,7 @@ import logging import traceback +from datetime import datetime, timezone from http import HTTPStatus from io import BytesIO from typing import Optional @@ -341,7 +342,10 @@ def get_application_details(application_number): application = ApplicationService.get_application(application_number=application_number, account_id=account_id) if not application: return error_response(http_status=HTTPStatus.NOT_FOUND, message=ErrorMessage.APPLICATION_NOT_FOUND.value) - return jsonify(ApplicationService.serialize(application)), HTTPStatus.OK + app_dict = ApplicationService.serialize(application) + # Only fetch GCP blob creation times when viewing a single application (not when listing) + ApplicationService.enrich_document_added_on_from_gcp(app_dict) + return jsonify(app_dict), HTTPStatus.OK except AuthException as auth_exception: return exception_response(auth_exception) @@ -803,7 +807,10 @@ def update_registration_supporting_document(application_number): document = DocumentService.upload_document(filename, file.content_type, file.read()) document["documentType"] = document_type document["uploadStep"] = upload_step - document["uploadDate"] = upload_date + # Store upload date for application-stage docs (no registration yet, so not in documents table) + now_iso = datetime.now(timezone.utc).isoformat() + document["uploadDate"] = upload_date if upload_date else now_iso + document["addedOn"] = now_iso application = ApplicationService.update_document_list(application=application, document=document) diff --git a/strr-api/src/strr_api/responses/DocumentResponse.py b/strr-api/src/strr_api/responses/DocumentResponse.py index f3037fb4c..6d51351ca 100644 --- a/strr-api/src/strr_api/responses/DocumentResponse.py +++ b/strr-api/src/strr_api/responses/DocumentResponse.py @@ -21,10 +21,12 @@ class Document(BaseModel): @classmethod def from_db(cls, source: models.Document): """Return a Document object from a database model.""" + # Use added_on when set, otherwise document upload time from documents.created + added_on = source.added_on if source.added_on is not None else getattr(source, "created", None) return cls( registrationId=source.eligibility.registration_id, documentId=source.id, fileName=source.file_name, fileType=source.file_type, - addedOn=source.added_on, + addedOn=added_on, ) diff --git a/strr-api/src/strr_api/responses/RegistrationSerializer.py b/strr-api/src/strr_api/responses/RegistrationSerializer.py index 33d27cf3c..cac17fc5f 100644 --- a/strr-api/src/strr_api/responses/RegistrationSerializer.py +++ b/strr-api/src/strr_api/responses/RegistrationSerializer.py @@ -82,13 +82,15 @@ def serialize(cls, registration: Registration): documents = [] if registration.documents: for doc in registration.documents: + # Use added_on when set, otherwise document upload time from documents.created + added_on_value = doc.added_on if doc.added_on is not None else getattr(doc, "created", None) documents.append( { "fileKey": doc.path, "fileName": doc.file_name, "fileType": doc.file_type, "documentType": doc.document_type, - "addedOn": doc.added_on.isoformat() if doc.added_on else None, + "addedOn": added_on_value.isoformat() if added_on_value else None, } ) registration_data["documents"] = documents diff --git a/strr-api/src/strr_api/services/application_service.py b/strr-api/src/strr_api/services/application_service.py index 5806cd830..734fddf03 100644 --- a/strr-api/src/strr_api/services/application_service.py +++ b/strr-api/src/strr_api/services/application_service.py @@ -46,6 +46,7 @@ from strr_api.models.rental import PropertyContact from strr_api.services.email_service import EmailService from strr_api.services.events_service import EventsService +from strr_api.services.gcp_storage_service import GCPStorageService from strr_api.services.registration_service import RegistrationService from strr_api.services.user_service import UserService @@ -101,6 +102,24 @@ def serialize(application: Application) -> dict: ) return app_dict + @staticmethod + def enrich_document_added_on_from_gcp(app_dict: dict) -> dict: + """ + For application-stage docs with no addedOn, set addedOn from GCP blob creation time. + Call this only when returning a single application (GET /applications/), not when listing, + to avoid N GCP calls per page. + """ + registration_docs = app_dict.get("registration", {}).get("documents", []) + for doc_item in registration_docs: + if doc_item.get("addedOn"): + continue + file_key = doc_item.get("fileKey") + if file_key: + created_iso = GCPStorageService.get_registration_document_creation_time(file_key) + if created_iso: + doc_item["addedOn"] = created_iso + return app_dict + @staticmethod def save_application(account_id: int, request_json: dict, application: Application) -> Application: """Saves an application to db.""" diff --git a/strr-api/src/strr_api/services/gcp_storage_service.py b/strr-api/src/strr_api/services/gcp_storage_service.py index f45b653c4..0bcce2116 100644 --- a/strr-api/src/strr_api/services/gcp_storage_service.py +++ b/strr-api/src/strr_api/services/gcp_storage_service.py @@ -108,6 +108,23 @@ def fetch_registration_document(cls, blob_name): logger.error(traceback.format_exc()) raise ExternalServiceException(message="Error fetching registration document from gcp bucket.") from e + @classmethod + def get_registration_document_creation_time(cls, blob_name): + """ + Return the blob creation time (when uploaded to GCP) as ISO string, or None if not found/error. + Used to expose addedOn for application-stage documents that have no date in application_json. + """ + try: + registration_documents_bucket = cls.registration_documents_bucket() + blob = registration_documents_bucket.blob(blob_name) + blob.reload() + if blob.time_created: + return blob.time_created.isoformat() + return None + except Exception as e: + logger.debug("Could not get creation time for blob %s: %s", blob_name, e) + return None + @classmethod def upload_file(cls, file_type, file_contents, bucket_id): """Save the document to the specified bucket.""" diff --git a/strr-api/tests/unit/resources/test_registration_applications.py b/strr-api/tests/unit/resources/test_registration_applications.py index d392730df..283c50cdf 100644 --- a/strr-api/tests/unit/resources/test_registration_applications.py +++ b/strr-api/tests/unit/resources/test_registration_applications.py @@ -10,6 +10,7 @@ from strr_api.enums.enum import PaymentStatus, RegistrationStatus from strr_api.models import Application, Events from strr_api.models.application import ApplicationSerializer +from strr_api.services import ApplicationService from tests.unit.utils.auth_helpers import PUBLIC_USER, STRR_EXAMINER, create_header CREATE_HOST_REGISTRATION_REQUEST = os.path.join( @@ -480,6 +481,112 @@ def test_post_and_delete_registration_documents(session, client, jwt): assert rv.status_code == HTTPStatus.NO_CONTENT +@patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE) +def test_put_application_documents_includes_added_on(session, client, jwt): + """PUT /applications//documents sets addedOn (and uploadDate) on the appended document.""" + with open(CREATE_HOST_REGISTRATION_REQUEST) as f: + headers = create_header(jwt, [PUBLIC_USER], "Account-Id") + headers["Account-Id"] = ACCOUNT_ID + json_data = json.load(f) + rv = client.post("/applications", json=json_data, headers=headers) + response_json = rv.json + application_number = response_json.get("header").get("applicationNumber") + + with patch( + "strr_api.services.gcp_storage_service.GCPStorageService.upload_registration_document", + return_value="put-doc-file-key-456", + ): + with open(MOCK_DOCUMENT_UPLOAD, "rb") as df: + data = { + "file": (df, MOCK_DOCUMENT_UPLOAD), + "documentType": "BC_DRIVERS_LICENSE", + "uploadStep": "step1", + "uploadDate": "2025-06-01T12:00:00", + } + rv = client.put( + f"/applications/{application_number}/documents", + content_type="multipart/form-data", + data=data, + headers=headers, + ) + assert rv.status_code == HTTPStatus.OK + response_json = rv.json + docs = response_json.get("registration", {}).get("documents", []) + doc_uploaded = next((d for d in docs if d.get("fileKey") == "put-doc-file-key-456"), None) + assert doc_uploaded is not None + assert doc_uploaded.get("addedOn") is not None + assert doc_uploaded.get("uploadDate") is not None + + rv = client.get(f"/applications/{application_number}", headers=headers) + assert rv.status_code == HTTPStatus.OK + response_json = rv.json + docs = response_json.get("registration", {}).get("documents", []) + doc_uploaded = next((d for d in docs if d.get("fileKey") == "put-doc-file-key-456"), None) + assert doc_uploaded is not None + assert doc_uploaded.get("addedOn") is not None + + +@patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE) +def test_application_serializer_added_on_from_upload_date(mock_invoice, session, client, jwt): + """ApplicationSerializer.to_dict sets addedOn from stored uploadDate when no registration.""" + with open(CREATE_HOST_REGISTRATION_REQUEST) as f: + headers = create_header(jwt, [PUBLIC_USER], "Account-Id") + headers["Account-Id"] = ACCOUNT_ID + json_data = json.load(f) + json_data.setdefault("registration", {})["documents"] = [ + { + "fileKey": "stored-doc-key", + "fileName": "stored.pdf", + "fileType": "application/pdf", + "documentType": "BC_DRIVERS_LICENSE", + "uploadDate": "2025-01-15T14:30:00", + } + ] + rv = client.post("/applications", json=json_data, headers=headers) + assert rv.status_code == HTTPStatus.OK + application_number = rv.json.get("header").get("applicationNumber") + + application = Application.find_by_application_number(application_number=application_number) + app_dict = ApplicationSerializer.to_dict(application) + docs = app_dict.get("registration", {}).get("documents", []) + doc_stored = next((d for d in docs if d.get("fileKey") == "stored-doc-key"), None) + assert doc_stored is not None + assert doc_stored.get("addedOn") == "2025-01-15T14:30:00" + + +@patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE) +@patch("strr_api.services.gcp_storage_service.GCPStorageService.get_registration_document_creation_time") +def test_application_serialize_gcp_fallback_for_missing_added_on( + mock_gcp_creation_time, mock_invoice, session, client, jwt +): + """enrich_document_added_on_from_gcp (used only for GET single application) uses GCP when doc has no addedOn.""" + mock_gcp_creation_time.return_value = "2025-02-01T10:00:00Z" + with open(CREATE_HOST_REGISTRATION_REQUEST) as f: + headers = create_header(jwt, [PUBLIC_USER], "Account-Id") + headers["Account-Id"] = ACCOUNT_ID + json_data = json.load(f) + json_data.setdefault("registration", {})["documents"] = [ + { + "fileKey": "gcp-fallback-key", + "fileName": "no-date.pdf", + "fileType": "application/pdf", + "documentType": "BC_DRIVERS_LICENSE", + } + ] + rv = client.post("/applications", json=json_data, headers=headers) + assert rv.status_code == HTTPStatus.OK + application_number = rv.json.get("header").get("applicationNumber") + + application = Application.find_by_application_number(application_number=application_number) + app_dict = ApplicationService.serialize(application) + ApplicationService.enrich_document_added_on_from_gcp(app_dict) + docs = app_dict.get("registration", {}).get("documents", []) + doc_no_date = next((d for d in docs if d.get("fileKey") == "gcp-fallback-key"), None) + assert doc_no_date is not None + assert doc_no_date.get("addedOn") == "2025-02-01T10:00:00Z" + mock_gcp_creation_time.assert_called_once_with("gcp-fallback-key") + + @patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE) def test_search_applications(session, client, jwt): with open(CREATE_HOST_REGISTRATION_MINIMUM_FIELDS_REQUEST) as f: diff --git a/strr-api/tests/unit/resources/test_registrations.py b/strr-api/tests/unit/resources/test_registrations.py index dbd554fd7..14d73c3f3 100644 --- a/strr-api/tests/unit/resources/test_registrations.py +++ b/strr-api/tests/unit/resources/test_registrations.py @@ -16,7 +16,8 @@ RegistrationType, ) from strr_api.exceptions import ExternalServiceException -from strr_api.models import Application, Events, Registration, User +from strr_api.models import Application, Document, Events, Registration, User +from strr_api.responses import RegistrationSerializer from tests.unit.utils.auth_helpers import PUBLIC_USER, STRR_EXAMINER, create_header from tests.unit.utils.mocks import ( fake_document, @@ -392,6 +393,97 @@ def test_get_registration_by_id_unauthorized(client): assert rv.status_code == HTTPStatus.UNAUTHORIZED +@patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE) +def test_get_registration_by_id_includes_document_added_on(session, client, jwt): + """GET /registrations/ returns documents with addedOn (from DB created when added_on is null).""" + with open(CREATE_HOST_REGISTRATION_REQUEST) as f: + headers = create_header(jwt, [PUBLIC_USER], "Account-Id") + headers["Account-Id"] = ACCOUNT_ID + json_data = json.load(f) + rv = client.post("/applications", json=json_data, headers=headers) + response_json = rv.json + application_number = response_json.get("header").get("applicationNumber") + + application = Application.find_by_application_number(application_number=application_number) + application.payment_status = PaymentStatus.COMPLETED.value + application.status = Application.Status.FULL_REVIEW + application.save() + + staff_headers = create_header(jwt, [STRR_EXAMINER], "Account-Id") + rv = client.put(f"/applications/{application_number}/assign", headers=staff_headers) + assert rv.status_code == HTTPStatus.OK + status_update_request = {"status": Application.Status.FULL_REVIEW_APPROVED} + rv = client.put(f"/applications/{application_number}/status", json=status_update_request, headers=staff_headers) + assert rv.status_code == HTTPStatus.OK + response_json = rv.json + registration_id = response_json.get("header").get("registrationId") + + with patch( + "strr_api.services.gcp_storage_service.GCPStorageService.upload_registration_document", + return_value="test-file-key-123", + ): + with open(MOCK_DOCUMENT_UPLOAD, "rb") as df: + data = {"file": (df, MOCK_DOCUMENT_UPLOAD), "documentType": "BC_DRIVERS_LICENSE"} + rv = client.post( + f"/registrations/{registration_id}/documents", + content_type="multipart/form-data", + data=data, + headers=staff_headers, + ) + assert rv.status_code == HTTPStatus.CREATED + + rv = client.get(f"/registrations/{registration_id}", headers=headers) + assert rv.status_code == HTTPStatus.OK + response_json = rv.json + documents = response_json.get("documents", []) + assert len(documents) >= 1 + doc_with_key = next((d for d in documents if d.get("fileKey") == "test-file-key-123"), None) + assert doc_with_key is not None + assert doc_with_key.get("addedOn") is not None + + +@patch.object(RegistrationSerializer, "populate_host_registration_details") +def test_registration_serializer_document_added_on_uses_created_when_added_on_null(mock_populate, session): + """RegistrationSerializer uses doc.created for addedOn when doc.added_on is null.""" + user = User( + username="serializer_test_user", + firstname="Test", + lastname="User", + iss="test", + sub="sub_serializer", + idp_userid="serializer_test_id", + login_source="test", + ) + user.save() + registration = Registration( + user_id=user.id, + sbc_account_id=ACCOUNT_ID, + status=RegistrationStatus.ACTIVE, + registration_type=RegistrationType.HOST.value, + registration_number="H9999999", + start_date=datetime.now(), + expiry_date=datetime.now() + timedelta(days=365), + ) + registration.save() + doc = Document( + registration_id=registration.id, + file_name="test.pdf", + file_type="application/pdf", + path="test-path-key", + document_type=Document.DocumentType.BC_DRIVERS_LICENSE, + added_on=None, + ) + doc.save() + # Ensure doc has created (from BaseModel) + registration.documents = [doc] + # Minimal registration has no rental_property; avoid populate_host_registration_details + result = RegistrationSerializer.serialize(registration) + assert "documents" in result + assert len(result["documents"]) == 1 + assert result["documents"][0].get("addedOn") is not None + assert result["documents"][0]["fileKey"] == "test-path-key" + + @patch("strr_api.services.strr_pay.create_invoice", return_value=MOCK_INVOICE_RESPONSE) def test_cancel_registration(session, client, jwt): with open(CREATE_HOST_REGISTRATION_REQUEST) as f: diff --git a/strr-api/tests/unit/services/test_gcp_storage_service.py b/strr-api/tests/unit/services/test_gcp_storage_service.py new file mode 100644 index 000000000..01f53bded --- /dev/null +++ b/strr-api/tests/unit/services/test_gcp_storage_service.py @@ -0,0 +1,77 @@ +# Copyright © 2025 Province of British Columbia +# +# Licensed under the BSD 3 Clause License, (the "License"); +# you may not use this file except in compliance with the License. +# The template for the license can be found here +# https://opensource.org/license/bsd-3-clause/ +# +# Redistribution and use in source and binary forms, +# with or without modification, are permitted provided that the +# following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# 3. Neither the name of the copyright holder nor the names of its contributors +# may be used to endorse or promote products derived from this software +# without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE +# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR +# CONSEQUENTIAL DAMAGES HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN +# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING +# IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +"""Tests for GCPStorageService (registration document creation time).""" +from datetime import datetime, timezone +from unittest.mock import MagicMock, patch + +import pytest + +from strr_api.services.gcp_storage_service import GCPStorageService + + +@patch("strr_api.services.gcp_storage_service.GCPStorageService.registration_documents_bucket") +def test_get_registration_document_creation_time_returns_iso_when_blob_exists(mock_bucket): + """get_registration_document_creation_time returns blob time_created as ISO string when blob exists.""" + mock_blob = MagicMock() + created_dt = datetime(2025, 1, 15, 10, 30, 0, tzinfo=timezone.utc) + mock_blob.time_created = created_dt + mock_bucket.return_value.blob.return_value = mock_blob + + result = GCPStorageService.get_registration_document_creation_time("some-blob-key") + + assert result == "2025-01-15T10:30:00+00:00" + mock_blob.reload.assert_called_once() + + +@patch("strr_api.services.gcp_storage_service.GCPStorageService.registration_documents_bucket") +def test_get_registration_document_creation_time_returns_none_when_blob_missing(mock_bucket): + """get_registration_document_creation_time returns None when blob does not exist or reload fails.""" + mock_blob = MagicMock() + mock_blob.reload.side_effect = Exception("Blob not found") + mock_bucket.return_value.blob.return_value = mock_blob + + result = GCPStorageService.get_registration_document_creation_time("missing-key") + + assert result is None + + +@patch("strr_api.services.gcp_storage_service.GCPStorageService.registration_documents_bucket") +def test_get_registration_document_creation_time_returns_none_when_time_created_is_none(mock_bucket): + """get_registration_document_creation_time returns None when blob has no time_created.""" + mock_blob = MagicMock() + mock_blob.time_created = None + mock_bucket.return_value.blob.return_value = mock_blob + + result = GCPStorageService.get_registration_document_creation_time("no-time-key") + + assert result is None + mock_blob.reload.assert_called_once() From ff9ae7c2b13b875869c4e3c9671058efb6d6339e Mon Sep 17 00:00:00 2001 From: Karim El Jazzar Date: Wed, 4 Feb 2026 12:16:28 -0800 Subject: [PATCH 2/2] combined lines using walrus operator --- strr-api/src/strr_api/models/application.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/strr-api/src/strr_api/models/application.py b/strr-api/src/strr_api/models/application.py index 9df359598..03cbecef5 100644 --- a/strr-api/src/strr_api/models/application.py +++ b/strr-api/src/strr_api/models/application.py @@ -600,8 +600,7 @@ def to_dict(application: Application) -> dict: application_dict["header"]["nocEndDate"] = application.noc.end_date.strftime("%Y-%m-%d") # Set addedOn for registration documents (application-stage docs live only in application_json, not in DB) - registration_docs = application_dict.get("registration", {}).get("documents") - if registration_docs: + if registration_docs := application_dict.get("registration", {}).get("documents"): # First: use stored addedOn or uploadDate so application-stage docs show a date when we have one for doc_item in registration_docs: doc_item["addedOn"] = doc_item.get("addedOn") or doc_item.get("uploadDate")