eli-313 actions properties returned in snake case (#213)

* WIP

* WIP

* factory created for suggested actions

* unit tests

* exclude none test

* urllink is httpurl everywhere

* test for not null

* code refactoring

* code cleanup

---------

Co-authored-by: Robert <[email protected]>

Author: Karthikeyannhs

src/eligibility_signposting_api/model/eligibility.py

@@ -6,6 +6,8 @@
 from functools import total_ordering
 from typing import NewType, Self
 
+from pydantic import HttpUrl
+
 NHSNumber = NewType("NHSNumber", str)
 DateOfBirth = NewType("DateOfBirth", date)
 Postcode = NewType("Postcode", str)
@@ -17,7 +19,7 @@
 ActionType = NewType("ActionType", str)
 ActionCode = NewType("ActionCode", str)
 ActionDescription = NewType("ActionDescription", str)
-UrlLink = NewType("UrlLink", str)
+UrlLink = NewType("UrlLink", HttpUrl)
 UrlLabel = NewType("UrlLabel", str)
 
 
@@ -79,17 +81,12 @@ class SuggestedAction:
     url_label: UrlLabel | None
 
 
-@dataclass
-class SuggestedActions:
-    actions: list[SuggestedAction]
-
-
 @dataclass
 class Condition:
     condition_name: ConditionName
     status: Status
     cohort_results: list[CohortGroupResult]
-    actions: SuggestedActions | None = None
+    actions: list[SuggestedAction] | None = None
 
 
 @dataclass
@@ -104,7 +101,7 @@ class CohortGroupResult:
 class IterationResult:
     status: Status
     cohort_results: list[CohortGroupResult]
-    actions: SuggestedActions | None
+    actions: list[SuggestedAction] | None
 
 
 @dataclass

src/eligibility_signposting_api/model/rules.py

@@ -9,7 +9,7 @@
 from operator import attrgetter
 from typing import Literal, NewType
 
-from pydantic import BaseModel, Field, RootModel, field_serializer, field_validator, model_validator
+from pydantic import BaseModel, Field, HttpUrl, RootModel, field_serializer, field_validator, model_validator
 
 from eligibility_signposting_api.config.contants import MAGIC_COHORT_LABEL, RULE_STOP_DEFAULT
 
@@ -132,7 +132,7 @@ class AvailableAction(BaseModel):
     action_type: str = Field(..., alias="ActionType")
     action_code: str = Field(..., alias="ExternalRoutingCode")
     action_description: str | None = Field(None, alias="ActionDescription")
-    url_link: str | None = Field(None, alias="UrlLink")
+    url_link: HttpUrl | None = Field(None, alias="UrlLink")
     url_label: str | None = Field(None, alias="UrlLabel")
 
     model_config = {"populate_by_name": True}

src/eligibility_signposting_api/services/calculators/eligibility_calculator.py

@@ -23,7 +23,6 @@
     IterationResult,
     Status,
     SuggestedAction,
-    SuggestedActions,
     UrlLabel,
     UrlLink,
 )
