Add details of breast implants or augmentation form

[swebberuk]

Added "Add breast augmentation history" page which can be accessed from the "Record medical information" page.

Ready for someone to review. I've added some questions that may need to be resolved before can be merged.

Description

https://pr-742.manage-breast-screening.non-live.screening.nhs.uk/mammograms/874758d2-3bfe-4374-911d-570b7d0f3616/record-medical-information/breast-augmentation-history/

Jira link

https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11447

Review notes

Review checklist

• Check database queries are correctly scoped to current_provider

[swebberuk]

Do we need implants_have_been_removed or can we determine that from checking if removal_year is not null?

For implanted medical devices we only store removal_year, not a separate boolean to flag if they've been removed.

Whichever approach we prefer, we should probably be consistent for both "breast augmentation" and "implanted medical devices".

[MatMoore]

Does checking yes require a year to be entered? I'm wondering if there is a case where the participant reports the implants were removed but doesn't know the year. In that case just storing null for the year would lose that information.

[swebberuk]

For right_breast_procedures and left_breast_procedures should we use ExcludesOtherOptionsValidator to enforce that if NO_PROCEDURES is selected then no other option can be? e.g.:

validators=[
  ExcludesOtherOptionsValidator(
    Procedure.NO_PROCEDURES.value, Procedure.NO_PROCEDURES.label
  )
],

Reason I didn't add it is that wasn't sure if it would be needed:

1. This restriction is currently being enforced using exclusive_choices on the MultipleChoiceField.
2. The ExcludesOtherOptionsValidator doesn't get enforced with how the save operation is currently implemented. This validation is enforced if I do breast_augmentation_history.full_clean().

[MatMoore]

I was thinking we should be calling full_clean() when we save the form though, because there are other model-level validations that can be set, and we're currently ignoring them on all our forms. For example validation of the blank param and any min/max constraints we decide to set at model level.

Django's ModelForm (which we're not using) propagates all these errors back to the form. We could decide to handle this differently in our forms but I was thinking of doing the same thing in our forms. We just need to be careful of not duplicating error messages if there are redundant validations in place.

If full_clean() is being called then I think validating on the model does makes sense because the validation is still relevant even if data is being entered in other ways. On the other hand we still need exclusive_choices at the form level in order to get the client side validation.

[swebberuk]

Have pushed change which specifies ExcludesOtherOptionsValidator for right_breast_procedures and left_breast_procedures. Have not made any changes to start calling full_clean().

[swebberuk]

Should procedure_year and removal_year have blank=True as well as null=True? Without it then when I do breast_augmentation_history.full_clean() I get This field cannot be blank., even though null=True. As the save operation doesn't call full_clean then it is not a problem for the current implementation.

[MatMoore]

Yes I think so.

[swebberuk]

Pushed change which has added blank=True to procedure_year and removal_year.

[swebberuk]

This year validation is the same as is used for "implanted medical devices".

I was going to try to share this logic by adding a form_utils that contained:

import datetime

MAX_YEAR_AGE = 80


def get_year_range():
    max_year = datetime.date.today().year
    min_year = max_year - MAX_YEAR_AGE
    return min_year, max_year


def build_year_range_messages(min_year, max_year):
    year_outside_range_error_message = (
        f"Year should be between {min_year} and {max_year}."
    )
    return {
        "min_value": year_outside_range_error_message,
        "max_value": year_outside_range_error_message,
        "invalid": "Enter year as a number.",
    }

But I now think it would be better to wait for the DTOSS-11451 "benign lump form" changes to be merged and then use the YearField introduced by that: https://github.com/NHSDigital/dtos-manage-breast-screening/pull/739/files#diff-1f1677042e1017ce081317c0d921d614917032f04daa9edd2f5cf4dcf56be807R40 Once that has been merged I can go back and update this form, and ImplantedMedicalDeviceHistoryForm, to use YearField.

[edwardhorsford]

Per my comment on slack, it would probably be good to look at these year validation messages. None are specific to any one form though. That could come as a later ticket though.

We'd probably want more tightly scoped ones like Year cannot be in the future rather than specifying 2026 explicitly.

[MatMoore]

Yep sounds good. I'll do the same for my ticket.

[swebberuk]

Okay, as discussed on Slack, have made the removal year optional and updated text to be "Year removed (if available)".

[swebberuk]

As "Year removed" is not required, even when "Implants have been removed" has been checked, we can no longer do:

self.given_field_value(
  "removal_status", RemovalStatusChoices.HAS_BEEN_REMOVED
).require_field("removal_year")

Therefore have added extra logic to clean to set removal_year to None if the checkbox has not been checked. This deals with the scenario where user checks the checkbox, enters a removal year and then unchecks the checkbox.

[swebberuk]

Removed the default value for implants_have_been_removed, as felt safer to force it to be specified on creation.

[github-actions]

The review app at this URL has been deleted:
https://pr-742.manage-breast-screening.non-live.screening.nhs.uk

[MatMoore]

Looks good, I would just recommend switching to BooleanField for the lone checkbox.