From 6435ec69a13e0f7b1ebd4e79bed2c315fa3c9723 Mon Sep 17 00:00:00 2001 From: James Graham Date: Fri, 3 Jul 2020 15:48:53 +0100 Subject: [PATCH 01/11] fix: Show 'add person' button to all users See #33 --- people/templates/people/person/list.html | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/people/templates/people/person/list.html b/people/templates/people/person/list.html index 6dfa8da..c2bd1ec 100644 --- a/people/templates/people/person/list.html +++ b/people/templates/people/person/list.html @@ -11,10 +11,8 @@
- {% if request.user.is_staff %} - New Person - {% endif %} + New Person From a94db2713ea8cdff9051f940a214c9f42615d975 Mon Sep 17 00:00:00 2001 From: James Graham Date: Fri, 14 Aug 2020 17:38:22 +0100 Subject: [PATCH 02/11] fix: Only allow users to create rels as source There is now no field for users to define the source of a relationship The source is always the person in the URL And only that user or staff can access the form --- breccia_mapper/urls.py | 8 +++-- people/forms.py | 8 ++++- people/permissions.py | 7 ++-- people/views/relationship.py | 66 +++++++++++++++++------------------- 4 files changed, 50 insertions(+), 39 deletions(-) diff --git a/breccia_mapper/urls.py b/breccia_mapper/urls.py index 2806827..0aade11 100644 --- a/breccia_mapper/urls.py +++ b/breccia_mapper/urls.py @@ -19,7 +19,11 @@ from django.urls import include, path from . import views urlpatterns = [ - path('admin/', admin.site.urls), + path('admin/', + admin.site.urls), + + path('select2/', + include('django_select2.urls')), path('', include('django.contrib.auth.urls')), @@ -36,4 +40,4 @@ urlpatterns = [ path('', include('activities.urls')), -] +] # yapf: disable diff --git a/people/forms.py b/people/forms.py index 5f7c5a3..7ddf909 100644 --- a/people/forms.py +++ b/people/forms.py @@ -8,7 +8,7 @@ from django import forms from django.forms.widgets import SelectDateWidget from django.utils import timezone -from django_select2.forms import Select2Widget, Select2MultipleWidget +from django_select2.forms import ModelSelect2Widget, Select2Widget, Select2MultipleWidget from . import models @@ -60,6 +60,12 @@ class PersonForm(forms.ModelForm): years=get_date_year_range()) +class RelationshipForm(forms.Form): + target = forms.ModelChoiceField( + models.Person.objects.all(), + widget=ModelSelect2Widget(search_fields=['name__icontains'])) + + class DynamicAnswerSetBase(forms.Form): field_class = forms.ModelChoiceField field_widget = None diff --git a/people/permissions.py b/people/permissions.py index 2e6c585..5e17f19 100644 --- a/people/permissions.py +++ b/people/permissions.py @@ -23,7 +23,9 @@ class UserIsLinkedPersonMixin(UserPassesTestMixin): test_person = self.get_object() if not isinstance(test_person, models.Person): - raise AttributeError('View incorrectly configured: \'related_person_field\' must be defined.') + raise AttributeError( + 'View incorrectly configured: \'related_person_field\' must be defined.' + ) return test_person @@ -34,4 +36,5 @@ class UserIsLinkedPersonMixin(UserPassesTestMixin): Require that user is either staff or is the linked person. """ user = self.request.user - return user.is_authenticated and (user.is_staff or self.get_test_person() == user.person) + return user.is_authenticated and ( + user.is_staff or self.get_test_person() == user.person) diff --git a/people/views/relationship.py b/people/views/relationship.py index 8709b99..3ea46d9 100644 --- a/people/views/relationship.py +++ b/people/views/relationship.py @@ -2,9 +2,11 @@ Views for displaying or manipulating instances of :class:`Relationship`. """ +from django.db import IntegrityError +from django.forms import ValidationError from django.urls import reverse from django.utils import timezone -from django.views.generic import CreateView, DetailView +from django.views.generic import CreateView, DetailView, FormView from people import forms, models, permissions @@ -18,7 +20,7 @@ class RelationshipDetailView(permissions.UserIsLinkedPersonMixin, DetailView): related_person_field = 'source' -class RelationshipCreateView(permissions.UserIsLinkedPersonMixin, CreateView): +class RelationshipCreateView(permissions.UserIsLinkedPersonMixin, FormView): """ View for creating a :class:`Relationship`. @@ -26,53 +28,45 @@ class RelationshipCreateView(permissions.UserIsLinkedPersonMixin, CreateView): """ model = models.Relationship template_name = 'people/relationship/create.html' - 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')) + form_class = forms.RelationshipForm + def get_person(self) -> models.Person: return models.Person.objects.get(pk=self.kwargs.get('person_pk')) - def get(self, request, *args, **kwargs): - self.person = models.Person.objects.get(pk=self.kwargs.get('person_pk')) + def get_test_person(self) -> models.Person: + return self.get_person() - return super().get(request, *args, **kwargs) + def form_valid(self, form): + try: + self.object = models.Relationship.objects.create( + source=self.get_person(), target=form.cleaned_data['target']) - def post(self, request, *args, **kwargs): - self.person = models.Person.objects.get(pk=self.kwargs.get('person_pk')) + except IntegrityError: + form.add_error( + None, + ValidationError('This relationship already exists', + code='already-exists')) + return self.form_invalid(form) - return super().post(request, *args, **kwargs) - - def get_initial(self): - initial = super().get_initial() - - initial['source'] = self.request.user.person - initial['target'] = self.person - - return initial + return super().form_valid(form) def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - context['person'] = self.person + context['person'] = self.get_person() return context def get_success_url(self): - return reverse('people:relationship.update', kwargs={'relationship_pk': self.object.pk}) + return reverse('people:relationship.update', + kwargs={'relationship_pk': self.object.pk}) class RelationshipUpdateView(permissions.UserIsLinkedPersonMixin, CreateView): """ - View for creating a :class:`Relationship`. + 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 @@ -83,18 +77,21 @@ class RelationshipUpdateView(permissions.UserIsLinkedPersonMixin, CreateView): """ Get the person instance which should be used for access control checks. """ - relationship = models.Relationship.objects.get(pk=self.kwargs.get('relationship_pk')) + 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.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.relationship = models.Relationship.objects.get( + pk=self.kwargs.get('relationship_pk')) self.person = self.relationship.source return super().post(request, *args, **kwargs) @@ -122,7 +119,8 @@ class RelationshipUpdateView(permissions.UserIsLinkedPersonMixin, CreateView): 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(pk=self.object.pk): + for answer_set in self.relationship.answer_sets.exclude( + pk=self.object.pk): answer_set.replaced_timestamp = now_date answer_set.save() From 5035b121a6a07dd534b9fed2e0fa45cf69fb5074 Mon Sep 17 00:00:00 2001 From: James Graham Date: Thu, 26 Nov 2020 12:59:30 +0000 Subject: [PATCH 03/11] refactor: migrate to question sets for person qs This means we're starting to use the same system for person questions as for relationship questions --- people/admin.py | 27 +++- people/forms.py | 1 - .../0022_refactor_person_questions.py | 55 ++++++++ people/migrations/0023_remove_person_role.py | 65 +++++++++ people/migrations/utils/__init__.py | 0 people/migrations/utils/question_sets.py | 23 +++ people/models/person.py | 132 +++++++++++++++--- people/templates/people/person/detail.html | 32 ++++- 8 files changed, 304 insertions(+), 31 deletions(-) create mode 100644 people/migrations/0022_refactor_person_questions.py create mode 100644 people/migrations/0023_remove_person_role.py create mode 100644 people/migrations/utils/__init__.py create mode 100644 people/migrations/utils/question_sets.py diff --git a/people/admin.py b/people/admin.py index a6f4ae9..82c6f46 100644 --- a/people/admin.py +++ b/people/admin.py @@ -21,19 +21,34 @@ class OrganisationAdmin(admin.ModelAdmin): pass -@admin.register(models.Role) -class RoleAdmin(admin.ModelAdmin): - pass - - @admin.register(models.Theme) class ThemeAdmin(admin.ModelAdmin): pass +class PersonQuestionChoiceInline(admin.TabularInline): + model = models.PersonQuestionChoice + + +@admin.register(models.PersonQuestion) +class PersonQuestionAdmin(admin.ModelAdmin): + inlines = [ + PersonQuestionChoiceInline, + ] + + +class PersonAnswerSetInline(admin.TabularInline): + model = models.PersonAnswerSet + readonly_fields = [ + 'question_answers', + ] + + @admin.register(models.Person) class PersonAdmin(admin.ModelAdmin): - pass + inlines = [ + PersonAnswerSetInline, + ] class RelationshipQuestionChoiceInline(admin.TabularInline): diff --git a/people/forms.py b/people/forms.py index 7ddf909..2b9850f 100644 --- a/people/forms.py +++ b/people/forms.py @@ -40,7 +40,6 @@ class PersonForm(forms.ModelForm): 'organisation_started_date', 'job_title', 'disciplines', - 'role', 'themes', ] widgets = { diff --git a/people/migrations/0022_refactor_person_questions.py b/people/migrations/0022_refactor_person_questions.py new file mode 100644 index 0000000..2da386b --- /dev/null +++ b/people/migrations/0022_refactor_person_questions.py @@ -0,0 +1,55 @@ +# Generated by Django 2.2.10 on 2020-11-23 14:29 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('people', '0021_refactor_person_disciplines'), + ] + + operations = [ + migrations.CreateModel( + name='PersonQuestion', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('version', models.PositiveSmallIntegerField(default=1)), + ('text', models.CharField(max_length=255)), + ('order', models.SmallIntegerField(default=0)), + ], + options={ + 'ordering': ['order', 'text'], + }, + ), + migrations.CreateModel( + name='PersonQuestionChoice', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('text', models.CharField(max_length=255)), + ('order', models.SmallIntegerField(default=0)), + ('question', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='answers', to='people.PersonQuestion')), + ], + options={ + 'ordering': ['question__order', 'order', 'text'], + }, + ), + migrations.CreateModel( + name='PersonAnswerSet', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('timestamp', models.DateTimeField(auto_now_add=True)), + ('replaced_timestamp', models.DateTimeField(blank=True, editable=False, null=True)), + ('person', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='answer_sets', to='people.Person')), + ('question_answers', models.ManyToManyField(to='people.PersonQuestionChoice')), + ], + options={ + 'ordering': ['timestamp'], + }, + ), + migrations.AddConstraint( + model_name='personquestionchoice', + constraint=models.UniqueConstraint(fields=('question', 'text'), name='unique_question_answer'), + ), + ] diff --git a/people/migrations/0023_remove_person_role.py b/people/migrations/0023_remove_person_role.py new file mode 100644 index 0000000..f67c4a0 --- /dev/null +++ b/people/migrations/0023_remove_person_role.py @@ -0,0 +1,65 @@ +# Generated by Django 2.2.10 on 2020-11-25 15:50 + +from django.core.exceptions import ObjectDoesNotExist +from django.db import migrations +from django.utils import timezone + +from .utils.question_sets import port_question + + +def migrate_forward(apps, schema_editor): + Person = apps.get_model('people', 'Person') + Role = apps.get_model('people', 'Role') + + role_question = port_question(apps, 'Role', + Role.objects.values_list('name', flat=True)) + + for person in Person.objects.all(): + try: + prev_set = person.answer_sets.latest('timestamp') + + except ObjectDoesNotExist: + prev_set = None + + try: + + answer_set = person.answer_sets.create() + answer_set.question_answers.add( + role_question.answers.get(text=person.role.name)) + + prev_set.replaced_timestamp = timezone.datetime.now() + + except AttributeError: + pass + + +def migrate_backward(apps, schema_editor): + Person = apps.get_model('people', 'Person') + Role = apps.get_model('people', 'Role') + + for person in Person.objects.all(): + try: + current_answers = person.answer_sets.latest('timestamp') + role_answer = current_answers.question_answers.get( + question__text='Role') + person.role, _ = Role.objects.get_or_create(name=role_answer.text) + person.save() + + except ObjectDoesNotExist: + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ('people', '0022_refactor_person_questions'), + ] + + operations = [ + migrations.RunPython(migrate_forward, migrate_backward), + migrations.RemoveField( + model_name='person', + name='role', + ), + migrations.DeleteModel('Role'), + ] diff --git a/people/migrations/utils/__init__.py b/people/migrations/utils/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/people/migrations/utils/question_sets.py b/people/migrations/utils/question_sets.py new file mode 100644 index 0000000..6dfa082 --- /dev/null +++ b/people/migrations/utils/question_sets.py @@ -0,0 +1,23 @@ + +import typing + +from django.core.exceptions import ObjectDoesNotExist + + +def port_question(apps, question_text: str, + answers_text: typing.Iterable[str]): + PersonQuestion = apps.get_model('people', 'PersonQuestion') + + try: + prev_question = PersonQuestion.objects.filter( + text=question_text).latest('version') + question = PersonQuestion.objects.create( + text=question_text, version=prev_question.version + 1) + + except ObjectDoesNotExist: + question = PersonQuestion.objects.create(text=question_text) + + for answer_text in answers_text: + question.answers.get_or_create(text=answer_text) + + return question diff --git a/people/models/person.py b/people/models/person.py index 6127f66..c7f0ec5 100644 --- a/people/models/person.py +++ b/people/models/person.py @@ -1,10 +1,12 @@ import logging +import typing from django.conf import settings from django.contrib.auth.models import AbstractUser from django.core.exceptions import ValidationError from django.db import models from django.urls import reverse +from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ from django_countries.fields import CountryField @@ -18,9 +20,11 @@ logger = logging.getLogger(__name__) # pylint: disable=invalid-name __all__ = [ 'User', 'Organisation', - 'Role', 'Theme', + 'PersonQuestion', + 'PersonQuestionChoice', 'Person', + 'PersonAnswerSet', ] @@ -36,7 +40,7 @@ class User(AbstractUser): """ return hasattr(self, 'person') - def send_welcome_email(self): + def send_welcome_email(self) -> None: """Send a welcome email to a new user.""" # Get exported data from settings.py first context = settings_export(None) @@ -71,16 +75,6 @@ class Organisation(models.Model): return self.name -class Role(models.Model): - """ - Role which a :class:`Person` holds within the project. - """ - name = models.CharField(max_length=255, blank=False, null=False) - - def __str__(self) -> str: - return self.name - - class Theme(models.Model): """ Project theme within which a :class:`Person` works. @@ -91,6 +85,79 @@ class Theme(models.Model): return self.name +class PersonQuestion(models.Model): + """Question which may be asked about a person.""" + 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:`PersonQuestionChoice`s for this question into Django choices. + """ + return [ + [choice.pk, str(choice)] for choice in self.answers.all() + ] + + @property + def slug(self) -> str: + return slugify(self.text) + + def __str__(self) -> str: + return self.text + + +class PersonQuestionChoice(models.Model): + """ + Allowed answer to a :class:`PersonQuestion`. + """ + 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(PersonQuestion, 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) + + @property + def slug(self) -> str: + return slugify(self.text) + + def __str__(self) -> str: + return self.text + + class Person(models.Model): """ A person may be a member of the BRECcIA core team or an external stakeholder. @@ -168,13 +235,6 @@ class Person(models.Model): #: Discipline(s) within which this person works disciplines = models.CharField(max_length=255, blank=True, null=True) - #: Role this person holds within the project - role = models.ForeignKey(Role, - on_delete=models.PROTECT, - related_name='holders', - blank=True, - null=True) - #: Project themes within this person works themes = models.ManyToManyField(Theme, related_name='people', blank=True) @@ -183,8 +243,42 @@ class Person(models.Model): return self.relationships_as_source.all().union( self.relationships_as_target.all()) + @property + def current_answers(self) -> 'PersonAnswerSet': + return self.answer_sets.last() + def get_absolute_url(self): return reverse('people:person.detail', kwargs={'pk': self.pk}) def __str__(self) -> str: return self.name + + +class PersonAnswerSet(models.Model): + """ + The answers to the person questions at a particular point in time. + """ + + class Meta: + ordering = [ + 'timestamp', + ] + + #: Person to which this answer set belongs + person = models.ForeignKey(Person, + on_delete=models.CASCADE, + related_name='answer_sets', + blank=False, null=False) + + #: Answers to :class:`PersonQuestion`s + question_answers = models.ManyToManyField(PersonQuestionChoice) + + #: When were these answers collected? + timestamp = models.DateTimeField(auto_now_add=True, + editable=False) + + replaced_timestamp = models.DateTimeField(blank=True, null=True, + editable=False) + + def get_absolute_url(self): + return self.person.get_absolute_url() diff --git a/people/templates/people/person/detail.html b/people/templates/people/person/detail.html index 5e49e83..27f7fd4 100644 --- a/people/templates/people/person/detail.html +++ b/people/templates/people/person/detail.html @@ -58,11 +58,6 @@
{{ person.job_title }}
{% endif %} - {% if person.role %} -
Role
-
{{ person.role }}
- {% endif %} - {% if person.disciplines %}
Discipline(s)
{{ person.disciplines }}
@@ -79,6 +74,33 @@ {% endif %} + {% with person.current_answers as answer_set %} +
+ + + + + + + + + {% for answer in answer_set.question_answers.all %} + + + + + + {% empty %} + + + + {% endfor %} + +
QuestionAnswer
{{ answer.question }}{{ answer }}
No records
+ +

Last updated: {{ answer_set.timestamp }}

+ {% endwith %} + Update From f95e06aa18b4d74bc8824f06b7ed856e8d2f6873 Mon Sep 17 00:00:00 2001 From: James Graham Date: Thu, 26 Nov 2020 14:10:38 +0000 Subject: [PATCH 04/11] refactor: move age and gender to question set --- people/forms.py | 2 - people/migrations/0024_remove_age_gender.py | 120 ++++++++++++++++++++ people/models/person.py | 28 ----- people/templates/people/person/detail.html | 10 -- 4 files changed, 120 insertions(+), 40 deletions(-) create mode 100644 people/migrations/0024_remove_age_gender.py diff --git a/people/forms.py b/people/forms.py index 2b9850f..645fd1a 100644 --- a/people/forms.py +++ b/people/forms.py @@ -32,8 +32,6 @@ class PersonForm(forms.ModelForm): model = models.Person fields = [ 'name', - 'gender', - 'age_group', 'nationality', 'country_of_residence', 'organisation', diff --git a/people/migrations/0024_remove_age_gender.py b/people/migrations/0024_remove_age_gender.py new file mode 100644 index 0000000..4bbba8c --- /dev/null +++ b/people/migrations/0024_remove_age_gender.py @@ -0,0 +1,120 @@ +# Generated by Django 2.2.10 on 2020-11-26 13:03 + +from django.core.exceptions import ObjectDoesNotExist +from django.db import migrations + +from backports.db.models.enums import TextChoices +from .utils.question_sets import port_question + + +class GenderChoices(TextChoices): + MALE = 'M', 'Male' + FEMALE = 'F', 'Female' + OTHER = 'O', 'Other' + PREFER_NOT_TO_SAY = 'N', 'Prefer not to say' + + +class AgeGroupChoices(TextChoices): + LTE_25 = '<=25', '25 or under' + BETWEEN_26_30 = '26-30', '26-30' + BETWEEN_31_35 = '31-35', '31-35' + BETWEEN_36_40 = '36-40', '36-40' + BETWEEN_41_45 = '41-45', '41-45' + BETWEEN_46_50 = '46-50', '46-50' + BETWEEN_51_55 = '51-55', '51-55' + BETWEEN_56_60 = '56-60', '56-60' + GTE_61 = '>=61', '61 or older' + PREFER_NOT_TO_SAY = 'N', 'Prefer not to say' + + +def migrate_forward(apps, schema_editor): + Person = apps.get_model('people', 'Person') + + gender_question = port_question(apps, 'Gender', GenderChoices.labels) + age_question = port_question(apps, 'Age', AgeGroupChoices.labels) + + for person in Person.objects.all(): + try: + answer_set = person.answer_sets.latest('timestamp') + + except ObjectDoesNotExist: + answer_set = person.answer_sets.create() + + try: + gender = [ + item for item in GenderChoices if item.value == person.gender + ][0] + answer_set.question_answers.filter( + question__text=gender_question.text).delete() + answer_set.question_answers.add( + gender_question.answers.get(text__iexact=gender.label)) + + except (AttributeError, IndexError): + pass + + try: + age = [ + item for item in AgeGroupChoices + if item.value == person.age_group + ][0] + answer_set.question_answers.filter( + question__text=age_question.text).delete() + answer_set.question_answers.add( + age_question.answers.get(text__iexact=age.label)) + + except (AttributeError, IndexError): + pass + + +def migrate_backward(apps, schema_editor): + Person = apps.get_model('people', 'Person') + + for person in Person.objects.all(): + try: + current_answers = person.answer_sets.latest('timestamp') + age_answer = current_answers.question_answers.get( + question__text='Age') + + person.age_group = [ + item for item in AgeGroupChoices + if item.label == age_answer.text + ][0].value + + person.save() + + except ObjectDoesNotExist: + pass + + try: + current_answers = person.answer_sets.latest('timestamp') + gender_answer = current_answers.question_answers.get( + question__text='Gender') + person.gender = [ + + item for item in GenderChoices + if item.label == gender_answer.text + ][0].value + + person.save() + + except ObjectDoesNotExist: + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ('people', '0023_remove_person_role'), + ] + + operations = [ + migrations.RunPython(migrate_forward, migrate_backward), + migrations.RemoveField( + model_name='person', + name='age_group', + ), + migrations.RemoveField( + model_name='person', + name='gender', + ), + ] diff --git a/people/models/person.py b/people/models/person.py index c7f0ec5..dbcef92 100644 --- a/people/models/person.py +++ b/people/models/person.py @@ -186,34 +186,6 @@ class Person(models.Model): ############################################################### # Data collected for analysis of community makeup and structure - class GenderChoices(TextChoices): - MALE = 'M', _('Male') - FEMALE = 'F', _('Female') - OTHER = 'O', _('Other') - PREFER_NOT_TO_SAY = 'N', _('Prefer not to say') - - gender = models.CharField(max_length=1, - choices=GenderChoices.choices, - blank=True, - null=False) - - class AgeGroupChoices(TextChoices): - LTE_25 = '<=25', _('25 or under') - BETWEEN_26_30 = '26-30', _('26-30') - BETWEEN_31_35 = '31-35', _('31-35') - BETWEEN_36_40 = '36-40', _('36-40') - BETWEEN_41_45 = '41-45', _('41-45') - BETWEEN_46_50 = '46-50', _('46-50') - BETWEEN_51_55 = '51-55', _('51-55') - BETWEEN_56_60 = '56-60', _('56-60') - GTE_61 = '>=61', _('61 or older') - PREFER_NOT_TO_SAY = 'N', _('Prefer not to say') - - age_group = models.CharField(max_length=5, - choices=AgeGroupChoices.choices, - blank=True, - null=False) - nationality = CountryField(blank=True, null=True) country_of_residence = CountryField(blank=True, null=True) diff --git a/people/templates/people/person/detail.html b/people/templates/people/person/detail.html index 27f7fd4..30c8430 100644 --- a/people/templates/people/person/detail.html +++ b/people/templates/people/person/detail.html @@ -23,16 +23,6 @@ {% endif %}
- {% if person.gender %} -
Gender
-
{{ person.get_gender_display }}
- {% endif %} - - {% if person.age_group %} -
Age Group
-
{{ person.get_age_group_display }}
- {% endif %} - {% if person.nationality %}
Nationality
{{ person.nationality.name }}
From d6e42cc18d421a9f381d9a81a170d6642d3c0d3e Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 2 Dec 2020 14:25:49 +0000 Subject: [PATCH 05/11] style: fix pyflakes warnings of unused imports --- export/serializers/__init__.py | 6 ++++++ export/views/__init__.py | 7 +++++++ people/views/__init__.py | 7 +++++++ 3 files changed, 20 insertions(+) diff --git a/export/serializers/__init__.py b/export/serializers/__init__.py index 104caf6..3b877e4 100644 --- a/export/serializers/__init__.py +++ b/export/serializers/__init__.py @@ -2,3 +2,9 @@ from . import ( activities, people ) + + +__all__ = [ + 'activities', + 'people', +] diff --git a/export/views/__init__.py b/export/views/__init__.py index da2cdd2..02b4be7 100644 --- a/export/views/__init__.py +++ b/export/views/__init__.py @@ -4,3 +4,10 @@ from . import ( activities, people ) + + +__all__ = [ + 'activities', + 'people', + 'ExportListView', +] diff --git a/people/views/__init__.py b/people/views/__init__.py index 39dd5c9..e7ddb60 100644 --- a/people/views/__init__.py +++ b/people/views/__init__.py @@ -7,3 +7,10 @@ from . import ( person, relationship ) + + +__all__ = [ + 'network', + 'person', + 'relationship', +] From 91a47b4fdc4437110b3d0e365e6350cb75822af2 Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 2 Dec 2020 14:40:59 +0000 Subject: [PATCH 06/11] refactor: move static person qs to answerset --- .../0025_rename_relationship_target.py | 26 +++ .../0026_move_static_person_questions.py | 148 ++++++++++++++++++ people/models/person.py | 90 +++++------ people/models/relationship.py | 52 ++++-- 4 files changed, 259 insertions(+), 57 deletions(-) create mode 100644 people/migrations/0025_rename_relationship_target.py create mode 100644 people/migrations/0026_move_static_person_questions.py diff --git a/people/migrations/0025_rename_relationship_target.py b/people/migrations/0025_rename_relationship_target.py new file mode 100644 index 0000000..2c06a96 --- /dev/null +++ b/people/migrations/0025_rename_relationship_target.py @@ -0,0 +1,26 @@ +# Generated by Django 2.2.10 on 2020-11-27 08:49 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('people', '0024_remove_age_gender'), + ] + + operations = [ + migrations.RemoveConstraint( + model_name='relationship', + name='unique_relationship', + ), + migrations.RenameField( + model_name='relationship', + old_name='target', + new_name='target_person', + ), + migrations.AddConstraint( + model_name='relationship', + constraint=models.UniqueConstraint(fields=('source', 'target_person'), name='unique_relationship'), + ), + ] diff --git a/people/migrations/0026_move_static_person_questions.py b/people/migrations/0026_move_static_person_questions.py new file mode 100644 index 0000000..b584848 --- /dev/null +++ b/people/migrations/0026_move_static_person_questions.py @@ -0,0 +1,148 @@ +# Generated by Django 2.2.10 on 2020-12-02 13:31 + +from django.core.exceptions import ObjectDoesNotExist +from django.db import migrations, models +import django.db.models.deletion +import django_countries.fields + + +def migrate_forward(apps, schema_editor): + Person = apps.get_model('people', 'Person') + PersonAnswerset = apps.get_model('people', 'PersonAnswerSet') + + fields = { + 'country_of_residence', + 'disciplines', + 'job_title', + 'nationality', + 'organisation', + 'organisation_started_date', + 'themes', + } + + for person in Person.objects.all(): + try: + answer_set = person.answer_sets.last() + + except ObjectDoesNotExist: + answer_set = person.answer_sets.create() + + for field in fields: + value = getattr(person, field) + try: + setattr(answer_set, field, value) + + except TypeError: + # Cannot directly set an m2m field + m2m = getattr(answer_set, field) + m2m.set(value.all()) + + answer_set.save() + + +def migrate_backward(apps, schema_editor): + Person = apps.get_model('people', 'Person') + PersonAnswerset = apps.get_model('people', 'PersonAnswerSet') + + fields = { + 'country_of_residence', + 'disciplines', + 'job_title', + 'nationality', + 'organisation', + 'organisation_started_date', + 'themes', + } + + for person in Person.objects.all(): + try: + answer_set = person.answer_sets.last() + + for field in fields: + value = getattr(answer_set, field) + try: + setattr(person, field, value) + + except TypeError: + # Cannot directly set an m2m field + m2m = getattr(person, field) + m2m.set(value.all()) + + person.save() + + except ObjectDoesNotExist: + pass + + +class Migration(migrations.Migration): + + dependencies = [ + ('people', '0025_rename_relationship_target'), + ] + + operations = [ + migrations.AddField( + model_name='personanswerset', + name='country_of_residence', + field=django_countries.fields.CountryField(blank=True, max_length=2, null=True), + ), + migrations.AddField( + model_name='personanswerset', + name='disciplines', + field=models.CharField(blank=True, max_length=255, null=True), + ), + migrations.AddField( + model_name='personanswerset', + name='job_title', + field=models.CharField(blank=True, max_length=255), + ), + migrations.AddField( + model_name='personanswerset', + name='nationality', + field=django_countries.fields.CountryField(blank=True, max_length=2, null=True), + ), + migrations.AddField( + model_name='personanswerset', + name='organisation', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='members', to='people.Organisation'), + ), + migrations.AddField( + model_name='personanswerset', + name='organisation_started_date', + field=models.DateField(null=True, verbose_name='Date started at this organisation'), + ), + migrations.AddField( + model_name='personanswerset', + name='themes', + field=models.ManyToManyField(blank=True, related_name='people', to='people.Theme'), + ), + migrations.RunPython(migrate_forward, migrate_backward), + migrations.RemoveField( + model_name='person', + name='country_of_residence', + ), + migrations.RemoveField( + model_name='person', + name='disciplines', + ), + migrations.RemoveField( + model_name='person', + name='job_title', + ), + migrations.RemoveField( + model_name='person', + name='nationality', + ), + migrations.RemoveField( + model_name='person', + name='organisation', + ), + migrations.RemoveField( + model_name='person', + name='organisation_started_date', + ), + migrations.RemoveField( + model_name='person', + name='themes', + ), + ] diff --git a/people/models/person.py b/people/models/person.py index dbcef92..e0b367d 100644 --- a/people/models/person.py +++ b/people/models/person.py @@ -13,8 +13,6 @@ from django_countries.fields import CountryField from django_settings_export import settings_export from post_office import mail -from backports.db.models.enums import TextChoices - logger = logging.getLogger(__name__) # pylint: disable=invalid-name __all__ = [ @@ -180,11 +178,51 @@ class Person(models.Model): 'self', related_name='relationship_sources', through='Relationship', - through_fields=('source', 'target'), + through_fields=('source', 'target_person'), symmetrical=False) - ############################################################### - # Data collected for analysis of community makeup and structure + @property + def relationships(self): + return self.relationships_as_source.all().union( + self.relationships_as_target.all()) + + @property + def current_answers(self) -> 'PersonAnswerSet': + return self.answer_sets.last() + + def get_absolute_url(self): + return reverse('people:person.detail', kwargs={'pk': self.pk}) + + def __str__(self) -> str: + return self.name + + +class PersonAnswerSet(models.Model): + """The answers to the person questions at a particular point in time.""" + class Meta: + ordering = [ + 'timestamp', + ] + + #: Person to which this answer set belongs + person = models.ForeignKey(Person, + on_delete=models.CASCADE, + related_name='answer_sets', + blank=False, + null=False) + + #: Answers to :class:`PersonQuestion`s + question_answers = models.ManyToManyField(PersonQuestionChoice) + + #: When were these answers collected? + timestamp = models.DateTimeField(auto_now_add=True, editable=False) + + replaced_timestamp = models.DateTimeField(blank=True, + null=True, + editable=False) + + ################## + # Static questions nationality = CountryField(blank=True, null=True) @@ -210,47 +248,5 @@ class Person(models.Model): #: Project themes within this person works themes = models.ManyToManyField(Theme, related_name='people', blank=True) - @property - def relationships(self): - return self.relationships_as_source.all().union( - self.relationships_as_target.all()) - - @property - def current_answers(self) -> 'PersonAnswerSet': - return self.answer_sets.last() - - def get_absolute_url(self): - return reverse('people:person.detail', kwargs={'pk': self.pk}) - - def __str__(self) -> str: - return self.name - - -class PersonAnswerSet(models.Model): - """ - The answers to the person questions at a particular point in time. - """ - - class Meta: - ordering = [ - 'timestamp', - ] - - #: Person to which this answer set belongs - person = models.ForeignKey(Person, - on_delete=models.CASCADE, - related_name='answer_sets', - blank=False, null=False) - - #: Answers to :class:`PersonQuestion`s - question_answers = models.ManyToManyField(PersonQuestionChoice) - - #: When were these answers collected? - timestamp = models.DateTimeField(auto_now_add=True, - editable=False) - - replaced_timestamp = models.DateTimeField(blank=True, null=True, - editable=False) - def get_absolute_url(self): return self.person.get_absolute_url() diff --git a/people/models/relationship.py b/people/models/relationship.py index 2776f71..a739190 100644 --- a/people/models/relationship.py +++ b/people/models/relationship.py @@ -4,6 +4,7 @@ Models describing relationships between people. import typing +from django.core.exceptions import ObjectDoesNotExist from django.db import models from django.urls import reverse from django.utils.text import slugify @@ -48,7 +49,7 @@ class RelationshipQuestion(models.Model): return [ [choice.pk, str(choice)] for choice in self.answers.all() ] - + @property def slug(self) -> str: return slugify(self.text) @@ -93,6 +94,19 @@ class RelationshipQuestionChoice(models.Model): return self.text +# class ExternalPerson(models.Model): +# """Model representing a person external to the project. + +# These will never need to be linked to a :class:`User` as they +# will never log in to the system. +# """ +# name = models.CharField(max_length=255, +# blank=False, null=False) + +# def __str__(self) -> str: +# return self.name + + class Relationship(models.Model): """ A directional relationship between two people allowing linked questions. @@ -100,7 +114,7 @@ class Relationship(models.Model): class Meta: constraints = [ - models.UniqueConstraint(fields=['source', 'target'], + models.UniqueConstraint(fields=['source', 'target_person'], name='unique_relationship'), ] @@ -110,16 +124,34 @@ class Relationship(models.Model): 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) - + target_person = models.ForeignKey(Person, + related_name='relationships_as_target', + on_delete=models.CASCADE, + blank=False, + null=False) + # blank=True, + # null=True) + + # target_external_person = models.ForeignKey( + # ExternalPerson, + # related_name='relationships_as_target', + # on_delete=models.CASCADE, + # blank=True, + # null=True) + #: 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 target(self) -> Person: + if self.target_person: + return self.target_person + + raise ObjectDoesNotExist('Relationship has no target linked') + @property def current_answers(self) -> 'RelationshipAnswerSet': return self.answer_sets.last() @@ -137,8 +169,8 @@ class Relationship(models.Model): @raise Relationship.DoesNotExist: When the reverse relationship is not known """ - return type(self).objects.get(source=self.target, - target=self.source) + return type(self).objects.get(source=self.target_person, + target_person=self.source) class RelationshipAnswerSet(models.Model): @@ -163,7 +195,7 @@ class RelationshipAnswerSet(models.Model): #: When were these answers collected? timestamp = models.DateTimeField(auto_now_add=True, editable=False) - + replaced_timestamp = models.DateTimeField(blank=True, null=True, editable=False) From 0288b0320d1d0e5c144799356b8ed574ffbe6112 Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 2 Dec 2020 14:57:07 +0000 Subject: [PATCH 07/11] refactor: move questions on person detail page --- people/templates/people/person/detail.html | 132 ++++++++++----------- 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/people/templates/people/person/detail.html b/people/templates/people/person/detail.html index 30c8430..367bac5 100644 --- a/people/templates/people/person/detail.html +++ b/people/templates/people/person/detail.html @@ -22,81 +22,81 @@ {% endif %} -
- {% if person.nationality %} -
Nationality
-
{{ person.nationality.name }}
- {% endif %} - {% if person.country_of_residence %} -
Country of Residence
-
{{ person.country_of_residence.name }}
- {% endif %} + {% with person.current_answers as answer_set %} +
+
- {% if person.organisation %} -
Organisation
-
{{ person.organisation }}
+ + + + + + + - {% if person.organisation_started_date %} -
Started Date
-
{{ person.organisation_started_date }}
+ + {% if answer_set.nationality %} + {% endif %} - {% endif %} - {% if person.job_title %} -
Job Title
-
{{ person.job_title }}
- {% endif %} + {% if answer_set.country_of_residence %} + + {% endif %} - {% if person.disciplines %} -
Discipline(s)
-
{{ person.disciplines }}
- {% endif %} + {% if answer_set.organisation %} + + {% endif %} - {% if person.themes.exists %} -
Project Themes
-
- {% for theme in person.themes.all %} - {{ theme }}{% if not forloop.last %}, {% endif %} - {% endfor %} -
- {% endif %} - + {% if answer_set.organisation_started_date %} + + {% endif %} + + {% if answer_set.job_title %} + + {% endif %} + + {% if answer_set.disciplines %} + + {% endif %} + + {% if answer_set.themes.exists %} + + + + + {% endif %} + + {% for answer in answer_set.question_answers.all %} + + + + + + {% empty %} + + + + {% endfor %} + +
QuestionAnswer
Nationality{{ answer_set.nationality.name }}
Country of Residence{{ answer_set.country_of_residence.name }}
Organisation{{ answer_set.organisation }}
Organisation Started Date{{ answer_set.organisation_started_date }}
Job Title{{ answer_set.job_title }}
Discipline(s){{ answer_set.disciplines }}
Project Themes + {% for theme in answer_set.themes.all %} + {{ theme }}{% if not forloop.last %}, {% endif %} + {% endfor %} +
{{ answer.question }}{{ answer }}
No records
+ +

Last updated: {{ answer_set.timestamp }}

+ {% endwith %} + + Update + + {% if person.user == request.user %} + Change Password + {% endif %} {% endif %} - {% with person.current_answers as answer_set %} - - - - - - - - - - {% for answer in answer_set.question_answers.all %} - - - - - - {% empty %} - - - - {% endfor %} - -
QuestionAnswer
{{ answer.question }}{{ answer }}
No records
- -

Last updated: {{ answer_set.timestamp }}

- {% endwith %} - - Update - - Change Password -
From 2027d9d3abd70983a590cc708bf6b592767fff4f Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 2 Dec 2020 14:57:27 +0000 Subject: [PATCH 08/11] fix: remove refactored questions from person form --- people/forms.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/people/forms.py b/people/forms.py index 645fd1a..5e6a2a1 100644 --- a/people/forms.py +++ b/people/forms.py @@ -32,13 +32,6 @@ class PersonForm(forms.ModelForm): model = models.Person fields = [ 'name', - 'nationality', - 'country_of_residence', - 'organisation', - 'organisation_started_date', - 'job_title', - 'disciplines', - 'themes', ] widgets = { 'nationality': Select2Widget(), From 6bb4f09454ddef135fe5a32e99b04319036b2c57 Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 2 Dec 2020 15:53:30 +0000 Subject: [PATCH 09/11] refactor: update forms to match moved questions --- people/forms.py | 66 ++++++++++++++++++++++++++++++------------ people/views/person.py | 42 +++++++++++++++++++++++---- 2 files changed, 84 insertions(+), 24 deletions(-) diff --git a/people/forms.py b/people/forms.py index 5e6a2a1..38561aa 100644 --- a/people/forms.py +++ b/people/forms.py @@ -25,29 +25,12 @@ def get_date_year_range() -> typing.Iterable[int]: class PersonForm(forms.ModelForm): - """ - Form for creating / updating an instance of :class:`Person`. - """ + """Form for creating / updating an instance of :class:`Person`.""" class Meta: model = models.Person fields = [ 'name', ] - widgets = { - 'nationality': Select2Widget(), - 'country_of_residence': Select2Widget(), - 'themes': Select2MultipleWidget(), - } - help_texts = { - 'organisation_started_date': - 'If you don\'t know the exact date, an approximate date is okay.', - } - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - - self.fields['organisation_started_date'].widget = SelectDateWidget( - years=get_date_year_range()) class RelationshipForm(forms.Form): @@ -60,11 +43,12 @@ class DynamicAnswerSetBase(forms.Form): field_class = forms.ModelChoiceField field_widget = None field_required = True + question_model = None def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - for question in models.RelationshipQuestion.objects.all(): + for question in self.question_model.objects.all(): field = self.field_class(label=question, queryset=question.answers, widget=self.field_widget, @@ -72,6 +56,47 @@ class DynamicAnswerSetBase(forms.Form): self.fields['question_{}'.format(question.pk)] = field +class PersonAnswerSetForm(forms.ModelForm, DynamicAnswerSetBase): + """Form for variable person attributes. + + Dynamic fields inspired by https://jacobian.org/2010/feb/28/dynamic-form-generation/ + """ + class Meta: + model = models.PersonAnswerSet + fields = [ + 'nationality', + 'country_of_residence', + 'organisation', + 'organisation_started_date', + 'job_title', + 'disciplines', + 'themes', + ] + widgets = { + 'nationality': Select2Widget(), + 'country_of_residence': Select2Widget(), + 'themes': Select2MultipleWidget(), + } + help_texts = { + 'organisation_started_date': + 'If you don\'t know the exact date, an approximate date is okay.', + } + + question_model = models.PersonQuestion + + def save(self, commit=True) -> models.PersonAnswerSet: + # Save Relationship model + self.instance = super().save(commit=commit) + + if commit: + # Save answers to relationship questions + for key, value in self.cleaned_data.items(): + if key.startswith('question_') and value: + self.instance.question_answers.add(value) + + return self.instance + + class RelationshipAnswerSetForm(forms.ModelForm, DynamicAnswerSetBase): """ Form to allow users to describe a relationship. @@ -84,6 +109,8 @@ class RelationshipAnswerSetForm(forms.ModelForm, DynamicAnswerSetBase): 'relationship', ] + question_model = models.RelationshipQuestion + def save(self, commit=True) -> models.RelationshipAnswerSet: # Save Relationship model self.instance = super().save(commit=commit) @@ -104,6 +131,7 @@ class NetworkFilterForm(DynamicAnswerSetBase): field_class = forms.ModelMultipleChoiceField field_widget = Select2MultipleWidget field_required = False + question_model = models.RelationshipQuestion def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) diff --git a/people/views/person.py b/people/views/person.py index 92e7ee9..c495a86 100644 --- a/people/views/person.py +++ b/people/views/person.py @@ -3,6 +3,7 @@ Views for displaying or manipulating instances of :class:`Person`. """ from django.contrib.auth.mixins import LoginRequiredMixin +from django.utils import timezone from django.views.generic import CreateView, DetailView, ListView, UpdateView from people import forms, models, permissions @@ -55,9 +56,40 @@ class ProfileView(permissions.UserIsLinkedPersonMixin, DetailView): class PersonUpdateView(permissions.UserIsLinkedPersonMixin, UpdateView): - """ - View for updating a :class:`Person` record. - """ - model = models.Person + """View for updating a :class:`Person` record.""" + model = models.PersonAnswerSet template_name = 'people/person/update.html' - form_class = forms.PersonForm + form_class = forms.PersonAnswerSetForm + + def get_test_person(self) -> models.Person: + """Get the person instance which should be used for access control checks.""" + return models.Person.objects.get(pk=self.kwargs.get('pk')) + + def get(self, request, *args, **kwargs): + self.person = models.Person.objects.get(pk=self.kwargs.get('pk')) + + return super().get(request, *args, **kwargs) + + def post(self, request, *args, **kwargs): + self.person = models.Person.objects.get(pk=self.kwargs.get('pk')) + + return super().post(request, *args, **kwargs) + + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + + context['person'] = self.person + + return context + + def form_valid(self, form): + """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.person.answer_sets.exclude(pk=self.object.pk): + answer_set.replaced_timestamp = now_date + answer_set.save() + + return response From e045b084d0b30438fcc36e963d4b17b862cc4946 Mon Sep 17 00:00:00 2001 From: James Graham Date: Mon, 7 Dec 2020 14:19:00 +0000 Subject: [PATCH 10/11] refactor: extract code share between question sets --- people/models/person.py | 91 ++++------------------------- people/models/question.py | 103 +++++++++++++++++++++++++++++++++ people/models/relationship.py | 105 +++++----------------------------- 3 files changed, 128 insertions(+), 171 deletions(-) create mode 100644 people/models/question.py diff --git a/people/models/person.py b/people/models/person.py index e0b367d..aebc680 100644 --- a/people/models/person.py +++ b/people/models/person.py @@ -1,18 +1,18 @@ import logging -import typing from django.conf import settings from django.contrib.auth.models import AbstractUser from django.core.exceptions import ValidationError from django.db import models from django.urls import reverse -from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ from django_countries.fields import CountryField from django_settings_export import settings_export from post_office import mail +from .question import AnswerSet, Question, QuestionChoice + logger = logging.getLogger(__name__) # pylint: disable=invalid-name __all__ = [ @@ -83,77 +83,18 @@ class Theme(models.Model): return self.name -class PersonQuestion(models.Model): +class PersonQuestion(Question): """Question which may be asked about a person.""" - 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:`PersonQuestionChoice`s for this question into Django choices. - """ - return [ - [choice.pk, str(choice)] for choice in self.answers.all() - ] - - @property - def slug(self) -> str: - return slugify(self.text) - - def __str__(self) -> str: - return self.text -class PersonQuestionChoice(models.Model): - """ - Allowed answer to a :class:`PersonQuestion`. - """ - class Meta: - constraints = [ - models.UniqueConstraint(fields=['question', 'text'], - name='unique_question_answer') - ] - ordering = [ - 'question__order', - 'order', - 'text', - ] - +class PersonQuestionChoice(QuestionChoice): + """Allowed answer to a :class:`PersonQuestion`.""" #: Question to which this answer belongs - question = models.ForeignKey(PersonQuestion, related_name='answers', + question = models.ForeignKey(PersonQuestion, + 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) - - @property - def slug(self) -> str: - return slugify(self.text) - - def __str__(self) -> str: - return self.text + blank=False, + null=False) class Person(models.Model): @@ -197,13 +138,8 @@ class Person(models.Model): return self.name -class PersonAnswerSet(models.Model): +class PersonAnswerSet(AnswerSet): """The answers to the person questions at a particular point in time.""" - class Meta: - ordering = [ - 'timestamp', - ] - #: Person to which this answer set belongs person = models.ForeignKey(Person, on_delete=models.CASCADE, @@ -214,13 +150,6 @@ class PersonAnswerSet(models.Model): #: Answers to :class:`PersonQuestion`s question_answers = models.ManyToManyField(PersonQuestionChoice) - #: When were these answers collected? - timestamp = models.DateTimeField(auto_now_add=True, editable=False) - - replaced_timestamp = models.DateTimeField(blank=True, - null=True, - editable=False) - ################## # Static questions diff --git a/people/models/question.py b/people/models/question.py new file mode 100644 index 0000000..700a4df --- /dev/null +++ b/people/models/question.py @@ -0,0 +1,103 @@ +"""Base models for configurable questions and response sets.""" +import typing + +from django.db import models +from django.utils.text import slugify + + +class Question(models.Model): + """Questions from which a survey form can be created.""" + class Meta: + abstract = True + 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:`QuestionChoice`s for this question into Django choices.""" + return [[choice.pk, str(choice)] for choice in self.answers.all()] + + @property + def slug(self) -> str: + return slugify(self.text) + + def __str__(self) -> str: + return self.text + + +class QuestionChoice(models.Model): + """Allowed answer to a :class:`Question`.""" + class Meta: + abstract = True + constraints = [ + models.UniqueConstraint(fields=['question', 'text'], + name='unique_question_answer') + ] + ordering = [ + 'question__order', + 'order', + 'text', + ] + + #: Question to which this answer belongs + #: This foreign key must be added to each concrete subclass + # question = models.ForeignKey(Question, + # 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) + + @property + def slug(self) -> str: + return slugify(self.text) + + def __str__(self) -> str: + return self.text + + +class AnswerSet(models.Model): + """The answers to a set of questions at a particular point in time.""" + class Meta: + abstract = True + ordering = [ + 'timestamp', + ] + + #: Entity to which this answer set belongs + #: This foreign key must be added to each concrete subclass + # person = models.ForeignKey(Person, + # on_delete=models.CASCADE, + # related_name='answer_sets', + # blank=False, + # null=False) + + #: Answers to :class:`Question`s + #: This many to many relation must be added to each concrete subclass + # question_answers = models.ManyToManyField(QuestionChoice) + + #: When were these answers collected? + timestamp = models.DateTimeField(auto_now_add=True, editable=False) + + #: When were these answers replaced? - happens when another set is collected + replaced_timestamp = models.DateTimeField(blank=True, + null=True, + editable=False) diff --git a/people/models/relationship.py b/people/models/relationship.py index a739190..ce68a3c 100644 --- a/people/models/relationship.py +++ b/people/models/relationship.py @@ -2,14 +2,12 @@ Models describing relationships between people. """ -import typing - from django.core.exceptions import ObjectDoesNotExist from django.db import models from django.urls import reverse -from django.utils.text import slugify from .person import Person +from .question import AnswerSet, Question, QuestionChoice __all__ = [ 'RelationshipQuestion', @@ -19,79 +17,19 @@ __all__ = [ ] -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:`RelationshipQuestionChoice`s for this question into Django choices. - """ - return [ - [choice.pk, str(choice)] for choice in self.answers.all() - ] - - @property - def slug(self) -> str: - return slugify(self.text) - - def __str__(self) -> str: - return self.text +class RelationshipQuestion(Question): + """Question which may be asked about a relationship.""" -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', - ] +class RelationshipQuestionChoice(QuestionChoice): + """Allowed answer to a :class:`RelationshipQuestion`.""" #: Question to which this answer belongs - question = models.ForeignKey(RelationshipQuestion, related_name='answers', + 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) - - @property - def slug(self) -> str: - return slugify(self.text) - - def __str__(self) -> str: - return self.text + blank=False, + null=False) # class ExternalPerson(models.Model): @@ -129,8 +67,8 @@ class Relationship(models.Model): on_delete=models.CASCADE, blank=False, null=False) - # blank=True, - # null=True) + # blank=True, + # null=True) # target_external_person = models.ForeignKey( # ExternalPerson, @@ -173,31 +111,18 @@ class Relationship(models.Model): target_person=self.source) -class RelationshipAnswerSet(models.Model): - """ - The answers to the relationship questions at a particular point in time. - """ - - class Meta: - ordering = [ - 'timestamp', - ] +class RelationshipAnswerSet(AnswerSet): + """The answers to the relationship questions at a particular point in time.""" #: Relationship to which this answer set belongs relationship = models.ForeignKey(Relationship, on_delete=models.CASCADE, related_name='answer_sets', - blank=False, null=False) + 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, - editable=False) - - replaced_timestamp = models.DateTimeField(blank=True, null=True, - editable=False) - def get_absolute_url(self): return self.relationship.get_absolute_url() From 7e8dba480688526fe62623c1d9a4c27433b0e17b Mon Sep 17 00:00:00 2001 From: James Graham Date: Mon, 7 Dec 2020 17:01:53 +0000 Subject: [PATCH 11/11] feat: allow multiple choice questions in forms --- people/forms.py | 29 +++++++++++++++---- .../0027_multiple_choice_questions.py | 23 +++++++++++++++ people/models/question.py | 5 ++++ 3 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 people/migrations/0027_multiple_choice_questions.py diff --git a/people/forms.py b/people/forms.py index 38561aa..80c6fad 100644 --- a/people/forms.py +++ b/people/forms.py @@ -49,10 +49,17 @@ class DynamicAnswerSetBase(forms.Form): super().__init__(*args, **kwargs) for question in self.question_model.objects.all(): - field = self.field_class(label=question, - queryset=question.answers, - widget=self.field_widget, - required=self.field_required) + field_class = self.field_class + field_widget = self.field_widget + + if question.is_multiple_choice: + field_class = forms.ModelMultipleChoiceField + field_widget = Select2MultipleWidget + + field = field_class(label=question, + queryset=question.answers, + widget=field_widget, + required=self.field_required) self.fields['question_{}'.format(question.pk)] = field @@ -92,7 +99,12 @@ class PersonAnswerSetForm(forms.ModelForm, DynamicAnswerSetBase): # Save answers to relationship questions for key, value in self.cleaned_data.items(): if key.startswith('question_') and value: - self.instance.question_answers.add(value) + try: + self.instance.question_answers.add(value) + + except TypeError: + # Value is a QuerySet - multiple choice question + self.instance.question_answers.add(*value.all()) return self.instance @@ -119,7 +131,12 @@ class RelationshipAnswerSetForm(forms.ModelForm, DynamicAnswerSetBase): # Save answers to relationship questions for key, value in self.cleaned_data.items(): if key.startswith('question_') and value: - self.instance.question_answers.add(value) + try: + self.instance.question_answers.add(value) + + except TypeError: + # Value is a QuerySet - multiple choice question + self.instance.question_answers.add(*value.all()) return self.instance diff --git a/people/migrations/0027_multiple_choice_questions.py b/people/migrations/0027_multiple_choice_questions.py new file mode 100644 index 0000000..c176787 --- /dev/null +++ b/people/migrations/0027_multiple_choice_questions.py @@ -0,0 +1,23 @@ +# Generated by Django 2.2.10 on 2020-12-07 16:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('people', '0026_move_static_person_questions'), + ] + + operations = [ + migrations.AddField( + model_name='personquestion', + name='is_multiple_choice', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='relationshipquestion', + name='is_multiple_choice', + field=models.BooleanField(default=False), + ), + ] diff --git a/people/models/question.py b/people/models/question.py index 700a4df..7379fb3 100644 --- a/people/models/question.py +++ b/people/models/question.py @@ -22,6 +22,11 @@ class Question(models.Model): #: Text of question text = models.CharField(max_length=255, blank=False, null=False) + #: Should people be able to select multiple responses to this question? + is_multiple_choice = models.BooleanField(default=False, + blank=False, + null=False) + #: Position of this question in the list order = models.SmallIntegerField(default=0, blank=False, null=False)