Add option to proceed with appointment

[swebberuk]

Provide option to proceed with appointment even when participant has had previous mammogram in last six months.

Description

Added reason_for_continuing column to participants_participantreportedmammogram table.

Added "Proceed with this appointment" link to the "This appointment should not proceed" page. Clicking on link takes user to new page:

Added "Reason for continuing" to Mammogram history:

Jira link

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

Review notes

Review checklist

• Check database queries are correctly scoped to current_provider

[colinrotherham]

You might have noticed HTML5 in-browser form validation sometimes pops up?

We need to make sure the novalidate boolean attributes appears on every form

(Because we do the validation ourselves, and it might conflict)

Suggested change

	<form action="{{ url('mammograms:attended_not_screened', kwargs={'appointment_pk': presented_appointment.pk}) }}" method="post">
	<form action="{{ url('mammograms:attended_not_screened', kwargs={'appointment_pk': presented_appointment.pk}) }}" method="post" novalidate>

[edwardhorsford]

Is there some way we can automate that certain attributes are populated on all forms? I think we're likely to forget this sometimes if they need to be manually applied.

[MatMoore]

Couple of ideas:

• we could hook something into our accessibility baseline check in the playwright tests
• we could create a form_tag macro, and import it from the form layout

Would suggest creating another ticket for this though

[malcolmbaig]

Looks good — some feedback left inline.

[malcolmbaig]

I think it's worth declaring form fields higher up at the class-level (eg — like we do in SymptomForm, LumpForm, etc), unless there's a reason not to. The fields syntax in the init block seems to work just fine, but I think:

• The class-level declaration is more idiomatic
• It's probably better for things like introspection and IDE tooling
• field logic in the init function is typically expected when there's some dynamic behaviour or customisation needed for a given field

[malcolmbaig]

Building on this — could this form be implemented as a ModelForm? If so, benefits would be:

• your model and fields declarations would make it clear at a glance that this is updating a ParticipantReportedMammogram, and only its reason_for_continuing field is affected.
• you get the save function for free and don't need to write your own update

[swebberuk]

I've moved the fields outside of the init function. Thanks.

I hadn't considered using ModelForm. I can look into using it if we feel that would be best.

The reason I used the update function is because the associated view inherits from UpdateWithAuditView. The form_valid function of UpdateWithAuditView calls form.update(). The reason I used UpdateWithAuditView is that it ensures an audit record is created and a success message is shown on redirect. Wanted to be consistent with other parts of the app.

[malcolmbaig]

I think we can skip these steps, given the focus of this scenario is the proceed anyway functionality. We test the edit functionality already in the test case above.

Additionally, I think a good change would be to move those steps related to the edit page to the test_adding_a_mammogram_at_the_same_provider scenario. It's a fairly generic bit of navigation that isn't specifically tied to "adding a mammogram within the last six months".

[swebberuk]

I agree we can skip those steps, as already tested elsewhere. I've removed them from test_adding_a_mammogram_within_last_six_months_proceed_anyway. Thanks.

I've left those steps in test_adding_a_mammogram_within_last_six_months_do_not_proceed. It is tied to "adding a mammogram within the last six months". when_i_click_edit_previous_mammogram_details clicks the "Edit previous mammogram details" link that appears on the "This appointment should not proceed" page. That page is only shown when the mammogram is within the last six months.

[MatMoore]

If it's not too much trouble, can you squash those code review commits into the commits they're amending? Just leaves us with a cleaner git history for anyone looking back on old commits.

Other than that everything looks good to me.

As discussed offline, ModelForm is good for simple CRUD code, though in other places I think we decided against using it as we were overriding a lot of the default behaviour. Happy to go with either approach here though.

If we use ModelForm:

• the built-in CreateView, UpdateView, DeleteView will all work with it
• our Create/Update/DeleteWithAuditView classes would need changing, perhaps by moving to save methods for all forms
• the model's clean methods would be called automatically, which we're missing at the moment

I never got around to customising the templates for ModelChoiceField and ModelMultipleChoiceField like I did the other field classes, so if we do make use of ModelForm we might want to wire those up to the design system in the same way.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud