Merge pull request #305 from edx/schen/ECOM-5402

ECOM-5402 Delete the drupal node when program is hard deleted

course_discovery/apps/course_metadata/apps.py

@@ -4,3 +4,8 @@ from django.apps import AppConfig
 class CourseMetadataConfig(AppConfig):
     name = 'course_discovery.apps.course_metadata'
     verbose_name = 'Course Metadata'
+
+    def ready(self):
+        super(CourseMetadataConfig, self).ready()
+        # noinspection PyUnresolvedReferences
+        import course_discovery.apps.course_metadata.signals  # pylint: disable=unused-variable

course_discovery/apps/course_metadata/publishers.py

@@ -89,6 +89,27 @@ class MarketingSitePublisher(object):
                 'title': program_before.title,
             }
 
+    def _get_api_client(self, program):
+        if not program.partner.has_marketing_site:
+            return
+
+        if not (program.partner.marketing_site_api_username and program.partner.marketing_site_api_password):
+            msg = 'Marketing Site API credentials are not properly configured for Partner [{partner}]!'.format(
+                partner=program.partner.short_code)
+            raise ProgramPublisherException(msg)
+
+        if self.data_before and \
+                all(self.data_before[key] == getattr(program, key) for key in ['title', 'status', 'type']):
+            # We don't need to publish to marketing site because
+            # nothing we care about has changed. This would save at least 4 network calls
+            return
+
+        return MarketingSiteAPIClient(
+            program.partner.marketing_site_api_username,
+            program.partner.marketing_site_api_password,
+            program.partner.marketing_site_url_root
+        )
+
     def _get_node_data(self, program, user_id):
         return {
             'type': str(program.type).lower(),
@@ -128,29 +149,13 @@ class MarketingSitePublisher(object):
         else:
             raise ProgramPublisherException("Marketing site page creation failed!")
 
-    def publish_program(self, program):
-        if not program.partner.has_marketing_site:
-            return
-
-        if not (program.partner.marketing_site_api_username and program.partner.marketing_site_api_password):
-            msg = 'Marketing Site API credentials are not properly configured for Partner [{partner}]!'.format(
-                partner=program.partner.short_code)
-            raise ProgramPublisherException(msg)
-
-        if self.data_before and \
-                self.data_before.get('title') == program.title and \
-                self.data_before.get('status') == program.status and \
-                self.data_before.get('type') == program.type:
-            # We don't need to publish to marketing site because
-            # nothing we care about has changed. This would save at least 4 network calls
-            return
-
-        api_client = MarketingSiteAPIClient(
-            program.partner.marketing_site_api_username,
-            program.partner.marketing_site_api_password,
-            program.partner.marketing_site_url_root
-        )
+    def _delete_node(self, api_client, nid):
+        node_url = '{root}/node.json/{nid}'.format(root=api_client.api_url, nid=nid)
+        api_client.api_session.delete(node_url)
 
+    def publish_program(self, program):
+        api_client = self._get_api_client(program)
+        if api_client:
             node_data = self._get_node_data(program, api_client.user_id)
             nid = self._get_node_id(api_client, program.uuid)
             if nid:
@@ -159,3 +164,10 @@ class MarketingSitePublisher(object):
             else:
                 # We should create a new node
                 self._create_node(api_client, node_data)
+
+    def delete_program(self, program):
+        api_client = self._get_api_client(program)
+        if api_client:
+            nid = self._get_node_id(api_client, program.uuid)
+            if nid:
+                self._delete_node(api_client, nid)

course_discovery/apps/course_metadata/signals.py

+from django.db.models.signals import pre_delete
+from django.dispatch import receiver
+import waffle
+
+from course_discovery.apps.course_metadata.models import Program
+from course_discovery.apps.course_metadata.publishers import MarketingSitePublisher
+
+
+@receiver(pre_delete, sender=Program)
+def delete_program(sender, instance, **kwargs):  # pylint: disable=unused-argument
+    if waffle.switch_is_active('publish_program_to_marketing_site') and \
+            instance.partner.has_marketing_site:
+        publisher = MarketingSitePublisher()
+        publisher.delete_program(instance)

course_discovery/apps/course_metadata/tests/mixins.py

 import json
 
+from django.test import TestCase
 from factory.fuzzy import FuzzyText, FuzzyInteger
 import responses
 
 from course_discovery.apps.core.tests.utils import FuzzyUrlRoot
 
 
-class MarketingSiteAPIClientTestMixin(object):
+class MarketingSiteAPIClientTestMixin(TestCase):
     """
     The mixing to help mock the responses for marketing site API Client
     """
@@ -70,6 +71,9 @@ class MarketingSiteAPIClientTestMixin(object):
             match_querystring=True
         )
 
