Remove contact field from interactions in main API

Author: reupen

changelog/interaction/remove-contact-main.removal.api.rst

@@ -0,0 +1 @@
+``GET,POST /v3/interaction``, ``GET,PATCH /v3/interaction/<id>``: The deprecated ``contact`` field was removed. Please use ``contacts`` instead.

datahub/interaction/serializers.py

@@ -2,7 +2,6 @@
 
 from django.utils.translation import ugettext_lazy
 from rest_framework import serializers
-from rest_framework.settings import api_settings
 
 from datahub.company.models import Company, Contact
 from datahub.company.serializers import NestedAdviserField
@@ -28,32 +27,6 @@
 from datahub.metadata.models import Service, Team
 
 
-class _ManyRelatedAsSingleItemField(NestedRelatedField):
-    """
-    Serialiser field that makes a to-many field behave like a to-one field.
-
-    Use for temporary backwards compatibility when migrating a to-one field to be a to-many field
-    (so that a to-one field can be emulated using a to-many field).
-
-    This isn't intended to be used in any other way as if the to-many field contains multiple
-    items, only one of them will be returned, and all of them will overwritten on updates.
-
-    TODO Remove this once contact has been removed from interactions.
-    """
-
-    def run_validation(self, data=serializers.empty):
-        """Validate a user-provided value and return the internal value (converted to a list)."""
-        validated_value = super().run_validation(data)
-        return [validated_value] if validated_value else []
-
-    def to_representation(self, value):
-        """Converts a query set to a dict representation of the first item in the query set."""
-        if not value.exists():
-            return None
-
-        return super().to_representation(value.first())
-
-
 class InteractionSerializer(serializers.ModelSerializer):
     """V3 interaction serialiser."""
 
@@ -76,38 +49,22 @@ class InteractionSerializer(serializers.ModelSerializer):
         'invalid_when_no_policy_feedback': ugettext_lazy(
             'This field is only valid when policy feedback has been provided.',
         ),
-        'one_contact_field': ugettext_lazy(
-            'Only one of contact and contacts should be provided.',
-        ),
         'too_many_contacts_for_event_service_delivery': ugettext_lazy(
             'Only one contact can be provided for event service deliveries.',
         ),
     }
 
     company = NestedRelatedField(Company)
-    # TODO Remove contact following deprecation period
-    contact = _ManyRelatedAsSingleItemField(
-        Contact,
-        extra_fields=(
-            'name',
-            'first_name',
-            'last_name',
-            'job_title',
-        ),
-        source='contacts',
-        required=False,
-    )
-    # TODO Make required once contact has been removed
     contacts = NestedRelatedField(
         Contact,
         many=True,
+        allow_empty=False,
         extra_fields=(
             'name',
             'first_name',
             'last_name',
             'job_title',
         ),
-        required=False,
     )
     dit_adviser = NestedAdviserField()
     created_by = NestedAdviserField(read_only=True)
@@ -131,24 +88,6 @@ class InteractionSerializer(serializers.ModelSerializer):
         required=False,
     )
 
-    def to_internal_value(self, data):
-        """
-        Checks that contact and contacts haven't both been provided.
-
-        Note: On serialisers, to_internal_value() is called before validate().
-
-        TODO Remove once contact removed from the API.
-        """
-        if 'contact' in data and 'contacts' in data:
-            error = {
-                api_settings.NON_FIELD_ERRORS_KEY: [
-                    self.error_messages['one_contact_field'],
-                ],
-            }
-            raise serializers.ValidationError(error, code='one_contact_field')
-
-        return super().to_internal_value(data)
-
     def validate(self, data):
         """
         Removes the semi-virtual field is_event from the data.
@@ -182,8 +121,6 @@ class Meta:
         fields = (
             'id',
             'company',
-            # TODO Remove contact following deprecation period
-            'contact',
             'contacts',
             'created_on',
             'created_by',
@@ -216,14 +153,6 @@ class Meta:
             HasAssociatedInvestmentProjectValidator(),
             ContactsBelongToCompanyValidator(),
             RulesBasedValidator(
-                # Because contacts could come from either contact or contacts, this has to be at
-                # the object level
-                # TODO Remove once contact has been removed and required=False removed from
-                #  contacts
-                ValidationRule(
-                    'required',
-                    OperatorRule('contacts', bool),
-                ),
                 ValidationRule(
                     'required',
                     OperatorRule('communication_channel', bool),

datahub/interaction/test/test_serializers.py

@@ -46,9 +46,9 @@ def test_invalid_kind_is_rejected(self, kind):
             'company': {
                 'id': company.pk,
             },
-            'contact': {
+            'contacts': [{
                 'id': contact.pk,
-            },
+            }],
             'date': '2017-04-18',
             'dit_adviser': {
                 'id': adviser.pk,
@@ -70,48 +70,6 @@ def test_invalid_kind_is_rejected(self, kind):
             'kind': [f'"{kind}" is not a valid choice.'],
         }
 
-    def test_contact_copied_to_contacts(self):
-        """
-        Test that the value provided in the contact field is copied to contacts when an
-        interaction is created.
-
-        TODO: remove once the contacts field has fully replaced the contact field.
-        """
-        company = CompanyFactory()
-        contact = ContactFactory(company=company)
-        communication_channel = random_obj_for_model(CommunicationChannel)
-
-        url = reverse('api-v3:interaction:collection')
-        request_data = {
-            'kind': Interaction.KINDS.interaction,
-            'communication_channel': communication_channel.pk,
-            'subject': 'whatever',
-            'date': date.today().isoformat(),
-            'dit_adviser': {
-                'id': self.user.pk,
-            },
-            'company': {
-                'id': company.pk,
-            },
-            'contact': {
-                'id': contact.pk,
-            },
-            'service': {
-                'id': random_obj_for_model(ServiceModel).pk,
-            },
-            'dit_team': {
-                'id': self.user.dit_team.pk,
-            },
-            'was_policy_feedback_provided': False,
-        }
-
-        api_client = self.create_api_client()
-        response = api_client.post(url, request_data)
-        assert response.status_code == status.HTTP_201_CREATED
-        interaction = Interaction.objects.get(pk=response.json()['id'])
-        assert interaction.contact == contact
-        assert list(interaction.contacts.all()) == [contact]
-
     def test_contacts_copied_to_contact(self):
         """
         Test that the value provided in the contacts field is copied to contact when an
