From 4e1ead423c3af3f0e556d3c8d7699ea678210e7f Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Mon, 29 Dec 2025 14:10:57 +0000 Subject: [PATCH 01/14] eli-579 adding a base class for derived field calculations, with aim of making it compatible with the token parsing logic --- .../processors/derived_values/base.py | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 src/eligibility_signposting_api/services/processors/derived_values/base.py diff --git a/src/eligibility_signposting_api/services/processors/derived_values/base.py b/src/eligibility_signposting_api/services/processors/derived_values/base.py new file mode 100644 index 00000000..8a5f0803 --- /dev/null +++ b/src/eligibility_signposting_api/services/processors/derived_values/base.py @@ -0,0 +1,66 @@ +from abc import ABC, abstractmethod +from dataclasses import dataclass +from typing import Any + + +@dataclass +class DerivedValueContext: + """Context object containing all data needed for derived value calculation. + + Attributes: + person_data: List of person attribute dictionaries + attribute_name: The condition/vaccine type (e.g., 'COVID', 'RSV') + source_attribute: The source attribute to derive from (e.g., 'LAST_SUCCESSFUL_DATE') + function_args: Arguments passed to the function (e.g., number of days) + date_format: Optional date format string for output formatting + """ + + person_data: list[dict[str, Any]] + attribute_name: str + source_attribute: str | None + function_args: str | None + date_format: str | None + + +class DerivedValueHandler(ABC): + """Abstract base class for derived value handlers. + + Derived value handlers compute values that don't exist directly in the data + but are calculated from existing attributes. Each handler is responsible for + a specific type of calculation (e.g., adding days to a date). + + To create a new derived value handler: + 1. Subclass DerivedValueHandler + 2. Set the `function_name` class attribute to the token function name (e.g., 'ADD_DAYS') + 3. Implement the `calculate` method + 4. Register the handler with the DerivedValueRegistry + """ + + function_name: str = "" + + @abstractmethod + def calculate(self, context: DerivedValueContext) -> str: + """Calculate the derived value. + + Args: + context: DerivedValueContext containing all necessary data + + Returns: + The calculated value as a string + + Raises: + ValueError: If the calculation cannot be performed + """ + + @abstractmethod + def get_source_attribute(self, target_attribute: str) -> str: + """Get the source attribute name needed for this derived value. + + For example, NEXT_DOSE_DUE derives from LAST_SUCCESSFUL_DATE. + + Args: + target_attribute: The target derived attribute name (e.g., 'NEXT_DOSE_DUE') + + Returns: + The source attribute name to use for calculation + """ From 9b4f10d52aeb6e1ad93beb5216c2697be855bc13 Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Mon, 29 Dec 2025 14:55:48 +0000 Subject: [PATCH 02/14] eli-579 adding an add_days derivation --- .../derived_values/add_days_handler.py | 167 +++++++++++ .../processors/derived_values/registry.py | 162 +++++++++++ .../processors/test_derived_values.py | 272 ++++++++++++++++++ 3 files changed, 601 insertions(+) create mode 100644 src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py create mode 100644 src/eligibility_signposting_api/services/processors/derived_values/registry.py create mode 100644 tests/unit/services/processors/test_derived_values.py diff --git a/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py b/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py new file mode 100644 index 00000000..5442016d --- /dev/null +++ b/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py @@ -0,0 +1,167 @@ +from datetime import UTC, datetime, timedelta +from typing import ClassVar + +from eligibility_signposting_api.services.processors.derived_values.base import ( + DerivedValueContext, + DerivedValueHandler, +) + + +class AddDaysHandler(DerivedValueHandler): + """Handler for adding days to a date value. + + This handler calculates derived dates by adding a configurable number of days + to a source date attribute. It supports: + - Default days value for all vaccine types + - Vaccine-specific days configuration + - Configurable mapping of derived attributes to source attributes + + Example token: [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]] + This would add 91 days to COVID's LAST_SUCCESSFUL_DATE to calculate NEXT_DOSE_DUE. + + The number of days can be specified in three ways (in order of precedence): + 1. In the token itself: :ADD_DAYS(91) + 2. In the vaccine_type_days configuration + 3. Using the default_days value + """ + + function_name: str = "ADD_DAYS" + + # Mapping of derived attribute names to their source attributes + DERIVED_ATTRIBUTE_SOURCES: ClassVar[dict[str, str]] = { + "NEXT_DOSE_DUE": "LAST_SUCCESSFUL_DATE", + } + + def __init__( + self, + default_days: int = 91, + vaccine_type_days: dict[str, int] | None = None, + ) -> None: + """Initialize the AddDaysHandler. + + Args: + default_days: Default number of days to add when not specified + in token or vaccine_type_days. Defaults to 91. + vaccine_type_days: Dictionary mapping vaccine types to their + specific days values. E.g., {"COVID": 91, "FLU": 365} + """ + self.default_days = default_days + self.vaccine_type_days = vaccine_type_days or {} + + def get_source_attribute(self, target_attribute: str) -> str: + """Get the source attribute for a derived attribute. + + Args: + target_attribute: The derived attribute name (e.g., 'NEXT_DOSE_DUE') + + Returns: + The source attribute name (e.g., 'LAST_SUCCESSFUL_DATE') + """ + return self.DERIVED_ATTRIBUTE_SOURCES.get(target_attribute, target_attribute) + + def calculate(self, context: DerivedValueContext) -> str: + """Calculate a date with added days. + + Args: + context: DerivedValueContext containing: + - person_data: List of attribute dictionaries + - attribute_name: Vaccine type (e.g., 'COVID') + - source_attribute: The source date attribute + - function_args: Optional days override from token + - date_format: Optional output date format + + Returns: + The calculated date as a formatted string + + Raises: + ValueError: If source date is not found or invalid + """ + source_date = self._find_source_date(context) + if not source_date: + return "" + + days_to_add = self._get_days_to_add(context) + calculated_date = self._add_days_to_date(source_date, days_to_add) + + return self._format_date(calculated_date, context.date_format) + + def _find_source_date(self, context: DerivedValueContext) -> str | None: + """Find the source date value from person data. + + Args: + context: The derived value context + + Returns: + The source date string or None if not found + """ + source_attr = context.source_attribute + if not source_attr: + return None + + for attribute in context.person_data: + if attribute.get("ATTRIBUTE_TYPE") == context.attribute_name: + return attribute.get(source_attr) + + return None + + def _get_days_to_add(self, context: DerivedValueContext) -> int: + """Determine the number of days to add. + + Priority: + 1. Function argument from token (e.g., :ADD_DAYS(91)) + 2. Vaccine-specific configuration + 3. Default days + + Args: + context: The derived value context + + Returns: + Number of days to add + """ + # Priority 1: Token argument + if context.function_args: + try: + return int(context.function_args) + except ValueError: + pass + + # Priority 2: Vaccine-specific configuration + if context.attribute_name in self.vaccine_type_days: + return self.vaccine_type_days[context.attribute_name] + + # Priority 3: Default + return self.default_days + + def _add_days_to_date(self, date_str: str, days: int) -> datetime: + """Parse a date string and add days. + + Args: + date_str: Date in YYYYMMDD format + days: Number of days to add + + Returns: + The calculated datetime + + Raises: + ValueError: If date format is invalid + """ + try: + date_obj = datetime.strptime(date_str, "%Y%m%d").replace(tzinfo=UTC) + return date_obj + timedelta(days=days) + except ValueError as e: + message = f"Invalid date format: {date_str}" + raise ValueError(message) from e + + def _format_date(self, date_obj: datetime, date_format: str | None) -> str: + """Format a datetime object. + + Args: + date_obj: The datetime to format + date_format: Optional strftime format string + + Returns: + Formatted date string. If no format specified, returns YYYYMMDD. + """ + if date_format: + return date_obj.strftime(date_format) + return date_obj.strftime("%Y%m%d") diff --git a/src/eligibility_signposting_api/services/processors/derived_values/registry.py b/src/eligibility_signposting_api/services/processors/derived_values/registry.py new file mode 100644 index 00000000..ae9b2b8d --- /dev/null +++ b/src/eligibility_signposting_api/services/processors/derived_values/registry.py @@ -0,0 +1,162 @@ +from typing import ClassVar + +from eligibility_signposting_api.services.processors.derived_values.base import ( + DerivedValueContext, + DerivedValueHandler, +) + + +class DerivedValueRegistry: + """Registry for derived value handlers. + + This class manages the registration and lookup of derived value handlers. + It provides a centralized way to: + - Register new derived value handlers + - Look up handlers by function name + - Check if an attribute is a derived value + + Example usage: + registry = DerivedValueRegistry() + registry.register(AddDaysHandler(default_days=91)) + + # Check if a token uses a derived value + if registry.has_handler("ADD_DAYS"): + handler = registry.get_handler("ADD_DAYS") + result = handler.calculate(context) + """ + + # Class-level default handlers - these can be configured at startup + _default_handlers: ClassVar[dict[str, DerivedValueHandler]] = {} + + def __init__(self) -> None: + """Initialize the registry with default handlers.""" + self._handlers: dict[str, DerivedValueHandler] = {} + # Copy default handlers to instance + for name, handler in self._default_handlers.items(): + self._handlers[name] = handler + + @classmethod + def register_default(cls, handler: DerivedValueHandler) -> None: + """Register a handler as a default for all registry instances. + + This is useful for configuring handlers at application startup. + + Args: + handler: The derived value handler to register + """ + cls._default_handlers[handler.function_name] = handler + + @classmethod + def clear_defaults(cls) -> None: + """Clear all default handlers. Useful for testing.""" + cls._default_handlers.clear() + + @classmethod + def get_default_handlers(cls) -> dict[str, DerivedValueHandler]: + """Get a copy of the default handlers. Useful for testing.""" + return cls._default_handlers.copy() + + @classmethod + def set_default_handlers(cls, handlers: dict[str, DerivedValueHandler]) -> None: + """Set the default handlers. Useful for testing.""" + cls._default_handlers = handlers + + def register(self, handler: DerivedValueHandler) -> None: + """Register a derived value handler. + + Args: + handler: The handler to register. Its function_name attribute + will be used as the lookup key. + """ + self._handlers[handler.function_name] = handler + + def get_handler(self, function_name: str) -> DerivedValueHandler | None: + """Get a handler by function name. + + Args: + function_name: The function name (e.g., 'ADD_DAYS') + + Returns: + The handler or None if not found + """ + return self._handlers.get(function_name.upper()) + + def has_handler(self, function_name: str) -> bool: + """Check if a handler exists for a function name. + + Args: + function_name: The function name to check + + Returns: + True if a handler is registered + """ + return function_name.upper() in self._handlers + + def is_derived_attribute(self, attribute_value: str) -> bool: + """Check if an attribute value represents a derived attribute. + + This checks across all registered handlers. + + Args: + attribute_value: The attribute to check (e.g., 'NEXT_DOSE_DUE') + + Returns: + True if any handler can derive this attribute + """ + for handler in self._handlers.values(): + source = handler.get_source_attribute(attribute_value) + if source != attribute_value: + return True + return False + + def get_source_attribute(self, function_name: str, target_attribute: str) -> str: + """Get the source attribute for a derived attribute. + + Args: + function_name: The function name of the handler + target_attribute: The target derived attribute + + Returns: + The source attribute name, or the target if no handler found + """ + handler = self.get_handler(function_name) + if handler: + return handler.get_source_attribute(target_attribute) + return target_attribute + + def calculate( + self, + function_name: str, + context: DerivedValueContext, + ) -> str: + """Calculate a derived value. + + Args: + function_name: The function name (e.g., 'ADD_DAYS') + context: The context containing all data needed for calculation + + Returns: + The calculated value as a string + + Raises: + ValueError: If no handler found for the function name + """ + handler = self.get_handler(function_name) + if not handler: + message = f"No handler registered for function: {function_name}" + raise ValueError(message) + + return handler.calculate(context) + + +# Create a singleton instance for convenience +_registry = DerivedValueRegistry() + + +def get_registry() -> DerivedValueRegistry: + """Get the global derived value registry. + + Returns: + The singleton DerivedValueRegistry instance + """ + return _registry diff --git a/tests/unit/services/processors/test_derived_values.py b/tests/unit/services/processors/test_derived_values.py new file mode 100644 index 00000000..3b59da86 --- /dev/null +++ b/tests/unit/services/processors/test_derived_values.py @@ -0,0 +1,272 @@ +from unittest.mock import MagicMock + +import pytest + +from eligibility_signposting_api.services.processors.derived_values import ( + AddDaysHandler, + DerivedValueContext, + DerivedValueRegistry, +) + + +class TestAddDaysHandler: + """Tests for the AddDaysHandler class.""" + + def test_calculate_adds_default_days_to_date(self): + """Test that calculate adds the default days to a date.""" + handler = AddDaysHandler(default_days=91) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}], + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args=None, + date_format=None, + ) + + result = handler.calculate(context) + + # 2025-01-01 + 91 days = 2025-04-02 + assert result == "20250402" + + def test_calculate_with_function_args_override(self): + """Test that function args override default days.""" + handler = AddDaysHandler(default_days=91) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}], + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args="30", # Override with 30 days + date_format=None, + ) + + result = handler.calculate(context) + + # 2025-01-01 + 30 days = 2025-01-31 + assert result == "20250131" + + def test_calculate_with_vaccine_specific_days(self): + """Test that vaccine-specific days are used when configured.""" + handler = AddDaysHandler( + default_days=91, + vaccine_type_days={"FLU": 365}, + ) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "FLU", "LAST_SUCCESSFUL_DATE": "20250101"}], + attribute_name="FLU", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args=None, + date_format=None, + ) + + result = handler.calculate(context) + + # 2025-01-01 + 365 days = 2026-01-01 + assert result == "20260101" + + def test_calculate_with_date_format(self): + """Test that date format is applied to output.""" + handler = AddDaysHandler(default_days=91) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}], + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args=None, + date_format="%d %B %Y", + ) + + result = handler.calculate(context) + + assert result == "02 April 2025" + + def test_calculate_returns_empty_when_source_not_found(self): + """Test that empty string is returned when source date not found.""" + handler = AddDaysHandler(default_days=91) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "COVID"}], # No LAST_SUCCESSFUL_DATE + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args=None, + date_format=None, + ) + + result = handler.calculate(context) + + assert result == "" + + def test_calculate_returns_empty_when_vaccine_not_found(self): + """Test that empty string is returned when vaccine type not found.""" + handler = AddDaysHandler(default_days=91) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "FLU", "LAST_SUCCESSFUL_DATE": "20250101"}], + attribute_name="COVID", # Looking for COVID but data has FLU + source_attribute="LAST_SUCCESSFUL_DATE", + function_args=None, + date_format=None, + ) + + result = handler.calculate(context) + + assert result == "" + + def test_calculate_with_invalid_date_raises_error(self): + """Test that invalid date format raises ValueError.""" + handler = AddDaysHandler(default_days=91) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "invalid"}], + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args=None, + date_format=None, + ) + + with pytest.raises(ValueError, match="Invalid date format"): + handler.calculate(context) + + def test_get_source_attribute_maps_derived_to_source(self): + """Test that get_source_attribute maps derived attributes correctly.""" + handler = AddDaysHandler() + + assert handler.get_source_attribute("NEXT_DOSE_DUE") == "LAST_SUCCESSFUL_DATE" + + def test_get_source_attribute_returns_original_if_not_mapped(self): + """Test that unmapped attributes return themselves.""" + handler = AddDaysHandler() + + assert handler.get_source_attribute("UNKNOWN_ATTR") == "UNKNOWN_ATTR" + + def test_function_args_priority_over_vaccine_config(self): + """Test that function args take priority over vaccine-specific config.""" + handler = AddDaysHandler( + default_days=91, + vaccine_type_days={"COVID": 120}, + ) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}], + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args="30", # Should take priority over 120 + date_format=None, + ) + + result = handler.calculate(context) + + # 2025-01-01 + 30 days = 2025-01-31 + assert result == "20250131" + + +class TestDerivedValueRegistry: + """Tests for the DerivedValueRegistry class.""" + + def test_register_and_get_handler(self): + """Test registering and retrieving a handler.""" + registry = DerivedValueRegistry() + handler = AddDaysHandler() + registry.register(handler) + + retrieved = registry.get_handler("ADD_DAYS") + + assert retrieved is handler + + def test_get_handler_case_insensitive(self): + """Test that handler lookup is case insensitive.""" + registry = DerivedValueRegistry() + handler = AddDaysHandler() + registry.register(handler) + + assert registry.get_handler("add_days") is handler + assert registry.get_handler("Add_Days") is handler + + def test_has_handler_returns_true_when_exists(self): + """Test has_handler returns True for registered handlers.""" + registry = DerivedValueRegistry() + registry.register(AddDaysHandler()) + + assert registry.has_handler("ADD_DAYS") is True + + def test_has_handler_returns_false_when_not_exists(self): + """Test has_handler returns False for unregistered handlers.""" + registry = DerivedValueRegistry() + + assert registry.has_handler("UNKNOWN") is False + + def test_calculate_delegates_to_correct_handler(self): + """Test that calculate delegates to the correct handler.""" + registry = DerivedValueRegistry() + + # Create a mock handler + mock_handler = MagicMock() + mock_handler.function_name = "TEST_FUNC" + mock_handler.calculate.return_value = "mock_result" + + # Register both real and mock handlers + registry.register(AddDaysHandler(default_days=91)) + registry.register(mock_handler) + + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}], + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args=None, + date_format=None, + ) + + # Call with the mock handler's function name + result = registry.calculate(function_name="TEST_FUNC", context=context) + + # Verify the mock handler was called with the context + mock_handler.calculate.assert_called_once_with(context) + assert result == "mock_result" + + def test_calculate_raises_for_unknown_function(self): + """Test that calculate raises ValueError for unknown functions.""" + registry = DerivedValueRegistry() + + context = DerivedValueContext( + person_data=[], + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args=None, + date_format=None, + ) + + with pytest.raises(ValueError, match="No handler registered"): + registry.calculate( + function_name="UNKNOWN", + context=context, + ) + + def test_is_derived_attribute_returns_true_for_derived(self): + """Test is_derived_attribute for known derived attributes.""" + registry = DerivedValueRegistry() + registry.register(AddDaysHandler()) + + assert registry.is_derived_attribute("NEXT_DOSE_DUE") is True + + def test_is_derived_attribute_returns_false_for_non_derived(self): + """Test is_derived_attribute for non-derived attributes.""" + registry = DerivedValueRegistry() + registry.register(AddDaysHandler()) + + assert registry.is_derived_attribute("LAST_SUCCESSFUL_DATE") is False + + def test_default_handlers_are_registered(self): + """Test that default handlers from the module are registered.""" + registry = DerivedValueRegistry() + + # The default ADD_DAYS handler should be registered via __init__.py + assert registry.has_handler("ADD_DAYS") + + def test_clear_defaults_removes_default_handlers(self): + """Test that clear_defaults removes all default handlers.""" + # Save current defaults using public method + saved_defaults = DerivedValueRegistry.get_default_handlers() + + try: + DerivedValueRegistry.clear_defaults() + + # New registry should have no handlers + registry = DerivedValueRegistry() + assert not registry.has_handler("ADD_DAYS") + finally: + # Restore defaults for other tests using public method + DerivedValueRegistry.set_default_handlers(saved_defaults) From c0230702400a9246b753c739554925de17a911a9 Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Mon, 29 Dec 2025 15:02:58 +0000 Subject: [PATCH 03/14] eli-579 amending token parser to parse out function details --- .../services/processors/token_parser.py | 79 +++++++++++++-- .../services/processors/test_token_parser.py | 8 +- .../processors/test_token_parser_functions.py | 96 +++++++++++++++++++ 3 files changed, 173 insertions(+), 10 deletions(-) create mode 100644 tests/unit/services/processors/test_token_parser_functions.py diff --git a/src/eligibility_signposting_api/services/processors/token_parser.py b/src/eligibility_signposting_api/services/processors/token_parser.py index f593e028..f11cb334 100644 --- a/src/eligibility_signposting_api/services/processors/token_parser.py +++ b/src/eligibility_signposting_api/services/processors/token_parser.py @@ -13,20 +13,30 @@ class ParsedToken: Example: "PERSON" or "TARGET" attribute_name : str Example: "POSTCODE" or "RSV" - attribute_value : int + attribute_value : str | None Example: "LAST_SUCCESSFUL_DATE" if attribute_level is TARGET - format : str + format : str | None Example: "%d %B %Y" if DATE formatting is used + function_name : str | None + Example: "ADD_DAYS" for derived value functions + function_args : str | None + Example: "91" for ADD_DAYS(91) """ attribute_level: str attribute_name: str attribute_value: str | None format: str | None + function_name: str | None = None + function_args: str | None = None class TokenParser: MIN_TOKEN_PARTS = 2 + # Pattern for function calls like ADD_DAYS(91) - captures function name and args + FUNCTION_PATTERN = re.compile(r":([A-Z_]+)\(([^()]*)\)", re.IGNORECASE) + # Pattern for DATE format - special case as it's already supported + DATE_PATTERN = re.compile(r":DATE\(([^()]*)\)", re.IGNORECASE) @staticmethod def parse(token: str) -> ParsedToken: @@ -35,8 +45,15 @@ def parse(token: str) -> ParsedToken: Strip the surrounding [[ ]] Check for empty body after stripping, e.g., '[[]]' Check for empty parts created by leading/trailing dots or tokens with no dot - Check if the name contains a date format + Check if the name contains a date format or function call Return a ParsedToken object + + Supported formats: + - [[PERSON.AGE]] - Simple person attribute + - [[TARGET.COVID.LAST_SUCCESSFUL_DATE]] - Target attribute + - [[PERSON.DATE_OF_BIRTH:DATE(%d %B %Y)]] - With date formatting + - [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]] - Derived value function + - [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91):DATE(%d %B %Y)]] - Function with date format """ token_body = token[2:-2] @@ -53,14 +70,20 @@ def parse(token: str) -> ParsedToken: token_level = token_parts[0].upper() token_name = token_parts[-1] - format_match = re.search(r":DATE\(([^()]*)\)", token_name, re.IGNORECASE) - if not format_match and len(token_name.split(":")) > 1: - message = "Invalid token format." - raise ValueError(message) + # Extract function call (e.g., ADD_DAYS(91)) + function_name, function_args = TokenParser._extract_function(token_name) + # Extract date format + format_match = TokenParser.DATE_PATTERN.search(token_name) format_str = format_match.group(1) if format_match else None - last_part = re.sub(r":DATE\([^)]*\)", "", token_name, flags=re.IGNORECASE) + # Validate format - if there's a colon but no valid pattern, it's invalid + if not format_match and not function_name and len(token_name.split(":")) > 1: + message = "Invalid token format." + raise ValueError(message) + + # Remove function and date patterns to get the clean attribute name + last_part = TokenParser._clean_attribute_name(token_name) if len(token_parts) == TokenParser.MIN_TOKEN_PARTS: name = last_part.upper() @@ -69,4 +92,42 @@ def parse(token: str) -> ParsedToken: name = token_parts[1].upper() value = last_part.upper() - return ParsedToken(attribute_level=token_level, attribute_name=name, attribute_value=value, format=format_str) + return ParsedToken( + attribute_level=token_level, + attribute_name=name, + attribute_value=value, + format=format_str, + function_name=function_name, + function_args=function_args, + ) + + @staticmethod + def _extract_function(token_name: str) -> tuple[str | None, str | None]: + """Extract function name and arguments from token name. + + Args: + token_name: The last part of the token (e.g., 'NEXT_DOSE_DUE:ADD_DAYS(91)') + + Returns: + Tuple of (function_name, function_args) or (None, None) if no function + """ + # Find all function matches (excluding DATE which is handled separately) + for match in TokenParser.FUNCTION_PATTERN.finditer(token_name): + func_name = match.group(1).upper() + if func_name != "DATE": + return func_name, match.group(2) + return None, None + + @staticmethod + def _clean_attribute_name(token_name: str) -> str: + """Remove function calls and date formatting from token name. + + Args: + token_name: The raw token name with potential modifiers + + Returns: + Clean attribute name + """ + # Remove date format and other function calls + without_date = TokenParser.DATE_PATTERN.sub("", token_name) + return TokenParser.FUNCTION_PATTERN.sub("", without_date) diff --git a/tests/unit/services/processors/test_token_parser.py b/tests/unit/services/processors/test_token_parser.py index d81465b3..f144f440 100644 --- a/tests/unit/services/processors/test_token_parser.py +++ b/tests/unit/services/processors/test_token_parser.py @@ -47,7 +47,6 @@ def test_parse_invalid_tokens_raises_error(self, token): "[[PERSON.DATE_OF_BIRTH:DATE(]]", "[[PERSON.DATE_OF_BIRTH:DATE)]]", "[[PERSON.DATE_OF_BIRTH:DATE]]", - "[[PERSON.DATE_OF_BIRTH:INVALID_FORMAT(abc)]]", "[[PERSON.DATE_OF_BIRTH:INVALID_FORMAT(a (b) c)]]", "[[PERSON.DATE_OF_BIRTH:DATE(a (b) c)]]", ], @@ -55,3 +54,10 @@ def test_parse_invalid_tokens_raises_error(self, token): def test_parse_invalid_token_format_raises_error(self, token): with pytest.raises(ValueError, match="Invalid token format."): TokenParser.parse(token) + + def test_parse_function_token_valid(self): + """Test that valid function tokens are parsed correctly.""" + # This used to be invalid, but now we support custom functions + parsed = TokenParser.parse("[[PERSON.DATE_OF_BIRTH:SOME_FUNC(abc)]]") + assert parsed.function_name == "SOME_FUNC" + assert parsed.function_args == "abc" diff --git a/tests/unit/services/processors/test_token_parser_functions.py b/tests/unit/services/processors/test_token_parser_functions.py new file mode 100644 index 00000000..cad0535c --- /dev/null +++ b/tests/unit/services/processors/test_token_parser_functions.py @@ -0,0 +1,96 @@ +"""Tests for TokenParser with derived value function support.""" + +from dataclasses import dataclass + +import pytest + +from eligibility_signposting_api.services.processors.token_parser import TokenParser + + +@dataclass +class ExpectedTokenResult: + """Expected result for a parsed token.""" + + level: str + name: str + value: str | None + function: str | None + args: str | None + date_format: str | None + + +class TestTokenParserWithFunctions: + """Tests for parsing tokens with function calls like ADD_DAYS.""" + + @pytest.mark.parametrize( + ("token", "expected"), + [ + # Basic ADD_DAYS function + ( + "[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]]", + ExpectedTokenResult("TARGET", "COVID", "NEXT_DOSE_DUE", "ADD_DAYS", "91", None), + ), + # ADD_DAYS with date format + ( + "[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91):DATE(%d %B %Y)]]", + ExpectedTokenResult("TARGET", "COVID", "NEXT_DOSE_DUE", "ADD_DAYS", "91", "%d %B %Y"), + ), + # Different vaccine type + ( + "[[TARGET.RSV.NEXT_DOSE_DUE:ADD_DAYS(365)]]", + ExpectedTokenResult("TARGET", "RSV", "NEXT_DOSE_DUE", "ADD_DAYS", "365", None), + ), + # Case insensitive function name + ( + "[[TARGET.COVID.NEXT_DOSE_DUE:add_days(91)]]", + ExpectedTokenResult("TARGET", "COVID", "NEXT_DOSE_DUE", "ADD_DAYS", "91", None), + ), + # Empty args (use default) + ( + "[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS()]]", + ExpectedTokenResult("TARGET", "COVID", "NEXT_DOSE_DUE", "ADD_DAYS", "", None), + ), + # Person level with function (hypothetical future use) + ( + "[[PERSON.SOME_DATE:ADD_DAYS(30)]]", + ExpectedTokenResult("PERSON", "SOME_DATE", None, "ADD_DAYS", "30", None), + ), + ], + ) + def test_parse_tokens_with_functions(self, token: str, expected: ExpectedTokenResult): + """Test parsing tokens with function calls.""" + parsed_token = TokenParser.parse(token) + + assert parsed_token.attribute_level == expected.level + assert parsed_token.attribute_name == expected.name + assert parsed_token.attribute_value == expected.value + assert parsed_token.function_name == expected.function + assert parsed_token.function_args == expected.args + assert parsed_token.format == expected.date_format + + def test_parse_without_function_has_none_function_fields(self): + """Test that tokens without functions have None for function fields.""" + parsed = TokenParser.parse("[[TARGET.COVID.LAST_SUCCESSFUL_DATE]]") + + assert parsed.function_name is None + assert parsed.function_args is None + + def test_parse_date_format_not_treated_as_function(self): + """Test that DATE format is not treated as a derived function.""" + parsed = TokenParser.parse("[[PERSON.DATE_OF_BIRTH:DATE(%d %B %Y)]]") + + assert parsed.function_name is None + assert parsed.format == "%d %B %Y" + + @pytest.mark.parametrize( + "token", + [ + "[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS]]", # Missing parentheses + "[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(]]", # Unclosed parenthesis + "[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS)]]", # No opening parenthesis + ], + ) + def test_parse_invalid_function_format_raises_error(self, token): + """Test that malformed function calls raise errors.""" + with pytest.raises(ValueError, match="Invalid token format"): + TokenParser.parse(token) From 28d2e75f40a93293afe4189a074d673ea0c399ab Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Mon, 29 Dec 2025 15:05:26 +0000 Subject: [PATCH 04/14] eli-579 amending token processor to use new token parser --- .../services/processors/token_processor.py | 80 ++++++ .../processors/test_token_processor.py | 29 +- .../test_token_processor_derived.py | 261 ++++++++++++++++++ 3 files changed, 366 insertions(+), 4 deletions(-) create mode 100644 tests/unit/services/processors/test_token_processor_derived.py diff --git a/src/eligibility_signposting_api/services/processors/token_processor.py b/src/eligibility_signposting_api/services/processors/token_processor.py index 4a54f76e..47c34d28 100644 --- a/src/eligibility_signposting_api/services/processors/token_processor.py +++ b/src/eligibility_signposting_api/services/processors/token_processor.py @@ -7,6 +7,10 @@ from eligibility_signposting_api.config.constants import ALLOWED_CONDITIONS from eligibility_signposting_api.model.person import Person +from eligibility_signposting_api.services.processors.derived_values import ( + DerivedValueContext, + DerivedValueRegistry, +) from eligibility_signposting_api.services.processors.token_parser import ParsedToken, TokenParser TARGET_ATTRIBUTE_LEVEL = "TARGET" @@ -21,6 +25,7 @@ "BOOKED_APPOINTMENT_PROVIDER", "LAST_INVITE_DATE", "LAST_INVITE_STATUS", + "NEXT_DOSE_DUE", } @@ -85,13 +90,88 @@ def get_token_replacement(token: str, person_data: list[dict], present_attribute if TokenProcessor.should_replace_with_empty(parsed_token, present_attributes): return "" + # Check if this is a derived value (has a function like ADD_DAYS) + if parsed_token.function_name: + return TokenProcessor.get_derived_value(parsed_token, person_data, present_attributes, token) + found_attribute, key_to_replace = TokenProcessor.find_matching_attribute(parsed_token, person_data) if not found_attribute or not key_to_replace: TokenProcessor.handle_token_not_found(parsed_token, token) + # handle_token_not_found always raises, but the type checker needs help + msg = "Unreachable" + raise RuntimeError(msg) # pragma: no cover return TokenProcessor.apply_formatting(found_attribute, key_to_replace, parsed_token.format) + @staticmethod + def get_derived_value( + parsed_token: ParsedToken, + person_data: list[dict], + present_attributes: set, + token: str, + ) -> str: + """Calculate a derived value using the registered handler. + + Args: + parsed_token: The parsed token containing function information + person_data: List of person attribute dictionaries + present_attributes: Set of attribute types present in person data + token: The original token string for error messages + + Returns: + The calculated derived value as a string + + Raises: + ValueError: If no handler is registered or attribute not found + """ + registry = DerivedValueRegistry() + + function_name = parsed_token.function_name + if not function_name: + message = f"No function specified in token '{token}'." + raise ValueError(message) + + if not registry.has_handler(function_name): + message = f"Unknown function '{function_name}' in token '{token}'." + raise ValueError(message) + + # For TARGET level tokens, validate the condition is allowed + if parsed_token.attribute_level == TARGET_ATTRIBUTE_LEVEL: + is_allowed_condition = parsed_token.attribute_name in ALLOWED_CONDITIONS.__args__ + is_allowed_target_attr = parsed_token.attribute_value in ALLOWED_TARGET_ATTRIBUTES + + # If condition is not allowed, raise error + if not is_allowed_condition: + TokenProcessor.handle_token_not_found(parsed_token, token) + + # If vaccine type is not in person data but is allowed, return empty + if parsed_token.attribute_name not in present_attributes: + if is_allowed_target_attr: + return "" + TokenProcessor.handle_token_not_found(parsed_token, token) + + try: + target_attribute = parsed_token.attribute_value or parsed_token.attribute_name + source_attribute = registry.get_source_attribute(function_name, target_attribute) + + context = DerivedValueContext( + person_data=person_data, + attribute_name=parsed_token.attribute_name, + source_attribute=source_attribute, + function_args=parsed_token.function_args, + date_format=parsed_token.format, + ) + + return registry.calculate( + function_name=function_name, + context=context, + ) + except ValueError as e: + # Re-raise with more context + message = f"Error calculating derived value for token '{token}': {e}" + raise ValueError(message) from e + @staticmethod def should_replace_with_empty(parsed_token: ParsedToken, present_attributes: set) -> bool: is_target_level = parsed_token.attribute_level == TARGET_ATTRIBUTE_LEVEL diff --git a/tests/unit/services/processors/test_token_processor.py b/tests/unit/services/processors/test_token_processor.py index 1a6785bb..a4fe2fdf 100644 --- a/tests/unit/services/processors/test_token_processor.py +++ b/tests/unit/services/processors/test_token_processor.py @@ -330,11 +330,7 @@ def test_valid_token_valid_format_should_replace_with_date_formatting(self): @pytest.mark.parametrize( "token_format", [ - ":INVALID_DATE_FORMATTER(%ABC)", - ":INVALID_DATE_FORMATTER(19900327)", ":()", - ":FORMAT(DATE)", - ":FORMAT(BLAH)", ":DATE[%d %B %Y]", ":DATE(%A, (%d) %B %Y)", ], @@ -354,6 +350,31 @@ def test_valid_token_invalid_format_should_raise_error(self, token_format: str): with pytest.raises(ValueError, match="Invalid token format."): TokenProcessor.find_and_replace_tokens(person, condition) + @pytest.mark.parametrize( + ("token_format", "func_name"), + [ + (":INVALID_DATE_FORMATTER(%ABC)", "INVALID_DATE_FORMATTER"), + (":INVALID_DATE_FORMATTER(19900327)", "INVALID_DATE_FORMATTER"), + (":FORMAT(DATE)", "FORMAT"), + (":FORMAT(BLAH)", "FORMAT"), + ], + ) + def test_unknown_function_raises_error(self, token_format: str, func_name: str): + """Test that unknown function names raise ValueError with appropriate message.""" + person = Person([{"ATTRIBUTE_TYPE": "PERSON", "AGE": "30", "DATE_OF_BIRTH": "19900327"}]) + + condition = Condition( + condition_name=ConditionName("You had your RSV vaccine"), + status=Status.actionable, + status_text=StatusText(f"Your birthday is on [[PERSON.DATE_OF_BIRTH{token_format}]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + with pytest.raises(ValueError, match=f"Unknown function '{func_name}'"): + TokenProcessor.find_and_replace_tokens(person, condition) + @pytest.mark.parametrize( ("token_format", "expected"), [ diff --git a/tests/unit/services/processors/test_token_processor_derived.py b/tests/unit/services/processors/test_token_processor_derived.py new file mode 100644 index 00000000..0ce9ef03 --- /dev/null +++ b/tests/unit/services/processors/test_token_processor_derived.py @@ -0,0 +1,261 @@ +"""Tests for TokenProcessor with derived value support.""" + +import re + +import pytest + +from eligibility_signposting_api.model.eligibility_status import ( + Condition, + ConditionName, + Status, + StatusText, +) +from eligibility_signposting_api.model.person import Person +from eligibility_signposting_api.services.processors.token_processor import TokenProcessor + + +class TestTokenProcessorDerivedValues: + """Tests for TokenProcessor handling derived values.""" + + def test_next_dose_due_basic_replacement(self): + """Test basic NEXT_DOSE_DUE token replacement.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("COVID"), + status=Status.actionable, + status_text=StatusText("Next dose due: [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + # 2025-01-01 + 91 days = 2025-04-02 + assert result.status_text == "Next dose due: 20250402" + + def test_next_dose_due_with_date_format(self): + """Test NEXT_DOSE_DUE with date formatting.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("COVID"), + status=Status.actionable, + status_text=StatusText("You can book from [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91):DATE(%d %B %Y)]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + assert result.status_text == "You can book from 02 April 2025" + + def test_next_dose_due_different_days(self): + """Test NEXT_DOSE_DUE with different number of days.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "RSV", "LAST_SUCCESSFUL_DATE": "20250601"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("RSV"), + status=Status.actionable, + status_text=StatusText("Next dose: [[TARGET.RSV.NEXT_DOSE_DUE:ADD_DAYS(365):DATE(%d/%m/%Y)]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + # 2025-06-01 + 365 days = 2026-06-01 + assert result.status_text == "Next dose: 01/06/2026" + + def test_missing_vaccine_data_returns_empty(self): + """Test that missing vaccine data returns empty string for derived values.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "PERSON", "AGE": "30"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("Next COVID dose: [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]]"), + status=Status.actionable, + status_text=StatusText("status"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + assert result.condition_name == "Next COVID dose: " + + def test_missing_last_successful_date_returns_empty(self): + """Test that missing source date returns empty string.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "COVID"}, # No LAST_SUCCESSFUL_DATE + ] + ) + + condition = Condition( + condition_name=ConditionName("COVID"), + status=Status.actionable, + status_text=StatusText("Next dose: [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + assert result.status_text == "Next dose: " + + def test_mixed_regular_and_derived_tokens(self): + """Test mixing regular tokens with derived value tokens.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "PERSON", "AGE": "65"}, + {"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("COVID"), + status=Status.actionable, + status_text=StatusText( + "At age [[PERSON.AGE]], your next dose is from " + "[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91):DATE(%d %B %Y)]]" + ), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + assert result.status_text == "At age 65, your next dose is from 02 April 2025" + + def test_unknown_function_raises_error(self): + """Test that unknown function name raises ValueError.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("COVID"), + status=Status.actionable, + status_text=StatusText("[[TARGET.COVID.SOMETHING:UNKNOWN_FUNC(123)]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + with pytest.raises(ValueError, match="Unknown function 'UNKNOWN_FUNC'"): + TokenProcessor.find_and_replace_tokens(person, condition) + + def test_multiple_derived_tokens(self): + """Test multiple derived value tokens in same text.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}, + {"ATTRIBUTE_TYPE": "FLU", "LAST_SUCCESSFUL_DATE": "20250601"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("Vaccines"), + status=Status.actionable, + status_text=StatusText( + "COVID: [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]], FLU: [[TARGET.FLU.NEXT_DOSE_DUE:ADD_DAYS(365)]]" + ), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + assert result.status_text == "COVID: 20250402, FLU: 20260601" + + def test_derived_value_uses_default_days_without_args(self): + """Test that empty function args uses default days from handler config.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("COVID"), + status=Status.actionable, + status_text=StatusText("Next dose: [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS()]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + # Should use the default 91 days configured in __init__.py + # 2025-01-01 + 91 days = 2025-04-02 + assert result.status_text == "Next dose: 20250402" + + def test_case_insensitive_function_name(self): + """Test that function names are case insensitive.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("COVID"), + status=Status.actionable, + status_text=StatusText("[[TARGET.COVID.NEXT_DOSE_DUE:add_days(91)]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + result = TokenProcessor.find_and_replace_tokens(person, condition) + + assert result.status_text == "20250402" + + def test_not_allowed_condition_with_derived_raises_error(self): + """Test that non-allowed conditions raise error for derived values.""" + person = Person( + [ + {"ATTRIBUTE_TYPE": "YELLOW_FEVER", "LAST_SUCCESSFUL_DATE": "20250101"}, + ] + ) + + condition = Condition( + condition_name=ConditionName("YELLOW_FEVER"), + status=Status.actionable, + status_text=StatusText("[[TARGET.YELLOW_FEVER.NEXT_DOSE_DUE:ADD_DAYS(91)]]"), + cohort_results=[], + suitability_rules=[], + actions=[], + ) + + expected_error = re.escape( + "Invalid attribute name 'NEXT_DOSE_DUE' in token '[[TARGET.YELLOW_FEVER.NEXT_DOSE_DUE:ADD_DAYS(91)]]'." + ) + with pytest.raises(ValueError, match=expected_error): + TokenProcessor.find_and_replace_tokens(person, condition) From 8f45ba6fe31f8651e0d911a1e97f9dbfd84cb696 Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Mon, 29 Dec 2025 15:06:35 +0000 Subject: [PATCH 05/14] eli-579 initialising the AddDaysHandler in the registry on app start --- .../processors/derived_values/__init__.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/eligibility_signposting_api/services/processors/derived_values/__init__.py diff --git a/src/eligibility_signposting_api/services/processors/derived_values/__init__.py b/src/eligibility_signposting_api/services/processors/derived_values/__init__.py new file mode 100644 index 00000000..8ab87caf --- /dev/null +++ b/src/eligibility_signposting_api/services/processors/derived_values/__init__.py @@ -0,0 +1,28 @@ +from eligibility_signposting_api.services.processors.derived_values.add_days_handler import AddDaysHandler +from eligibility_signposting_api.services.processors.derived_values.base import ( + DerivedValueContext, + DerivedValueHandler, +) +from eligibility_signposting_api.services.processors.derived_values.registry import ( + DerivedValueRegistry, + get_registry, +) + +__all__ = [ + "AddDaysHandler", + "DerivedValueContext", + "DerivedValueHandler", + "DerivedValueRegistry", + "get_registry", +] + +# Register default handlers +DerivedValueRegistry.register_default( + AddDaysHandler( + default_days=91, + vaccine_type_days={ + "COVID": 91, # 91 days between COVID vaccinations + # Add other vaccine-specific configurations here as needed. + }, + ) +) From 50668e8fdf14eb4533708ba43441beb8dc1648f2 Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Mon, 29 Dec 2025 15:07:22 +0000 Subject: [PATCH 06/14] eli-579 amending error message match now that the invalid date format syntax has changed. Going to loop back round and check if we need to specifically re-enable what it was actually testing --- tests/unit/services/calculators/test_eligibility_calculator.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/services/calculators/test_eligibility_calculator.py b/tests/unit/services/calculators/test_eligibility_calculator.py index c235aa48..2c993855 100644 --- a/tests/unit/services/calculators/test_eligibility_calculator.py +++ b/tests/unit/services/calculators/test_eligibility_calculator.py @@ -1436,7 +1436,8 @@ def test_eligibility_status_with_invalid_tokens_raises_attribute_error(faker: Fa calculator = EligibilityCalculator(person_rows, campaign_configs) - with pytest.raises(ValueError, match="Invalid token."): + # INVALID_DATE_FORMAT is now parsed as a function name and raises unknown function error + with pytest.raises(ValueError, match="Unknown function 'INVALID_DATE_FORMAT'"): calculator.get_eligibility_status("Y", ["ALL"], "ALL") From 8b8ed0fbfa90a602fdf9110904e456f4051e05eb Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Tue, 30 Dec 2025 10:48:51 +0000 Subject: [PATCH 07/14] eli-579 adding an integration tests to make sure add_days plus add_days with date formatting work --- tests/integration/conftest.py | 160 +++++++++++++++ .../in_process/test_derived_values.py | 185 ++++++++++++++++++ 2 files changed, 345 insertions(+) create mode 100644 tests/integration/in_process/test_derived_values.py diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 1edf3517..01f5f4b6 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -21,10 +21,16 @@ from eligibility_signposting_api.model.campaign_config import ( AvailableAction, CampaignConfig, + CommsRouting, EndDate, + RuleAttributeLevel, + RuleAttributeName, RuleCode, + RuleDescription, RuleEntry, RuleName, + RuleOperator, + RulePriority, RuleText, RuleType, StartDate, @@ -1119,6 +1125,160 @@ def get_secret_previous(self, secret_name: str) -> dict[str, str]: # noqa: ARG0 return {} +@pytest.fixture +def person_with_covid_vaccination( + person_table: Any, faker: Faker, hashing_service: HashingService +) -> Generator[eligibility_status.NHSNumber]: + """ + Creates a person with a COVID vaccination record. + LAST_SUCCESSFUL_DATE is set to 2026-01-28 (20260128). + Used for testing derived values like NEXT_DOSE_DUE with ADD_DAYS function. + """ + nhs_num = faker.nhs_number() + nhs_number = eligibility_status.NHSNumber(nhs_num) + nhs_num_hash = hashing_service.hash_with_current_secret(nhs_num) + + date_of_birth = eligibility_status.DateOfBirth(datetime.date(1960, 5, 15)) + + for row in ( + rows := person_rows_builder( + nhs_number=nhs_num_hash, + date_of_birth=date_of_birth, + postcode="SW1A", + cohorts=["covid_eligible"], + vaccines={"COVID": {"LAST_SUCCESSFUL_DATE": "20260128", "CONDITION_NAME": "COVID"}}, + ).data + ): + person_table.put_item(Item=row) + + yield nhs_number + + for row in rows: + person_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) + + +@pytest.fixture(scope="class") +def campaign_config_with_derived_values(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[CampaignConfig]: + """ + Creates a campaign config that uses the ADD_DAYS derived value function. + Contains actions for: + - DateOfLastVaccination: Shows the raw LAST_SUCCESSFUL_DATE + - DateOfNextEarliestVaccination: Shows NEXT_DOSE_DUE derived by adding 91 days + """ + campaign: CampaignConfig = rule.CampaignConfigFactory.build( + target="COVID", + iterations=[ + rule.IterationFactory.build( + actions_mapper=rule.ActionsMapperFactory.build( + root={ + "VACCINATION_DATES": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfLastVaccination", + ActionDescription="[[TARGET.COVID.LAST_SUCCESSFUL_DATE]]", + ), + "NEXT_DOSE_DATE": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfNextEarliestVaccination", + ActionDescription="[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]]", + ), + } + ), + iteration_cohorts=[ + rule.IterationCohortFactory.build( + cohort_label="covid_eligible", + cohort_group="covid_vaccination", + positive_description="Eligible for COVID vaccination", + negative_description="Not eligible for COVID vaccination", + ), + ], + iteration_rules=[ + rule.IterationRuleFactory.build( + type=RuleType.redirect, + name=RuleName("Provide vaccination dates"), + description=RuleDescription("Provide vaccination dates to patient"), + priority=RulePriority(10), + operator=RuleOperator.is_not_null, + attribute_level=RuleAttributeLevel.TARGET, + attribute_target="COVID", + attribute_name=RuleAttributeName("LAST_SUCCESSFUL_DATE"), + comms_routing=CommsRouting("VACCINATION_DATES|NEXT_DOSE_DATE"), + ), + ], + default_comms_routing="VACCINATION_DATES", + default_not_eligible_routing="VACCINATION_DATES", + default_not_actionable_routing="VACCINATION_DATES", + ) + ], + ) + campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} + s3_client.put_object( + Bucket=rules_bucket, Key=f"{campaign.name}.json", Body=json.dumps(campaign_data), ContentType="application/json" + ) + yield campaign + s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") + + +@pytest.fixture(scope="class") +def campaign_config_with_derived_values_formatted( + s3_client: BaseClient, rules_bucket: BucketName +) -> Generator[CampaignConfig]: + """ + Creates a campaign config that uses ADD_DAYS with DATE formatting. + The NEXT_DOSE_DUE is formatted as "29 April 2026" instead of raw "20260429". + """ + campaign: CampaignConfig = rule.CampaignConfigFactory.build( + target="COVID", + iterations=[ + rule.IterationFactory.build( + actions_mapper=rule.ActionsMapperFactory.build( + root={ + "VACCINATION_DATES": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfLastVaccination", + ActionDescription="[[TARGET.COVID.LAST_SUCCESSFUL_DATE:DATE(%d %B %Y)]]", + ), + "NEXT_DOSE_DATE": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfNextEarliestVaccination", + ActionDescription="[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91):DATE(%d %B %Y)]]", + ), + } + ), + iteration_cohorts=[ + rule.IterationCohortFactory.build( + cohort_label="covid_eligible", + cohort_group="covid_vaccination", + positive_description="Eligible for COVID vaccination", + negative_description="Not eligible for COVID vaccination", + ), + ], + iteration_rules=[ + rule.IterationRuleFactory.build( + type=RuleType.redirect, + name=RuleName("Provide vaccination dates"), + description=RuleDescription("Provide vaccination dates to patient"), + priority=RulePriority(10), + operator=RuleOperator.is_not_null, + attribute_level=RuleAttributeLevel.TARGET, + attribute_target="COVID", + attribute_name=RuleAttributeName("LAST_SUCCESSFUL_DATE"), + comms_routing=CommsRouting("VACCINATION_DATES|NEXT_DOSE_DATE"), + ), + ], + default_comms_routing="VACCINATION_DATES", + default_not_eligible_routing="VACCINATION_DATES", + default_not_actionable_routing="VACCINATION_DATES", + ) + ], + ) + campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} + s3_client.put_object( + Bucket=rules_bucket, Key=f"{campaign.name}.json", Body=json.dumps(campaign_data), ContentType="application/json" + ) + yield campaign + s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") + + @pytest.fixture def hashing_service() -> HashingService: secret_repo = StubSecretRepo( diff --git a/tests/integration/in_process/test_derived_values.py b/tests/integration/in_process/test_derived_values.py new file mode 100644 index 00000000..313f8c11 --- /dev/null +++ b/tests/integration/in_process/test_derived_values.py @@ -0,0 +1,185 @@ +""" +Integration tests for derived values functionality. + +These tests verify the end-to-end flow of the ADD_DAYS derived value function, +demonstrating how NEXT_DOSE_DUE is calculated from LAST_SUCCESSFUL_DATE. + +Example API response format: +{ + "processedSuggestions": [ + { + "actions": [ + { + "actionType": "DataValue", + "actionCode": "DateOfLastVaccination", + "description": "20260128" + }, + { + "actionType": "DataValue", + "actionCode": "DateOfNextEarliestVaccination", + "description": "20260429" + } + ] + } + ] +} +""" + +from http import HTTPStatus + +from botocore.client import BaseClient +from brunns.matchers.data import json_matching as is_json_that +from brunns.matchers.werkzeug import is_werkzeug_response as is_response +from flask.testing import FlaskClient +from hamcrest import ( + assert_that, + has_key, +) + +from eligibility_signposting_api.model.campaign_config import CampaignConfig +from eligibility_signposting_api.model.eligibility_status import NHSNumber + + +class TestDerivedValues: + """Integration tests for the ADD_DAYS derived value functionality.""" + + def test_add_days_calculates_next_dose_due_from_last_successful_date( + self, + client: FlaskClient, + person_with_covid_vaccination: NHSNumber, + campaign_config_with_derived_values: CampaignConfig, # noqa: ARG002 + secretsmanager_client: BaseClient, # noqa: ARG002 + ): + """ + Test that the ADD_DAYS function correctly calculates the next dose date. + + Given: + - A person with COVID vaccination on 2026-01-28 (20260128) + - A campaign config with actions using: + - [[TARGET.COVID.LAST_SUCCESSFUL_DATE]] for DateOfLastVaccination + - [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]] for DateOfNextEarliestVaccination + + Expected: + - DateOfLastVaccination shows "20260128" + - DateOfNextEarliestVaccination shows "20260429" (2026-01-28 + 91 days = 2026-04-29) + + This demonstrates the use case from the requirement: + "actions": [ + {"actionType": "DataValue", "actionCode": "DateOfLastVaccination", "description": "20260128"}, + {"actionType": "DataValue", "actionCode": "DateOfNextEarliestVaccination", "description": "20260429"} + ] + """ + # Given + headers = {"nhs-login-nhs-number": str(person_with_covid_vaccination)} + + # When + response = client.get( + f"/patient-check/{person_with_covid_vaccination}?includeActions=Y", + headers=headers, + ) + + # Then - verify response is successful + assert_that( + response, + is_response().with_status_code(HTTPStatus.OK).and_text(is_json_that(has_key("processedSuggestions"))), + ) + + # Extract the processed suggestions + body = response.get_json() + assert body is not None + processed_suggestions = body.get("processedSuggestions", []) + + # Find the COVID condition + covid_suggestion = next( + (s for s in processed_suggestions if s.get("condition") == "COVID"), + None, + ) + assert covid_suggestion is not None, "Expected COVID condition in response" + + # Extract actions + actions = covid_suggestion.get("actions", []) + expected_actions_count = 2 + assert len(actions) >= expected_actions_count, ( + f"Expected at least {expected_actions_count} actions, got {len(actions)}" + ) + + # Find the vaccination date actions by action code + date_of_last = next( + (a for a in actions if a.get("actionCode") == "DateOfLastVaccination"), + None, + ) + date_of_next = next( + (a for a in actions if a.get("actionCode") == "DateOfNextEarliestVaccination"), + None, + ) + + # Verify DateOfLastVaccination shows the raw date + assert date_of_last is not None, "Expected DateOfLastVaccination action" + assert date_of_last["description"] == "20260128", ( + f"Expected DateOfLastVaccination to be '20260128', got '{date_of_last['description']}'" + ) + + # Verify DateOfNextEarliestVaccination shows the calculated date (2026-01-28 + 91 days = 2026-04-29) + assert date_of_next is not None, "Expected DateOfNextEarliestVaccination action" + assert date_of_next["description"] == "20260429", ( + f"Expected DateOfNextEarliestVaccination to be '20260429' (20260128 + 91 days), " + f"got '{date_of_next['description']}'" + ) + + # Verify action types are DataValue as per requirement + assert date_of_last["actionType"] == "DataValue" + assert date_of_next["actionType"] == "DataValue" + + def test_add_days_with_formatted_date_output( + self, + client: FlaskClient, + person_with_covid_vaccination: NHSNumber, + campaign_config_with_derived_values_formatted: CampaignConfig, # noqa: ARG002 + secretsmanager_client: BaseClient, # noqa: ARG002 + ): + """ + Test that ADD_DAYS can be combined with DATE formatting. + + Given: + - A person with COVID vaccination on 2026-01-28 + - A campaign config using [[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91):DATE(%d %B %Y)]] + + Expected: + - DateOfNextEarliestVaccination shows "29 April 2026" (formatted output) + """ + # Given + headers = {"nhs-login-nhs-number": str(person_with_covid_vaccination)} + + # When + response = client.get( + f"/patient-check/{person_with_covid_vaccination}?includeActions=Y", + headers=headers, + ) + + # Then + assert_that( + response, + is_response().with_status_code(HTTPStatus.OK).and_text(is_json_that(has_key("processedSuggestions"))), + ) + + body = response.get_json() + assert body is not None + processed_suggestions = body.get("processedSuggestions", []) + + covid_suggestion = next( + (s for s in processed_suggestions if s.get("condition") == "COVID"), + None, + ) + assert covid_suggestion is not None + + actions = covid_suggestion.get("actions", []) + date_of_next = next( + (a for a in actions if a.get("actionCode") == "DateOfNextEarliestVaccination"), + None, + ) + + # Verify the formatted date output + assert date_of_next is not None, "Expected DateOfNextEarliestVaccination action" + assert date_of_next["description"] == "29 April 2026", ( + f"Expected formatted date '29 April 2026', got '{date_of_next['description']}'" + ) From e9cd520b60a5db1f43081e72cda4758605527e4b Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Tue, 30 Dec 2025 12:06:54 +0000 Subject: [PATCH 08/14] eli-579 file formatting --- .../services/processors/derived_values/add_days_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py b/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py index 5442016d..d0972a3e 100644 --- a/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py +++ b/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py @@ -41,9 +41,9 @@ def __init__( Args: default_days: Default number of days to add when not specified - in token or vaccine_type_days. Defaults to 91. + in token or vaccine_type_days. Defaults to 91. vaccine_type_days: Dictionary mapping vaccine types to their - specific days values. E.g., {"COVID": 91, "FLU": 365} + specific days values. E.g., {"COVID": 91, "FLU": 365} """ self.default_days = default_days self.vaccine_type_days = vaccine_type_days or {} From 236a9a1f4d77a599524737be94a8d6f70c218969 Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Fri, 2 Jan 2026 13:41:07 +0000 Subject: [PATCH 09/14] chore - bumping test data date --- tests/integration/conftest.py | 162 +--------------------------------- 1 file changed, 1 insertion(+), 161 deletions(-) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 01f5f4b6..eb76ff2b 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -21,16 +21,10 @@ from eligibility_signposting_api.model.campaign_config import ( AvailableAction, CampaignConfig, - CommsRouting, EndDate, - RuleAttributeLevel, - RuleAttributeName, RuleCode, - RuleDescription, RuleEntry, RuleName, - RuleOperator, - RulePriority, RuleText, RuleType, StartDate, @@ -830,7 +824,7 @@ def inactive_iteration_config(s3_client: BaseClient, rules_bucket: BucketName) - ) campaign.start_date = StartDate(datetime.date(2025, 1, 1)) - campaign.end_date = EndDate(datetime.date(2026, 1, 1)) + campaign.end_date = EndDate(datetime.date(2027, 1, 1)) campaign.iterations[0].iteration_date = data[1] campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} @@ -1125,160 +1119,6 @@ def get_secret_previous(self, secret_name: str) -> dict[str, str]: # noqa: ARG0 return {} -@pytest.fixture -def person_with_covid_vaccination( - person_table: Any, faker: Faker, hashing_service: HashingService -) -> Generator[eligibility_status.NHSNumber]: - """ - Creates a person with a COVID vaccination record. - LAST_SUCCESSFUL_DATE is set to 2026-01-28 (20260128). - Used for testing derived values like NEXT_DOSE_DUE with ADD_DAYS function. - """ - nhs_num = faker.nhs_number() - nhs_number = eligibility_status.NHSNumber(nhs_num) - nhs_num_hash = hashing_service.hash_with_current_secret(nhs_num) - - date_of_birth = eligibility_status.DateOfBirth(datetime.date(1960, 5, 15)) - - for row in ( - rows := person_rows_builder( - nhs_number=nhs_num_hash, - date_of_birth=date_of_birth, - postcode="SW1A", - cohorts=["covid_eligible"], - vaccines={"COVID": {"LAST_SUCCESSFUL_DATE": "20260128", "CONDITION_NAME": "COVID"}}, - ).data - ): - person_table.put_item(Item=row) - - yield nhs_number - - for row in rows: - person_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) - - -@pytest.fixture(scope="class") -def campaign_config_with_derived_values(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[CampaignConfig]: - """ - Creates a campaign config that uses the ADD_DAYS derived value function. - Contains actions for: - - DateOfLastVaccination: Shows the raw LAST_SUCCESSFUL_DATE - - DateOfNextEarliestVaccination: Shows NEXT_DOSE_DUE derived by adding 91 days - """ - campaign: CampaignConfig = rule.CampaignConfigFactory.build( - target="COVID", - iterations=[ - rule.IterationFactory.build( - actions_mapper=rule.ActionsMapperFactory.build( - root={ - "VACCINATION_DATES": AvailableAction( - ActionType="DataValue", - ExternalRoutingCode="DateOfLastVaccination", - ActionDescription="[[TARGET.COVID.LAST_SUCCESSFUL_DATE]]", - ), - "NEXT_DOSE_DATE": AvailableAction( - ActionType="DataValue", - ExternalRoutingCode="DateOfNextEarliestVaccination", - ActionDescription="[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]]", - ), - } - ), - iteration_cohorts=[ - rule.IterationCohortFactory.build( - cohort_label="covid_eligible", - cohort_group="covid_vaccination", - positive_description="Eligible for COVID vaccination", - negative_description="Not eligible for COVID vaccination", - ), - ], - iteration_rules=[ - rule.IterationRuleFactory.build( - type=RuleType.redirect, - name=RuleName("Provide vaccination dates"), - description=RuleDescription("Provide vaccination dates to patient"), - priority=RulePriority(10), - operator=RuleOperator.is_not_null, - attribute_level=RuleAttributeLevel.TARGET, - attribute_target="COVID", - attribute_name=RuleAttributeName("LAST_SUCCESSFUL_DATE"), - comms_routing=CommsRouting("VACCINATION_DATES|NEXT_DOSE_DATE"), - ), - ], - default_comms_routing="VACCINATION_DATES", - default_not_eligible_routing="VACCINATION_DATES", - default_not_actionable_routing="VACCINATION_DATES", - ) - ], - ) - campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} - s3_client.put_object( - Bucket=rules_bucket, Key=f"{campaign.name}.json", Body=json.dumps(campaign_data), ContentType="application/json" - ) - yield campaign - s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") - - -@pytest.fixture(scope="class") -def campaign_config_with_derived_values_formatted( - s3_client: BaseClient, rules_bucket: BucketName -) -> Generator[CampaignConfig]: - """ - Creates a campaign config that uses ADD_DAYS with DATE formatting. - The NEXT_DOSE_DUE is formatted as "29 April 2026" instead of raw "20260429". - """ - campaign: CampaignConfig = rule.CampaignConfigFactory.build( - target="COVID", - iterations=[ - rule.IterationFactory.build( - actions_mapper=rule.ActionsMapperFactory.build( - root={ - "VACCINATION_DATES": AvailableAction( - ActionType="DataValue", - ExternalRoutingCode="DateOfLastVaccination", - ActionDescription="[[TARGET.COVID.LAST_SUCCESSFUL_DATE:DATE(%d %B %Y)]]", - ), - "NEXT_DOSE_DATE": AvailableAction( - ActionType="DataValue", - ExternalRoutingCode="DateOfNextEarliestVaccination", - ActionDescription="[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91):DATE(%d %B %Y)]]", - ), - } - ), - iteration_cohorts=[ - rule.IterationCohortFactory.build( - cohort_label="covid_eligible", - cohort_group="covid_vaccination", - positive_description="Eligible for COVID vaccination", - negative_description="Not eligible for COVID vaccination", - ), - ], - iteration_rules=[ - rule.IterationRuleFactory.build( - type=RuleType.redirect, - name=RuleName("Provide vaccination dates"), - description=RuleDescription("Provide vaccination dates to patient"), - priority=RulePriority(10), - operator=RuleOperator.is_not_null, - attribute_level=RuleAttributeLevel.TARGET, - attribute_target="COVID", - attribute_name=RuleAttributeName("LAST_SUCCESSFUL_DATE"), - comms_routing=CommsRouting("VACCINATION_DATES|NEXT_DOSE_DATE"), - ), - ], - default_comms_routing="VACCINATION_DATES", - default_not_eligible_routing="VACCINATION_DATES", - default_not_actionable_routing="VACCINATION_DATES", - ) - ], - ) - campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} - s3_client.put_object( - Bucket=rules_bucket, Key=f"{campaign.name}.json", Body=json.dumps(campaign_data), ContentType="application/json" - ) - yield campaign - s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") - - @pytest.fixture def hashing_service() -> HashingService: secret_repo = StubSecretRepo( From c39bc7d424844ead3bf47ae7fb9f53fb1017d95e Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Fri, 2 Jan 2026 14:02:00 +0000 Subject: [PATCH 10/14] eli-579 re-adding fixtures after panic removing them due to non-related flakey test --- tests/integration/conftest.py | 113 ++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index eb76ff2b..01b85c95 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -653,6 +653,38 @@ def persisted_person_with_no_person_attribute_type( person_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) +@pytest.fixture +def person_with_covid_vaccination( + person_table: Any, faker: Faker, hashing_service: HashingService +) -> Generator[eligibility_status.NHSNumber]: + """ + Fixture for a person with a COVID vaccination on 2026-01-28. + Used for testing derived values (ADD_DAYS function). + """ + nhs_num = faker.nhs_number() + nhs_number = eligibility_status.NHSNumber(nhs_num) + nhs_num_hash = hashing_service.hash_with_current_secret(nhs_num) + + date_of_birth = eligibility_status.DateOfBirth(faker.date_of_birth(minimum_age=77, maximum_age=77)) + + for row in ( + rows := person_rows_builder( + nhs_number=nhs_num_hash, + date_of_birth=date_of_birth, + postcode="HP1", + cohorts=["cohort_label1"], + vaccines={"COVID": {"LAST_SUCCESSFUL_DATE": "20260128"}}, + icb="QE1", + ).data + ): + person_table.put_item(Item=row) + + yield nhs_number + + for row in rows: + person_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) + + @pytest.fixture(scope="session") def rules_bucket(s3_client: BaseClient) -> Generator[BucketName]: bucket_name = BucketName(os.getenv("RULES_BUCKET_NAME", "test-rules-bucket")) @@ -981,6 +1013,87 @@ def campaign_config_with_invalid_tokens(s3_client: BaseClient, rules_bucket: Buc s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") +@pytest.fixture +def campaign_config_with_derived_values(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[CampaignConfig]: + """Campaign config with derived values for testing ADD_DAYS function.""" + campaign: CampaignConfig = rule.CampaignConfigFactory.build( + target="COVID", + iterations=[ + rule.IterationFactory.build( + default_comms_routing="DERIVED_VALUES_TEST|DERIVED_VALUES_NEXT_DOSE", + actions_mapper=rule.ActionsMapperFactory.build( + root={ + "DERIVED_VALUES_TEST": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfLastVaccination", + ActionDescription="[[TARGET.COVID.LAST_SUCCESSFUL_DATE]]", + ), + "DERIVED_VALUES_NEXT_DOSE": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfNextEarliestVaccination", + ActionDescription="[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]]", + ), + } + ), + iteration_rules=[], + iteration_cohorts=[ + rule.IterationCohortFactory.build( + cohort_label="cohort_label1", + cohort_group="cohort_group1", + positive_description="Positive Description", + negative_description="Negative Description", + ) + ], + ) + ], + ) + campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} + s3_client.put_object( + Bucket=rules_bucket, Key=f"{campaign.name}.json", Body=json.dumps(campaign_data), ContentType="application/json" + ) + yield campaign + s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") + + +@pytest.fixture +def campaign_config_with_derived_values_formatted( + s3_client: BaseClient, rules_bucket: BucketName +) -> Generator[CampaignConfig]: + """Campaign config with derived values and date formatting.""" + campaign: CampaignConfig = rule.CampaignConfigFactory.build( + target="COVID", + iterations=[ + rule.IterationFactory.build( + default_comms_routing="DERIVED_VALUES_FORMATTED", + actions_mapper=rule.ActionsMapperFactory.build( + root={ + "DERIVED_VALUES_FORMATTED": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfNextEarliestVaccination", + ActionDescription="[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91):DATE(%d %B %Y)]]", + ), + } + ), + iteration_rules=[], + iteration_cohorts=[ + rule.IterationCohortFactory.build( + cohort_label="cohort_label1", + cohort_group="cohort_group1", + positive_description="Positive Description", + negative_description="Negative Description", + ) + ], + ) + ], + ) + campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} + s3_client.put_object( + Bucket=rules_bucket, Key=f"{campaign.name}.json", Body=json.dumps(campaign_data), ContentType="application/json" + ) + yield campaign + s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") + + @pytest.fixture(scope="class") def multiple_campaign_configs(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[list[CampaignConfig]]: """Create and upload multiple campaign configs to S3, then clean up after tests.""" From c9ce52bd7de09bc1c93637597b28a6ca25831aca Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Tue, 6 Jan 2026 16:18:36 +0000 Subject: [PATCH 11/14] eli-579 safer handling of non-numeric values if passed to ADD_DAYS --- .../derived_values/add_days_handler.py | 7 +- .../processors/test_derived_values.py | 71 ++++++++++++------- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py b/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py index d0972a3e..3203fe3f 100644 --- a/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py +++ b/src/eligibility_signposting_api/services/processors/derived_values/add_days_handler.py @@ -118,12 +118,13 @@ def _get_days_to_add(self, context: DerivedValueContext) -> int: Returns: Number of days to add """ - # Priority 1: Token argument + # Priority 1: Token argument (if non-empty) if context.function_args: try: return int(context.function_args) - except ValueError: - pass + except ValueError as e: + message = f"Invalid days argument '{context.function_args}' for ADD_DAYS function. Expected an integer." + raise ValueError(message) from e # Priority 2: Vaccine-specific configuration if context.attribute_name in self.vaccine_type_days: diff --git a/tests/unit/services/processors/test_derived_values.py b/tests/unit/services/processors/test_derived_values.py index 3b59da86..0b6393c7 100644 --- a/tests/unit/services/processors/test_derived_values.py +++ b/tests/unit/services/processors/test_derived_values.py @@ -1,6 +1,6 @@ from unittest.mock import MagicMock -import pytest +from hamcrest import assert_that, calling, equal_to, is_, raises, same_instance from eligibility_signposting_api.services.processors.derived_values import ( AddDaysHandler, @@ -26,7 +26,7 @@ def test_calculate_adds_default_days_to_date(self): result = handler.calculate(context) # 2025-01-01 + 91 days = 2025-04-02 - assert result == "20250402" + assert_that(result, is_(equal_to("20250402"))) def test_calculate_with_function_args_override(self): """Test that function args override default days.""" @@ -42,7 +42,7 @@ def test_calculate_with_function_args_override(self): result = handler.calculate(context) # 2025-01-01 + 30 days = 2025-01-31 - assert result == "20250131" + assert_that(result, is_(equal_to("20250131"))) def test_calculate_with_vaccine_specific_days(self): """Test that vaccine-specific days are used when configured.""" @@ -61,7 +61,7 @@ def test_calculate_with_vaccine_specific_days(self): result = handler.calculate(context) # 2025-01-01 + 365 days = 2026-01-01 - assert result == "20260101" + assert_that(result, is_(equal_to("20260101"))) def test_calculate_with_date_format(self): """Test that date format is applied to output.""" @@ -76,7 +76,7 @@ def test_calculate_with_date_format(self): result = handler.calculate(context) - assert result == "02 April 2025" + assert_that(result, is_(equal_to("02 April 2025"))) def test_calculate_returns_empty_when_source_not_found(self): """Test that empty string is returned when source date not found.""" @@ -91,7 +91,7 @@ def test_calculate_returns_empty_when_source_not_found(self): result = handler.calculate(context) - assert result == "" + assert_that(result, is_(equal_to(""))) def test_calculate_returns_empty_when_vaccine_not_found(self): """Test that empty string is returned when vaccine type not found.""" @@ -106,7 +106,7 @@ def test_calculate_returns_empty_when_vaccine_not_found(self): result = handler.calculate(context) - assert result == "" + assert_that(result, is_(equal_to(""))) def test_calculate_with_invalid_date_raises_error(self): """Test that invalid date format raises ValueError.""" @@ -119,20 +119,38 @@ def test_calculate_with_invalid_date_raises_error(self): date_format=None, ) - with pytest.raises(ValueError, match="Invalid date format"): - handler.calculate(context) + assert_that( + calling(handler.calculate).with_args(context), + raises(ValueError, pattern="Invalid date format"), + ) + + def test_calculate_with_invalid_function_args_raises_error(self): + """Test that non-integer function args raises ValueError.""" + handler = AddDaysHandler(default_days=91) + context = DerivedValueContext( + person_data=[{"ATTRIBUTE_TYPE": "COVID", "LAST_SUCCESSFUL_DATE": "20250101"}], + attribute_name="COVID", + source_attribute="LAST_SUCCESSFUL_DATE", + function_args="abc", # Invalid: not an integer + date_format=None, + ) + + assert_that( + calling(handler.calculate).with_args(context), + raises(ValueError, pattern="Invalid days argument 'abc' for ADD_DAYS function"), + ) def test_get_source_attribute_maps_derived_to_source(self): """Test that get_source_attribute maps derived attributes correctly.""" handler = AddDaysHandler() - assert handler.get_source_attribute("NEXT_DOSE_DUE") == "LAST_SUCCESSFUL_DATE" + assert_that(handler.get_source_attribute("NEXT_DOSE_DUE"), is_(equal_to("LAST_SUCCESSFUL_DATE"))) def test_get_source_attribute_returns_original_if_not_mapped(self): """Test that unmapped attributes return themselves.""" handler = AddDaysHandler() - assert handler.get_source_attribute("UNKNOWN_ATTR") == "UNKNOWN_ATTR" + assert_that(handler.get_source_attribute("UNKNOWN_ATTR"), is_(equal_to("UNKNOWN_ATTR"))) def test_function_args_priority_over_vaccine_config(self): """Test that function args take priority over vaccine-specific config.""" @@ -151,7 +169,7 @@ def test_function_args_priority_over_vaccine_config(self): result = handler.calculate(context) # 2025-01-01 + 30 days = 2025-01-31 - assert result == "20250131" + assert_that(result, is_(equal_to("20250131"))) class TestDerivedValueRegistry: @@ -165,7 +183,7 @@ def test_register_and_get_handler(self): retrieved = registry.get_handler("ADD_DAYS") - assert retrieved is handler + assert_that(retrieved, same_instance(handler)) # type: ignore[call-overload] def test_get_handler_case_insensitive(self): """Test that handler lookup is case insensitive.""" @@ -173,21 +191,21 @@ def test_get_handler_case_insensitive(self): handler = AddDaysHandler() registry.register(handler) - assert registry.get_handler("add_days") is handler - assert registry.get_handler("Add_Days") is handler + assert_that(registry.get_handler("add_days"), same_instance(handler)) # type: ignore[call-overload] + assert_that(registry.get_handler("Add_Days"), same_instance(handler)) # type: ignore[call-overload] def test_has_handler_returns_true_when_exists(self): """Test has_handler returns True for registered handlers.""" registry = DerivedValueRegistry() registry.register(AddDaysHandler()) - assert registry.has_handler("ADD_DAYS") is True + assert_that(registry.has_handler("ADD_DAYS"), is_(True)) def test_has_handler_returns_false_when_not_exists(self): """Test has_handler returns False for unregistered handlers.""" registry = DerivedValueRegistry() - assert registry.has_handler("UNKNOWN") is False + assert_that(registry.has_handler("UNKNOWN"), is_(False)) def test_calculate_delegates_to_correct_handler(self): """Test that calculate delegates to the correct handler.""" @@ -215,7 +233,7 @@ def test_calculate_delegates_to_correct_handler(self): # Verify the mock handler was called with the context mock_handler.calculate.assert_called_once_with(context) - assert result == "mock_result" + assert_that(result, is_(equal_to("mock_result"))) def test_calculate_raises_for_unknown_function(self): """Test that calculate raises ValueError for unknown functions.""" @@ -229,32 +247,31 @@ def test_calculate_raises_for_unknown_function(self): date_format=None, ) - with pytest.raises(ValueError, match="No handler registered"): - registry.calculate( - function_name="UNKNOWN", - context=context, - ) + assert_that( + calling(registry.calculate).with_args(function_name="UNKNOWN", context=context), + raises(ValueError, pattern="No handler registered"), + ) def test_is_derived_attribute_returns_true_for_derived(self): """Test is_derived_attribute for known derived attributes.""" registry = DerivedValueRegistry() registry.register(AddDaysHandler()) - assert registry.is_derived_attribute("NEXT_DOSE_DUE") is True + assert_that(registry.is_derived_attribute("NEXT_DOSE_DUE"), is_(True)) def test_is_derived_attribute_returns_false_for_non_derived(self): """Test is_derived_attribute for non-derived attributes.""" registry = DerivedValueRegistry() registry.register(AddDaysHandler()) - assert registry.is_derived_attribute("LAST_SUCCESSFUL_DATE") is False + assert_that(registry.is_derived_attribute("LAST_SUCCESSFUL_DATE"), is_(False)) def test_default_handlers_are_registered(self): """Test that default handlers from the module are registered.""" registry = DerivedValueRegistry() # The default ADD_DAYS handler should be registered via __init__.py - assert registry.has_handler("ADD_DAYS") + assert_that(registry.has_handler("ADD_DAYS"), is_(True)) def test_clear_defaults_removes_default_handlers(self): """Test that clear_defaults removes all default handlers.""" @@ -266,7 +283,7 @@ def test_clear_defaults_removes_default_handlers(self): # New registry should have no handlers registry = DerivedValueRegistry() - assert not registry.has_handler("ADD_DAYS") + assert_that(registry.has_handler("ADD_DAYS"), is_(False)) finally: # Restore defaults for other tests using public method DerivedValueRegistry.set_default_handlers(saved_defaults) From d656bcf8a423700eb78f033baeb220ec4c18ba02 Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Wed, 7 Jan 2026 10:46:19 +0000 Subject: [PATCH 12/14] eli-579 updating tests to use hamcrest --- poetry.lock | 3 --- .../processors/test_token_parser_functions.py | 24 +++++++++---------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/poetry.lock b/poetry.lock index 8197439f..ac0e6617 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1636,11 +1636,8 @@ files = [ {file = "lxml-5.4.0-cp36-cp36m-win_amd64.whl", hash = "sha256:7ce1a171ec325192c6a636b64c94418e71a1964f56d002cc28122fceff0b6121"}, {file = "lxml-5.4.0-cp37-cp37m-macosx_10_9_x86_64.whl", hash = "sha256:795f61bcaf8770e1b37eec24edf9771b307df3af74d1d6f27d812e15a9ff3872"}, {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_12_i686.manylinux2010_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:29f451a4b614a7b5b6c2e043d7b64a15bd8304d7e767055e8ab68387a8cacf4e"}, - {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:891f7f991a68d20c75cb13c5c9142b2a3f9eb161f1f12a9489c82172d1f133c0"}, {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:4aa412a82e460571fad592d0f93ce9935a20090029ba08eca05c614f99b0cc92"}, - {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_28_aarch64.whl", hash = "sha256:ac7ba71f9561cd7d7b55e1ea5511543c0282e2b6450f122672a2694621d63b7e"}, {file = "lxml-5.4.0-cp37-cp37m-manylinux_2_28_x86_64.whl", hash = "sha256:c5d32f5284012deaccd37da1e2cd42f081feaa76981f0eaa474351b68df813c5"}, - {file = "lxml-5.4.0-cp37-cp37m-musllinux_1_2_aarch64.whl", hash = "sha256:ce31158630a6ac85bddd6b830cffd46085ff90498b397bd0a259f59d27a12188"}, {file = "lxml-5.4.0-cp37-cp37m-musllinux_1_2_x86_64.whl", hash = "sha256:31e63621e073e04697c1b2d23fcb89991790eef370ec37ce4d5d469f40924ed6"}, {file = "lxml-5.4.0-cp37-cp37m-win32.whl", hash = "sha256:be2ba4c3c5b7900246a8f866580700ef0d538f2ca32535e991027bdaba944063"}, {file = "lxml-5.4.0-cp37-cp37m-win_amd64.whl", hash = "sha256:09846782b1ef650b321484ad429217f5154da4d6e786636c38e434fa32e94e49"}, diff --git a/tests/unit/services/processors/test_token_parser_functions.py b/tests/unit/services/processors/test_token_parser_functions.py index cad0535c..b80c489f 100644 --- a/tests/unit/services/processors/test_token_parser_functions.py +++ b/tests/unit/services/processors/test_token_parser_functions.py @@ -3,6 +3,7 @@ from dataclasses import dataclass import pytest +from hamcrest import assert_that, calling, equal_to, is_, none, raises from eligibility_signposting_api.services.processors.token_parser import TokenParser @@ -61,26 +62,26 @@ def test_parse_tokens_with_functions(self, token: str, expected: ExpectedTokenRe """Test parsing tokens with function calls.""" parsed_token = TokenParser.parse(token) - assert parsed_token.attribute_level == expected.level - assert parsed_token.attribute_name == expected.name - assert parsed_token.attribute_value == expected.value - assert parsed_token.function_name == expected.function - assert parsed_token.function_args == expected.args - assert parsed_token.format == expected.date_format + assert_that(parsed_token.attribute_level, is_(equal_to(expected.level))) + assert_that(parsed_token.attribute_name, is_(equal_to(expected.name))) + assert_that(parsed_token.attribute_value, is_(equal_to(expected.value))) + assert_that(parsed_token.function_name, is_(equal_to(expected.function))) + assert_that(parsed_token.function_args, is_(equal_to(expected.args))) + assert_that(parsed_token.format, is_(equal_to(expected.date_format))) def test_parse_without_function_has_none_function_fields(self): """Test that tokens without functions have None for function fields.""" parsed = TokenParser.parse("[[TARGET.COVID.LAST_SUCCESSFUL_DATE]]") - assert parsed.function_name is None - assert parsed.function_args is None + assert_that(parsed.function_name, is_(none())) + assert_that(parsed.function_args, is_(none())) def test_parse_date_format_not_treated_as_function(self): """Test that DATE format is not treated as a derived function.""" parsed = TokenParser.parse("[[PERSON.DATE_OF_BIRTH:DATE(%d %B %Y)]]") - assert parsed.function_name is None - assert parsed.format == "%d %B %Y" + assert_that(parsed.function_name, is_(none())) + assert_that(parsed.format, is_(equal_to("%d %B %Y"))) @pytest.mark.parametrize( "token", @@ -92,5 +93,4 @@ def test_parse_date_format_not_treated_as_function(self): ) def test_parse_invalid_function_format_raises_error(self, token): """Test that malformed function calls raise errors.""" - with pytest.raises(ValueError, match="Invalid token format"): - TokenParser.parse(token) + assert_that(calling(TokenParser.parse).with_args(token), raises(ValueError, "Invalid token format")) From afea6a3146376df334d088c7bd847ca4355ee32c Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Wed, 7 Jan 2026 10:53:55 +0000 Subject: [PATCH 13/14] eli-579 refactoring tests to use hamcrest --- .../in_process/test_derived_values.py | 61 ++++++++----------- 1 file changed, 24 insertions(+), 37 deletions(-) diff --git a/tests/integration/in_process/test_derived_values.py b/tests/integration/in_process/test_derived_values.py index 313f8c11..1e268112 100644 --- a/tests/integration/in_process/test_derived_values.py +++ b/tests/integration/in_process/test_derived_values.py @@ -33,7 +33,13 @@ from flask.testing import FlaskClient from hamcrest import ( assert_that, + greater_than_or_equal_to, + has_entries, + has_item, has_key, + has_length, + is_not, + none, ) from eligibility_signposting_api.model.campaign_config import CampaignConfig @@ -86,7 +92,7 @@ def test_add_days_calculates_next_dose_due_from_last_successful_date( # Extract the processed suggestions body = response.get_json() - assert body is not None + assert_that(body, is_not(none())) processed_suggestions = body.get("processedSuggestions", []) # Find the COVID condition @@ -94,42 +100,27 @@ def test_add_days_calculates_next_dose_due_from_last_successful_date( (s for s in processed_suggestions if s.get("condition") == "COVID"), None, ) - assert covid_suggestion is not None, "Expected COVID condition in response" + assert_that(covid_suggestion, is_not(none())) # Extract actions - actions = covid_suggestion.get("actions", []) + actions = covid_suggestion.get("actions", []) # type: ignore[union-attr] expected_actions_count = 2 - assert len(actions) >= expected_actions_count, ( - f"Expected at least {expected_actions_count} actions, got {len(actions)}" - ) - - # Find the vaccination date actions by action code - date_of_last = next( - (a for a in actions if a.get("actionCode") == "DateOfLastVaccination"), - None, - ) - date_of_next = next( - (a for a in actions if a.get("actionCode") == "DateOfNextEarliestVaccination"), - None, - ) + assert_that(actions, has_length(greater_than_or_equal_to(expected_actions_count))) # Verify DateOfLastVaccination shows the raw date - assert date_of_last is not None, "Expected DateOfLastVaccination action" - assert date_of_last["description"] == "20260128", ( - f"Expected DateOfLastVaccination to be '20260128', got '{date_of_last['description']}'" + assert_that( + actions, + has_item(has_entries(actionType="DataValue", actionCode="DateOfLastVaccination", description="20260128")), ) # Verify DateOfNextEarliestVaccination shows the calculated date (2026-01-28 + 91 days = 2026-04-29) - assert date_of_next is not None, "Expected DateOfNextEarliestVaccination action" - assert date_of_next["description"] == "20260429", ( - f"Expected DateOfNextEarliestVaccination to be '20260429' (20260128 + 91 days), " - f"got '{date_of_next['description']}'" + assert_that( + actions, + has_item( + has_entries(actionType="DataValue", actionCode="DateOfNextEarliestVaccination", description="20260429") + ), ) - # Verify action types are DataValue as per requirement - assert date_of_last["actionType"] == "DataValue" - assert date_of_next["actionType"] == "DataValue" - def test_add_days_with_formatted_date_output( self, client: FlaskClient, @@ -163,23 +154,19 @@ def test_add_days_with_formatted_date_output( ) body = response.get_json() - assert body is not None + assert_that(body, is_not(none())) processed_suggestions = body.get("processedSuggestions", []) covid_suggestion = next( (s for s in processed_suggestions if s.get("condition") == "COVID"), None, ) - assert covid_suggestion is not None + assert_that(covid_suggestion, is_not(none())) - actions = covid_suggestion.get("actions", []) - date_of_next = next( - (a for a in actions if a.get("actionCode") == "DateOfNextEarliestVaccination"), - None, - ) + actions = covid_suggestion.get("actions", []) # type: ignore[union-attr] # Verify the formatted date output - assert date_of_next is not None, "Expected DateOfNextEarliestVaccination action" - assert date_of_next["description"] == "29 April 2026", ( - f"Expected formatted date '29 April 2026', got '{date_of_next['description']}'" + assert_that( + actions, + has_item(has_entries(actionCode="DateOfNextEarliestVaccination", description="29 April 2026")), ) From 42c377677c03943086a730d6502ff1760a7637f8 Mon Sep 17 00:00:00 2001 From: Edd Almond <102675624+eddalmond1@users.noreply.github.com> Date: Thu, 8 Jan 2026 13:04:47 +0000 Subject: [PATCH 14/14] eli-579 adding integration test to test that functions can use multiple instances a function --- tests/integration/conftest.py | 49 +++++++++++++ .../in_process/test_derived_values.py | 72 +++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 5724dd5d..e76dc9c6 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -1094,6 +1094,55 @@ def campaign_config_with_derived_values_formatted( s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") +@pytest.fixture +def campaign_config_with_multiple_add_days( + s3_client: BaseClient, rules_bucket: BucketName +) -> Generator[CampaignConfig]: + """Campaign config with multiple actions using ADD_DAYS with different parameters.""" + campaign: CampaignConfig = rule.CampaignConfigFactory.build( + target="COVID", + iterations=[ + rule.IterationFactory.build( + default_comms_routing="DERIVED_LAST_DATE|DERIVED_NEXT_DOSE_91|DERIVED_NEXT_DOSE_61", + actions_mapper=rule.ActionsMapperFactory.build( + root={ + "DERIVED_LAST_DATE": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfLastVaccination", + ActionDescription="[[TARGET.COVID.LAST_SUCCESSFUL_DATE]]", + ), + "DERIVED_NEXT_DOSE_91": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfNextDoseAt91Days", + ActionDescription="[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(91)]]", + ), + "DERIVED_NEXT_DOSE_61": AvailableAction( + ActionType="DataValue", + ExternalRoutingCode="DateOfNextDoseAt61Days", + ActionDescription="[[TARGET.COVID.NEXT_DOSE_DUE:ADD_DAYS(61)]]", + ), + } + ), + iteration_rules=[], + iteration_cohorts=[ + rule.IterationCohortFactory.build( + cohort_label="cohort_label1", + cohort_group="cohort_group1", + positive_description="Positive Description", + negative_description="Negative Description", + ) + ], + ) + ], + ) + campaign_data = {"CampaignConfig": campaign.model_dump(by_alias=True)} + s3_client.put_object( + Bucket=rules_bucket, Key=f"{campaign.name}.json", Body=json.dumps(campaign_data), ContentType="application/json" + ) + yield campaign + s3_client.delete_object(Bucket=rules_bucket, Key=f"{campaign.name}.json") + + @pytest.fixture(scope="class") def multiple_campaign_configs(s3_client: BaseClient, rules_bucket: BucketName) -> Generator[list[CampaignConfig]]: """Create and upload multiple campaign configs to S3, then clean up after tests.""" diff --git a/tests/integration/in_process/test_derived_values.py b/tests/integration/in_process/test_derived_values.py index 1e268112..29574e92 100644 --- a/tests/integration/in_process/test_derived_values.py +++ b/tests/integration/in_process/test_derived_values.py @@ -32,6 +32,7 @@ from brunns.matchers.werkzeug import is_werkzeug_response as is_response from flask.testing import FlaskClient from hamcrest import ( + all_of, assert_that, greater_than_or_equal_to, has_entries, @@ -170,3 +171,74 @@ def test_add_days_with_formatted_date_output( actions, has_item(has_entries(actionCode="DateOfNextEarliestVaccination", description="29 April 2026")), ) + + +class TestMultipleActionsWithAddDays: + """Test that multiple actions can use ADD_DAYS with different parameters.""" + + def test_multiple_actions_with_different_add_days_parameters( + self, + client: FlaskClient, + campaign_config_with_multiple_add_days: CampaignConfig, # noqa: ARG002 + person_with_covid_vaccination: NHSNumber, + ): + """ + Test that multiple actions can use the same function (ADD_DAYS) with different parameters. + + This test verifies that when a campaign has multiple actions that use ADD_DAYS with different + parameters (e.g., ADD_DAYS(91) and ADD_DAYS(61)), both calculations are performed correctly. + + Given: + - A person with a COVID vaccination date of 2026-01-28 (20260128) + - A campaign config with three actions: + 1. DateOfLastVaccination: Returns the raw LAST_SUCCESSFUL_DATE + 2. DateOfNextDoseAt91Days: Returns NEXT_DOSE_DUE with ADD_DAYS(91) + 3. DateOfNextDoseAt61Days: Returns NEXT_DOSE_DUE with ADD_DAYS(61) + + When: + - The /patient-check endpoint is called with includeActions=Y + + Then: + - DateOfLastVaccination should be "20260128" (raw date) + - DateOfNextDoseAt91Days should be "20260429" (2026-01-28 + 91 days) + - DateOfNextDoseAt61Days should be "20260330" (2026-01-28 + 61 days) + + This addresses the reviewer's suggestion to test multiple actions using the same + function with different parameters. + """ + # Given + headers = {"nhs-login-nhs-number": str(person_with_covid_vaccination)} + + # When + response = client.get( + f"/patient-check/{person_with_covid_vaccination}?includeActions=Y", + headers=headers, + ) + + # Then + assert_that( + response, + is_response().with_status_code(HTTPStatus.OK).and_text(is_json_that(has_key("processedSuggestions"))), + ) + + body = response.get_json() + assert_that(body, is_not(none())) + processed_suggestions = body.get("processedSuggestions", []) + + covid_suggestion = next( + (s for s in processed_suggestions if s.get("condition") == "COVID"), + None, + ) + assert_that(covid_suggestion, is_not(none())) + + actions = covid_suggestion.get("actions", []) # type: ignore[union-attr] + + # Verify all three actions are present with correct values + assert_that( + actions, + all_of( + has_item(has_entries(actionCode="DateOfLastVaccination", description="20260128")), + has_item(has_entries(actionCode="DateOfNextDoseAt91Days", description="20260429")), + has_item(has_entries(actionCode="DateOfNextDoseAt61Days", description="20260330")), + ), + )