From 91a47b4fdc4437110b3d0e365e6350cb75822af2 Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 2 Dec 2020 14:40:59 +0000 Subject: [PATCH] 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)