Update the corresponding drupal node when instructor is updated in publisher

course_discovery/apps/api/v1/tests/test_views/test_people.py

@@ -99,7 +99,7 @@ class PersonViewSetTests(SerializationMixin, APITestCase):
                             (
                                 people_logger.name,
                                 'ERROR',
-                                'An error occurred while adding the person [{}]-[{}]-[{}].'.format(
+                                'An error occurred while adding the person [{}]-[{}]-[{}] in discovery.'.format(
                                     data['given_name'], data['family_name'], self.expected_node['id']
                                 )
                             )
@@ -178,11 +178,8 @@ class PersonViewSetTests(SerializationMixin, APITestCase):
             }
         }
 
-    def test_update(self):
-        """Verify that people data can be updated using endpoint."""
-        url = reverse('api:v1:person-detail', kwargs={'uuid': self.person.uuid})
-
-        data = {
+    def _update_person_data(self):
+        return {
             'given_name': "updated",
             'family_name': "name",
             'bio': "updated bio",
@@ -197,6 +194,62 @@ class PersonViewSetTests(SerializationMixin, APITestCase):
             }
         }
 
+    def test_update_without_drupal_client_settings(self):
+        """ Verify that if credentials are missing api will return the error. """
+        url = reverse('api:v1:person-detail', kwargs={'uuid': self.person.uuid})
+        self.partner.marketing_site_api_username = None
+        self.partner.save()
+        data = self._update_person_data()
+
+        with LogCapture(people_logger.name) as log_capture:
+            response = self.client.patch(url, data, format='json')
+            self.assertEqual(response.status_code, 400)
+            log_capture.check(
+                (
+                    people_logger.name,
+                    'ERROR',
+                    'An error occurred while updating the person [{}]-[{}] on the marketing site.'.format(
+                        data['given_name'], data['family_name']
+                    )
+                )
+            )
+
+    def test_update_with_api_exception(self):
+        """ Verify that if the serializer fails, error message is logged and update fails"""
+        url = reverse('api:v1:person-detail', kwargs={'uuid': self.person.uuid})
+        data = self._update_person_data()
+        with mock.patch.object(MarketingSitePeople, 'update_person', return_value={}):
+            with mock.patch(
+                'course_discovery.apps.api.v1.views.people.PersonViewSet.perform_update',
+                side_effect=IntegrityError
+            ):
+                with LogCapture(people_logger.name) as log_capture:
+                    response = self.client.patch(url, self._update_person_data(), format='json')
+                    self.assertEqual(response.status_code, 400)
+                    log_capture.check(
+                        (
+                            people_logger.name,
+                            'ERROR',
+                            'An error occurred while updating the person [{}]-[{}] in discovery.'.format(
+                                data['given_name'], data['family_name']
+                            )
+                        )
+                    )
+
+    def test_update_without_waffle_switch(self):
+        """ Verify update endpoint shows error message if waffle switch is disabled. """
+        url = reverse('api:v1:person-detail', kwargs={'uuid': self.person.uuid})
+        toggle_switch('publish_person_to_marketing_site', False)
+        response = self.client.patch(url, self._update_person_data(), format='json')
+        self.assertEqual(response.status_code, 400)
+
+    def test_update(self):
+        """Verify that people data can be updated using endpoint."""
+        url = reverse('api:v1:person-detail', kwargs={'uuid': self.person.uuid})
+
+        data = self._update_person_data()
+
+        with mock.patch.object(MarketingSitePeople, 'update_person', return_value={}):
             response = self.client.patch(url, data, format='json')
             self.assertEqual(response.status_code, 200)
 

course_discovery/apps/api/v1/views/people.py

@@ -28,7 +28,9 @@ class PersonViewSet(viewsets.ModelViewSet):
     pagination_class = PageNumberPagination
 
     def create(self, request, *args, **kwargs):
-        """ Create a new person. """
+        """
+        Create a person in discovery and also create a person node in drupal
+        """
         person_data = request.data
 
         partner = request.site.partner
@@ -36,14 +38,13 @@ class PersonViewSet(viewsets.ModelViewSet):
         serializer = self.get_serializer(data=person_data)
         serializer.is_valid(raise_exception=True)
 
-        if waffle.switch_is_active('publish_person_to_marketing_site'):
+        if not waffle.switch_is_active('publish_person_to_marketing_site'):
+            return Response('publish_person_to_marketing_site is disabled.', status=status.HTTP_400_BAD_REQUEST)
         try:
             marketing_person = MarketingSitePeople()
             response = marketing_person.publish_person(
-                    partner, {
-                        'given_name': serializer.validated_data['given_name'],
-                        'family_name': serializer.validated_data['family_name']
-                    }
+                partner,
+                self._get_person_data(serializer)
             )
             serializer.validated_data.pop('uuid')
             serializer.validated_data['uuid'] = response['uuid']
