-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| import pytest | ||
| from django.urls import reverse | ||
| from pytest_django.asserts import assertRedirects | ||
|
|
||
| from manage_breast_screening.participants.models import AppointmentNote | ||
| from manage_breast_screening.participants.tests.factories import AppointmentFactory | ||
|
|
||
|
|
||
| @pytest.mark.django_db | ||
| class TestAppointmentNoteView: | ||
| def test_delete_link_not_shown_when_note_does_not_exist(self, clinical_user_client): | ||
| appointment = AppointmentFactory.create( | ||
| clinic_slot__clinic__setting__provider=clinical_user_client.current_provider | ||
| ) | ||
| response = clinical_user_client.http.get( | ||
| reverse( | ||
| "mammograms:appointment_note", | ||
| kwargs={"pk": appointment.pk}, | ||
| ) | ||
| ) | ||
| assert response.status_code == 200 | ||
| assert "Delete appointment note" not in response.content.decode() | ||
|
|
||
| def test_delete_link_shown_when_note_exists(self, clinical_user_client): | ||
| appointment = AppointmentFactory.create( | ||
| clinic_slot__clinic__setting__provider=clinical_user_client.current_provider | ||
| ) | ||
| AppointmentNote.objects.create(appointment=appointment, content="Existing note") | ||
| response = clinical_user_client.http.get( | ||
| reverse( | ||
| "mammograms:appointment_note", | ||
| kwargs={"pk": appointment.pk}, | ||
| ) | ||
| ) | ||
| assert response.status_code == 200 | ||
| assert "Delete appointment note" in response.content.decode() | ||
|
|
||
| @pytest.mark.parametrize( | ||
| "client_fixture", ["clinical_user_client", "administrative_user_client"] | ||
| ) | ||
| def test_users_can_save_note(self, request, client_fixture): | ||
| client = request.getfixturevalue(client_fixture) | ||
| appointment = AppointmentFactory.create( | ||
| clinic_slot__clinic__setting__provider=client.current_provider | ||
| ) | ||
|
|
||
| note_content = "Participant prefers left arm blood pressure readings." | ||
| response = client.http.post( | ||
| reverse( | ||
| "mammograms:appointment_note", | ||
| kwargs={"pk": appointment.pk}, | ||
| ), | ||
| {"content": note_content}, | ||
| ) | ||
|
|
||
| assertRedirects( | ||
| response, | ||
| reverse("mammograms:appointment_note", kwargs={"pk": appointment.pk}), | ||
| ) | ||
| saved_note = AppointmentNote.objects.get(appointment=appointment) | ||
| assert saved_note.content == note_content | ||
|
|
||
| @pytest.mark.parametrize( | ||
| "client_fixture", ["clinical_user_client", "administrative_user_client"] | ||
| ) | ||
| def test_users_can_update_note(self, request, client_fixture): | ||
| client = request.getfixturevalue(client_fixture) | ||
| appointment = AppointmentFactory.create( | ||
| clinic_slot__clinic__setting__provider=client.current_provider | ||
| ) | ||
| note = AppointmentNote.objects.create( | ||
| appointment=appointment, content="Original note" | ||
| ) | ||
|
|
||
| updated_content = "Updated note content" | ||
| response = client.http.post( | ||
| reverse("mammograms:appointment_note", kwargs={"pk": appointment.pk}), | ||
| {"content": updated_content}, | ||
| ) | ||
|
|
||
| assertRedirects( | ||
| response, | ||
| reverse("mammograms:appointment_note", kwargs={"pk": appointment.pk}), | ||
| ) | ||
| updated_note = AppointmentNote.objects.get(pk=note.pk) | ||
| assert updated_note.content == updated_content | ||
| assert AppointmentNote.objects.count() == 1 | ||
|
|
||
|
|
||
| @pytest.mark.django_db | ||
| class TestDeleteAppointmentNoteView: | ||
| def test_get_redirects_when_note_does_not_exist(self, clinical_user_client): | ||
| appointment = AppointmentFactory.create( | ||
| clinic_slot__clinic__setting__provider=clinical_user_client.current_provider | ||
| ) | ||
| response = clinical_user_client.http.get( | ||
| reverse( | ||
| "mammograms:delete_appointment_note", | ||
| kwargs={"pk": appointment.pk}, | ||
| ) | ||
| ) | ||
| assertRedirects( | ||
| response, | ||
| reverse("mammograms:appointment_note", kwargs={"pk": appointment.pk}), | ||
| ) | ||
|
|
||
| def test_post_redirects_when_note_does_not_exist(self, clinical_user_client): | ||
| appointment = AppointmentFactory.create( | ||
| clinic_slot__clinic__setting__provider=clinical_user_client.current_provider | ||
| ) | ||
| response = clinical_user_client.http.post( | ||
| reverse( | ||
| "mammograms:delete_appointment_note", | ||
| kwargs={"pk": appointment.pk}, | ||
| ) | ||
| ) | ||
| assertRedirects( | ||
| response, | ||
| reverse("mammograms:appointment_note", kwargs={"pk": appointment.pk}), | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import logging | ||
|
|
||
| from django.contrib import messages | ||
| from django.shortcuts import redirect | ||
| from django.urls import reverse | ||
| from django.views.generic import FormView | ||
|
|
||
| from manage_breast_screening.core.services.auditor import Auditor | ||
| from manage_breast_screening.core.views.generic import DeleteWithAuditView | ||
| from manage_breast_screening.participants.models import AppointmentNote | ||
|
|
||
| from ..forms import AppointmentNoteForm | ||
| from ..presenters import AppointmentPresenter, present_secondary_nav | ||
| from .mixins import AppointmentMixin | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class AppointmentNoteView(AppointmentMixin, FormView): | ||
| template_name = "mammograms/show/appointment_note.jinja" | ||
| form_class = AppointmentNoteForm | ||
|
|
||
| def get_context_data(self, **kwargs): | ||
| context = super().get_context_data(**kwargs) | ||
| appointment = self.appointment | ||
| appointment_presenter = AppointmentPresenter( | ||
| appointment, tab_description="Note" | ||
| ) | ||
|
|
||
| context.update( | ||
| { | ||
| "heading": appointment_presenter.participant.full_name, | ||
| "caption": appointment_presenter.caption, | ||
| "page_title": appointment_presenter.page_title, | ||
| "presented_appointment": appointment_presenter, | ||
| "secondary_nav_items": present_secondary_nav( | ||
| appointment.pk, current_tab="note" | ||
| ), | ||
| } | ||
| ) | ||
| return context | ||
|
|
||
| def get_form_kwargs(self): | ||
| kwargs = super().get_form_kwargs() | ||
| try: | ||
| kwargs["instance"] = self.appointment.note | ||
| except AppointmentNote.DoesNotExist: | ||
| kwargs["instance"] = AppointmentNote(appointment=self.appointment) | ||
| return kwargs | ||
|
|
||
| def form_valid(self, form): | ||
| is_new_note = form.instance._state.adding | ||
| note = form.save() | ||
| auditor = Auditor.from_request(self.request) | ||
| if is_new_note: | ||
| auditor.audit_create(note) | ||
| else: | ||
| auditor.audit_update(note) | ||
| messages.add_message( | ||
| self.request, | ||
| messages.SUCCESS, | ||
| "Appointment note saved", | ||
| ) | ||
| return redirect("mammograms:appointment_note", pk=self.appointment.pk) | ||
|
|
||
|
|
||
| class DeleteAppointmentNoteView(DeleteWithAuditView): | ||
| def get_thing_name(self, object): | ||
| return "appointment note" | ||
|
|
||
| def get_success_message_content(self, object): | ||
| return "Appointment note deleted" | ||
|
|
||
| def get_object(self): | ||
| provider = self.request.user.current_provider | ||
| appointment = provider.appointments.get(pk=self.kwargs["pk"]) | ||
| return appointment.note | ||
|
|
||
| def get_success_url(self): | ||
| return reverse("mammograms:appointment_note", kwargs={"pk": self.kwargs["pk"]}) | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| def get(self, request, *args, **kwargs): | ||
| try: | ||
| return super().get(request, *args, **kwargs) | ||
| except AppointmentNote.DoesNotExist: | ||
| return redirect(self.get_success_url()) | ||
|
|
||
| def post(self, request, *args, **kwargs): | ||
| try: | ||
| return super().post(request, *args, **kwargs) | ||
| except AppointmentNote.DoesNotExist: | ||
| return redirect(self.get_success_url()) | ||
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?_statejust 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.