refactor(people): Move relationship answers to answer set

Answers to the relationship questions have been moved from the relationship
to another model RelationshipAnswerSet.  A new answer set is created each
time a user answers the relationship questions.

Resolves #16
Resolves #17
Resolves #18

people/admin.py

@@ -31,11 +31,6 @@ class PersonAdmin(admin.ModelAdmin):
     pass
 
 
-@admin.register(models.RelationshipQuestionChoice)
-class RelationshipQuestionChoiceAdmin(admin.ModelAdmin):
-    pass
-
-
 class RelationshipQuestionChoiceInline(admin.TabularInline):
     model = models.RelationshipQuestionChoice
 

people/forms.py

@@ -25,22 +25,17 @@ class PersonForm(forms.ModelForm):
         }
 
 
-class RelationshipForm(forms.ModelForm):
+class RelationshipAnswerSetForm(forms.ModelForm):
     """
-    Form to allow users to describe a relationship - includes :class:`RelationshipQuestion`s.
+    Form to allow users to describe a relationship.
 
     Dynamic fields inspired by https://jacobian.org/2010/feb/28/dynamic-form-generation/
     """
     class Meta:
-        model = models.Relationship
+        model = models.RelationshipAnswerSet
         fields = [
-            'source',
+            'relationship',
-            'target',
         ]
-        widgets = {
-            'source': Select2Widget(),
-            'target': Select2Widget(),
-        }
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
@@ -53,7 +48,7 @@ class RelationshipForm(forms.ModelForm):
                                       choices=choices)
             self.fields['question_{}'.format(question.pk)] = field
 
-    def save(self, commit=True) -> models.Relationship:
+    def save(self, commit=True) -> models.RelationshipAnswerSet:
         # Save Relationship model
         self.instance = super().save(commit=commit)
 
@@ -61,7 +56,9 @@ class RelationshipForm(forms.ModelForm):
             # Save answers to relationship questions
             for key, value in self.cleaned_data.items():
                 if key.startswith('question_'):
-                    answer = models.RelationshipQuestionChoice.objects.get(pk=value)
+                    question_id = key.replace('question_', '', 1)
+                    answer = models.RelationshipQuestionChoice.objects.get(pk=value,
+                                                                           question__pk=question_id)
                     self.instance.question_answers.add(answer)
 
         return self.instance

people/migrations/0016_add_answer_set.py

@@ -0,0 +1,78 @@
+# Generated by Django 2.2.10 on 2020-03-04 12:09
+import logging
+
+from django.db import migrations, models
+import django.db.models.deletion
+import django.utils.timezone
+
+logger = logging.getLogger(__name__)
+
+
+def forward_migration(apps, schema_editor):
+    """
+    Move existing data forward into answer sets from the relationship.
+    """
+    Relationship = apps.get_model('people', 'Relationship')
+
+    for relationship in Relationship.objects.all():
+        answer_set = relationship.answer_sets.first()
+        if answer_set is None:
+            answer_set = relationship.answer_sets.create()
+
+        for answer in relationship.question_answers.all():
+            answer_set.question_answers.add(answer)
+
+            
+def backward_migration(apps, schema_editor):
+    """
+    Move data backward from answer sets onto the relationship.
+    """
+    Relationship = apps.get_model('people', 'Relationship')
+
+    for relationship in Relationship.objects.all():
+        answer_set = relationship.answer_sets.last()
+
+        try:
+            for answer in answer_set.question_answers.all():
+                relationship.question_answers.add(answer)
+
+        except AttributeError:
+            pass
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('people', '0015_shrink_name_fields_to_255'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='relationship',
+            name='created',
+            field=models.DateTimeField(auto_now_add=True, default=django.utils.timezone.now),
+            preserve_default=False,
+        ),
+        migrations.AddField(
+            model_name='relationship',
+            name='expired',
+            field=models.DateTimeField(blank=True, null=True),
+        ),
+        migrations.CreateModel(
+            name='RelationshipAnswerSet',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('timestamp', models.DateTimeField(auto_now_add=True)),
+                ('question_answers', models.ManyToManyField(to='people.RelationshipQuestionChoice')),
+                ('relationship', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='answer_sets', to='people.Relationship')),
+            ],
+            options={
+                'ordering': ['timestamp'],
+            },
+        ),
+        migrations.RunPython(forward_migration, backward_migration),
+        migrations.RemoveField(
+            model_name='relationship',
+            name='question_answers',
+        ),
+    ]

