Merge pull request #1556 from uktrade/removal/interaction-contact-model-state

reupen · web-flow · commit 032486405e01 · 2019-04-18T16:31:27.000+01:00
Remove Interaction.contact from state
diff --git a/changelog/interaction/contact-model-state.removal.rst b/changelog/interaction/contact-model-state.removal.rst
@@ -0,0 +1 @@
+The deprecated ``interaction_interaction.contact`` column is being prepared for removal and will shortly be removed. Please use the ``interaction_interaction_contacts`` table instead.
diff --git a/datahub/cleanup/management/commands/delete_old_records.py b/datahub/cleanup/management/commands/delete_old_records.py
@@ -82,7 +82,6 @@ class Command(BaseCleanupCommand):
                 # Contacts are not deleted if they have any related interactions, investment
                 # projects, OMIS orders or OMIS quotes. We wait for those records to expire
                 # before we delete the related contacts.
-                Interaction._meta.get_field('contact').remote_field: (),
                 Contact._meta.get_field('interactions'): (),
                 Contact._meta.get_field('investment_projects'): (),
                 Contact._meta.get_field('orders'): (),
diff --git a/datahub/cleanup/test/commands/factories.py b/datahub/cleanup/test/commands/factories.py
@@ -1,5 +1,4 @@
 from datahub.core.test.factories import to_many_field
-from datahub.interaction.test.factories import CompanyInteractionFactory
 from datahub.investment.project.test.factories import InvestmentProjectFactory
 
 
@@ -17,19 +16,3 @@ class ShallowInvestmentProjectFactory(InvestmentProjectFactory):
     def client_contacts(self):
         """No client contacts."""
         return []
-
-
-class CompanyInteractionFactoryWithoutContacts(CompanyInteractionFactory):
-    """
-    Same as CompanyInteractionFactory but with reduced dependencies
-    so that we can test specific references without extra noise.
-
-    TODO: Remove once Interaction.contact has been removed.
-    """
-
-    contact = None
-
-    @to_many_field
-    def contacts(self):
-        """Default to no contacts."""
-        return []
diff --git a/datahub/cleanup/test/commands/test_delete_old_records.py b/datahub/cleanup/test/commands/test_delete_old_records.py
@@ -24,7 +24,6 @@
     ORDER_MODIFIED_ON_CUT_OFF,
 )
 from datahub.cleanup.query_utils import get_relations_to_delete