@@ -154,50 +112,6 @@ def test_contacts_copied_to_contact(self):
         assert interaction.contact == contacts[0]
         assert set(interaction.contacts.all()) == set(contacts)
 
-    def test_providing_contact_and_contacts_returns_an_error(self):
-        """
-        Test that if both contact and contacts are provided, an error is returned.
-
-        TODO: remove once the contacts field has fully replaced the contact field.
-        """
-        company = CompanyFactory()
-        contact = ContactFactory()
-        communication_channel = random_obj_for_model(CommunicationChannel)
-
-        url = reverse('api-v3:interaction:collection')
-        request_data = {
-            'kind': Interaction.KINDS.interaction,
-            'communication_channel': communication_channel.pk,
-            'subject': 'whatever',
-            'date': date.today().isoformat(),
-            'dit_adviser': {
-                'id': self.user.pk,
-            },
-            'company': {
-                'id': company.pk,
-            },
-            'contact': {
-                'id': contact.pk,
-            },
-            'contacts': [{
-                'id': contact.pk,
-            }],
-            'service': {
-                'id': random_obj_for_model(ServiceModel).pk,
-            },
-            'dit_team': {
-                'id': self.user.dit_team.pk,
-            },
-            'was_policy_feedback_provided': False,
-        }
-
-        api_client = self.create_api_client()
-        response = api_client.post(url, request_data)
-        assert response.status_code == status.HTTP_400_BAD_REQUEST
-        assert response.json() == {
-            'non_field_errors': ['Only one of contact and contacts should be provided.'],
-        }
-
     def test_error_returned_if_contacts_dont_belong_to_company(self):
         """
         Test that an error is returned if the contacts don't belong to the specified company.
@@ -289,29 +203,6 @@ def test_date_validation(self):
             'Datetime has wrong format. Use one of these formats instead: YYYY-MM-DD.',
         ]
 
-    def test_contact_copied_to_contacts(self):
-        """
-        Test that the value provided in the contact field is copied to contacts when an
-        interaction is updated.
-
-        TODO: remove once the contacts field has fully replaced the contact field.
-        """
-        interaction = CompanyInteractionFactory(contacts=[])
-        new_contact = ContactFactory(company=interaction.company)
-
-        url = reverse('api-v3:interaction:item', kwargs={'pk': interaction.pk})
-        data = {
-            'contact': {
-                'id': new_contact.pk,
-            },
-        }
-        response = self.api_client.patch(url, data=data)
-
-        assert response.status_code == status.HTTP_200_OK
-        interaction.refresh_from_db()
-        assert interaction.contact == new_contact
-        assert list(interaction.contacts.all()) == [new_contact]
-
     def test_contacts_copied_to_contact(self):
         """
         Test that the value provided in the contact field is copied to contacts when an
@@ -335,31 +226,6 @@ def test_contacts_copied_to_contact(self):
         assert interaction.contact == new_contacts[0]
         assert set(interaction.contacts.all()) == set(new_contacts)
 
-    def test_providing_contact_and_contacts_returns_an_error(self):
-        """
-        Test that if both contact and contacts are provided, an error is returned.
-
-        TODO: remove once the contacts field has fully replaced the contact field.
-        """
-        interaction = CompanyInteractionFactory(contacts=[])
-        new_contact = ContactFactory(company=interaction.company)
-
-        url = reverse('api-v3:interaction:item', kwargs={'pk': interaction.pk})
-        data = {
-            'contact': {
-                'id': new_contact.pk,
-            },
-            'contacts': [{
-                'id': new_contact.pk,
-            }],
-        }
-        response = self.api_client.patch(url, data=data)
-
-        assert response.status_code == status.HTTP_400_BAD_REQUEST
-        assert response.json() == {
-            'non_field_errors': ['Only one of contact and contacts should be provided.'],
-        }
-
     @pytest.mark.parametrize(
         'request_data',
         (