diff --git a/README.md b/README.md index e393c52c..28b9197d 100644 --- a/README.md +++ b/README.md @@ -70,29 +70,29 @@ The following software packages, or their equivalents, are expected to be instal #### Environment variables - Local -| Variable | Default | Description | -|--------------------------|------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `AWS_ACCESS_KEY_ID` | `dummy_key` | AWS Access Key | -| `AWS_DEFAULT_REGION` | `eu-west-1` | AWS Region | -| `AWS_SECRET_ACCESS_KEY` | `dummy_secret` | AWS Secret Access Key | -| `DYNAMODB_ENDPOINT` | `http://localhost:4566` | Endpoint for the app to access DynamoDB | -| `S3_ENDPOINT` | `http://localhost:4566` | Endpoint for the app to access S3 | -| `ELIGIBILITY_TABLE_NAME` | `test_eligibility_datastore` | AWS DynamoDB table for person data. | -| `LOG_LEVEL` | `WARNING` | Logging level. Must be one of `DEBUG`, `INFO`, `WARNING`, `ERROR` or `CRITICAL` as per [Logging Levels](https://docs.python.org/3/library/logging.html#logging-levels) | -| `RULES_BUCKET_NAME` | `test-rules-bucket` | AWS S3 bucket from which to read rules. | +| Variable | Default | Description | +|-------------------------|------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `AWS_ACCESS_KEY_ID` | `dummy_key` | AWS Access Key | +| `AWS_DEFAULT_REGION` | `eu-west-1` | AWS Region | +| `AWS_SECRET_ACCESS_KEY` | `dummy_secret` | AWS Secret Access Key | +| `DYNAMODB_ENDPOINT` | `http://localhost:4566` | Endpoint for the app to access DynamoDB | +| `S3_ENDPOINT` | `http://localhost:4566` | Endpoint for the app to access S3 | +| `PERSON_TABLE_NAME` | `test_eligibility_datastore` | AWS DynamoDB table for person data. | +| `LOG_LEVEL` | `WARNING` | Logging level. Must be one of `DEBUG`, `INFO`, `WARNING`, `ERROR` or `CRITICAL` as per [Logging Levels](https://docs.python.org/3/library/logging.html#logging-levels) | +| `RULES_BUCKET_NAME` | `test-rules-bucket` | AWS S3 bucket from which to read rules. | #### Environment variables - DEV, PROD or PRE-PROD -| Variable | Default | Description | Comments | -|--------------------------|------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------| -| `AWS_DEFAULT_REGION` | `eu-west-1` | AWS Region | | -| `AWS_ACCESS_KEY_ID` | None | AWS Access Key | **AWS_ACCESS_KEY_ID** is set to None,
because it is provided by the AWS environment automatically. | -| `AWS_SECRET_ACCESS_KEY` | None | AWS Secret Access Key | **AWS_SECRET_ACCESS_KEY** is set to None,
because it is provided by the AWS environment automatically. | -| `DYNAMODB_ENDPOINT` | None | Endpoint for the app to access DynamoDB | **DYNAMODB_ENDPOINT** are set to None,
since we are using aws service default endpoints which are provided automatically. | -| `S3_ENDPOINT` | None | Endpoint for the app to access S3 | **S3_ENDPOINT** are set to None,
since we are using aws service default endpoints which are provided automatically. | -| `ELIGIBILITY_TABLE_NAME` | `test_eligibility_datastore` | AWS DynamoDB table for person data. | | -| `LOG_LEVEL` | `WARNING` | Logging level. Must be one of `DEBUG`, `INFO`, `WARNING`, `ERROR` or `CRITICAL` as per [Logging Levels](https://docs.python.org/3/library/logging.html#logging-levels) | | -| `RULES_BUCKET_NAME` | `test-rules-bucket` | AWS S3 bucket from which to read rules. | | +| Variable | Default | Description | Comments | +|-------------------------|------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------| +| `AWS_DEFAULT_REGION` | `eu-west-1` | AWS Region | | +| `AWS_ACCESS_KEY_ID` | None | AWS Access Key | **AWS_ACCESS_KEY_ID** is set to None,
because it is provided by the AWS environment automatically. | +| `AWS_SECRET_ACCESS_KEY` | None | AWS Secret Access Key | **AWS_SECRET_ACCESS_KEY** is set to None,
because it is provided by the AWS environment automatically. | +| `DYNAMODB_ENDPOINT` | None | Endpoint for the app to access DynamoDB | **DYNAMODB_ENDPOINT** are set to None,
since we are using aws service default endpoints which are provided automatically. | +| `S3_ENDPOINT` | None | Endpoint for the app to access S3 | **S3_ENDPOINT** are set to None,
since we are using aws service default endpoints which are provided automatically. | +| `PERSON_TABLE_NAME` | `test_eligibility_datastore` | AWS DynamoDB table for person data. | | +| `LOG_LEVEL` | `WARNING` | Logging level. Must be one of `DEBUG`, `INFO`, `WARNING`, `ERROR` or `CRITICAL` as per [Logging Levels](https://docs.python.org/3/library/logging.html#logging-levels) | | +| `RULES_BUCKET_NAME` | `test-rules-bucket` | AWS S3 bucket from which to read rules. | | ## Usage @@ -201,8 +201,8 @@ graph TB end subgraph "Data Access Layer" direction TB - RepoElig["repos/eligibility_repo.py"] - RepoRules["repos/rules_repo.py"] + PersonRepo["repos/person_repo.py"] + CampaignRepo["repos/campaign_repo.py"] Factory["repos/factory.py, exceptions.py"] end subgraph "Models" @@ -216,10 +216,10 @@ graph TB App -->|injects| View View -->|calls| Service Service -->|calls| Operators - Service -->|calls| RepoElig - Service -->|calls| RepoRules - RepoElig -->|uses| DynamoDB - RepoRules -->|uses| S3Bucket + Service -->|calls| PersonRepo + Service -->|calls| CampaignRepo + PersonRepo -->|uses| DynamoDB + CampaignRepo -->|uses| S3Bucket View -->|uses| ResponseModel App -->|reads| Config Service -->|uses| ModelElig diff --git a/infrastructure/modules/lambda/lambda.tf b/infrastructure/modules/lambda/lambda.tf index c4fb95ca..a91878ca 100644 --- a/infrastructure/modules/lambda/lambda.tf +++ b/infrastructure/modules/lambda/lambda.tf @@ -14,7 +14,7 @@ resource "aws_lambda_function" "eligibility_signposting_lambda" { environment { variables = { - ELIGIBILITY_TABLE_NAME = var.eligibility_status_table_name, + PERSON_TABLE_NAME = var.eligibility_status_table_name, RULES_BUCKET_NAME = var.eligibility_rules_bucket_name, ENV = var.environment } diff --git a/src/eligibility_signposting_api/config.py b/src/eligibility_signposting_api/config.py index 8ff9a317..1922bc06 100644 --- a/src/eligibility_signposting_api/config.py +++ b/src/eligibility_signposting_api/config.py @@ -7,8 +7,8 @@ from pythonjsonlogger.json import JsonFormatter from yarl import URL -from eligibility_signposting_api.repos.eligibility_repo import TableName -from eligibility_signposting_api.repos.rules_repo import BucketName +from eligibility_signposting_api.repos.campaign_repo import BucketName +from eligibility_signposting_api.repos.person_repo import TableName LOG_LEVEL = logging.getLevelNamesMapping().get(os.getenv("LOG_LEVEL", ""), logging.WARNING) @@ -19,7 +19,7 @@ @cache def config() -> dict[str, Any]: - eligibility_table_name = TableName(os.getenv("ELIGIBILITY_TABLE_NAME", "test_eligibility_datastore")) + person_table_name = TableName(os.getenv("PERSON_TABLE_NAME", "test_eligibility_datastore")) rules_bucket_name = BucketName(os.getenv("RULES_BUCKET_NAME", "test-rules-bucket")) aws_default_region = AwsRegion(os.getenv("AWS_DEFAULT_REGION", "eu-west-1")) log_level = LOG_LEVEL @@ -30,7 +30,7 @@ def config() -> dict[str, Any]: "aws_default_region": aws_default_region, "aws_secret_access_key": None, "dynamodb_endpoint": None, - "eligibility_table_name": eligibility_table_name, + "person_table_name": person_table_name, "s3_endpoint": None, "rules_bucket_name": rules_bucket_name, "log_level": log_level, @@ -41,7 +41,7 @@ def config() -> dict[str, Any]: "aws_default_region": aws_default_region, "aws_secret_access_key": AwsSecretAccessKey(os.getenv("AWS_SECRET_ACCESS_KEY", "dummy_secret")), "dynamodb_endpoint": URL(os.getenv("DYNAMODB_ENDPOINT", "http://localhost:4566")), - "eligibility_table_name": eligibility_table_name, + "person_table_name": person_table_name, "s3_endpoint": URL(os.getenv("S3_ENDPOINT", "http://localhost:4566")), "rules_bucket_name": rules_bucket_name, "log_level": log_level, diff --git a/src/eligibility_signposting_api/repos/__init__.py b/src/eligibility_signposting_api/repos/__init__.py index 8f2aaa31..d66efe6c 100644 --- a/src/eligibility_signposting_api/repos/__init__.py +++ b/src/eligibility_signposting_api/repos/__init__.py @@ -1,5 +1,5 @@ -from .eligibility_repo import EligibilityRepo +from .campaign_repo import CampaignRepo from .exceptions import NotFoundError -from .rules_repo import RulesRepo +from .person_repo import PersonRepo -__all__ = ["EligibilityRepo", "NotFoundError", "RulesRepo"] +__all__ = ["CampaignRepo", "NotFoundError", "PersonRepo"] diff --git a/src/eligibility_signposting_api/repos/rules_repo.py b/src/eligibility_signposting_api/repos/campaign_repo.py similarity index 98% rename from src/eligibility_signposting_api/repos/rules_repo.py rename to src/eligibility_signposting_api/repos/campaign_repo.py index 8a9bb7e1..8a2a212f 100644 --- a/src/eligibility_signposting_api/repos/rules_repo.py +++ b/src/eligibility_signposting_api/repos/campaign_repo.py @@ -11,7 +11,7 @@ @service -class RulesRepo: +class CampaignRepo: """Repository class for Campaign Rules, which we can use to calculate a person's eligibility for vaccination. These rules are stored as JSON files in AWS S3.""" diff --git a/src/eligibility_signposting_api/repos/eligibility_repo.py b/src/eligibility_signposting_api/repos/person_repo.py similarity index 75% rename from src/eligibility_signposting_api/repos/eligibility_repo.py rename to src/eligibility_signposting_api/repos/person_repo.py index 15582d74..02d7c396 100644 --- a/src/eligibility_signposting_api/repos/eligibility_repo.py +++ b/src/eligibility_signposting_api/repos/person_repo.py @@ -13,25 +13,25 @@ TableName = NewType("TableName", str) -@service(qualifier="eligibility_table") -def eligibility_table_factory( +@service(qualifier="person_table") +def person_table_factory( dynamodb_resource: Annotated[ServiceResource, Inject(qualifier="dynamodb")], - eligibility_table_name: Annotated[TableName, Inject(param="eligibility_table_name")], + person_table_name: Annotated[TableName, Inject(param="person_table_name")], ) -> Any: - table = dynamodb_resource.Table(eligibility_table_name) # type: ignore[reportAttributeAccessIssue] - logger.info("eligibility_table %r", table, extra={"table": table}) + table = dynamodb_resource.Table(person_table_name) # type: ignore[reportAttributeAccessIssue] + logger.info("person_table %r", table, extra={"table": table}) return table @service -class EligibilityRepo: +class PersonRepo: """Repository class for the data held about a person which may be relevant to calculating their eligibility for vaccination. This data is held in a handful of records in a single Dynamodb table. """ - def __init__(self, table: Annotated[Any, Inject(qualifier="eligibility_table")]) -> None: + def __init__(self, table: Annotated[Any, Inject(qualifier="person_table")]) -> None: super().__init__() self.table = table diff --git a/src/eligibility_signposting_api/services/calculators/__init__.py b/src/eligibility_signposting_api/services/calculators/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py new file mode 100644 index 00000000..5cab648a --- /dev/null +++ b/src/eligibility_signposting_api/services/calculators/eligibility_calculator.py @@ -0,0 +1,251 @@ +from __future__ import annotations + +from _operator import attrgetter +from collections import defaultdict +from collections.abc import Collection, Iterator, Mapping +from dataclasses import dataclass, field +from functools import cached_property +from itertools import groupby +from typing import Any + +from hamcrest.core.string_description import StringDescription +from wireup import service + +from eligibility_signposting_api.model import eligibility, rules +from eligibility_signposting_api.services.rules.operators import OperatorRegistry + +Row = Collection[Mapping[str, Any]] + + +@service +class EligibilityCalculatorFactory: + @staticmethod + def get(person_data: Row, campaign_configs: Collection[rules.CampaignConfig]) -> EligibilityCalculator: + return EligibilityCalculator(person_data=person_data, campaign_configs=campaign_configs) + + +@dataclass +class EligibilityCalculator: + person_data: Row + campaign_configs: Collection[rules.CampaignConfig] + + results: dict[eligibility.ConditionName, eligibility.Condition] = field(default_factory=dict) + + @cached_property + def condition_names(self) -> set[eligibility.ConditionName]: + return { + eligibility.ConditionName(cc.target) + for cc in self.campaign_configs + if cc.campaign_live and cc.current_iteration + } + + def evaluate_eligibility(self) -> eligibility.EligibilityStatus: + """Calculate a person's eligibility for vaccination.""" + + # Get all iterations for which the person is base eligible, i.e. those which *might* provide eligibility + # due to cohort membership. + base_eligible_campaigns = self.get_base_eligible_campaigns() + + # Evaluate iteration rules to see if the person is actionable, not actionable (due to "F" rules), + # or not eligible (due to "S" rules") + evaluations = self.evaluate_for_base_eligible_campaigns(base_eligible_campaigns) + + # Add all not base eligible conditions to result set. + self.get_not_base_eligible_conditions(base_eligible_campaigns) + # Add all base eligible conditions to result set. + self.get_base_eligible_conditions(evaluations) + + return eligibility.EligibilityStatus(conditions=list(self.results.values())) + + def get_base_eligible_campaigns(self) -> list[rules.CampaignConfig]: + """Get all campaigns for which the person is base eligible, i.e. those which *might* provide eligibility. + + Build and return a collection of campaigns for which the person is base eligible (using cohorts). + Also build and return a set of conditions in the campaigns while we are here. + """ + base_eligible_campaigns: list[rules.CampaignConfig] = [] + + for campaign_config in (cc for cc in self.campaign_configs if cc.campaign_live and cc.current_iteration): + base_eligible = self.evaluate_base_eligibility(campaign_config.current_iteration) + if base_eligible: + base_eligible_campaigns.append(campaign_config) + + return base_eligible_campaigns + + def evaluate_base_eligibility(self, iteration: rules.Iteration | None) -> set[str]: + """Return cohorts for which person is base eligible.""" + if not iteration: + return set() + iteration_cohorts: set[str] = { + cohort.cohort_label for cohort in iteration.iteration_cohorts if cohort.cohort_label + } + + cohorts_row: Mapping[str, dict[str, dict[str, dict[str, Any]]]] = next( + (r for r in self.person_data if r.get("ATTRIBUTE_TYPE", "") == "COHORTS"), {} + ) + person_cohorts = set(cohorts_row.get("COHORT_MAP", {}).get("cohorts", {}).get("M", {}).keys()) + + return iteration_cohorts.intersection(person_cohorts) + + def get_not_base_eligible_conditions( + self, + base_eligible_campaigns: Collection[rules.CampaignConfig], + ) -> None: + """Get conditions where the person is not base eligible, + i.e. is not is the cohort for any campaign iteration.""" + + # for each condition: + # if the person isn't base eligible for any iteration, + # the person is not (base) eligible for the condition + for condition_name in self.condition_names: + if condition_name not in {eligibility.ConditionName(cc.target) for cc in base_eligible_campaigns}: + self.results[condition_name] = eligibility.Condition( + condition_name=condition_name, status=eligibility.Status.not_eligible, reasons=[] + ) + + def evaluate_for_base_eligible_campaigns( + self, base_eligible_campaigns: Collection[rules.CampaignConfig] + ) -> dict[eligibility.ConditionName, dict[eligibility.Status, list[eligibility.Reason]]]: + """Evaluate iteration rules to see if the person is actionable, not actionable (due to "F" rules), + or not eligible (due to "S" rules"). + + For each condition, evaluate all iterations for inclusion or exclusion.""" + priority_getter = attrgetter("priority") + + base_eligible_evaluations: dict[ + eligibility.ConditionName, dict[eligibility.Status, list[eligibility.Reason]] + ] = defaultdict(dict) + for condition_name, iteration in [ + (eligibility.ConditionName(cc.target), cc.current_iteration) + for cc in base_eligible_campaigns + if cc.current_iteration + ]: + # Until we see a worse status, we assume someone is actionable for this iteration. + worst_status_so_far_for_condition = eligibility.Status.actionable + exclusion_reasons, actionable_reasons = [], [] + for _priority, iteration_rule_group in groupby( + sorted(iteration.iteration_rules, key=priority_getter), key=priority_getter + ): + worst_status_so_far_for_condition, group_actionable_reasons, group_exclusion_reasons = ( + self.evaluate_priority_group(iteration_rule_group, worst_status_so_far_for_condition) + ) + actionable_reasons.extend(group_actionable_reasons) + exclusion_reasons.extend(group_exclusion_reasons) + condition_entry = base_eligible_evaluations.setdefault(condition_name, {}) + condition_status_entry = condition_entry.setdefault(worst_status_so_far_for_condition, []) + condition_status_entry.extend( + actionable_reasons + if worst_status_so_far_for_condition is eligibility.Status.actionable + else exclusion_reasons + ) + return base_eligible_evaluations + + def evaluate_priority_group( + self, + iteration_rule_group: Iterator[rules.IterationRule], + worst_status_so_far_for_condition: eligibility.Status, + ) -> tuple[eligibility.Status, list[eligibility.Reason], list[eligibility.Reason]]: + actionable_reasons, exclusion_reasons = [], [] + exclude_capable_rules = [ + ir for ir in iteration_rule_group if ir.type in (rules.RuleType.filter, rules.RuleType.suppression) + ] + best_status_so_far_for_priority_group = ( + eligibility.Status.not_eligible if exclude_capable_rules else eligibility.Status.actionable + ) + for iteration_rule in exclude_capable_rules: + exclusion, reason = self.evaluate_exclusion(iteration_rule) + if exclusion: + best_status_so_far_for_priority_group = self.best_status( + iteration_rule.type, best_status_so_far_for_priority_group + ) + exclusion_reasons.append(reason) + else: + best_status_so_far_for_priority_group = eligibility.Status.actionable + actionable_reasons.append(reason) + return ( + self.worst_status(best_status_so_far_for_priority_group, worst_status_so_far_for_condition), + actionable_reasons, + exclusion_reasons, + ) + + @staticmethod + def worst_status(*statuses: eligibility.Status) -> eligibility.Status: + """Pick the worst status from those given. + + Here "worst" means furthest from being able to access vaccination, so not-eligible is "worse" than + not-actionable, and not-actionable is "worse" than actionable. + """ + return min(statuses) + + @staticmethod + def best_status(rule_type: rules.RuleType, status: eligibility.Status) -> eligibility.Status: + """Pick the best status between the existing status, and the status implied by + the rule excluding the person from vaccination. + + Here "best" means closest to being able to access vaccination, so not-actionable is "better" than + not-eligible, and actionable is "better" than not-actionable. + """ + return max( + status, + eligibility.Status.not_eligible + if rule_type == rules.RuleType.filter + else eligibility.Status.not_actionable, + ) + + def get_base_eligible_conditions( + self, + base_eligible_evaluations: Mapping[ + eligibility.ConditionName, Mapping[eligibility.Status, list[eligibility.Reason]] + ], + ) -> None: + """Get conditions where the person is base eligible, but may be either actionable, not actionable, + or not eligible.""" + + # for each condition for which the person is base eligible: + # what is the "best" status, i.e. closest to actionable? Add the condition to the result with that status. + for condition_name, reasons_by_status in base_eligible_evaluations.items(): + best_status = max(reasons_by_status.keys()) + self.results[condition_name] = eligibility.Condition( + condition_name=condition_name, status=best_status, reasons=reasons_by_status[best_status] + ) + + def evaluate_exclusion(self, iteration_rule: rules.IterationRule) -> tuple[bool, eligibility.Reason]: + """Evaluate if a particular rule excludes this person. Return the result, and the reason for the result.""" + attribute_value = self.get_attribute_value(iteration_rule) + exclusion, reason = self.evaluate_rule(iteration_rule, attribute_value) + reason = eligibility.Reason( + rule_name=eligibility.RuleName(iteration_rule.name), + rule_type=eligibility.RuleType(iteration_rule.type), + rule_result=eligibility.RuleResult( + f"Rule {iteration_rule.name!r} ({iteration_rule.description!r}) " + f"{'' if exclusion else 'not '}excluding - " + f"{iteration_rule.attribute_name!r} {iteration_rule.comparator!r} {reason}" + ), + ) + return exclusion, reason + + def get_attribute_value(self, iteration_rule: rules.IterationRule) -> str | None: + """Pull out the correct attribute for a rule from the person's data.""" + match iteration_rule.attribute_level: + case rules.RuleAttributeLevel.PERSON: + person: Mapping[str, str | None] | None = next( + (r for r in self.person_data if r.get("ATTRIBUTE_TYPE", "") == "PERSON"), None + ) + attribute_value = person.get(iteration_rule.attribute_name) if person else None + case _: # pragma: no cover + msg = f"{iteration_rule.attribute_level} not implemented" + raise NotImplementedError(msg) + return attribute_value + + @staticmethod + def evaluate_rule(iteration_rule: rules.IterationRule, attribute_value: str | None) -> tuple[bool, str]: + """Evaluate a rule against a person data attribute. Return the result, and the reason for the result.""" + matcher_class = OperatorRegistry.get(iteration_rule.operator) + matcher = matcher_class(rule_value=iteration_rule.comparator) + + reason = StringDescription() + if matcher.matches(attribute_value): + matcher.describe_match(attribute_value, reason) + return True, str(reason) + matcher.describe_mismatch(attribute_value, reason) + return False, str(reason) diff --git a/src/eligibility_signposting_api/services/eligibility_services.py b/src/eligibility_signposting_api/services/eligibility_services.py index a4e5e2f8..9352226a 100644 --- a/src/eligibility_signposting_api/services/eligibility_services.py +++ b/src/eligibility_signposting_api/services/eligibility_services.py @@ -1,16 +1,10 @@ import logging -from collections import defaultdict -from collections.abc import Collection, Iterator, Mapping -from itertools import groupby -from operator import attrgetter -from typing import Any -from hamcrest.core.string_description import StringDescription from wireup import service -from eligibility_signposting_api.model import eligibility, rules -from eligibility_signposting_api.repos import EligibilityRepo, NotFoundError, RulesRepo -from eligibility_signposting_api.services.rules.operators import OperatorRegistry +from eligibility_signposting_api.model import eligibility +from eligibility_signposting_api.repos import CampaignRepo, NotFoundError, PersonRepo +from eligibility_signposting_api.services.calculators import eligibility_calculator as calculator logger = logging.getLogger(__name__) @@ -21,17 +15,23 @@ class UnknownPersonError(Exception): @service class EligibilityService: - def __init__(self, eligibility_repo: EligibilityRepo, rules_repo: RulesRepo) -> None: + def __init__( + self, + person_repo: PersonRepo, + campaign_repo: CampaignRepo, + calculator_factory: calculator.EligibilityCalculatorFactory, + ) -> None: super().__init__() - self.eligibility_repo = eligibility_repo - self.rules_repo = rules_repo + self.person_repo = person_repo + self.campaign_repo = campaign_repo + self.calculator_factory = calculator_factory def get_eligibility_status(self, nhs_number: eligibility.NHSNumber | None = None) -> eligibility.EligibilityStatus: """Calculate a person's eligibility for vaccination given an NHS number.""" if nhs_number: try: - person_data = self.eligibility_repo.get_eligibility_data(nhs_number) - campaign_configs = list(self.rules_repo.get_campaign_configs()) + person_data = self.person_repo.get_eligibility_data(nhs_number) + campaign_configs = list(self.campaign_repo.get_campaign_configs()) logger.debug( "got person_data for %r", nhs_number, @@ -44,247 +44,7 @@ def get_eligibility_status(self, nhs_number: eligibility.NHSNumber | None = None except NotFoundError as e: raise UnknownPersonError from e else: - return self.evaluate_eligibility(campaign_configs, person_data) + calc: calculator.EligibilityCalculator = self.calculator_factory.get(person_data, campaign_configs) + return calc.evaluate_eligibility() raise UnknownPersonError # pragma: no cover - - @staticmethod - def evaluate_eligibility( - campaign_configs: Collection[rules.CampaignConfig], person_data: Collection[Mapping[str, Any]] - ) -> eligibility.EligibilityStatus: - """Calculate a person's eligibility for vaccination.""" - - # Get all iterations for which the person is base eligible, i.e. those which *might* provide eligibility - # due to cohort membership. - base_eligible_campaigns, condition_names = EligibilityService.get_base_eligible_campaigns( - campaign_configs, person_data - ) - # Evaluate iteration rules to see if the person is actionable, not actionable (due to "F" rules), - # or not eligible (due to "S" rules") - evaluations = EligibilityService.evaluate_for_base_eligible_campaigns(base_eligible_campaigns, person_data) - - conditions: dict[eligibility.ConditionName, eligibility.Condition] = {} - # Add all not base eligible conditions to result set. - conditions |= EligibilityService.get_not_base_eligible_conditions(base_eligible_campaigns, condition_names) - # Add all base eligible conditions to result set. - conditions |= EligibilityService.get_base_eligible_conditions(evaluations) - - return eligibility.EligibilityStatus(conditions=list(conditions.values())) - - @staticmethod - def get_base_eligible_campaigns( - campaign_configs: Collection[rules.CampaignConfig], person_data: Collection[Mapping[str, Any]] - ) -> tuple[list[rules.CampaignConfig], set[eligibility.ConditionName]]: - """Get all campaigns for which the person is base eligible, i.e. those which *might* provide eligibility. - - Build and return a collection of campaigns for which the person is base eligible (using cohorts). - Also build and return a set of conditions in the campaigns while we are here. - """ - condition_names: set[eligibility.ConditionName] = set() - base_eligible_campaigns: list[rules.CampaignConfig] = [] - - for campaign_config in (cc for cc in campaign_configs if cc.campaign_live and cc.current_iteration): - condition_name = eligibility.ConditionName(campaign_config.target) - condition_names.add(condition_name) - base_eligible = EligibilityService.evaluate_base_eligibility(campaign_config.current_iteration, person_data) - if base_eligible: - base_eligible_campaigns.append(campaign_config) - - return base_eligible_campaigns, condition_names - - @staticmethod - def evaluate_base_eligibility( - iteration: rules.Iteration | None, person_data: Collection[Mapping[str, Any]] - ) -> set[str]: - """Return cohorts for which person is base eligible.""" - if not iteration: - return set() - iteration_cohorts: set[str] = { - cohort.cohort_label for cohort in iteration.iteration_cohorts if cohort.cohort_label - } - - cohorts_row: Mapping[str, dict[str, dict[str, dict[str, Any]]]] = next( - (r for r in person_data if r.get("ATTRIBUTE_TYPE", "") == "COHORTS"), {} - ) - person_cohorts = set(cohorts_row.get("COHORT_MAP", {}).get("cohorts", {}).get("M", {}).keys()) - - return iteration_cohorts.intersection(person_cohorts) - - @staticmethod - def get_not_base_eligible_conditions( - base_eligible_campaigns: Collection[rules.CampaignConfig], - condition_names: Collection[eligibility.ConditionName], - ) -> dict[eligibility.ConditionName, eligibility.Condition]: - """Get conditions where the person is not base eligible, - i.e. is not is the cohort for any campaign iteration.""" - - # for each condition: - # if the person isn't base eligible for any iteration, - # the person is not (base) eligible for the condition - not_eligible_conditions: dict[eligibility.ConditionName, eligibility.Condition] = {} - for condition_name in condition_names: - if condition_name not in {eligibility.ConditionName(cc.target) for cc in base_eligible_campaigns}: - not_eligible_conditions[condition_name] = eligibility.Condition( - condition_name=condition_name, status=eligibility.Status.not_eligible, reasons=[] - ) - return not_eligible_conditions - - @staticmethod - def evaluate_for_base_eligible_campaigns( - base_eligible_campaigns: Collection[rules.CampaignConfig], - person_data: Collection[Mapping[str, Any]], - ) -> dict[eligibility.ConditionName, dict[eligibility.Status, list[eligibility.Reason]]]: - """Evaluate iteration rules to see if the person is actionable, not actionable (due to "F" rules), - or not eligible (due to "S" rules"). - - For each condition, evaluate all iterations for inclusion or exclusion.""" - priority_getter = attrgetter("priority") - - base_eligible_evaluations: dict[ - eligibility.ConditionName, dict[eligibility.Status, list[eligibility.Reason]] - ] = defaultdict(dict) - for condition_name, iteration in [ - (eligibility.ConditionName(cc.target), cc.current_iteration) - for cc in base_eligible_campaigns - if cc.current_iteration - ]: - # Until we see a worse status, we assume someone is actionable for this iteration. - worst_status_so_far_for_condition = eligibility.Status.actionable - exclusion_reasons, actionable_reasons = [], [] - for _priority, iteration_rule_group in groupby( - sorted(iteration.iteration_rules, key=priority_getter), key=priority_getter - ): - worst_status_so_far_for_condition, group_actionable_reasons, group_exclusion_reasons = ( - EligibilityService.evaluate_priority_group( - iteration_rule_group, person_data, worst_status_so_far_for_condition - ) - ) - actionable_reasons.extend(group_actionable_reasons) - exclusion_reasons.extend(group_exclusion_reasons) - condition_entry = base_eligible_evaluations.setdefault(condition_name, {}) - condition_status_entry = condition_entry.setdefault(worst_status_so_far_for_condition, []) - condition_status_entry.extend( - actionable_reasons - if worst_status_so_far_for_condition is eligibility.Status.actionable - else exclusion_reasons - ) - return base_eligible_evaluations - - @staticmethod - def evaluate_priority_group( - iteration_rule_group: Iterator[rules.IterationRule], - person_data: Collection[Mapping[str, Any]], - worst_status_so_far_for_condition: eligibility.Status, - ) -> tuple[eligibility.Status, list[eligibility.Reason], list[eligibility.Reason]]: - actionable_reasons, exclusion_reasons = [], [] - exclude_capable_rules = [ - ir for ir in iteration_rule_group if ir.type in (rules.RuleType.filter, rules.RuleType.suppression) - ] - best_status_so_far_for_priority_group = ( - eligibility.Status.not_eligible if exclude_capable_rules else eligibility.Status.actionable - ) - for iteration_rule in exclude_capable_rules: - exclusion, reason = EligibilityService.evaluate_exclusion(iteration_rule, person_data) - if exclusion: - best_status_so_far_for_priority_group = EligibilityService.best_status( - iteration_rule.type, best_status_so_far_for_priority_group - ) - exclusion_reasons.append(reason) - else: - best_status_so_far_for_priority_group = eligibility.Status.actionable - actionable_reasons.append(reason) - return ( - EligibilityService.worst_status(best_status_so_far_for_priority_group, worst_status_so_far_for_condition), - actionable_reasons, - exclusion_reasons, - ) - - @staticmethod - def worst_status(*statuses: eligibility.Status) -> eligibility.Status: - """Pick the worst status from those given. - - Here "worst" means furthest from being able to access vaccination, so not-eligible is "worse" than - not-actionable, and not-actionable is "worse" than actionable. - """ - return min(statuses) - - @staticmethod - def best_status(rule_type: rules.RuleType, status: eligibility.Status) -> eligibility.Status: - """Pick the best status between the existing status, and the status implied by - the rule excluding the person from vaccination. - - Here "best" means closest to being able to access vaccination, so not-actionable is "better" than - not-eligible, and actionable is "better" than not-actionable. - """ - return max( - status, - eligibility.Status.not_eligible - if rule_type == rules.RuleType.filter - else eligibility.Status.not_actionable, - ) - - @staticmethod - def get_base_eligible_conditions( - base_eligible_evaluations: Mapping[ - eligibility.ConditionName, Mapping[eligibility.Status, list[eligibility.Reason]] - ], - ) -> dict[eligibility.ConditionName, eligibility.Condition]: - """Get conditions where the person is base eligible, but may be either actionable, not actionable, - or not eligible.""" - - # for each condition for which the person is base eligible: - # what is the "best" status, i.e. closest to actionable? Add the condition to the result with that status. - eligible_conditions: dict[eligibility.ConditionName, eligibility.Condition] = {} - for condition_name, reasons_by_status in base_eligible_evaluations.items(): - best_status = max(reasons_by_status.keys()) - eligible_conditions[condition_name] = eligibility.Condition( - condition_name=condition_name, status=best_status, reasons=reasons_by_status[best_status] - ) - return eligible_conditions - - @staticmethod - def evaluate_exclusion( - iteration_rule: rules.IterationRule, person_data: Collection[Mapping[str, str | None]] - ) -> tuple[bool, eligibility.Reason]: - """Evaluate if a particular rule excludes this person. Return the result, and the reason for the result.""" - attribute_value = EligibilityService.get_attribute_value(iteration_rule, person_data) - exclusion, reason = EligibilityService.evaluate_rule(iteration_rule, attribute_value) - reason = eligibility.Reason( - rule_name=eligibility.RuleName(iteration_rule.name), - rule_type=eligibility.RuleType(iteration_rule.type), - rule_result=eligibility.RuleResult( - f"Rule {iteration_rule.name!r} ({iteration_rule.description!r}) " - f"{'' if exclusion else 'not '}excluding - " - f"{iteration_rule.attribute_name!r} {iteration_rule.comparator!r} {reason}" - ), - ) - return exclusion, reason - - @staticmethod - def get_attribute_value( - iteration_rule: rules.IterationRule, person_data: Collection[Mapping[str, str | None]] - ) -> str | None: - """Pull out the correct attribute for a rule from the person's data.""" - match iteration_rule.attribute_level: - case rules.RuleAttributeLevel.PERSON: - person: Mapping[str, str | None] | None = next( - (r for r in person_data if r.get("ATTRIBUTE_TYPE", "") == "PERSON"), None - ) - attribute_value = person.get(iteration_rule.attribute_name) if person else None - case _: # pragma: no cover - msg = f"{iteration_rule.attribute_level} not implemented" - raise NotImplementedError(msg) - return attribute_value - - @staticmethod - def evaluate_rule(iteration_rule: rules.IterationRule, attribute_value: str | None) -> tuple[bool, str]: - """Evaluate a rule against a person data attribute. Return the result, and the reason for the result.""" - matcher_class = OperatorRegistry.get(iteration_rule.operator) - matcher = matcher_class(rule_value=iteration_rule.comparator) - - reason = StringDescription() - if matcher.matches(attribute_value): - matcher.describe_match(attribute_value, reason) - return True, str(reason) - matcher.describe_mismatch(attribute_value, reason) - return False, str(reason) diff --git a/tests/fixtures/builders/repos/eligibility.py b/tests/fixtures/builders/repos/person.py similarity index 98% rename from tests/fixtures/builders/repos/eligibility.py rename to tests/fixtures/builders/repos/person.py index 191fb6c1..c700be9c 100644 --- a/tests/fixtures/builders/repos/eligibility.py +++ b/tests/fixtures/builders/repos/person.py @@ -7,7 +7,7 @@ from eligibility_signposting_api.model import eligibility -def eligibility_rows_builder( +def person_rows_builder( nhs_number: eligibility.NHSNumber, *, date_of_birth: eligibility.DateOfBirth | None = None, diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 33929af6..250d8f62 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -17,10 +17,10 @@ from yarl import URL from eligibility_signposting_api.model import eligibility, rules -from eligibility_signposting_api.repos.eligibility_repo import TableName -from eligibility_signposting_api.repos.rules_repo import BucketName +from eligibility_signposting_api.repos.campaign_repo import BucketName +from eligibility_signposting_api.repos.person_repo import TableName from tests.fixtures.builders.model import rule -from tests.fixtures.builders.repos.eligibility import eligibility_rows_builder +from tests.fixtures.builders.repos.person import person_rows_builder if TYPE_CHECKING: from pytest_docker.plugin import Services @@ -200,9 +200,9 @@ def wait_for_function_active(function_name, lambda_client): @pytest.fixture(scope="session") -def eligibility_table(dynamodb_resource: ServiceResource) -> Generator[Any]: +def person_table(dynamodb_resource: ServiceResource) -> Generator[Any]: table = dynamodb_resource.create_table( - TableName=TableName(os.getenv("ELIGIBILITY_TABLE_NAME", "test_eligibility_datastore")), + TableName=TableName(os.getenv("PERSON_TABLE_NAME", "test_eligibility_datastore")), KeySchema=[ {"AttributeName": "NHS_NUMBER", "KeyType": "HASH"}, {"AttributeName": "ATTRIBUTE_TYPE", "KeyType": "RANGE"}, @@ -219,33 +219,31 @@ def eligibility_table(dynamodb_resource: ServiceResource) -> Generator[Any]: @pytest.fixture -def persisted_person(eligibility_table: Any, faker: Faker) -> Generator[eligibility.NHSNumber]: +def persisted_person(person_table: Any, faker: Faker) -> Generator[eligibility.NHSNumber]: nhs_number = eligibility.NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = eligibility.DateOfBirth(faker.date_of_birth(minimum_age=18, maximum_age=65)) - for row in (rows := eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"])): - eligibility_table.put_item(Item=row) + for row in (rows := person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"])): + person_table.put_item(Item=row) yield nhs_number for row in rows: - eligibility_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) + person_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) @pytest.fixture -def persisted_77yo_person(eligibility_table: Any, faker: Faker) -> Generator[eligibility.NHSNumber]: +def persisted_77yo_person(person_table: Any, faker: Faker) -> Generator[eligibility.NHSNumber]: nhs_number = eligibility.NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = eligibility.DateOfBirth(faker.date_of_birth(minimum_age=77, maximum_age=77)) - for row in ( - rows := eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1", "cohort2"]) - ): - eligibility_table.put_item(Item=row) + for row in (rows := person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1", "cohort2"])): + person_table.put_item(Item=row) yield nhs_number for row in rows: - eligibility_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) + person_table.delete_item(Key={"NHS_NUMBER": row["NHS_NUMBER"], "ATTRIBUTE_TYPE": row["ATTRIBUTE_TYPE"]}) @pytest.fixture(scope="session") diff --git a/tests/integration/repo/test_rules_repo.py b/tests/integration/repo/test_campaign_repo.py similarity index 92% rename from tests/integration/repo/test_rules_repo.py rename to tests/integration/repo/test_campaign_repo.py index da69b97d..accc9082 100644 --- a/tests/integration/repo/test_rules_repo.py +++ b/tests/integration/repo/test_campaign_repo.py @@ -6,7 +6,7 @@ from hamcrest import assert_that, has_item from eligibility_signposting_api.model.rules import CampaignConfig -from eligibility_signposting_api.repos.rules_repo import BucketName, RulesRepo +from eligibility_signposting_api.repos.campaign_repo import BucketName, CampaignRepo from tests.fixtures.builders.model.rule import CampaignConfigFactory from tests.fixtures.matchers.rules import is_campaign_config, is_iteration, is_iteration_rule @@ -24,7 +24,7 @@ def campaign_config(s3_client: BaseClient, bucket: BucketName) -> Generator[Camp def test_get_campaign_config(s3_client: BaseClient, bucket: BucketName, campaign_config: CampaignConfig): # Given - repo = RulesRepo(s3_client, bucket) + repo = CampaignRepo(s3_client, bucket) # When actual = list(repo.get_campaign_configs()) diff --git a/tests/integration/repo/test_eligibility_repo.py b/tests/integration/repo/test_person_repo.py similarity index 76% rename from tests/integration/repo/test_eligibility_repo.py rename to tests/integration/repo/test_person_repo.py index 15ece5c0..69b63f99 100644 --- a/tests/integration/repo/test_eligibility_repo.py +++ b/tests/integration/repo/test_person_repo.py @@ -6,12 +6,12 @@ from eligibility_signposting_api.model.eligibility import NHSNumber from eligibility_signposting_api.repos import NotFoundError -from eligibility_signposting_api.repos.eligibility_repo import EligibilityRepo +from eligibility_signposting_api.repos.person_repo import PersonRepo -def test_person_found(eligibility_table: Any, persisted_person: NHSNumber): +def test_person_found(person_table: Any, persisted_person: NHSNumber): # Given - repo = EligibilityRepo(eligibility_table) + repo = PersonRepo(person_table) # When actual = repo.get_eligibility_data(persisted_person) @@ -28,10 +28,10 @@ def test_person_found(eligibility_table: Any, persisted_person: NHSNumber): ) -def test_person_not_found(eligibility_table: Any, faker: Faker): +def test_person_not_found(person_table: Any, faker: Faker): # Given nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") - repo = EligibilityRepo(eligibility_table) + repo = PersonRepo(person_table) # When, Then with pytest.raises(NotFoundError): diff --git a/tests/unit/services/calculators/__init__.py b/tests/unit/services/calculators/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/services/calculators/test_eligibility_calculator.py b/tests/unit/services/calculators/test_eligibility_calculator.py new file mode 100644 index 00000000..89540c09 --- /dev/null +++ b/tests/unit/services/calculators/test_eligibility_calculator.py @@ -0,0 +1,40 @@ +from faker import Faker +from hamcrest import assert_that, has_item + +from eligibility_signposting_api.model.eligibility import ConditionName, NHSNumber, Status +from eligibility_signposting_api.services.calculators.eligibility_calculator import EligibilityCalculator +from tests.fixtures.builders.model import rule as rule_builder +from tests.fixtures.builders.repos.person import person_rows_builder +from tests.fixtures.matchers.eligibility import is_condition, is_eligibility_status + + +def test_not_base_eligible(faker: Faker): + # Given + nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") + + person_rows = person_rows_builder(nhs_number, cohorts=["cohort1"]) + campaign_configs = [ + ( + rule_builder.CampaignConfigFactory.build( + target="RSV", + iterations=[ + rule_builder.IterationFactory.build( + iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort2")] + ) + ], + ) + ) + ] + + calculator = EligibilityCalculator(person_rows, campaign_configs) + + # When + actual = calculator.evaluate_eligibility() + + # Then + assert_that( + actual, + is_eligibility_status().with_conditions( + has_item(is_condition().with_condition_name(ConditionName("RSV")).and_status(Status.not_eligible)) + ), + ) diff --git a/tests/unit/services/test_eligibility_services.py b/tests/unit/services/test_eligibility_services.py index 06c0ad3d..4137a839 100644 --- a/tests/unit/services/test_eligibility_services.py +++ b/tests/unit/services/test_eligibility_services.py @@ -6,19 +6,13 @@ from freezegun import freeze_time from hamcrest import assert_that, contains_exactly, empty, has_item, has_items +from eligibility_signposting_api.model import rules as rules_model from eligibility_signposting_api.model.eligibility import ConditionName, DateOfBirth, NHSNumber, Postcode, Status -from eligibility_signposting_api.model.rules import ( - CampaignConfig, - IterationDate, - IterationRule, - RuleComparator, - RulePriority, - RuleType, -) -from eligibility_signposting_api.repos import EligibilityRepo, NotFoundError, RulesRepo +from eligibility_signposting_api.repos import CampaignRepo, NotFoundError, PersonRepo from eligibility_signposting_api.services import EligibilityService, UnknownPersonError +from eligibility_signposting_api.services.calculators.eligibility_calculator import EligibilityCalculatorFactory from tests.fixtures.builders.model import rule as rule_builder -from tests.fixtures.builders.repos.eligibility import eligibility_rows_builder +from tests.fixtures.builders.repos.person import person_rows_builder from tests.fixtures.matchers.eligibility import is_condition, is_eligibility_status @@ -29,10 +23,10 @@ def faker() -> Faker: def test_eligibility_service_returns_from_repo(): # Given - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility = MagicMock(return_value=[]) - service = EligibilityService(eligibility_repo, rules_repo) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility = MagicMock(return_value=[]) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber("1234567890")) @@ -43,10 +37,10 @@ def test_eligibility_service_returns_from_repo(): def test_eligibility_service_for_nonexistent_nhs_number(): # Given - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock(side_effect=NotFoundError) - service = EligibilityService(eligibility_repo, rules_repo) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock(side_effect=NotFoundError) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When with pytest.raises(UnknownPersonError): @@ -57,13 +51,11 @@ def test_not_base_eligible(faker: Faker): # Given nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, cohorts=["cohort1"]) - ) - rules_repo.get_campaign_configs = MagicMock( + person_repo.get_eligibility_data = MagicMock(return_value=person_rows_builder(nhs_number, cohorts=["cohort1"])) + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", @@ -76,7 +68,7 @@ def test_not_base_eligible(faker: Faker): ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -95,12 +87,10 @@ def test_only_live_campaigns_considered(faker: Faker): # Given nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, cohorts=["cohort1"]) - ) - rules_repo.get_campaign_configs = MagicMock( + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock(return_value=person_rows_builder(nhs_number, cohorts=["cohort1"])) + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( name="Live", @@ -130,7 +120,7 @@ def test_only_live_campaigns_considered(faker: Faker): ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -149,12 +139,12 @@ def test_base_eligible_and_simple_rule_includes(faker: Faker): nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=76, maximum_age=79)) - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock( + return_value=person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) ) - rules_repo.get_campaign_configs = MagicMock( + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", @@ -168,7 +158,7 @@ def test_base_eligible_and_simple_rule_includes(faker: Faker): ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -187,12 +177,12 @@ def test_base_eligible_but_simple_rule_excludes(faker: Faker): nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=18, maximum_age=74)) - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock( + return_value=person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) ) - rules_repo.get_campaign_configs = MagicMock( + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", @@ -206,7 +196,7 @@ def test_base_eligible_but_simple_rule_excludes(faker: Faker): ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -226,12 +216,12 @@ def test_simple_rule_only_excludes_from_live_iteration(faker: Faker): nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=66, maximum_age=74)) - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock( + return_value=person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) ) - rules_repo.get_campaign_configs = MagicMock( + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", @@ -259,7 +249,7 @@ def test_simple_rule_only_excludes_from_live_iteration(faker: Faker): ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -278,23 +268,23 @@ def test_campaign_with_no_active_iteration_not_considered(faker: Faker): # Given nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock(return_value=eligibility_rows_builder(nhs_number)) - rules_repo.get_campaign_configs = MagicMock( + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock(return_value=person_rows_builder(nhs_number)) + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", iterations=[ rule_builder.IterationFactory.build( - iteration_date=IterationDate(datetime.date(2025, 4, 26)), + iteration_date=rules_model.IterationDate(datetime.date(2025, 4, 26)), ) ], ) ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -306,22 +296,22 @@ def test_campaign_with_no_active_iteration_not_considered(faker: Faker): @pytest.mark.parametrize( ("rule_type", "expected_status"), [ - (RuleType.suppression, Status.not_actionable), - (RuleType.filter, Status.not_eligible), - (RuleType.redirect, Status.actionable), + (rules_model.RuleType.suppression, Status.not_actionable), + (rules_model.RuleType.filter, Status.not_eligible), + (rules_model.RuleType.redirect, Status.actionable), ], ) -def test_rule_types_cause_correct_statuses(rule_type: RuleType, expected_status: Status, faker: Faker): +def test_rule_types_cause_correct_statuses(rule_type: rules_model.RuleType, expected_status: Status, faker: Faker): # Given nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=18, maximum_age=74)) - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock( + return_value=person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) ) - rules_repo.get_campaign_configs = MagicMock( + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", @@ -335,7 +325,7 @@ def test_rule_types_cause_correct_statuses(rule_type: RuleType, expected_status: ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -354,12 +344,12 @@ def test_multiple_rule_types_cause_correct_status(faker: Faker): nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=18, maximum_age=74)) - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock( + return_value=person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) ) - rules_repo.get_campaign_configs = MagicMock( + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", @@ -367,13 +357,13 @@ def test_multiple_rule_types_cause_correct_status(faker: Faker): rule_builder.IterationFactory.build( iteration_rules=[ rule_builder.PersonAgeSuppressionRuleFactory.build( - priority=RulePriority(5), type=RuleType.suppression + priority=rules_model.RulePriority(5), type=rules_model.RuleType.suppression ), rule_builder.PersonAgeSuppressionRuleFactory.build( - priority=RulePriority(10), type=RuleType.filter + priority=rules_model.RulePriority(10), type=rules_model.RuleType.filter ), rule_builder.PersonAgeSuppressionRuleFactory.build( - priority=RulePriority(15), type=RuleType.suppression + priority=rules_model.RulePriority(15), type=rules_model.RuleType.suppression ), ], iteration_cohorts=[rule_builder.IterationCohortFactory.build(cohort_label="cohort1")], @@ -383,7 +373,7 @@ def test_multiple_rule_types_cause_correct_status(faker: Faker): ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -402,65 +392,69 @@ def test_multiple_rule_types_cause_correct_status(faker: Faker): [ ( "two rules, both exclude, same priority, should exclude", - rule_builder.PersonAgeSuppressionRuleFactory.build(priority=RulePriority(5)), - rule_builder.PostcodeSuppressionRuleFactory.build(priority=RulePriority(5)), + rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)), + rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)), Status.not_actionable, ), ( "two rules, rule 1 excludes, same priority, should allow", - rule_builder.PersonAgeSuppressionRuleFactory.build(priority=RulePriority(5)), + rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)), rule_builder.PostcodeSuppressionRuleFactory.build( - priority=RulePriority(5), comparator=RuleComparator("NW1") + priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("NW1") ), Status.actionable, ), ( "two rules, rule 2 excludes, same priority, should allow", rule_builder.PersonAgeSuppressionRuleFactory.build( - priority=RulePriority(5), comparator=RuleComparator("-65") + priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("-65") ), - rule_builder.PostcodeSuppressionRuleFactory.build(priority=RulePriority(5)), + rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)), Status.actionable, ), ( "two rules, rule 1 excludes, different priority, should exclude", - rule_builder.PersonAgeSuppressionRuleFactory.build(priority=RulePriority(5)), + rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)), rule_builder.PostcodeSuppressionRuleFactory.build( - priority=RulePriority(10), comparator=RuleComparator("NW1") + priority=rules_model.RulePriority(10), comparator=rules_model.RuleComparator("NW1") ), Status.not_actionable, ), ( "two rules, rule 2 excludes, different priority, should exclude", rule_builder.PersonAgeSuppressionRuleFactory.build( - priority=RulePriority(5), comparator=RuleComparator("-65") + priority=rules_model.RulePriority(5), comparator=rules_model.RuleComparator("-65") ), - rule_builder.PostcodeSuppressionRuleFactory.build(priority=RulePriority(10)), + rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(10)), Status.not_actionable, ), ( "two rules, both excludes, different priority, should exclude", - rule_builder.PersonAgeSuppressionRuleFactory.build(priority=RulePriority(5)), - rule_builder.PostcodeSuppressionRuleFactory.build(priority=RulePriority(10)), + rule_builder.PersonAgeSuppressionRuleFactory.build(priority=rules_model.RulePriority(5)), + rule_builder.PostcodeSuppressionRuleFactory.build(priority=rules_model.RulePriority(10)), Status.not_actionable, ), ], ) def test_rules_with_same_priority_must_all_match_to_exclude( - test_comment: str, rule1: IterationRule, rule2: IterationRule, expected_status: Status, faker: Faker + test_comment: str, + rule1: rules_model.IterationRule, + rule2: rules_model.IterationRule, + expected_status: Status, + faker: Faker, ): # Given nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=66, maximum_age=74)) - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder( + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) + person_repo.get_eligibility_data = MagicMock( + return_value=person_rows_builder( nhs_number, date_of_birth=date_of_birth, postcode=Postcode("SW19 2BH"), cohorts=["cohort1"] ) ) - rules_repo.get_campaign_configs = MagicMock( + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", @@ -474,7 +468,7 @@ def test_rules_with_same_priority_must_all_match_to_exclude( ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -494,13 +488,13 @@ def test_multiple_conditions(faker: Faker): nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=76, maximum_age=78)) - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) + person_repo.get_eligibility_data = MagicMock( + return_value=person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) ) - rules_repo.get_campaign_configs = MagicMock( + campaign_repo.get_campaign_configs = MagicMock( return_value=[ rule_builder.CampaignConfigFactory.build( target="RSV", @@ -523,7 +517,7 @@ def test_multiple_conditions(faker: Faker): ] ) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) @@ -588,21 +582,21 @@ def test_multiple_conditions(faker: Faker): ], ) def test_multiple_campaigns_for_single_condition( - test_comment: str, campaign1: CampaignConfig, campaign2: CampaignConfig, faker: Faker + test_comment: str, campaign1: rules_model.CampaignConfig, campaign2: rules_model.CampaignConfig, faker: Faker ): # Given nhs_number = NHSNumber(f"5{faker.random_int(max=999999999):09d}") date_of_birth = DateOfBirth(faker.date_of_birth(minimum_age=76, maximum_age=78)) - eligibility_repo = MagicMock(spec=EligibilityRepo) - rules_repo = MagicMock(spec=RulesRepo) + person_repo = MagicMock(spec=PersonRepo) + campaign_repo = MagicMock(spec=CampaignRepo) - eligibility_repo.get_eligibility_data = MagicMock( - return_value=eligibility_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) + person_repo.get_eligibility_data = MagicMock( + return_value=person_rows_builder(nhs_number, date_of_birth=date_of_birth, cohorts=["cohort1"]) ) - rules_repo.get_campaign_configs = MagicMock(return_value=[campaign1, campaign2]) + campaign_repo.get_campaign_configs = MagicMock(return_value=[campaign1, campaign2]) - service = EligibilityService(eligibility_repo, rules_repo) + service = EligibilityService(person_repo, campaign_repo, EligibilityCalculatorFactory()) # When actual = service.get_eligibility_status(NHSNumber(nhs_number)) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 20da43fb..f2de9734 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -4,8 +4,8 @@ from yarl import URL from eligibility_signposting_api.config import LOG_LEVEL, AwsAccessKey, AwsRegion, AwsSecretAccessKey, config -from eligibility_signposting_api.repos.eligibility_repo import TableName -from eligibility_signposting_api.repos.rules_repo import BucketName +from eligibility_signposting_api.repos.campaign_repo import BucketName +from eligibility_signposting_api.repos.person_repo import TableName @pytest.fixture(autouse=True) @@ -27,7 +27,7 @@ def test_config_with_env_variable(monkeypatch): assert config_data_with_env["aws_secret_access_key"] is None assert config_data_with_env["aws_default_region"] == AwsRegion("eu-west-1") assert config_data_with_env["dynamodb_endpoint"] is None - assert config_data_with_env["eligibility_table_name"] == TableName("test_eligibility_datastore") + assert config_data_with_env["person_table_name"] == TableName("test_eligibility_datastore") assert config_data_with_env["s3_endpoint"] is None assert config_data_with_env["rules_bucket_name"] == BucketName("test-rules-bucket") assert config_data_with_env["log_level"] == LOG_LEVEL @@ -44,7 +44,7 @@ def test_config_without_env_variable(): assert config_data_without_env["aws_secret_access_key"] == AwsSecretAccessKey("dummy_secret") assert config_data_without_env["aws_default_region"] == AwsRegion("eu-west-1") assert config_data_without_env["dynamodb_endpoint"] == URL("http://localhost:4566") - assert config_data_without_env["eligibility_table_name"] == TableName("test_eligibility_datastore") + assert config_data_without_env["person_table_name"] == TableName("test_eligibility_datastore") assert config_data_without_env["s3_endpoint"] == URL("http://localhost:4566") assert config_data_without_env["rules_bucket_name"] == BucketName("test-rules-bucket") assert config_data_without_env["log_level"] == LOG_LEVEL