diff --git a/people/forms.py b/people/forms.py index 44d7078..706034b 100644 --- a/people/forms.py +++ b/people/forms.py @@ -45,7 +45,6 @@ class DynamicAnswerSetBase(forms.Form): def __init__(self, *args, as_filters: bool = False, **kwargs): super().__init__(*args, **kwargs) - initial = kwargs.get('initial', {}) field_order = [] for question in self.question_model.objects.all(): @@ -77,7 +76,7 @@ class DynamicAnswerSetBase(forms.Form): widget=field_widget, required=(self.field_required and not question.allow_free_text), - initial=initial.get(field_name, None), + initial=self.initial.get(field_name, None), help_text=question.help_text if not as_filters else '') self.fields[field_name] = field field_order.append(field_name) diff --git a/people/models/person.py b/people/models/person.py index e683127..452b226 100644 --- a/people/models/person.py +++ b/people/models/person.py @@ -208,20 +208,8 @@ class PersonAnswerSet(AnswerSet): if field.attname not in exclude_fields } - for answer in self.question_answers.all(): - question = answer.question - field_name = f'question_{question.pk}' - - if question.is_multiple_choice: - if field_name not in answers: - answers[field_name] = [] - - answers[field_name].append(answer.pk) - - else: - answers[field_name] = answer.pk - - return answers + # Add answers to dynamic questions + return super().as_dict(answers=answers) def get_absolute_url(self): return self.person.get_absolute_url() diff --git a/people/models/question.py b/people/models/question.py index d0daa2c..b8df075 100644 --- a/people/models/question.py +++ b/people/models/question.py @@ -144,3 +144,23 @@ class AnswerSet(models.Model): replaced_timestamp = models.DateTimeField(blank=True, null=True, editable=False) + + def as_dict(self, answers: typing.Optional[typing.Dict[str, typing.Any]] = None): + """Get the answers from this set as a dictionary for use in Form.initial.""" + if answers is None: + answers = {} + + for answer in self.question_answers.all(): + question = answer.question + field_name = f'question_{question.pk}' + + if question.is_multiple_choice: + if field_name not in answers: + answers[field_name] = [] + + answers[field_name].append(answer.pk) + + else: + answers[field_name] = answer.pk + + return answers diff --git a/people/templates/people/organisation-relationship/detail.html b/people/templates/people/organisation-relationship/detail.html index 3ed68d3..d4ea1ec 100644 --- a/people/templates/people/organisation-relationship/detail.html +++ b/people/templates/people/organisation-relationship/detail.html @@ -40,7 +40,7 @@
Update + href="{% url 'people:organisation.relationship.update' pk=relationship.pk %}">Update {% with relationship.current_answers as answer_set %} diff --git a/people/templates/people/person/includes/relationships_full.html b/people/templates/people/person/includes/relationships_full.html index 4c647ce..336e177 100644 --- a/people/templates/people/person/includes/relationships_full.html +++ b/people/templates/people/person/includes/relationships_full.html @@ -20,7 +20,7 @@ Relationship DetailUpdate + href="{% url 'people:relationship.update' pk=relationship.pk %}">Update @@ -55,7 +55,7 @@ Relationship DetailUpdate + href="{% url 'people:organisation.relationship.update' pk=relationship.pk %}">Update diff --git a/people/templates/people/relationship/detail.html b/people/templates/people/relationship/detail.html index c749476..71db9d0 100644 --- a/people/templates/people/relationship/detail.html +++ b/people/templates/people/relationship/detail.html @@ -46,7 +46,7 @@
Update + href="{% url 'people:relationship.update' pk=relationship.pk %}">Update {% with relationship.current_answers as answer_set %}
diff --git a/people/urls.py b/people/urls.py index 9f26151..dcd49b6 100644 --- a/people/urls.py +++ b/people/urls.py @@ -56,7 +56,7 @@ urlpatterns = [ views.relationship.RelationshipDetailView.as_view(), name='relationship.detail'), - path('relationships//update', + path('relationships//update', views.relationship.RelationshipUpdateView.as_view(), name='relationship.update'), @@ -70,7 +70,7 @@ urlpatterns = [ views.relationship.OrganisationRelationshipDetailView.as_view(), name='organisation.relationship.detail'), - path('organisation-relationships//update', + path('organisation-relationships//update', views.relationship.OrganisationRelationshipUpdateView.as_view(), name='organisation.relationship.update'), diff --git a/people/views/relationship.py b/people/views/relationship.py index d9e2a51..823c2e2 100644 --- a/people/views/relationship.py +++ b/people/views/relationship.py @@ -5,7 +5,7 @@ import typing from django.contrib.auth.mixins import LoginRequiredMixin from django.urls import reverse from django.utils import timezone -from django.views.generic import CreateView, DetailView, RedirectView +from django.views.generic import DetailView, RedirectView, UpdateView from people import forms, models, permissions @@ -31,73 +31,65 @@ class RelationshipCreateView(LoginRequiredMixin, RedirectView): source=self.request.user.person, target=target) return reverse('people:relationship.update', - kwargs={'relationship_pk': relationship.pk}) + kwargs={'pk': relationship.pk}) -class RelationshipUpdateView(permissions.UserIsLinkedPersonMixin, CreateView): - """ - View for updating the details of a relationship. +class RelationshipUpdateView(permissions.UserIsLinkedPersonMixin, UpdateView): + """View for updating the details of a relationship. Creates a new :class:`RelationshipAnswerSet` for the :class:`Relationship`. Displays / processes a form containing the :class:`RelationshipQuestion`s. """ - model = models.RelationshipAnswerSet + model = models.Relationship + context_object_name = 'relationship' template_name = 'people/relationship/update.html' form_class = forms.RelationshipAnswerSetForm def get_test_person(self) -> models.Person: - """ - Get the person instance which should be used for access control checks. - """ - relationship = models.Relationship.objects.get( - pk=self.kwargs.get('relationship_pk')) - - return relationship.source - - def get(self, request, *args, **kwargs): - self.relationship = models.Relationship.objects.get( - pk=self.kwargs.get('relationship_pk')) - self.person = self.relationship.source - - return super().get(request, *args, **kwargs) - - def post(self, request, *args, **kwargs): - self.relationship = models.Relationship.objects.get( - pk=self.kwargs.get('relationship_pk')) - self.person = self.relationship.source - - return super().post(request, *args, **kwargs) + """Get the person instance which should be used for access control checks.""" + return self.object.source def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - - context['person'] = self.person - context['relationship'] = self.relationship - + context['person'] = self.object.source return context def get_initial(self): - initial = super().get_initial() + try: + previous_answers = self.object.current_answers.as_dict() - initial['relationship'] = self.relationship + except AttributeError: + previous_answers = {} - return initial + previous_answers.update({ + 'relationship': self.object, + }) + + return previous_answers + + def get_form_kwargs(self) -> typing.Dict[str, typing.Any]: + """Remove instance from form kwargs as it's a person, but expects a PersonAnswerSet.""" + kwargs = super().get_form_kwargs() + kwargs.pop('instance') + + return kwargs def form_valid(self, form): - """ - Mark any previous answer sets as replaced. - """ + """Mark any previous answer sets as replaced.""" response = super().form_valid(form) now_date = timezone.now().date() # Shouldn't be more than one after initial updates after migration - for answer_set in self.relationship.answer_sets.exclude( + for answer_set in self.object.relationship.answer_sets.exclude( pk=self.object.pk): answer_set.replaced_timestamp = now_date answer_set.save() return response + def get_success_url(self) -> str: + return self.object.get_absolute_url() + class OrganisationRelationshipDetailView(permissions.UserIsLinkedPersonMixin, DetailView): @@ -121,59 +113,50 @@ class OrganisationRelationshipCreateView(LoginRequiredMixin, RedirectView): source=self.request.user.person, target=target) return reverse('people:organisation.relationship.update', - kwargs={'relationship_pk': relationship.pk}) + kwargs={'pk': relationship.pk}) class OrganisationRelationshipUpdateView(permissions.UserIsLinkedPersonMixin, - CreateView): - """ - View for updating the details of a Organisationrelationship. + UpdateView): + """View for updating the details of a Organisationrelationship. Creates a new :class:`OrganisationRelationshipAnswerSet` for the :class:`OrganisationRelationship`. Displays / processes a form containing the :class:`OrganisationRelationshipQuestion`s. """ - model = models.OrganisationRelationshipAnswerSet + model = models.OrganisationRelationship + context_object_name = 'relationship' template_name = 'people/relationship/update.html' form_class = forms.OrganisationRelationshipAnswerSetForm def get_test_person(self) -> models.Person: - """ - Get the person instance which should be used for access control checks. - """ - relationship = models.OrganisationRelationship.objects.get( - pk=self.kwargs.get('relationship_pk')) + """Get the person instance which should be used for access control checks.""" + return self.object.source - return relationship.source + def get_initial(self): + try: + previous_answers = self.object.current_answers.as_dict() - def get(self, request, *args, **kwargs): - self.relationship = models.OrganisationRelationship.objects.get( - pk=self.kwargs.get('relationship_pk')) - self.person = self.relationship.source + except AttributeError: + previous_answers = {} - return super().get(request, *args, **kwargs) + previous_answers.update({ + 'relationship': self.object, + }) - def post(self, request, *args, **kwargs): - self.relationship = models.OrganisationRelationship.objects.get( - pk=self.kwargs.get('relationship_pk')) - self.person = self.relationship.source + return previous_answers - return super().post(request, *args, **kwargs) + def get_form_kwargs(self) -> typing.Dict[str, typing.Any]: + """Remove instance from form kwargs as it's a person, but expects a PersonAnswerSet.""" + kwargs = super().get_form_kwargs() + kwargs.pop('instance') + + return kwargs def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - - context['person'] = self.person - context['relationship'] = self.relationship - + context['person'] = self.object.source return context - def get_initial(self): - initial = super().get_initial() - - initial['relationship'] = self.relationship - - return initial - def form_valid(self, form): """ Mark any previous answer sets as replaced. @@ -182,7 +165,7 @@ class OrganisationRelationshipUpdateView(permissions.UserIsLinkedPersonMixin, now_date = timezone.now().date() # Shouldn't be more than one after initial updates after migration - for answer_set in self.relationship.answer_sets.exclude( + for answer_set in self.object.relationship.answer_sets.exclude( pk=self.object.pk): answer_set.replaced_timestamp = now_date answer_set.save()