-from datahub.cleanup.test.commands.factories import CompanyInteractionFactoryWithoutContacts
 from datahub.company.test.factories import (
     CompanyFactory,
     ContactFactory,
@@ -226,18 +225,7 @@
         ],
         'relations': [
             {
-                'factory': CompanyInteractionFactoryWithoutContacts,
-                'field': 'contact',
-                'expired_objects_kwargs': [],
-                'unexpired_objects_kwargs': [
-                    {
-                        'created_on': CONTACT_DELETE_BEFORE_DATETIME - relativedelta(days=1),
-                        'modified_on': CONTACT_DELETE_BEFORE_DATETIME - relativedelta(days=1),
-                    },
-                ],
-            },
-            {
-                'factory': CompanyInteractionFactoryWithoutContacts,
+                'factory': CompanyInteractionFactory,
                 'field': 'contacts',
                 'expired_objects_kwargs': [],
                 'unexpired_objects_kwargs': [
diff --git a/datahub/cleanup/test/commands/test_delete_orphans.py b/datahub/cleanup/test/commands/test_delete_orphans.py
@@ -11,10 +11,7 @@
 
 from datahub.cleanup.management.commands import delete_orphans
 from datahub.cleanup.query_utils import get_relations_to_delete
-from datahub.cleanup.test.commands.factories import (
-    CompanyInteractionFactoryWithoutContacts,
-    ShallowInvestmentProjectFactory,
-)
+from datahub.cleanup.test.commands.factories import ShallowInvestmentProjectFactory
 from datahub.company.test.factories import (
     CompanyFactory,
     ContactFactory,
@@ -47,8 +44,7 @@
     'company.Contact': {
         'factory': ContactFactory,
         'dependent_models': (
-            (CompanyInteractionFactoryWithoutContacts, 'contact'),
-            (CompanyInteractionFactoryWithoutContacts, 'contacts'),
+            (CompanyInteractionFactory, 'contacts'),
             (OrderFactory, 'contact'),
             (QuoteFactory, 'accepted_by'),
             (InvestmentProjectFactory, 'client_contacts'),
diff --git a/datahub/interaction/admin.py b/datahub/interaction/admin.py
@@ -1,7 +1,6 @@
 import json
 
 from django.contrib import admin
-from django.db.transaction import atomic
 from django.utils.html import format_html
 from reversion.admin import VersionAdmin
 
@@ -121,7 +120,6 @@ class InteractionAdmin(BaseModelAdminMixin, VersionAdmin):
         'created_by',
         'modified_on',
         'modified_by',
-        'contact',
         'source',
     )
 
@@ -149,20 +147,6 @@ def pretty_source(self, obj):
 
     pretty_source.short_description = 'source'
 
-    @atomic
-    def save_model(self, request, obj, form, change):
-        """
-        Saves the object, while also:
-            - copying contacts to contact
-            - copying dit_adviser and dit_team to dit_participants
-        """
-        # TODO: Remove once the migration from contact to contacts is complete.
-        if 'contacts' in form.cleaned_data:
-            contacts = form.cleaned_data['contacts']
-            obj.contact = contacts[0] if contacts else None
-
-        super().save_model(request, obj, form, change)
-
     def changelist_view(self, request, extra_context=None):
         """
         Change list view with additional data added to the context.
diff --git a/datahub/interaction/migrations/0050_remove_contact_from_model_state.py b/datahub/interaction/migrations/0050_remove_contact_from_model_state.py
@@ -0,0 +1,21 @@
+# Generated by Django 2.1.7 on 2019-03-22 15:57
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('interaction', '0049_interaction_source'),
+    ]
+
+    operations = [
+        migrations.SeparateDatabaseAndState(
+            state_operations=[
+                migrations.RemoveField(
+                    model_name='interaction',
+                    name='contact',
+                ),
+            ],
+        ),
+    ]
diff --git a/datahub/interaction/models.py b/datahub/interaction/models.py
@@ -169,18 +169,8 @@ class Interaction(ArchivableModel, BaseModel):
         null=True,
         on_delete=models.CASCADE,
     )
-    # TODO: contact is being replaced with contacts, and contact will be removed once the
-    # migration to a to-many field is complete
-    contact = models.ForeignKey(
-        'company.Contact',
-        related_name='+',
-        blank=True,
-        null=True,
-        on_delete=models.CASCADE,
-    )
     contacts = models.ManyToManyField(
         'company.Contact',
-        # TODO: change related_name to interactions once this field has fully replaced contact
         related_name='%(class)ss',
         blank=True,
     )
diff --git a/datahub/interaction/serializers.py b/datahub/interaction/serializers.py
@@ -216,13 +216,6 @@ def validate(self, data):
         if 'is_event' in data:
             del data['is_event']
 
-        # Copies the first contact specific to contact (for backwards compatibility with
-        # anything consuming the database)
-        # TODO Remove following deprecation period.
-        if 'contacts' in data:
-            contacts = data['contacts']
-            data['contact'] = contacts[0] if contacts else None
-
         # If dit_participants has been provided, this copies the first participant to
         # dit_adviser and dit_team (for backwards compatibility).
         # TODO Remove once dit_adviser and dit_team removed from the database.
diff --git a/datahub/interaction/test/factories.py b/datahub/interaction/test/factories.py
@@ -22,10 +22,6 @@ class InteractionFactoryBase(factory.django.DjangoModelFactory):
     created_by = factory.SubFactory(AdviserFactory)
     modified_by = factory.SubFactory(AdviserFactory)
     company = factory.SubFactory(CompanyFactory)
-    contact = factory.SubFactory(
-        ContactFactory,
-        company=factory.SelfAttribute('..company'),
-    )
     subject = factory.Faker('sentence', nb_words=8)
     date = factory.Faker('past_datetime', start_date='-5y', tzinfo=utc)
     notes = factory.Faker('paragraph', nb_sentences=10)
@@ -42,7 +38,7 @@ def contacts(self):
 
         Defaults to the contact from the contact field.
         """
-        return [self.contact] if self.contact else []
+        return [ContactFactory(company=self.company)] if self.company else []
 
     @to_many_field
     def dit_participants(self):
diff --git a/datahub/interaction/test/test_admin.py b/datahub/interaction/test/test_admin.py
@@ -1,148 +1,15 @@
-from uuid import uuid4
-
 import pytest
 from django.contrib.admin import site
-from django.contrib.admin.templatetags.admin_urls import admin_urlname
-from django.urls import reverse
-from rest_framework import status
 
-from datahub.company.test.factories import CompanyFactory, ContactFactory
-from datahub.core.admin import get_change_url
-from datahub.core.test_utils import AdminTestMixin, random_obj_for_model
+from datahub.company.test.factories import ContactFactory
+from datahub.core.test_utils import AdminTestMixin
 from datahub.interaction.admin import InteractionAdmin
-from datahub.interaction.models import CommunicationChannel, Interaction
+from datahub.interaction.models import Interaction
 from datahub.interaction.test.factories import CompanyInteractionFactory
-from datahub.metadata.models import Service, Team
-
-
-class TestInteractionAdminContacts(AdminTestMixin):
-    """
-    Tests for the contacts and contact logic in interaction admin.
-
-    TODO: The app and update tests will be removed once the migration from contact to
-     contacts is complete.
-    """
-
-    def test_add(self):
-        """Test that adding an interaction also sets the contact field."""
-        company = CompanyFactory()
-        contacts = ContactFactory.create_batch(2, company=company)
-        communication_channel = random_obj_for_model(CommunicationChannel)
-
-        url = reverse(admin_urlname(Interaction._meta, 'add'))
-        data = {
-            'id': uuid4(),
-            'kind': Interaction.KINDS.interaction,
-            'status': Interaction.STATUSES.complete,
-            'communication_channel': communication_channel.pk,
-            'subject': 'whatever',
-            'date_0': '2018-01-01',
-            'date_1': '00:00:00',
-            'dit_adviser': self.user.pk,
-            'company': company.pk,
-            'contacts': [contact.pk for contact in contacts],
-            'service': random_obj_for_model(Service).pk,
-            'dit_team': random_obj_for_model(Team).pk,
-            'was_policy_feedback_provided': False,
-            'dit_participants-TOTAL_FORMS': 0,
-            'dit_participants-INITIAL_FORMS': 0,
-            'dit_participants-MAX_NUM_FORMS': 0,
-            'dit_participants-MIN_NUM_FORMS': 0,
-        }
-        response = self.client.post(url, data, follow=True)
-
-        assert response.status_code == status.HTTP_200_OK
-
-        assert Interaction.objects.count() == 1
-        interaction = Interaction.objects.first()
-
-        assert interaction.contact == contacts[0]
-        assert set(interaction.contacts.all()) == set(contacts)
-
-    def test_update_contact_to_non_null(self):
-        """
-        Test that updating an interaction with a value in contacts also sets the contact field.
-        """
-        interaction = CompanyInteractionFactory()
-        new_contacts = ContactFactory.create_batch(2, company=interaction.company)
-
-        url = get_change_url(interaction)
-        data = {
-            # Unchanged values
-            'id': interaction.pk,
-            'kind': Interaction.KINDS.interaction,
-            'status': Interaction.STATUSES.complete,
-            'communication_channel': interaction.communication_channel.pk,
-            'subject': interaction.subject,
-            'date_0': interaction.date.date().isoformat(),
-            'date_1': interaction.date.time().isoformat(),
-            'dit_adviser': interaction.dit_adviser.pk,
-            'company': interaction.company.pk,
-            'service': interaction.service.pk,
-            'dit_team': interaction.dit_team.pk,
-            'was_policy_feedback_provided': interaction.was_policy_feedback_provided,
-            'policy_feedback_notes': interaction.policy_feedback_notes,
-            'policy_areas': [],
-            'policy_issue_types': [],
-            'event': '',
-            'dit_participants-TOTAL_FORMS': 0,
-            'dit_participants-INITIAL_FORMS': 0,
-            'dit_participants-MAX_NUM_FORMS': 0,
-            'dit_participants-MIN_NUM_FORMS': 0,
-            'dit_participants-0-id': interaction.dit_participants.first().pk,
-            'dit_participants-0-interaction': interaction.pk,
-
-            # Changed values
-            'contacts': [contact.pk for contact in new_contacts],
-        }
-        response = self.client.post(url, data, follow=True)
-
-        assert response.status_code == status.HTTP_200_OK
-
-        interaction.refresh_from_db()
-        assert interaction.contact == new_contacts[0]
-        assert set(interaction.contacts.all()) == set(new_contacts)
-
-    def test_update_contact_to_null(self):
-        """Test that removing all contacts from an interaction clears the contact field."""
-        interaction = CompanyInteractionFactory()
-
-        url = get_change_url(interaction)
-        data = {
-            # Unchanged values
-            'id': interaction.pk,
-            'kind': Interaction.KINDS.interaction,
-            'status': Interaction.STATUSES.complete,
-            'communication_channel': interaction.communication_channel.pk,
-            'subject': interaction.subject,
-            'date_0': interaction.date.date().isoformat(),
-            'date_1': interaction.date.time().isoformat(),
-            'dit_adviser': interaction.dit_adviser.pk,
-            'company': interaction.company.pk,
-            'service': interaction.service.pk,
-            'dit_team': interaction.dit_team.pk,
-            'was_policy_feedback_provided': interaction.was_policy_feedback_provided,
-            'policy_feedback_notes': interaction.policy_feedback_notes,
-            'policy_areas': [],
-            'policy_issue_types': [],
-            'event': '',
-            'dit_participants-TOTAL_FORMS': 0,
-            'dit_participants-INITIAL_FORMS': 0,
-            'dit_participants-MAX_NUM_FORMS': 0,
-            'dit_participants-MIN_NUM_FORMS': 0,
-            'dit_participants-0-id': interaction.dit_participants.first().pk,
-            'dit_participants-0-interaction': interaction.pk,
-
-            # Changed values
-            'contacts': [],
-        }
-        response = self.client.post(url, data=data, follow=True)
 
-        assert response.status_code == status.HTTP_200_OK
 
-        interaction.refresh_from_db()
-        assert interaction.contact is None
-        assert interaction.contacts.count() == 0
+class TestInteractionAdmin(AdminTestMixin):
+    """Tests for interaction admin."""
 
     @pytest.mark.parametrize(
         'num_contacts,expected_display_value',
diff --git a/datahub/interaction/test/views/test_common.py b/datahub/interaction/test/views/test_common.py
diff --git a/datahub/interaction/test/views/test_interaction.py b/datahub/interaction/test/views/test_interaction.py
diff --git a/datahub/search/interaction/tests/test_signals.py b/datahub/search/interaction/tests/test_signals.py

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+The deprecated ``interaction_interaction.contact`` column is being prepared for removal and will shortly be removed. Please use the ``interaction_interaction_contacts`` table instead.