@@ -129,7 +128,7 @@ def get_redirect_rules(
     def evaluate_eligibility(self, *, include_actions_flag: bool = True) -> eligibility.EligibilityStatus:
         """Iterates over campaign groups, evaluates eligibility, and returns a consolidated status."""
         condition_results: dict[ConditionName, IterationResult] = {}
-        actions: SuggestedActions | None = SuggestedActions([])
+        actions: list[SuggestedAction] | None = []
 
         for condition_name, campaign_group in self.campaigns_grouped_by_condition_name:
             iteration_results: dict[str, tuple[Iteration, IterationResult]] = {}
@@ -165,12 +164,12 @@ def evaluate_eligibility(self, *, include_actions_flag: bool = True) -> eligibil
         final_result = self.build_condition_results(condition_results)
         return eligibility.EligibilityStatus(conditions=final_result)
 
-    def handle_redirect_rules(self, best_active_iteration: Iteration) -> SuggestedActions | None:
+    def handle_redirect_rules(self, best_active_iteration: Iteration) -> list[SuggestedAction] | None:
         redirect_rules, action_mapper, default_comms = self.get_redirect_rules(best_active_iteration)
         priority_getter = attrgetter("priority")
         sorted_rules_by_priority = sorted(redirect_rules, key=priority_getter)
 
-        actions: SuggestedActions | None = self.get_actions_from_comms(action_mapper, default_comms)
+        actions: list[SuggestedAction] | None = self.get_actions_from_comms(action_mapper, default_comms)
         for _, rule_group in groupby(sorted_rules_by_priority, key=priority_getter):
             rule_group_list = list(rule_group)
             matcher_matched_list = [
@@ -181,7 +180,7 @@ def handle_redirect_rules(self, best_active_iteration: Iteration) -> SuggestedAc
             comms_routing = rule_group_list[0].comms_routing
             if comms_routing and all(matcher_matched_list):
                 rule_actions = self.get_actions_from_comms(action_mapper, comms_routing)
-                if rule_actions and len(rule_actions.actions) > 0:
+                if rule_actions and len(rule_actions) > 0:
                     actions = rule_actions
                 break
 
@@ -330,12 +329,12 @@ def evaluate_rules_priority_group(
         return best_status, inclusion_reasons, exclusion_reasons, is_rule_stop
 
     @staticmethod
-    def get_actions_from_comms(action_mapper: ActionsMapper, comms: str) -> SuggestedActions | None:
-        suggested_actions: SuggestedActions = SuggestedActions([])
+    def get_actions_from_comms(action_mapper: ActionsMapper, comms: str) -> list[SuggestedAction] | None:
+        suggested_actions: list[SuggestedAction] = []
         for comm in comms.split("|"):
             action = action_mapper.get(comm)
             if action is not None:
-                suggested_actions.actions.append(
+                suggested_actions.append(
                     SuggestedAction(
                         action_type=ActionType(action.action_type),
                         action_code=ActionCode(action.action_code),

src/eligibility_signposting_api/views/eligibility.py

@@ -39,7 +39,7 @@ def check_eligibility(nhs_number: NHSNumber, eligibility_service: Injected[Eligi
     except UnknownPersonError:
         return handle_unknown_person_error(nhs_number)
     else:
-        eligibility_response = build_eligibility_response(eligibility_status)
+        eligibility_response: eligibility.EligibilityResponse = build_eligibility_response(eligibility_status)
         return make_response(
             eligibility_response.model_dump(by_alias=True, mode="json", exclude_none=True), HTTPStatus.OK
         )
@@ -103,11 +103,7 @@ def build_eligibility_response(eligibility_status: EligibilityStatus) -> eligibi
             statusText=eligibility.StatusText(f"{condition.status}"),  # pyright: ignore[reportCallIssue]
             eligibilityCohorts=build_eligibility_cohorts(condition),  # pyright: ignore[reportCallIssue]
             suitabilityRules=build_suitability_results(condition),  # pyright: ignore[reportCallIssue]
-            actions=(
-                condition.actions.actions
-                if condition.actions is not None and condition.actions.actions is not None
-                else None
-            ),
+            actions=build_actions(condition),
         )
 
         processed_suggestions.append(suggestions)
@@ -120,6 +116,24 @@ def build_eligibility_response(eligibility_status: EligibilityStatus) -> eligibi
     )
 
 
+def build_actions(condition: Condition) -> list[eligibility.Action] | None:
+    if condition.actions is not None:
+        return [
+            eligibility.Action(
+                actionType=eligibility.ActionType(action.action_type),
+                actionCode=eligibility.ActionCode(action.action_code),
+                description=eligibility.Description(action.action_description)
+                if action.action_description is not None
+                else None,
+                urlLink=eligibility.HttpUrl(action.url_link) if action.url_link is not None else None,
+                urlLabel=eligibility.UrlLabel(action.url_label) if action.url_label is not None else None,
+            )
+            for action in condition.actions
+        ]
+
+    return None
+
+
 def build_eligibility_cohorts(condition: Condition) -> list[eligibility.EligibilityCohort]:
     """Group Iteration cohorts and make only one entry per cohort group"""
 

src/eligibility_signposting_api/views/response_model/eligibility.py

@@ -5,8 +5,6 @@
 from pydantic import UUID4, BaseModel, Field, HttpUrl, field_serializer
 from pydantic_core.core_schema import SerializationInfo
 
-from eligibility_signposting_api.model.eligibility import SuggestedAction
-
 LastUpdated = NewType("LastUpdated", datetime)
 ConditionName = NewType("ConditionName", str)
 StatusText = NewType("StatusText", str)
@@ -17,6 +15,7 @@
 RuleText = NewType("RuleText", str)
 CohortCode = NewType("CohortCode", str)
 CohortText = NewType("CohortText", str)
+UrlLabel = NewType("UrlLabel", str)
 
 
 class Status(StrEnum):
@@ -50,8 +49,9 @@ class SuitabilityRule(BaseModel):
 class Action(BaseModel):
     action_type: ActionType = Field(..., alias="actionType")
     action_code: ActionCode = Field(..., alias="actionCode")
-    description: Description
-    url_link: HttpUrl = Field(..., alias="urlLink")
+    description: Description | None
+    url_link: HttpUrl | None = Field(..., alias="urlLink")
+    url_label: UrlLabel | None = Field(..., alias="urlLabel")
 
     model_config = {"populate_by_name": True}
 
@@ -62,7 +62,7 @@ class ProcessedSuggestion(BaseModel):
     status_text: StatusText = Field(..., alias="statusText")
     eligibility_cohorts: list[EligibilityCohort] = Field(..., alias="eligibilityCohorts")
     suitability_rules: list[SuitabilityRule] = Field(..., alias="suitabilityRules")
-    actions: list[SuggestedAction] | None
+    actions: list[Action] | None
 
     model_config = {"populate_by_name": True}
 

tests/fixtures/builders/model/eligibility.py

@@ -3,18 +3,25 @@
 
 from polyfactory import Use
 from polyfactory.factories import DataclassFactory
+from pydantic import HttpUrl
 
-from eligibility_signposting_api.model.eligibility import CohortGroupResult, Condition, EligibilityStatus
+from eligibility_signposting_api.model import eligibility
+from eligibility_signposting_api.model.eligibility import UrlLink
 
 
-class ConditionFactory(DataclassFactory[Condition]): ...
+class SuggestedActionFactory(DataclassFactory[eligibility.SuggestedAction]):
+    url_link = UrlLink(HttpUrl("https://test-example.com"))
 
 
-class EligibilityStatusFactory(DataclassFactory[EligibilityStatus]):
-    condition = Use(ConditionFactory.batch, size=2)
+class ConditionFactory(DataclassFactory[eligibility.Condition]):
+    actions = Use(SuggestedActionFactory.batch, size=2)
 
 
-class CohortResultFactory(DataclassFactory[CohortGroupResult]): ...
+class EligibilityStatusFactory(DataclassFactory[eligibility.EligibilityStatus]):
+    conditions = Use(ConditionFactory.batch, size=2)
+
+
+class CohortResultFactory(DataclassFactory[eligibility.CohortGroupResult]): ...
 
 
 def random_str(length: int) -> str:

tests/fixtures/matchers/eligibility.py

@@ -1,7 +1,7 @@
 from hamcrest.core.matcher import Matcher
 
 from eligibility_signposting_api.model.eligibility import CohortGroupResult, Condition, EligibilityStatus, Reason
-from eligibility_signposting_api.views.response_model.eligibility import EligibilityCohort, SuitabilityRule
+from eligibility_signposting_api.views.response_model.eligibility import Action, EligibilityCohort, SuitabilityRule
 
 from .meta import BaseAutoMatcher
 
@@ -24,6 +24,9 @@ class EligibilityCohortMatcher(BaseAutoMatcher[EligibilityCohort]): ...
 class SuitabilityRuleMatcher(BaseAutoMatcher[SuitabilityRule]): ...
 
 
+class ActionMatcher(BaseAutoMatcher[Action]): ...
+
+
 def is_eligibility_status() -> Matcher[EligibilityStatus]:
     return EligibilityStatusMatcher()
 
@@ -46,3 +49,7 @@ def is_eligibility_cohort() -> Matcher[EligibilityCohort]:
 
 def is_suitability_rule() -> Matcher[SuitabilityRule]:
     return SuitabilityRuleMatcher()
+
+
+def is_action() -> Matcher[Action]:
+    return ActionMatcher()

tests/integration/in_process/test_eligibility_endpoint.py

@@ -218,7 +218,7 @@ def test_actionable(
                                             "cohortText": "positive_description",
                                         }
                                     ],
-                                    "actions": [{"action_type": "defaultcomms", "action_code": "action_code"}],
+                                    "actions": [{"actionType": "defaultcomms", "actionCode": "action_code"}],
                                     "suitabilityRules": [],
                                     "statusText": "Status.actionable",
                                 }
@@ -355,7 +355,7 @@ def test_actionable_when_only_magic_cohort_is_present(
                                             "cohortText": "magic positive description",
                                         }
                                     ],
-                                    "actions": [{"action_type": "defaultcomms", "action_code": "action_code"}],
+                                    "actions": [{"actionType": "defaultcomms", "actionCode": "action_code"}],
                                     "suitabilityRules": [],
                                     "statusText": "Status.actionable",
                                 }
@@ -511,7 +511,7 @@ def test_actionable(
                                     "condition": "FLU",
                                     "status": "Actionable",
                                     "eligibilityCohorts": [],
-                                    "actions": [{"action_code": "action_code", "action_type": "defaultcomms"}],
+                                    "actions": [{"actionCode": "action_code", "actionType": "defaultcomms"}],
                                     "suitabilityRules": [],
                                     "statusText": "Status.actionable",
                                 }

tests/integration/lambda/test_app_running_as_lambda.py

@@ -153,7 +153,7 @@ def get_log_messages(flask_function: str, logs_client: BaseClient) -> list[str]:
     return [e["message"] for e in log_events["events"]]
 
 
-def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers(  # noqa: PLR0913
+def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers_and_check_if_audited(  # noqa: PLR0913
     lambda_client: BaseClient,  # noqa:ARG001
     persisted_person: NHSNumber,
     campaign_config: CampaignConfig,  # noqa:ARG001
@@ -176,6 +176,7 @@ def test_given_nhs_number_in_path_matches_with_nhs_number_in_headers(  # noqa: P
         is_response().with_status_code(HTTPStatus.OK).and_body(is_json_that(has_key("processedSuggestions"))),
     )
 
+    # Then - check if audited
     objects = s3_client.list_objects_v2(Bucket=audit_bucket).get("Contents", [])
     object_keys = [obj["Key"] for obj in objects]
     latest_key = sorted(object_keys)[-1]