refactor: allow admin config of static questions

Text and visibility set in admin panel are now respected everywhere

people/forms.py

@@ -52,8 +52,9 @@ class DynamicAnswerSetBase(forms.Form):
                 continue
 
             # Placeholder question for sorting hardcoded questions
-            if question.is_hardcoded and (question.text in self.Meta.fields):
+            if question.is_hardcoded and (question.hardcoded_field
-                field_order.append(question.text)
+                                          in self.Meta.fields):
+                field_order.append(question.hardcoded_field)
                 continue
 
             field_class = self.field_class

people/migrations/0051_refactor_hardcoded_questions.py

@@ -0,0 +1,81 @@
+# Generated by Django 2.2.10 on 2021-03-19 14:37
+
+from django.db import migrations, models
+from django.db.models import F
+
+
+def forward(apps, schema_editor):
+    """Move `text` field to `hardcoded_field`."""
+    models = [
+        'OrganisationQuestion',
+        'OrganisationRelationshipQuestion',
+        'PersonQuestion',
+        'RelationshipQuestion',
+    ]
+    models = map(lambda m: apps.get_model('people', m), models)
+
+    for model in models:
+        model.objects.filter(is_hardcoded=True).update(
+            hardcoded_field=F('text'))
+
+
+def backward(apps, schema_editor):
+    """Move `hardcoded_field` to `text` field."""
+    models = [
+        'OrganisationQuestion',
+        'OrganisationRelationshipQuestion',
+        'PersonQuestion',
+        'RelationshipQuestion',
+    ]
+    models = map(lambda m: apps.get_model('people', m), models)
+
+    for model in models:
+        model.objects.exclude(hardcoded_field='').update(
+            text=F('hardcoded_field'), is_hardcoded=True)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('people', '0050_relationship_remove_timestamps'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='organisationquestion',
+            name='hardcoded_field',
+            field=models.CharField(blank=True, help_text='Which hardcoded field does this question represent?', max_length=255),
+        ),
+        migrations.AddField(
+            model_name='organisationrelationshipquestion',
+            name='hardcoded_field',
+            field=models.CharField(blank=True, help_text='Which hardcoded field does this question represent?', max_length=255),
+        ),
+        migrations.AddField(
+            model_name='personquestion',
+            name='hardcoded_field',
+            field=models.CharField(blank=True, help_text='Which hardcoded field does this question represent?', max_length=255),
+        ),
+        migrations.AddField(
+            model_name='relationshipquestion',
+            name='hardcoded_field',
+            field=models.CharField(blank=True, help_text='Which hardcoded field does this question represent?', max_length=255),
+        ),
+        migrations.RunPython(forward, backward),
+        migrations.RemoveField(
+            model_name='organisationquestion',
+            name='is_hardcoded',
+        ),
+        migrations.RemoveField(
+            model_name='organisationrelationshipquestion',
+            name='is_hardcoded',
+        ),
+        migrations.RemoveField(
+            model_name='personquestion',
+            name='is_hardcoded',
+        ),
+        migrations.RemoveField(
+            model_name='relationshipquestion',
+            name='is_hardcoded',
+        ),
+    ]

people/models/organisation.py

@@ -61,6 +61,9 @@ class Organisation(models.Model):
 
 class OrganisationAnswerSet(AnswerSet):
     """The answers to the organisation questions at a particular point in time."""