@@ -59,7 +60,7 @@ class PersonViewSet(viewsets.ModelViewSet):
             self.perform_create(serializer)
         except Exception:  # pylint: disable=broad-except
             logger.exception(
-                    'An error occurred while adding the person [%s]-[%s]-[%s].',
+                'An error occurred while adding the person [%s]-[%s]-[%s] in discovery.',
                 serializer.validated_data['given_name'], serializer.validated_data['family_name'],
                 response['id']
             )
@@ -69,7 +70,56 @@ class PersonViewSet(viewsets.ModelViewSet):
         headers = self.get_success_headers(serializer.data)
         return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers)
 
-        return Response('publish_program_to_marketing_site is disabled.', status=status.HTTP_400_BAD_REQUEST)
+    def update(self, request, *args, **kwargs):  # pylint: disable=unused-argument
+        """
+        Updates a person in discovery and the corresponding person node in drupal
+        """
+        person_data = request.data
+
+        partner = request.site.partner
+        person_data['partner'] = partner.id
+        partial = kwargs.pop('partial', False)
+        instance = self.get_object()
+        serializer = self.get_serializer(instance, data=person_data, partial=partial)
+        serializer.is_valid(raise_exception=True)
+
+        if not waffle.switch_is_active('publish_person_to_marketing_site'):
+            return Response('publish_person_to_marketing_site is disabled.', status=status.HTTP_400_BAD_REQUEST)
+        try:
+            marketing_person = MarketingSitePeople()
+            marketing_person.update_person(
+                partner,
+                serializer.validated_data['uuid'],
+                self._get_person_data(serializer)
+            )
+
+        except (PersonToMarketingException, MarketingSiteAPIClientException):
+            logger.exception(
+                'An error occurred while updating the person [%s]-[%s] on the marketing site.',
+                serializer.validated_data['given_name'], serializer.validated_data['family_name']
+            )
+            return Response(
+                'Failed to update person data on the marketing site.',
+                status=status.HTTP_400_BAD_REQUEST
+            )
+
+        try:
+            self.perform_update(serializer)
+        except Exception:  # pylint: disable=broad-except
+            logger.exception(
+                'An error occurred while updating the person [%s]-[%s] in discovery.',
+                serializer.validated_data['given_name'], serializer.validated_data['family_name']
+            )
+            return Response('Failed to update person data.', status=status.HTTP_400_BAD_REQUEST)
+
+        headers = self.get_success_headers(serializer.data)
+        return Response(serializer.data, status=status.HTTP_200_OK, headers=headers)
+
+    def _get_person_data(self, serializer):
+        return {
+            'given_name': serializer.validated_data['given_name'],
+            'family_name': serializer.validated_data['family_name']
+        }
 
     def list(self, request, *args, **kwargs):
         """ Retrieve a list of all people. """

course_discovery/apps/course_metadata/people.py

@@ -22,6 +22,7 @@ class MarketingSitePeople(object):
         return {
             'field_person_first_middle_name': person['given_name'],
             'field_person_last_name': person['family_name'],
+            'title': person['given_name'] + ' ' + person['family_name'],
             'type': 'person',
         }
 
@@ -35,6 +36,26 @@ class MarketingSitePeople(object):
             logger.exception('Failed to create person node to marketing site [%s].', response.content)
             raise PersonToMarketingException("Marketing site Person page creation failed!")
 
+    def _get_node_id_from_uuid(self, api_client, uuid):
+        node_url = '{root}/node.json?uuid={uuid}'.format(root=api_client.api_url, uuid=uuid)
+        response = api_client.api_session.get(node_url)
+        if response.status_code == 200:
+            response_json = response.json()
+            person_list = response_json.get('list')
+            return person_list[0].get('nid') if person_list else None
+        else:
+            logger.exception('Failed to update person node on marketing site [%s].', response.content)
+            raise PersonToMarketingException("Marketing site Person page update failed!")
+
+    def update_person(self, partner, person_uuid, person):
+        api_client = self._get_api_client(partner)
+        node_id = self._get_node_id_from_uuid(api_client, person_uuid)
+        if node_id:
+            node_data = self._get_node_data(person)
+            return self._update_node(api_client, node_id, node_data)
+        else:
+            logger.info('Person with UUID [%s] does not exist on the marketing site', person_uuid)
+
     def publish_person(self, partner, person):
         api_client = self._get_api_client(partner)
         node_data = self._get_node_data(person)
@@ -46,6 +67,16 @@ class MarketingSitePeople(object):
         if api_client and node_id:
             self._delete_node(api_client, node_id)
 
+    def _update_node(self, api_client, node_id, node_data):
+        node_url = '{root}/node.json/{node_id}'.format(root=api_client.api_url, node_id=node_id)
+        response = api_client.api_session.put(node_url, data=json.dumps(node_data))
+        if response.status_code == 200:
+            response_json = response.json()
+            return response_json
+        else:
+            logger.exception('Failed to update person node on marketing site [%s].', response.content)
+            raise PersonToMarketingException("Marketing site Person page update failed!")
+
     def _delete_node(self, api_client, node_id):
         node_url = '{root}/node.json/{node_id}'.format(root=api_client.api_url, node_id=node_id)
         api_client.api_session.delete(node_url)