people/models/__init__.py

@@ -0,0 +1,2 @@
+from .person import *
+from .relationship import *

people/models/person.py

@@ -10,6 +10,15 @@ from django_countries.fields import CountryField
 
 from backports.db.models.enums import TextChoices
 
+__all__ = [
+    'User',
+    'Organisation',
+    'Role',
+    'Discipline',
+    'Theme',
+    'Person',
+]
+
 
 class User(AbstractUser):
     """
@@ -175,109 +184,3 @@ class Person(models.Model):
         return self.name
 
 
-class RelationshipQuestion(models.Model):
-    """
-    Question which may be asked about a relationship.
-    """
-    class Meta:
-        ordering = [
-            'order',
-            'text',
-        ]
-
-    #: Version number of this question - to allow modification without invalidating existing data
-    version = models.PositiveSmallIntegerField(default=1,
-                                               blank=False, null=False)
-
-    #: Text of question
-    text = models.CharField(max_length=255,
-                            blank=False, null=False)
-
-    #: Position of this question in the list
-    order = models.SmallIntegerField(default=0,
-                                     blank=False, null=False)
-
-    @property
-    def choices(self) -> typing.List[typing.List[str]]:
-        """
-        Convert the :class:`RelationshipQuestionChoices` for this question into Django choices.
-        """
-        return [
-            [choice.pk, str(choice)] for choice in self.answers.all()
-        ]
-
-    def __str__(self) -> str:
-        return self.text
-
-
-class RelationshipQuestionChoice(models.Model):
-    """
-    Allowed answer to a :class:`RelationshipQuestion`.
-    """
-    class Meta:
-        constraints = [
-            models.UniqueConstraint(fields=['question', 'text'],
-                                    name='unique_question_answer')
-        ]
-        ordering = [
-            'question__order',
-            'order',
-            'text',
-        ]
-
-    #: Question to which this answer belongs
-    question = models.ForeignKey(RelationshipQuestion, related_name='answers',
-                                 on_delete=models.CASCADE,
-                                 blank=False, null=False)
-
-    #: Text of answer
-    text = models.CharField(max_length=255,
-                            blank=False, null=False)
-
-    #: Position of this answer in the list
-    order = models.SmallIntegerField(default=0,
-                                     blank=False, null=False)
-
-    def __str__(self) -> str:
-        return self.text
-
-
-class Relationship(models.Model):
-    """
-    A directional relationship between two people allowing linked questions.
-    """
-
-    class Meta:
-        constraints = [
-            models.UniqueConstraint(fields=['source', 'target'],
-                                    name='unique_relationship'),
-        ]
-
-    #: Person reporting the relationship
-    source = models.ForeignKey(Person, related_name='relationships_as_source',
-                               on_delete=models.CASCADE,
-                               blank=False, null=False)
-
-    #: Person with whom the relationship is reported
-    target = models.ForeignKey(Person, related_name='relationships_as_target',
-                               on_delete=models.CASCADE,
-                               blank=False, null=False)
-
-    #: Answers to :class:`RelationshipQuestion`s
-    question_answers = models.ManyToManyField(RelationshipQuestionChoice)
-
-    def get_absolute_url(self):
-        return reverse('people:relationship.detail', kwargs={'pk': self.pk})
-
-    def __str__(self) -> str:
-        return f'{self.source} -> {self.target}'
-
-    @property
-    def reverse(self):
-        """
-        Get the reverse of this relationship.
-
-        @raise Relationship.DoesNotExist: When the reverse relationship is not known
-        """
-        return type(self).objects.get(source=self.target,
-                                      target=self.source)

people/models/relationship.py

@@ -0,0 +1,155 @@
+"""
+Models describing relationships between people.
+"""
+
+import typing
+
+from django.db import models
+from django.urls import reverse
+
+from .person import Person
+
+__all__ = [
+    'RelationshipQuestion',
+    'RelationshipQuestionChoice',
+    'RelationshipAnswerSet',
+    'Relationship',
+]
+
+
+class RelationshipQuestion(models.Model):
+    """
+    Question which may be asked about a relationship.
+    """
+    class Meta:
+        ordering = [
+            'order',
+            'text',
+        ]
+
+    #: Version number of this question - to allow modification without invalidating existing data
+    version = models.PositiveSmallIntegerField(default=1,
+                                               blank=False, null=False)
+
+    #: Text of question
+    text = models.CharField(max_length=255,
+                            blank=False, null=False)
+
+    #: Position of this question in the list
+    order = models.SmallIntegerField(default=0,
+                                     blank=False, null=False)
+
+    @property
+    def choices(self) -> typing.List[typing.List[str]]:
+        """
+        Convert the :class:`RelationshipQuestionChoices` for this question into Django choices.
+        """
+        return [
+            [choice.pk, str(choice)] for choice in self.answers.all()
+        ]
+
+    def __str__(self) -> str:
+        return self.text
+
+
+class RelationshipQuestionChoice(models.Model):
+    """
+    Allowed answer to a :class:`RelationshipQuestion`.
+    """
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(fields=['question', 'text'],
+                                    name='unique_question_answer')
+        ]
+        ordering = [
+            'question__order',
+            'order',
+            'text',
+        ]
+
+    #: Question to which this answer belongs
+    question = models.ForeignKey(RelationshipQuestion, related_name='answers',
+                                 on_delete=models.CASCADE,
+                                 blank=False, null=False)
+
+    #: Text of answer
+    text = models.CharField(max_length=255,
+                            blank=False, null=False)
+
+    #: Position of this answer in the list
+    order = models.SmallIntegerField(default=0,
+                                     blank=False, null=False)
+
+    def __str__(self) -> str:
+        return self.text
+
+
+class Relationship(models.Model):
+    """
+    A directional relationship between two people allowing linked questions.
+    """
+
+    class Meta:
+        constraints = [
+            models.UniqueConstraint(fields=['source', 'target'],
+                                    name='unique_relationship'),
+        ]
+
+    #: Person reporting the relationship
+    source = models.ForeignKey(Person, related_name='relationships_as_source',
+                               on_delete=models.CASCADE,
+                               blank=False, null=False)
+
+    #: Person with whom the relationship is reported
+    target = models.ForeignKey(Person, related_name='relationships_as_target',
+                               on_delete=models.CASCADE,
+                               blank=False, null=False)
+    
+    #: When was this relationship defined?
+    created = models.DateTimeField(auto_now_add=True)
+    
+    #: When was this marked as expired?  Default None means it has not expired
+    expired = models.DateTimeField(blank=True, null=True)
+
+    @property
+    def current_answers(self) -> 'RelationshipAnswerSet':
+        return self.answer_sets.last()
+
+    def get_absolute_url(self):
+        return reverse('people:relationship.detail', kwargs={'pk': self.pk})
+
+    def __str__(self) -> str:
+        return f'{self.source} -> {self.target}'
+
+    @property
+    def reverse(self):
+        """
+        Get the reverse of this relationship.
+
+        @raise Relationship.DoesNotExist: When the reverse relationship is not known
+        """
+        return type(self).objects.get(source=self.target,
+                                      target=self.source)
+
+
+class RelationshipAnswerSet(models.Model):
+    """
+    The answers to the relationship questions at a particular point in time.
+    """
+
+    class Meta:
+        ordering = [
+            'timestamp',
+        ]
+
+    #: Relationship to which this answer set belongs
+    relationship = models.ForeignKey(Relationship,
+                                     on_delete=models.CASCADE,
+                                     related_name='answer_sets',
+                                     blank=False, null=False)
+
+    #: Answers to :class:`RelationshipQuestion`s
+    question_answers = models.ManyToManyField(RelationshipQuestionChoice)
+
+    #: When were these answers collected?
+    timestamp = models.DateTimeField(auto_now_add=True)

people/templates/people/relationship/detail.html

@@ -43,6 +43,10 @@
 
     <hr>
 
+    <a class="btn btn-success"
+       href="{% url 'people:relationship.update' relationship_pk=relationship.pk %}">Update</a>
+
+    {% with relationship.current_answers as answer_set %}
         <table class="table table-borderless">
             <thead>
             <tr>
@@ -52,7 +56,7 @@
             </thead>
 
             <tbody>
-        {% for answer in relationship.question_answers.all %}
+            {% for answer in answer_set.question_answers.all %}
                 <tr>
                     <td>{{ answer.question }}</td>
                     <td>{{ answer }}</td>
@@ -66,4 +70,7 @@
             </tbody>
         </table>
 
+        Last updated: {{ answer_set.timestamp }}
+    {% endwith %}
+
 {% endblock %}

people/templates/people/relationship/update.html

@@ -0,0 +1,33 @@
+{% extends 'base.html' %}
+
+{% block content %}
+    <nav aria-label="breadcrumb">
+        <ol class="breadcrumb">
+            <li class="breadcrumb-item">
+                <a href="{% url 'people:person.list' %}">People</a>
+            </li>
+            <li class="breadcrumb-item">
+                <a href="{% url 'people:person.detail' pk=person.pk %}">{{ person }}</a>
+            </li>
+            <li class="breadcrumb-item">
+                <a href="{% url 'people:relationship.detail' pk=relationship.pk %}">{{ relationship.target }}</a>
+            </li>
+            <li class="breadcrumb-item active" aria-current="page">Update Relationship</li>
+        </ol>
+    </nav>
+
+    <hr>
+
+    <form class="form"
+          method="POST">
+        {% csrf_token %}
+
+        {% load bootstrap4 %}
+        {% bootstrap_form form %}
+
+        {% buttons %}
+            <button class="btn btn-success" type="submit">Submit</button>
+        {% endbuttons %}
+    </form>
+
+{% endblock %}

people/urls.py

@@ -33,4 +33,8 @@ urlpatterns = [
     path('relationships/<int:pk>',
          views.RelationshipDetailView.as_view(),
          name='relationship.detail'),
+
+    path('relationships/<int:relationship_pk>/update',
+         views.RelationshipUpdateView.as_view(),
+         name='relationship.update'),
 ]

people/views.py

@@ -3,7 +3,8 @@ Views for displaying or manipulating models in the 'people' app.
 """
 
 from django.http import HttpResponseRedirect
