-
Notifications
You must be signed in to change notification settings - Fork 4
Add mastectomy or lumpectomy history form #754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bd63fb1 to
c657e2a
Compare
b2d3bdc to
441eb64
Compare
| classes="nhsuk-input--width-4", | ||
| min_value=datetime.date.today().year - 80, | ||
| max_value=datetime.date.today().year, | ||
| error_messages={ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will replace this with YearField once #739 has been merged, so won't need to specify error messages.
441eb64 to
ee4eeb9
Compare
| {% block form %} | ||
| <p> | ||
| This is designed to capture information on prophylactic or elective surgery. | ||
| <a href="../breast-cancer-history/" class="nhsuk-link"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't know if href="../breast-cancer-history/" is the best way to refer to the "Add details of breast cancer" page? Should I be using medical_information_presenter.py add_breast_cancer_history_link instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think that would be better. The method is already implemented, and looking up all the routes via reverse means the tests will check that the links are actually going somewhere.
ee4eeb9 to
64f1894
Compare
MatMoore
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just a couple of minor things
|
|
||
| assert form.is_valid() | ||
|
|
||
| existing_log_count = AuditLog.objects.count() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is doing quite a lot of verification... I'm wondering if it's worth doing quite so much querying of the database state in our tests as it's going to slow down the runtime of the test suite.
I previously added a similar test to this audit log bit for something else, but we decided in review to drop it, and spoke about adding some kind of static analysis check instead. I haven't had a go at that yet but think we should probably leave it out here as well.
| self.when_i_click_on_add_mastectomy_or_lumpectomy() | ||
| self.then_i_see_the_add_mastectomy_or_lumpectomy_form() | ||
|
|
||
| self.and_i_click_save() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minor, but the "and" here breaks the pattern slightly. Could we add in an alias for the step so it reads more naturally?
Added "Add details of mastectomy or lumpectomy" page which can be accessed from the "Record medical information" page.
ff504df to
8eaef2c
Compare
work in progress, not yet ready for review
Added "Add details of mastectomy or lumpectomy" page which can be accessed from the "Record medical information" page.
Description
Jira link
https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11452
Review notes
Review checklist