-
Notifications
You must be signed in to change notification settings - Fork 4
Add link to delete appointment notes #823
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
- Put appointment note view logic into its own file - Add a view dealing with note deletion, including a confirmation step
If a user clicks on a stale delete link for an appointment note that has already been deleted, simply redirect them back to the note tab.
914174e to
1810cb9
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 👍🏻
|
|
||
| @property | ||
| def instance_is_saved(self): | ||
| return self.instance and not self.instance._state.adding |
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.
could this be simplified to self.instance.pk is not None?
_state just looks a bit iffy to me as it normally indicates private fields, although it is part of the documented API in this case. OK to leave it as is if you prefer.
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.
Started off as that but it looks like Django assigns uuid primary keys at instantiation time so this check wasn't working as intended.
|
|
||
| def get_success_url(self): | ||
| return reverse("mammograms:appointment_note", kwargs={"pk": self.kwargs["pk"]}) | ||
|
|
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.
Can this logic be moved into the superclass do you think? I've just added something similar for add and update in this PR #824
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.
Do you mean in DeleteWithAuditView? Worth considering — would be interested in seeing what that looks like.
Description
Jira link
https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11725
Review notes
na
Review checklist