From 963b7d70fe304337711240f6e9e1198ae78bcf5e Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Mon, 15 Jun 2026 14:06:54 +0000 Subject: [PATCH] fix terraform error and pass handler bool for dry runs --- applications/sharepoint_renamer/handler.py | 4 ++- .../sharepoint_renamer_request.py | 5 +++ .../lambda/sharepoint_renamer/variables.tf | 5 +++ .../sharepoint_renamer_orchestrator.py | 32 ++++++++++++------- tests/scripts/test_rename_sharepoint_files.py | 24 +++++++++++++- 5 files changed, 57 insertions(+), 13 deletions(-) create mode 100644 applications/sharepoint_renamer/sharepoint_renamer_request.py diff --git a/applications/sharepoint_renamer/handler.py b/applications/sharepoint_renamer/handler.py index 998458bc..67e64cf7 100644 --- a/applications/sharepoint_renamer/handler.py +++ b/applications/sharepoint_renamer/handler.py @@ -1,5 +1,6 @@ from typing import Any +from applications.sharepoint_renamer.sharepoint_renamer_request import SharepointRenamerRequest from orchestration.sharepoint_renamer_orchestrator import SharepointRenamerOrchestrator from utils.sharepoint.domna_sharepoint_client import DomnaSharepointClient from utils.sharepoint.domna_sites import DomnaSites @@ -8,6 +9,7 @@ CSV_PATH = "applications/sharepoint_renamer/sero_address_list.csv" def handler(event: dict[str, Any], context: Any) -> None: + request = SharepointRenamerRequest.model_validate(event) sp_client = DomnaSharepointClient(DomnaSites.SOCIAL_HOUSING_WAVE_3) - orchestrator = SharepointRenamerOrchestrator(sp_client, CSV_PATH) + orchestrator = SharepointRenamerOrchestrator(sp_client, CSV_PATH, dry_run=request.dry_run) orchestrator.run() diff --git a/applications/sharepoint_renamer/sharepoint_renamer_request.py b/applications/sharepoint_renamer/sharepoint_renamer_request.py new file mode 100644 index 00000000..6a10447b --- /dev/null +++ b/applications/sharepoint_renamer/sharepoint_renamer_request.py @@ -0,0 +1,5 @@ +from pydantic import BaseModel + + +class SharepointRenamerRequest(BaseModel): + dry_run: bool = False diff --git a/deployment/terraform/lambda/sharepoint_renamer/variables.tf b/deployment/terraform/lambda/sharepoint_renamer/variables.tf index 97cca538..192dbafa 100644 --- a/deployment/terraform/lambda/sharepoint_renamer/variables.tf +++ b/deployment/terraform/lambda/sharepoint_renamer/variables.tf @@ -1,3 +1,8 @@ +variable "lambda_name" { + type = string + description = "Logical name of the lambda (e.g. sharepoint_renamer)" +} + variable "stage" { description = "Deployment stage (e.g. dev, prod)" type = string diff --git a/orchestration/sharepoint_renamer_orchestrator.py b/orchestration/sharepoint_renamer_orchestrator.py index 764776ae..b73c41b5 100644 --- a/orchestration/sharepoint_renamer_orchestrator.py +++ b/orchestration/sharepoint_renamer_orchestrator.py @@ -1,9 +1,9 @@ import csv -import logging import os from typing import Optional from backend.pashub_fetcher.sharepoint_subfolders import SharepointSubfolders +from utilities.logger import setup_logger from utils.sharepoint.domna_sharepoint_client import DomnaSharepointClient BASE_PATH = ( @@ -12,7 +12,7 @@ BASE_PATH = ( ) ASSESSMENT_SUBFOLDER = "A. Assessment" -logger = logging.getLogger(__name__) +logger = setup_logger() def build_canonical_filename( @@ -58,9 +58,12 @@ def build_canonical_filename( class SharepointRenamerOrchestrator: - def __init__(self, sp_client: DomnaSharepointClient, csv_path: str) -> None: + def __init__( + self, sp_client: DomnaSharepointClient, csv_path: str, dry_run: bool = False + ) -> None: self._sp_client = sp_client self._csv_path = csv_path + self._dry_run = dry_run def run(self) -> None: with open(self._csv_path, newline="", encoding="utf-8-sig") as f: @@ -97,17 +100,24 @@ class SharepointRenamerOrchestrator: ) elif "file" in item: original_name: str = item["name"] - new_name = build_canonical_filename(uprn, address, postcode, original_name) + new_name = build_canonical_filename( + uprn, address, postcode, original_name + ) if new_name is None: continue - try: - self._sp_client.rename_file(item["id"], new_name) + if self._dry_run: logger.info( - f'Renamed: "{original_name}" → "{new_name}" (UPRN: {uprn})' - ) - except Exception as e: - logger.error( - f'Failed to rename "{original_name}" → "{new_name}" (UPRN: {uprn}): {e}' + f'Would rename: "{original_name}" → "{new_name}" (UPRN: {uprn})' ) + else: + try: + self._sp_client.rename_file(item["id"], new_name) + logger.info( + f'Renamed: "{original_name}" → "{new_name}" (UPRN: {uprn})' + ) + except Exception as e: + logger.error( + f'Failed to rename "{original_name}" → "{new_name}" (UPRN: {uprn}): {e}' + ) diff --git a/tests/scripts/test_rename_sharepoint_files.py b/tests/scripts/test_rename_sharepoint_files.py index 7b3e6587..5affea7e 100644 --- a/tests/scripts/test_rename_sharepoint_files.py +++ b/tests/scripts/test_rename_sharepoint_files.py @@ -21,9 +21,10 @@ def _make_package(name: str) -> dict[str, Any]: return {"name": name, "package": {}} -def _make_orchestrator(sp: MagicMock) -> SharepointRenamerOrchestrator: +def _make_orchestrator(sp: MagicMock, dry_run: bool = False) -> SharepointRenamerOrchestrator: orchestrator = SharepointRenamerOrchestrator.__new__(SharepointRenamerOrchestrator) orchestrator._sp_client = sp + orchestrator._dry_run = dry_run return orchestrator @@ -144,3 +145,24 @@ def test_skips_already_canonical_files() -> None: _make_orchestrator(sp)._process_folder("some/path", "500", "5 Pine Ln", "BB3 3CC") sp.rename_file.assert_not_called() + + +# --------------------------------------------------------------------------- +# _process_folder — dry_run=True logs intent but never calls rename_file +# --------------------------------------------------------------------------- + + +def test_dry_run_logs_would_rename_without_calling_api( + caplog: pytest.LogCaptureFixture, +) -> None: + sp = MagicMock() + sp.get_folders_in_path.return_value = { + "value": [_make_file("Survey.pdf", "id-1")] + } + + _make_orchestrator(sp, dry_run=True)._process_folder( + "some/path", "100", "1 High St", "AB1 2CD" + ) + + sp.rename_file.assert_not_called() + assert any("Would rename" in r.message for r in caplog.records)