+    def assert_responses_call_count(self, count):
+        self.assertEqual(len(responses.calls), count)
+
 
 class MarketingSitePublisherTestMixin(MarketingSiteAPIClientTestMixin):
     """
@@ -121,3 +125,12 @@ class MarketingSitePublisherTestMixin(MarketingSiteAPIClientTestMixin):
             content_type='application/json',
             status=status
         )
+
+    def mock_node_delete(self, status):
+        responses.add(
+            responses.DELETE,
+            '{root}/node.json/{nid}'.format(root=self.api_root, nid=self.nid),
+            body='',
+            content_type='text/html',
+            status=status
+        )

course_discovery/apps/course_metadata/tests/test_models.py

@@ -268,7 +268,7 @@ class AbstractValueModelTests(TestCase):
 
 
 @ddt.ddt
-class ProgramTests(MarketingSitePublisherTestMixin, TestCase):
+class ProgramTests(MarketingSitePublisherTestMixin):
     """Tests of the Program model."""
 
     def setUp(self):
@@ -395,7 +395,12 @@ class ProgramTests(MarketingSitePublisherTestMixin, TestCase):
     def test_save_without_publish(self):
         self.program.title = FuzzyText().fuzz()
         self.program.save()
-        self.assertEqual(len(responses.calls), 0)
+        self.assert_responses_call_count(0)
+
+    @responses.activate
+    def test_delete_without_publish(self):
+        self.program.delete()
+        self.assert_responses_call_count(0)
 
     @responses.activate
     def test_save_and_publish_success(self):
@@ -409,8 +414,7 @@ class ProgramTests(MarketingSitePublisherTestMixin, TestCase):
         toggle_switch('publish_program_to_marketing_site', True)
         self.program.title = FuzzyText().fuzz()
         self.program.save()
-        self.assertEqual(len(responses.calls), 6)
-        toggle_switch('publish_program_to_marketing_site', False)
+        self.assert_responses_call_count(6)
 
     @responses.activate
     def test_save_and_no_marketing_site(self):
@@ -419,8 +423,28 @@ class ProgramTests(MarketingSitePublisherTestMixin, TestCase):
         toggle_switch('publish_program_to_marketing_site', True)
         self.program.title = FuzzyText().fuzz()
         self.program.save()
-        self.assertEqual(len(responses.calls), 0)
-        toggle_switch('publish_program_to_marketing_site', False)
+        self.assert_responses_call_count(0)
+
+    @responses.activate
+    def test_delete_and_publish_success(self):
+        self.program.partner.marketing_site_url_root = self.api_root
+        self.program.partner.marketing_site_api_username = self.username
+        self.program.partner.marketing_site_api_password = self.password
+        self.program.save()
+        self.mock_api_client(200)
+        self.mock_node_retrieval(self.program.uuid)
+        self.mock_node_delete(204)
+        toggle_switch('publish_program_to_marketing_site', True)
+        self.program.delete()
+        self.assert_responses_call_count(5)
+
+    @responses.activate
+    def test_delete_and_no_marketing_site(self):
+        self.program.partner.marketing_site_url_root = None
+        self.program.save()
+        toggle_switch('publish_program_to_marketing_site', True)
+        self.program.delete()
+        self.assert_responses_call_count(0)
 
     def test_course_update_caught_exception(self):
         """ Test that the index update process failing will not cause the program save to error """

course_discovery/apps/course_metadata/tests/test_publishers.py

-from django.test import TestCase
 import responses
 
 from course_discovery.apps.course_metadata.publishers import (
@@ -14,7 +13,7 @@ from course_discovery.apps.course_metadata.tests.mixins import (
 from course_discovery.apps.course_metadata.models import Program
 
 
-class MarketingSiteAPIClientTests(MarketingSiteAPIClientTestMixin, TestCase):
+class MarketingSiteAPIClientTests(MarketingSiteAPIClientTestMixin):
     """
     Unit test cases for MarketinSiteAPIClient
     """
@@ -30,7 +29,7 @@ class MarketingSiteAPIClientTests(MarketingSiteAPIClientTestMixin, TestCase):
     def test_init_session(self):
         self.mock_login_response(200)
         session = self.api_client.init_session
-        self.assertEqual(len(responses.calls), 2)
+        self.assert_responses_call_count(2)
         self.assertIsNotNone(session)
 
     @responses.activate
@@ -44,7 +43,7 @@ class MarketingSiteAPIClientTests(MarketingSiteAPIClientTestMixin, TestCase):
         self.mock_login_response(200)
         self.mock_csrf_token_response(200)
         csrf_token = self.api_client.csrf_token
-        self.assertEqual(len(responses.calls), 3)
+        self.assert_responses_call_count(3)
         self.assertEqual(self.csrf_token, csrf_token)
 
     @responses.activate
@@ -59,7 +58,7 @@ class MarketingSiteAPIClientTests(MarketingSiteAPIClientTestMixin, TestCase):
         self.mock_login_response(200)
         self.mock_user_id_response(200)
         user_id = self.api_client.user_id
-        self.assertEqual(len(responses.calls), 3)
+        self.assert_responses_call_count(3)
         self.assertEqual(self.user_id, user_id)
 
     @responses.activate
@@ -74,7 +73,7 @@ class MarketingSiteAPIClientTests(MarketingSiteAPIClientTestMixin, TestCase):
         self.mock_login_response(200)
         self.mock_csrf_token_response(200)
         api_session = self.api_client.api_session
-        self.assertEqual(len(responses.calls), 3)
+        self.assert_responses_call_count(3)
         self.assertIsNotNone(api_session)
         self.assertEqual(api_session.headers.get('Content-Type'), 'application/json')
         self.assertEqual(api_session.headers.get('X-CSRF-Token'), self.csrf_token)
@@ -87,7 +86,7 @@ class MarketingSiteAPIClientTests(MarketingSiteAPIClientTestMixin, TestCase):
             self.api_client.api_session  # pylint: disable=pointless-statement
 
 
-class MarketingSitePublisherTests(MarketingSitePublisherTestMixin, TestCase):
+class MarketingSitePublisherTests(MarketingSitePublisherTestMixin):
     """
     Unit test cases for the MarketingSitePublisher
     """
@@ -125,7 +124,7 @@ class MarketingSitePublisherTests(MarketingSitePublisherTestMixin, TestCase):
         self.mock_node_retrieval(self.program.uuid)
         publisher = MarketingSitePublisher()
         node_id = publisher._get_node_id(self.api_client, self.program.uuid)  # pylint: disable=protected-access
-        self.assertEqual(len(responses.calls), 4)
+        self.assert_responses_call_count(4)
         self.assertEqual(node_id, self.nid)
 
     @responses.activate
@@ -143,7 +142,7 @@ class MarketingSitePublisherTests(MarketingSitePublisherTestMixin, TestCase):
         publisher = MarketingSitePublisher()
         publish_data = publisher._get_node_data(self.program, self.user_id)  # pylint: disable=protected-access
         publisher._edit_node(self.api_client, self.nid, publish_data)  # pylint: disable=protected-access
-        self.assertEqual(len(responses.calls), 4)
+        self.assert_responses_call_count(4)
 
     @responses.activate
     def test_edit_node_failed(self):
@@ -189,7 +188,7 @@ class MarketingSitePublisherTests(MarketingSitePublisherTestMixin, TestCase):
         self.mock_node_create(expected, 201)
         publisher = MarketingSitePublisher()
         publisher.publish_program(self.program)
-        self.assertEqual(len(responses.calls), 6)
+        self.assert_responses_call_count(6)
 
     @responses.activate
     def test_publish_program_edit(self):
@@ -198,7 +197,7 @@ class MarketingSitePublisherTests(MarketingSitePublisherTestMixin, TestCase):
         self.mock_node_edit(200)
         publisher = MarketingSitePublisher()
         publisher.publish_program(self.program)
-        self.assertEqual(len(responses.calls), 6)
+        self.assert_responses_call_count(6)
 
     @responses.activate
     def test_publish_modified_program(self):
@@ -208,11 +207,38 @@ class MarketingSitePublisherTests(MarketingSitePublisherTestMixin, TestCase):
         program_before = ProgramFactory()
         publisher = MarketingSitePublisher(program_before)
         publisher.publish_program(self.program)
-        self.assertEqual(len(responses.calls), 6)
+        self.assert_responses_call_count(6)
 
     @responses.activate
     def test_publish_unmodified_program(self):
         self.mock_api_client(200)
         publisher = MarketingSitePublisher(self.program)
         publisher.publish_program(self.program)
-        self.assertEqual(len(responses.calls), 0)
+        self.assert_responses_call_count(0)
+
+    @responses.activate
+    def test_publish_program_no_credential(self):
+        self.program.partner.marketing_site_api_password = None
+        self.program.partner.marketing_site_api_username = None
+        self.program.save()  # pylint: disable=no-member
+        publisher = MarketingSitePublisher()
+        with self.assertRaises(ProgramPublisherException):
+            publisher.publish_program(self.program)
+            self.assert_responses_call_count(0)
+
+    @responses.activate
+    def test_publish_delete_program(self):
+        self.mock_api_client(200)
+        self.mock_node_retrieval(self.program.uuid)
+        self.mock_node_delete(204)
+        publisher = MarketingSitePublisher()
+        publisher.delete_program(self.program)
+        self.assert_responses_call_count(5)
+
+    @responses.activate
+    def test_publish_delete_non_existent_program(self):
+        self.mock_api_client(200)
+        self.mock_node_retrieval(self.program.uuid, exists=False)
+        publisher = MarketingSitePublisher()
+        publisher.delete_program(self.program)
+        self.assert_responses_call_count(4)

course_discovery/apps/course_metadata/tests/test_signals.py

+# pylint: disable=no-member
+from unittest.mock import patch
+
+from django.test import TestCase
+
+from course_discovery.apps.course_metadata.tests import factories, toggle_switch
+
+
+MARKETING_SITE_PUBLISHERS_MODULE = 'course_discovery.apps.course_metadata.publishers.MarketingSitePublisher'
+
+
+@patch(MARKETING_SITE_PUBLISHERS_MODULE + '.delete_program')
+class SignalsTest(TestCase):
+    def setUp(self):
+        super(SignalsTest, self).setUp()
+        self.program = factories.ProgramFactory()
+
+    def test_delete_program_signal_no_publish(self, delete_program_mock):
+        toggle_switch('publish_program_to_marketing_site', False)
+        self.program.delete()
+        self.assertFalse(delete_program_mock.called)
+
+    def test_delete_program_signal_with_publish(self, delete_program_mock):
+        toggle_switch('publish_program_to_marketing_site', True)
+        self.program.delete()
+        delete_program_mock.assert_called_once_with(self.program)