-from django.views.generic import CreateView, DetailView, ListView, UpdateView
+from django.urls import reverse
+from django.views.generic import CreateView, DetailView, FormView, ListView, UpdateView
 
 from . import forms, models, permissions
 
@@ -80,11 +81,14 @@ class RelationshipCreateView(permissions.UserIsLinkedPersonMixin, CreateView):
     """
     model = models.Relationship
     template_name = 'people/relationship/create.html'
-    form_class = forms.RelationshipForm
+    fields = [
+        'source',
+        'target',
+    ]
 
     def get_test_person(self) -> models.Person:
         """
-        
+        Get the person instance which should be used for access control checks.
         """
         if self.request.method == 'POST':
             return models.Person.objects.get(pk=self.request.POST.get('source'))
@@ -116,10 +120,59 @@ class RelationshipCreateView(permissions.UserIsLinkedPersonMixin, CreateView):
 
         return context
 
+    def get_success_url(self):
+        return reverse('people:relationship.update', kwargs={'pk': self.object.pk})
+
+
+class RelationshipUpdateView(permissions.UserIsLinkedPersonMixin, CreateView):
+    """
+    View for creating a :class:`Relationship`.
+
+    Displays / processes a form containing the :class:`RelationshipQuestion`s.
+    """
+    model = models.RelationshipAnswerSet
+    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)
+
+    def get_context_data(self, **kwargs):
+        context = super().get_context_data(**kwargs)
+
+        context['person'] = self.person
+        context['relationship'] = self.relationship
+
+        return context
+    
+    def get_initial(self):
+        initial = super().get_initial()
+        
+        initial['relationship'] = self.relationship
+        
+        return initial
+
     def form_valid(self, form):
         """
-        Form is valid - create :class:`Relationship` and save answers to questions.
+        Don't rebind self.object to be the result of the form - it is a :class:`RelationshipAnswerSet`.
         """
-        self.object = form.save()
+        form.save()
         
-        return HttpResponseRedirect(self.object.get_absolute_url())
+        return HttpResponseRedirect(self.relationship.get_absolute_url())