From 6e139159e957e26e933f0edcf5bdb8001153434a Mon Sep 17 00:00:00 2001 From: Piotr Banaszkiewicz Date: Fri, 21 Apr 2023 23:16:44 +0200 Subject: [PATCH] [#2370] Test merging consents with "most recent" strategy --- amy/workshops/tests/test_person.py | 131 +++++++++++++++++++++-------- amy/workshops/utils/merge.py | 10 +++ 2 files changed, 104 insertions(+), 37 deletions(-) diff --git a/amy/workshops/tests/test_person.py b/amy/workshops/tests/test_person.py index bb0c40c35..3aa0f373a 100644 --- a/amy/workshops/tests/test_person.py +++ b/amy/workshops/tests/test_person.py @@ -761,6 +761,7 @@ def setUp(self): orcid="0000-0000-0000", is_active=True, ) + self.person_consent_active_terms(self.person_a) self.person_a.award_set.create( badge=self.swc_instructor, awarded=date(2016, 2, 16) ) @@ -794,6 +795,9 @@ def setUp(self): term.slug: iter(term.options) for term in Term.objects.active().prefetch_active_options() } + privacy_policy_only_term = next( + term_options_by_term_slug[TermEnum.PRIVACY_POLICY] + ) # consents for person_a person_a_consents_by_term_slug = { @@ -802,15 +806,38 @@ def setUp(self): .active() .select_related("term", "term_option") } - # no privacy policy consent - Consent.reconsent( + self.person_a_consent_privacy_policy = Consent.reconsent( + person_a_consents_by_term_slug[TermEnum.PRIVACY_POLICY], + privacy_policy_only_term, + ) + self.person_a_consent_privacy_policy.created_at = datetime( + 2023, 4, 10, tzinfo=timezone.utc + ) + self.person_a_consent_privacy_policy.save() + self.person_a_consent_may_contact = Consent.reconsent( person_a_consents_by_term_slug[TermEnum.MAY_CONTACT], next(term_options_by_term_slug[TermEnum.MAY_CONTACT]), ) - Consent.reconsent( + self.person_a_consent_may_contact.created_at = datetime( + 2023, 4, 11, tzinfo=timezone.utc + ) + self.person_a_consent_may_contact.save() + self.person_a_consent_public_profile = Consent.reconsent( person_a_consents_by_term_slug[TermEnum.PUBLIC_PROFILE], next(term_options_by_term_slug[TermEnum.PUBLIC_PROFILE]), ) + self.person_a_consent_public_profile.created_at = datetime( + 2023, 4, 11, tzinfo=timezone.utc + ) + self.person_a_consent_public_profile.save() + self.person_a_consent_may_publish_name = Consent.reconsent( + person_a_consents_by_term_slug[TermEnum.MAY_PUBLISH_NAME], + next(term_options_by_term_slug[TermEnum.MAY_PUBLISH_NAME]), + ) + self.person_a_consent_may_publish_name.created_at = datetime( + 2023, 4, 10, tzinfo=timezone.utc + ) + self.person_a_consent_may_publish_name.save() # create second person self.person_b = Person.objects.create( @@ -864,18 +891,38 @@ def setUp(self): .active() .select_related("term", "term_option") } - Consent.reconsent( + self.person_b_consent_privacy_policy = Consent.reconsent( person_b_consents_by_term_slug[TermEnum.PRIVACY_POLICY], - next(term_options_by_term_slug[TermEnum.PRIVACY_POLICY]), + privacy_policy_only_term, + ) + self.person_b_consent_privacy_policy.created_at = datetime( + 2023, 4, 11, tzinfo=timezone.utc ) - Consent.reconsent( + self.person_b_consent_privacy_policy.save() + self.person_b_consent_may_contact = Consent.reconsent( person_b_consents_by_term_slug[TermEnum.MAY_CONTACT], next(term_options_by_term_slug[TermEnum.MAY_CONTACT]), ) - Consent.reconsent( + self.person_b_consent_may_contact.created_at = datetime( + 2023, 4, 10, tzinfo=timezone.utc + ) + self.person_b_consent_may_contact.save() + self.person_b_consent_public_profile = Consent.reconsent( person_b_consents_by_term_slug[TermEnum.PUBLIC_PROFILE], next(term_options_by_term_slug[TermEnum.PUBLIC_PROFILE]), ) + self.person_b_consent_public_profile.created_at = datetime( + 2023, 4, 10, tzinfo=timezone.utc + ) + self.person_b_consent_public_profile.save() + self.person_b_consent_may_publish_name = Consent.reconsent( + person_b_consents_by_term_slug[TermEnum.MAY_PUBLISH_NAME], + next(term_options_by_term_slug[TermEnum.MAY_PUBLISH_NAME]), + ) + self.person_b_consent_may_publish_name.created_at = datetime( + 2023, 4, 11, tzinfo=timezone.utc + ) + self.person_b_consent_may_publish_name.save() # set up a strategy self.strategy = { @@ -906,7 +953,7 @@ def setUp(self): "trainingprogress_set": "combine", "comment_comments": "combine", # made by this person "comments": "combine", # regarding this person - "consent_set": "obj_a", + "consent_set": "most_recent", } base_url = reverse("persons_merge") query = urlencode({"person_a": self.person_a.pk, "person_b": self.person_b.pk}) @@ -937,7 +984,7 @@ def test_form_invalid_values(self): "occupation": "combine", "orcid": "combine", "is_active": "combine", - "consent_set": "combine", + "consent_set": "combine", # it actually accepts only "most_recent" } # fields additionally accepting "combine" passing = { @@ -1113,39 +1160,49 @@ def test_merging_comments_strategy3(self): set(comments), ) - def test_merging_consents_obj_a(self): + def test_merging_consents_most_recent(self): """Ensure consents regarding persons are correctly merged using `merge_objects`. - This test uses strategy 2 (object a).""" - self.strategy["consent_set"] = "obj_a" - expected_consents = list(Consent.objects.filter(person=self.person_a)) - rv = self.client.post(self.url, data=self.strategy) - self.assertEqual(rv.status_code, 302) - self.person_b.refresh_from_db() - self.assertCountEqual( - [ - (c.term, c.term_option, c.person) - for c in Consent.objects.filter(person=self.person_b) - ], - [(c.term, c.term_option, self.person_b) for c in expected_consents], - ) + This test uses "most_recent" strategy.""" + # Arrange + self.strategy["consent_set"] = "most_recent" - def test_merging_consents_obj_b(self): - """Ensure consents regarding persons are correctly merged using - `merge_objects`. - This test uses strategy 3 (object b).""" - self.strategy["consent_set"] = "obj_b" - expected_consents = list(Consent.objects.filter(person=self.person_b)) + # Act rv = self.client.post(self.url, data=self.strategy) + + # Assert self.assertEqual(rv.status_code, 302) - self.person_b.refresh_from_db() - self.assertCountEqual( - [ - (c.term, c.term_option, c.person) - for c in Consent.objects.filter(person=self.person_b) - ], - [(c.term, c.term_option, self.person_b) for c in expected_consents], - ) + + # Deleted consents because they are older from the pair of consents beloging to + # person A and B, and they are not assigned to person B. + with self.assertRaises(Consent.DoesNotExist): + self.person_a_consent_privacy_policy.refresh_from_db() + with self.assertRaises(Consent.DoesNotExist): + self.person_a_consent_may_publish_name.refresh_from_db() + + # Not deleted consents because they are newer or assigned to person B. + self.person_b_consent_privacy_policy.refresh_from_db() + self.person_a_consent_may_contact.refresh_from_db() + self.person_b_consent_may_contact.refresh_from_db() + self.person_a_consent_public_profile.refresh_from_db() + self.person_b_consent_public_profile.refresh_from_db() + self.person_b_consent_may_publish_name.refresh_from_db() + + # Archived because they are older from the pair. + self.assertTrue(self.person_b_consent_may_contact.is_archived()) + self.assertTrue(self.person_b_consent_public_profile.is_archived()) + + # Active + self.assertTrue(self.person_b_consent_privacy_policy.is_active()) + self.assertTrue(self.person_a_consent_may_contact.is_active()) + self.assertTrue(self.person_a_consent_public_profile.is_active()) + self.assertTrue(self.person_b_consent_may_publish_name.is_active()) + + # Assigned to base person (person B) + self.assertEqual(self.person_b_consent_privacy_policy.person, self.person_b) + self.assertEqual(self.person_a_consent_may_contact.person, self.person_b) + self.assertEqual(self.person_a_consent_public_profile.person, self.person_b) + self.assertEqual(self.person_b_consent_may_publish_name.person, self.person_b) def github_username_to_uid_mock(username): diff --git a/amy/workshops/utils/merge.py b/amy/workshops/utils/merge.py index b94b25b68..0a1f504a6 100644 --- a/amy/workshops/utils/merge.py +++ b/amy/workshops/utils/merge.py @@ -2,6 +2,7 @@ from django.db import IntegrityError, transaction from django_comments.models import Comment +from consents.models import Consent from workshops.utils.consents import archive_least_recent_active_consents @@ -131,8 +132,17 @@ def merge_objects( integrity_errors.append(str(e)) elif attr == "consent_set" and value == "most_recent": + # Special case: consents should be merge with a "most recent" strategy. archive_least_recent_active_consents(object_a, object_b, base_obj) + # Reassign consents to the base object + try: + Consent.objects.active().filter( + person__in=[object_a, object_b] + ).update(person=base_obj) + except IntegrityError as e: + integrity_errors.append(str(e)) + if "comments" in choices: value = choices["comments"] # special case: comments made regarding these objects