Merge pull request #58 from IATI/sk-paging-on-reporting-org

Add paging to GET /reporting-orgs, and two small fixes.

Author: simon-20

.gitignore

@@ -10,3 +10,11 @@ __pycache__/
 
 .ve
 .venv
+
+# Project-specific items
+/keys/
+/logs/
+
+/.env
+/test.db
+/test-audit-log-public-key.pem

CHANGELOG.md

@@ -19,6 +19,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Security
 
+## [0.2.6]
+
+### Added
+
+ - Added paging to GET /reporting-orgs, as per the specification.
+
+### Fixed
+
+ - Fixed the 'last' paging link when there are no results to point to page 1.
+
 ## [0.2.5]
 
 ### Fixed

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "register-your-data-api"
-version = "0.2.5"
+version = "0.2.6"
 requires-python = ">= 3.12.11"
 readme = "README.md"
 authors = [{name="IATI Secretariat", email="[email protected]"}]
@@ -12,7 +12,7 @@ dependencies = [
     "click",
     "cryptography==45.0.5",
     "fastapi[standard]==0.116.0",
-    "libsuitecrm @ git+https://github.com/iati/iati-registry-libsuitecrm@v0.4.0",
+    "libsuitecrm @ git+https://github.com/iati/iati-registry-libsuitecrm@v0.5.0",
     "prometheus-client>=0.22.1",
     "pyjwt>=2.10.0",
     "python-dotenv>=1.1.1",

requirements.txt

@@ -65,7 +65,7 @@ idna==3.10
     #   requests
 jinja2==3.1.6
     # via fastapi
-libsuitecrm @ git+https://github.com/iati/iati-registry-libsuitecrm@v0.4.0
+libsuitecrm @ git+https://github.com/iati/iati-registry-libsuitecrm@v0.5.0
     # via register-your-data-api (pyproject.toml)
 mako==1.3.10
     # via alembic

requirements_dev.txt

@@ -87,7 +87,7 @@ isort==6.0.1
     # via register-your-data-api (pyproject.toml)
 jinja2==3.1.6
     # via fastapi
-libsuitecrm @ git+https://github.com/iati/iati-registry-libsuitecrm@v0.4.0
+libsuitecrm @ git+https://github.com/iati/iati-registry-libsuitecrm@v0.5.0
     # via register-your-data-api (pyproject.toml)
 mako==1.3.10
     # via alembic

src/register_your_data_api/client_application_details_provider.py

@@ -1,3 +1,4 @@
+from logging import Logger
 from typing import Annotated
 from uuid import UUID
 
@@ -23,11 +24,17 @@ class ClientApplicationDetails(BaseModel):
 
 class ClientApplicationDetailsProvider:
 
+    _app_logger: Logger
+    _audit_logger: Logger
     _CLIENT_APPLICATION_DETAILS: dict[str, ClientApplicationDetails] = {}
 
-    def __init__(self, filename: str) -> None:
+    def __init__(self, filename: str, app_logger: Logger, audit_logger: Logger) -> None:
         """Initializes the provider by loading client application details from a JSON file."""
 
+        self._app_logger = app_logger
+
+        self._audit_logger = audit_logger
+
         try:
             with open(filename, "r") as file:
                 data = file.read()
@@ -46,6 +53,9 @@ def get_client_application_details(self, client_id: str) -> ClientApplicationDet
         """Retrieves details about the client application given its client ID."""
 
         if client_id not in self._CLIENT_APPLICATION_DETAILS:
-            raise ValueError("Unknown client application")
+            error_message = f"Unknown client application. Client id: {client_id} is not found in the list of clients"
+            self._app_logger.error(error_message)
+            self._audit_logger.error(error_message)
+            raise ValueError(error_message)
 
         return self._CLIENT_APPLICATION_DETAILS[client_id]

src/register_your_data_api/data_handling/data_schemas.py

@@ -231,10 +231,3 @@ class UserReportingOrgRelationSingleResponse(pydantic.BaseModel):
     data: UserReportingOrgRelation
     error: str | None = pydantic.Field(None)
     status: str
-
-
-class UserReportingOrgRelationListResponse(pydantic.BaseModel):
-
-    data: list[UserReportingOrgRelation | UserReportingOrgDiscoverableMetadataRelation]
-    error: str | None = pydantic.Field(None)
-    status: str

src/register_your_data_api/response_schemas.py

@@ -28,7 +28,7 @@ def create(
         if page_size == 0:
             raise ValueError("page_size must be greater than 0")
 
-        total_pages = -(-total_records // page_size)
+        total_pages = max(1, -(-total_records // page_size))
 
         next_page_url = None
         if page < total_pages:

src/register_your_data_api/routers/reporting_orgs.py

@@ -8,7 +8,7 @@
 from fastapi import Depends, Security
 from fastapi.exceptions import HTTPException
 from fastapi.responses import JSONResponse
-from libsuitecrm import Filter, SuiteCRM  # type: ignore
+from libsuitecrm import Filter, RequestFailed, SuiteCRM  # type: ignore
 
 from register_your_data_api.dependencies import get_suitecrm_audit_headers
 
@@ -37,7 +37,6 @@
     ReportingOrgUserCreateModel,
     UserReportingOrgDiscoverableMetadataRelation,
     UserReportingOrgRelation,
-    UserReportingOrgRelationListResponse,
     UserReportingOrgRelationSingleResponse,
 )
 from ..response_schemas import PaginatedResultsPage
@@ -51,62 +50,80 @@
 def get_reporting_orgs(
     request: starlette.requests.Request,
     user: auth_models.UserAndCredentials = Security(authz.get_user_authnz, scopes=["ryd", "ryd:reporting_org"]),
+    paging: PaginationQueryParams = fastapi.Depends(),
     include_actions: str = "no",
-) -> UserReportingOrgRelationListResponse:
+) -> PaginatedResultsPage[UserReportingOrgRelation | UserReportingOrgDiscoverableMetadataRelation]:
 
     context: Context = request.app.state.context
 
-    user_reporting_org_associations = user.validator.get_users_fine_grained_associations()
+    crm: SuiteCRM = context.suitecrm_client_factory.get_client()
+
+    try:
+        orgs_for_user = crm.get_relationship(
+            "Contacts",
+            user.user_id_crm,
+            "Accounts",
+            page_number=paging.page,
+            page_size=paging.page_size,
+            sort_field="name",
+            sort_dir="ascending",
+            filters=Filter().equal("iati_registry_discoverable", "1"),
+        )
+    except RequestFailed as e:
+        error_id = uuid.uuid4()
+        public_error_message = (
+            "There was a problem fetching the list of reporting orgs you are associated with. "
+            f"Please try again later, or contact IATI Support quoting error id: {error_id}"
+        )
+        context.app_logger.error(
+            f"Error: error id: {error_id} - user id: {user.user_id_crm} - GET /reporting-orgs - Problem when fetching "
+            "the list of reporting organisations for this user from SuiteCRM. "
+            f"Details: {str(e)}"
+        )
+        raise HTTPException(status_code=fastapi.status.HTTP_500_INTERNAL_SERVER_ERROR, detail=public_error_message)
+
+    total_records = orgs_for_user.get("meta", {}).get("total-records", 0)
 
     reporting_orgs_list: list[UserReportingOrgRelation | UserReportingOrgDiscoverableMetadataRelation] = []
 
-    if len(user_reporting_org_associations) > 0:
+    for reporting_org_from_suitecrm in orgs_for_user["data"]:
+        role_for_org = user.validator.get_user_role_for_reporting_org(reporting_org_from_suitecrm["id"])
 
-        crm: SuiteCRM = context.suitecrm_client_factory.get_client()
+        if role_for_org is None:
+            context.app_logger.info(
+                f"Info: user id: {user.user_id_crm} - GET /reporting-orgs - user is associated with organisation "
+                f"{reporting_org_from_suitecrm["id"]} in the CRM but has no role for that organisation in the FGA DB. "
+                "Organisation was omitted from the list returned to the user."
+            )
+            continue
 
-        fields = SUITECRM_REPORTING_ORG_FIELDS
+        reporting_org_obj: ReportingOrgMetadata | DiscoverableReportingOrgMetadata
 
-        # The OR search in SuiteCRM appears to be broken; you can't search for items where id = 'A' OR id = 'B' OR ...
-        # It appears this doesn't work when the field being searched on is the same in each case. So we have to fetch
-        # the details for reporting orgs the user is associated with one at a time.
-        suitecrm_collected_responses: list[dict[str, Any]] = []
-        for user_reporting_org_association in user_reporting_org_associations:
-            filters = Filter()
-            filters.equal("id", str(user_reporting_org_association.reporting_org))
-            filters.equal("iati_registry_discoverable", "1")
-            crm_reporting_org = crm.get_records("Accounts", fields=fields, filters=filters)
-            if "data" in crm_reporting_org and len(crm_reporting_org["data"]) > 0:
-                suitecrm_collected_responses.append(*crm_reporting_org["data"])
-
-        for reporting_org_from_suitecrm in suitecrm_collected_responses:
-            role_for_org = user.validator.get_user_role_for_reporting_org(reporting_org_from_suitecrm["id"])
-            reporting_org_obj: ReportingOrgMetadata | DiscoverableReportingOrgMetadata
-
-            if role_for_org == fga_models.FineGrainedAuthorisationRole.CONTRIBUTOR_PENDING:
-                reporting_org_obj = get_discoverable_reporting_org_meta_from_suitecrm_response(
-                    reporting_org_from_suitecrm["attributes"]
-                )
-            else:
-                reporting_org_obj = get_reporting_org_meta_from_suitecrm_response(
-                    reporting_org_from_suitecrm["attributes"]
-                )
+        if role_for_org == fga_models.FineGrainedAuthorisationRole.CONTRIBUTOR_PENDING:
+            reporting_org_obj = get_discoverable_reporting_org_meta_from_suitecrm_response(
+                reporting_org_from_suitecrm["attributes"]
+            )
+        else:
+            reporting_org_obj = get_reporting_org_meta_from_suitecrm_response(
+                reporting_org_from_suitecrm["attributes"]
+            )
 
-            reporting_orgs_list.append(
-                UserReportingOrgRelation(
-                    id=reporting_org_from_suitecrm["id"],
-                    user_role=get_fga_role_as_str(role_for_org),  # type: ignore
-                    metadata=reporting_org_obj,
-                    reporting_org_actions=(
-                        get_reporting_org_actions(crm, reporting_org_from_suitecrm["id"])
-                        if include_actions == "yes"
-                        else []
-                    ),
-                )
+        reporting_orgs_list.append(
+            UserReportingOrgRelation(
+                id=reporting_org_from_suitecrm["id"],
+                user_role=get_fga_role_as_str(role_for_org),
+                metadata=reporting_org_obj,
+                reporting_org_actions=(
+                    get_reporting_org_actions(crm, reporting_org_from_suitecrm["id"])
+                    if include_actions == "yes"
+                    else []
+                ),
             )
+        )
 
-        reporting_orgs_list.sort(key=lambda org: org.metadata.human_readable_name.lower())
+    reporting_orgs_list.sort(key=lambda org: org.metadata.human_readable_name.lower())
 
-    return UserReportingOrgRelationListResponse(status="success", error=None, data=reporting_orgs_list)
+    return PaginatedResultsPage.create(reporting_orgs_list, paging.page, paging.page_size, total_records, request)
 
 
 @router.get("/{org_id}")

src/register_your_data_api/util.py

@@ -168,7 +168,9 @@ def __del__(self) -> None:
             pass
 
     def _setup_client_application_details_provider(self) -> None:
-        self._client_app_details = ClientApplicationDetailsProvider(self._env["CLIENT_APPLICATION_DETAILS_FILE"])
+        self._client_app_details = ClientApplicationDetailsProvider(
+            self._env["CLIENT_APPLICATION_DETAILS_FILE"], self._app_logger, self._audit_logger
+        )
 
     def _setup_prom_metrics(self) -> None:
         """Add all the prometheus metrics"""