course_discovery/apps/course_metadata/tests/test_people.py

+import mock
 import responses
+from testfixtures import LogCapture
 
 from course_discovery.apps.core.tests.factories import PartnerFactory
 from course_discovery.apps.course_metadata.exceptions import PersonToMarketingException
@@ -6,6 +8,8 @@ from course_discovery.apps.course_metadata.people import MarketingSitePeople
 from course_discovery.apps.course_metadata.tests.mixins import MarketingSitePublisherTestMixin
 from course_discovery.apps.course_metadata.utils import MarketingSiteAPIClient
 
+LOGGER_NAME = 'course_discovery.apps.course_metadata.people'
+
 
 class MarketingSitePublisherTests(MarketingSitePublisherTestMixin):
     """
@@ -23,17 +27,23 @@ class MarketingSitePublisherTests(MarketingSitePublisherTestMixin):
             self.password,
             self.api_root
         )
+        self.uuid = '18d5542f-fa80-418e-b416-455cfdeb4d4e'
 
         self.expected_node = {
             'resource': 'node', ''
             'id': '28691',
-            'uuid': '18d5542f-fa80-418e-b416-455cfdeb4d4e',
+            'uuid': self.uuid,
             'uri': 'https://stage.edx.org/node/28691'
         }
         self.data = {
             'given_name': 'test',
             'family_name': 'user'
         }
+        self.updated_node_data = {
+            'given_name': 'updated test',
+            'family_name': 'user',
+            'title': 'updated test user'
+        }
 
     @responses.activate
     def test_create_node(self):
@@ -44,6 +54,68 @@ class MarketingSitePublisherTests(MarketingSitePublisherTestMixin):
         self.assertEqual(data, self.expected_node)
 
     @responses.activate
+    def test_update_node(self):
+        self.mock_api_client(200)
+        self.mock_node_edit(200)
+        people = MarketingSitePeople()
+        data = people._update_node(self.api_client, self.node_id, self.updated_node_data)  # pylint: disable=protected-access
+        self.assertEqual(data, {})
+
+    @responses.activate
+    def test_update_node_failed(self):
+        self.mock_api_client(200)
+        self.mock_node_edit(500)
+        people = MarketingSitePeople()
+        with self.assertRaises(PersonToMarketingException):
+            people._update_node(self.api_client, self.node_id, self.data)  # pylint: disable=protected-access
+
+    @responses.activate
+    def test_update_person(self):
+        self.mock_api_client(200)
+        self.mock_node_edit(200)
+        self.mock_node_retrieval('uuid', self.uuid, status=200)
+        people = MarketingSitePeople()
+        data = people.update_person(self.partner, self.uuid, self.updated_node_data)
+        self.assertEqual(data, {})
+
+    @responses.activate
+    def test_update_person_failed(self):
+        self.mock_api_client(200)
+        self.mock_node_edit(500)
+        self.mock_node_retrieval('uuid', self.uuid, status=200)
+        people = MarketingSitePeople()
+        with self.assertRaises(PersonToMarketingException):
+            people.update_person(self.partner, self.uuid, self.updated_node_data)
+
+    @responses.activate
+    def test_get_node_id_from_uuid(self):
+        self.mock_api_client(200)
+        self.mock_node_retrieval('uuid', self.uuid, status=200)
+        people = MarketingSitePeople()
+        data = people._get_node_id_from_uuid(self.api_client, self.uuid)  # pylint: disable=protected-access
+        self.assertEqual(data, self.node_id)
+
+    @responses.activate
+    def test_get_node_id_from_uuid_failed(self):
+        self.mock_api_client(200)
+        self.mock_node_retrieval('uuid', self.uuid, status=500)
+        people = MarketingSitePeople()
+        with self.assertRaises(PersonToMarketingException):
+            people._get_node_id_from_uuid(self.api_client, self.uuid)  # pylint: disable=protected-access
+
+    @mock.patch(
+        'course_discovery.apps.course_metadata.people.MarketingSitePeople._get_node_id_from_uuid',
+        mock.Mock(return_value=None)
+    )
+    def test_update_uuid_not_found(self):
+        self.mock_api_client(200)
+        people = MarketingSitePeople()
+        with LogCapture(LOGGER_NAME) as log:
+            people.update_person(self.partner, self.uuid, self.updated_node_data)
+            log.check((LOGGER_NAME, 'INFO',
+                       'Person with UUID [{}] does not exist on the marketing site'.format(self.uuid)))
+
+    @responses.activate
     def test_create_node_failed(self):
         self.mock_api_client(200)
         self.mock_node_create({}, 500)