+
+    question_model = OrganisationQuestion
+
     #: Organisation to which this answer set belongs
     organisation = models.ForeignKey(Organisation,
                                      on_delete=models.CASCADE,

people/models/person.py

@@ -132,6 +132,8 @@ class Person(models.Model):
 
 class PersonAnswerSet(AnswerSet):
     """The answers to the person questions at a particular point in time."""
+    question_model = PersonQuestion
+
     #: Person to which this answer set belongs
     person = models.ForeignKey(Person,
                                on_delete=models.CASCADE,

people/models/question.py

@@ -1,4 +1,5 @@
 """Base models for configurable questions and response sets."""
+import abc
 import typing
 
 from django.db import models
@@ -53,12 +54,16 @@ class Question(models.Model):
                                              blank=False,
                                              null=False)
 
-    #: Is this question hardcoded in an AnswerSet?
+    @property
-    is_hardcoded = models.BooleanField(
+    def is_hardcoded(self) -> bool:
-        help_text='Only the order field has any effect for a hardcoded question.',
+        return bool(self.hardcoded_field)
-        default=False,
+
-        blank=False,
+    hardcoded_field = models.CharField(
-        null=False)
+        help_text='Which hardcoded field does this question represent?',
+        max_length=255,
+        blank=True,
+        null=False
+    )
 
     #: Should people be able to add their own answers?
     allow_free_text = models.BooleanField(default=False,
@@ -126,6 +131,12 @@ class AnswerSet(models.Model):
         ]
         get_latest_by = 'timestamp'
 
+    @classmethod
+    @abc.abstractproperty
+    def question_model(cls) -> models.Model:
+        """Model representing questions to be answered in this AnswerSet."""
+        raise NotImplementedError
+
     #: Entity to which this answer set belongs
     #: This foreign key must be added to each concrete subclass
     # person = models.ForeignKey(Person,
@@ -134,9 +145,14 @@ class AnswerSet(models.Model):
     #                            blank=False,
     #                            null=False)
 
-    #: Answers to :class:`Question`s
+    @abc.abstractproperty
-    #: This many to many relation must be added to each concrete subclass
+    def question_answers(self) -> models.QuerySet:
-    # question_answers = models.ManyToManyField(QuestionChoice)
+        """Answers to :class:`Question`s.
+
+        This many to many relation must be added to each concrete subclass
+        question_answers = models.ManyToManyField(<X>QuestionChoice)
+        """
+        raise NotImplementedError
 
     #: When were these answers collected?
     timestamp = models.DateTimeField(auto_now_add=True, editable=False)
@@ -150,6 +166,31 @@ class AnswerSet(models.Model):
     def is_current(self) -> bool:
         return self.replaced_timestamp is None
 
+    def build_question_answers(self, show_all: bool = False) -> typing.Dict[str, str]:
+        """Collect answers to dynamic questions and join with commas."""
+        questions = self.question_model.objects.all()
+        if not show_all:
+            questions = questions.filter(answer_is_public=True)
+
+        question_answers = {}
+        try:
+            for question in questions:
+                if question.hardcoded_field:
+                    question_answers[question.text] = getattr(
+                        self, question.hardcoded_field)
+
+                else:
+                    answers = self.question_answers.filter(
+                        question=question)
+                    question_answers[question.text] = ', '.join(
+                        map(str, answers))
+
+        except AttributeError:
+            # No AnswerSet yet
+            pass
+
+        return question_answers
+
     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:

people/models/relationship.py

@@ -36,9 +36,7 @@ class RelationshipQuestionChoice(QuestionChoice):
 
 
 class Relationship(models.Model):
-    """
+    """A directional relationship between two people allowing linked questions."""
-    A directional relationship between two people allowing linked questions.
-    """
     class Meta:
         constraints = [
             models.UniqueConstraint(fields=['source', 'target'],
@@ -95,6 +93,8 @@ class Relationship(models.Model):
 class RelationshipAnswerSet(AnswerSet):
     """The answers to the relationship questions at a particular point in time."""
 
+    question_model = RelationshipQuestion
+
     #: Relationship to which this answer set belongs
     relationship = models.ForeignKey(Relationship,
                                      on_delete=models.CASCADE,
@@ -176,6 +176,8 @@ class OrganisationRelationship(models.Model):
 class OrganisationRelationshipAnswerSet(AnswerSet):
     """The answers to the organisation relationship questions at a particular point in time."""
 
+    question_model = OrganisationRelationshipQuestion
+
     #: OrganisationRelationship to which this answer set belongs
     relationship = models.ForeignKey(OrganisationRelationship,
                                      on_delete=models.CASCADE,

people/templates/people/organisation-relationship/detail.html

@@ -64,32 +64,7 @@
             </div>
 
         {% else %}
-            <table class="table table-borderless">
+            {% include 'people/includes/answer_set.html' %}
-                <thead>
-                <tr>
-                    <th>Question</th>
-                    <th>Answer</th>
-                </tr>
-                </thead>
-
-                <tbody>
-                {% for answer in answer_set.question_answers.all %}
-                    <tr>
-                        <td>{{ answer.question }}</td>
-                        <td>{{ answer }}</td>
-                    </tr>
-
-                {% empty %}
-                    <tr>
-                        <td>No records</td>
-                    </tr>
-                {% endfor %}
-                </tbody>
-            </table>
-
-            <p>
-                Last updated: {{ answer_set.timestamp }}
-            </p>
         {% endif %}
     {% endwith %}
 

people/templates/people/organisation/detail.html

@@ -54,49 +54,7 @@
 
     <hr>
 
-    <table class="table table-borderless">
+    {% include 'people/includes/answer_set.html' %}
-        <thead>
-        <tr>
-            <th width="50%">Question</th>
-            <th>Answer</th>
-        </tr>
-        </thead>
-
-        <tbody>
-        {% if answer_set.website %}
-            <tr><td>Website</td><td>
-                <a href="{{ answer_set.website }}">{{ answer_set.website }}</a>
-            </td></tr>
-        {% endif %}
-
-        {% if answer_set.countries %}
-            <tr><td>Countries</td><td>
-                {% for country in answer_set.countries %}
-                    {{ country.name }}{% if not forloop.last %},{% endif %}
-                {% endfor %}
-            </td></tr>
-        {% endif %}
-
-        {% if answer_set.hq_country %}
-            <tr><td>HQ Country</td><td>{{ answer_set.hq_country.name }}</td></tr>
-        {% endif %}
-
-        {% for question, answers in question_answers.items %}
-            <tr>
-                <td>{{ question }}</td>
-                <td>{{ answers }}</td>
-            </tr>
-        {% endfor %}
-
-        {% if answer_set is None %}
-            <tr>
-                <td colspan="2">No answers</td>
-            </tr>
-        {% endif %}
-        </tbody>
-    </table>
-
-    <p>Last updated: {{ answer_set.timestamp }}</p>
 
     <hr>
 

people/templates/people/person/detail_full.html

@@ -57,7 +57,7 @@
     {% endif %}
 
 
-    {% include 'people/person/includes/answer_set.html' %}
+    {% include 'people/includes/answer_set.html' %}
 
     <a class="btn btn-success"
         href="{% url 'people:person.update' pk=person.pk %}">Update</a>

people/templates/people/person/detail_partial.html

@@ -49,7 +49,7 @@
 
     <hr>
 
-    {% include 'people/person/includes/answer_set.html' %}
+    {% include 'people/includes/answer_set.html' %}
 
     <hr>
 

people/templates/people/relationship/detail.html

@@ -70,32 +70,7 @@
             </div>
 
         {% else %}
-            <table class="table table-borderless">
+            {% include 'people/includes/answer_set.html' %}
-                <thead>
-                <tr>
-                    <th>Question</th>
-                    <th>Answer</th>
-                </tr>
-                </thead>
-
-                <tbody>
-                {% for answer in answer_set.question_answers.all %}
-                    <tr>
-                        <td>{{ answer.question }}</td>
-                        <td>{{ answer }}</td>
-                    </tr>
-
-                {% empty %}
-                    <tr>
-                        <td>No records</td>
-                    </tr>
-                {% endfor %}
-                </tbody>
-            </table>
-
-            <p>
-                Last updated: {{ answer_set.timestamp }}
-            </p>
         {% endif %}
     {% endwith %}
 

people/views/organisation.py

@@ -117,42 +117,25 @@ class OrganisationDetailView(LoginRequiredMixin, DetailView):
     context_object_name = 'organisation'
     template_name = 'people/organisation/detail.html'
 
-    def build_question_answers(
-            self,
-            answer_set: models.OrganisationAnswerSet) -> typing.Dict[str, str]:
-        """Collect answers to dynamic questions and join with commas."""
-        show_all = self.request.user.is_superuser
-        questions = models.OrganisationQuestion.objects.filter(
-            is_hardcoded=False)
-        if not show_all:
-            questions = questions.filter(answer_is_public=True)
-
-        question_answers = {}
-        try:
-            for question in questions:
-                answers = answer_set.question_answers.filter(question=question)
-                question_answers[str(question)] = ', '.join(map(str, answers))
-
-        except AttributeError:
-            # No AnswerSet yet
-            pass
-
-        return question_answers
-
     def get_context_data(self,
                          **kwargs: typing.Any) -> typing.Dict[str, typing.Any]:
         """Add map marker to context."""
         context = super().get_context_data(**kwargs)
 
-        answerset = self.object.current_answers
+        answer_set = self.object.current_answers
-        context['answer_set'] = answerset
+        context['answer_set'] = answer_set
-        context['question_answers'] = self.build_question_answers(answerset)
         context['map_markers'] = [{
             'name': self.object.name,
-            'lat': getattr(answerset, 'latitude', None),
+            'lat': getattr(answer_set, 'latitude', None),
-            'lng': getattr(answerset, 'longitude', None),
+            'lng': getattr(answer_set, 'longitude', None),
         }]
 
+        context['question_answers'] = {}
+        if answer_set is not None:
+            show_all = self.request.user.is_superuser
+            context['question_answers'] = answer_set.build_question_answers(
+                show_all)
+
         context['relationship'] = None
         try:
             relationship = models.OrganisationRelationship.objects.get(

people/views/person.py

@@ -94,34 +94,6 @@ class ProfileView(LoginRequiredMixin, DetailView):
             # pk was not provided in URL
             return self.request.user.person
 
-    def build_question_answers(
-            self, answer_set: models.PersonAnswerSet) -> typing.Dict[str, str]:
-        """Collect answers to dynamic questions and join with commas."""
-        show_all = ((self.object.user == self.request.user)
-                    or self.request.user.is_superuser)
-        questions = models.PersonQuestion.objects.all()
-        if not show_all:
-            questions = questions.filter(answer_is_public=True)
-
-        question_answers = {}
-        try:
-            for question in questions:
-                if question.is_hardcoded:
-                    question_answers[str(question)] = getattr(
-                        answer_set, question.text)
-
-                else:
-                    answers = answer_set.question_answers.filter(
-                        question=question)
-                    question_answers[str(question)] = ', '.join(
-                        map(str, answers))
-
-        except AttributeError:
-            # No AnswerSet yet
-            pass
-
-        return question_answers
-
     def get_context_data(self,
                          **kwargs: typing.Any) -> typing.Dict[str, typing.Any]:
         """Add current :class:`PersonAnswerSet` to context."""
@@ -129,9 +101,15 @@ class ProfileView(LoginRequiredMixin, DetailView):
 
         answer_set = self.object.current_answers
         context['answer_set'] = answer_set
-        context['question_answers'] = self.build_question_answers(answer_set)
         context['map_markers'] = [get_map_data(self.object)]
 
+        context['question_answers'] = {}
+        if answer_set is not None:
+            show_all = ((self.object.user == self.request.user)
+                        or self.request.user.is_superuser)
+            context['question_answers'] = answer_set.build_question_answers(
+                show_all)
+
         context['relationship'] = None
         try:
             relationship = models.Relationship.objects.get(

people/views/relationship.py

@@ -19,6 +19,23 @@ class RelationshipDetailView(permissions.UserIsLinkedPersonMixin, DetailView):
     template_name = 'people/relationship/detail.html'
     related_person_field = 'source'
 
+    def get_context_data(self,
+                         **kwargs: typing.Any) -> typing.Dict[str, typing.Any]:
+        """Add current :class:`RelationshipAnswerSet` to context."""
+        context = super().get_context_data(**kwargs)
+
+        answer_set = self.object.current_answers
+        context['answer_set'] = answer_set
+
+        context['question_answers'] = {}
+        if answer_set is not None:
+            show_all = ((self.object.source == self.request.user)
+                        or self.request.user.is_superuser)
+            context['question_answers'] = answer_set.build_question_answers(
+                show_all)
+
+        return context
+
 
 class RelationshipCreateView(LoginRequiredMixin, RedirectView):
     """View for creating a :class:`Relationship`.
@@ -124,8 +141,7 @@ class OrganisationRelationshipEndView(RelationshipEndView):
     model = models.OrganisationRelationship
 
 
-class OrganisationRelationshipDetailView(permissions.UserIsLinkedPersonMixin,
+class OrganisationRelationshipDetailView(RelationshipDetailView):
-                                         DetailView):
     """View displaying details of an :class:`OrganisationRelationship`."""
     model = models.OrganisationRelationship
     template_name = 'people/organisation-relationship/detail.html'