Program marketing slug to be URL of published Drupal page

ECOM-7310

course_discovery/apps/course_metadata/publishers.py

@@ -2,7 +2,7 @@ import json
 from urllib.parse import urljoin
 
 from bs4 import BeautifulSoup
-from django.utils.functional import cached_property
+from django.utils.text import slugify
 
 from course_discovery.apps.course_metadata.choices import CourseRunStatus
 from course_discovery.apps.course_metadata.exceptions import (
@@ -255,6 +255,7 @@ class ProgramMarketingSitePublisher(BaseMarketingSitePublisher):
                     self.edit_node(node_id, node_data)
 
             if node_id:
+                self.get_and_delete_alias(slugify(obj.title))
                 self.update_node_alias(obj, node_id, previous_obj)
 
     def serialize_obj(self, obj):
@@ -292,65 +293,77 @@ class ProgramMarketingSitePublisher(BaseMarketingSitePublisher):
         """
         new_alias = self.alias(obj)
         previous_alias = self.alias(previous_obj) if previous_obj else None
+        new_alias_delete_path = self.alias_delete_path(self.get_alias_list_url(obj.marketing_slug))
 
-        if new_alias != previous_alias:
-            alias_add_url = '{}/add'.format(self.alias_api_base)
+        if new_alias != previous_alias or not new_alias_delete_path:
+            # Delete old alias before saving the new one.
+            if previous_obj and previous_obj.marketing_slug != obj.marketing_slug:
+                self.get_and_delete_alias(previous_obj.marketing_slug)
 
             headers = {
                 'content-type': 'application/x-www-form-urlencoded'
             }
 
             data = {
-                **self.alias_form_inputs,
+                **self.alias_form_inputs(self.alias_add_url),
                 'alias': new_alias,
                 'form_id': 'path_admin_form',
                 'op': 'Save',
                 'source': 'node/{}'.format(node_id),
             }
 
-            response = self.client.api_session.post(alias_add_url, headers=headers, data=data)
+            response = self.client.api_session.post(self.alias_add_url, headers=headers, data=data)
 
             if response.status_code != 200:
                 raise AliasCreateError
 
-            # Delete old alias after saving the new one.
-            if previous_obj:
-                alias_list_url = '{base}/list/{slug}'.format(
-                    base=self.alias_api_base,
-                    slug=previous_obj.marketing_slug
-                )
-                alias_delete_path = self.alias_delete_path(alias_list_url)
+    def get_and_delete_alias(self, slug):
+        """
+        Get the URL alias for the provided slug and delete it if exists.
 
-                if alias_delete_path:
-                    alias_delete_url = '{base}/{path}'.format(
-                        base=self.client.api_url,
-                        path=alias_delete_path.strip('/')
-                    )
+        Arguments:
+            slug (str): slug for which URL alias has to be fetched.
+        """
+        alias_list_url = self.get_alias_list_url(slug)
+        alias_delete_path = self.alias_delete_path(alias_list_url)
+        if alias_delete_path:
+            self.delete_alias(alias_delete_path)
+
+    def delete_alias(self, alias_delete_path):
+        """
+        Delete the url alias for provided path
+        """
+        headers = {
+            'content-type': 'application/x-www-form-urlencoded'
+        }
+        alias_delete_url = '{base}/{path}'.format(
+            base=self.client.api_url,
+            path=alias_delete_path.strip('/')
+        )
 
-                    data = {
-                        **self.alias_form_inputs,
-                        'confirm': 1,
-                        'form_id': 'path_admin_delete_confirm',
-                        'op': 'Confirm',
-                    }
+        data = {
+            **self.alias_form_inputs(alias_delete_url),
+            'confirm': 1,
+            'form_id': 'path_admin_delete_confirm',
+            'op': 'Confirm'
+        }
 
-                    response = self.client.api_session.post(alias_delete_url, headers=headers, data=data)
+        response = self.client.api_session.post(alias_delete_url, headers=headers, data=data)
 
-                    if response.status_code != 200:
-                        raise AliasDeleteError
+        if response.status_code != 200:
+            raise AliasDeleteError
 
     def alias(self, obj):
         return '{type}/{slug}'.format(type=obj.type.slug, slug=obj.marketing_slug)
 
-    @cached_property
-    def alias_form_inputs(self):
+    def alias_form_inputs(self, url):
         """
         Scrape input values from the form used to modify Drupal aliases.
 
         Raises:
             FormRetrievalError: If there's a problem getting the form from Drupal.
         """
-        response = self.client.api_session.get(self.alias_add_url)
+        response = self.client.api_session.get(url)
 
         if response.status_code != 200:
             raise FormRetrievalError
@@ -379,3 +392,9 @@ class ProgramMarketingSitePublisher(BaseMarketingSitePublisher):
         delete_element = html.select('.delete.last a')
 
         return delete_element[0].get('href') if delete_element else None
+
+    def get_alias_list_url(self, slug):
+        return '{base}/list/{slug}'.format(
+            base=self.alias_api_base,
+            slug=slug
+        )

course_discovery/apps/course_metadata/tests/mixins.py

@@ -148,6 +148,15 @@ class MarketingSitePublisherTestMixin(MarketingSiteAPIClientTestMixin):
                  '</input><input name="form_token" value="2"></input></form></html>'
         )
 
+    def mock_delete_alias_form(self, status=200):
+        responses.add(
+            responses.GET,
+            '{root}/admin/config/search/path/delete/foo'.format(root=self.api_root),
+            status=status,
+            body='<html><form><input name="form_build_id" value="1">'
+                 '</input><input name="form_token" value="2"></input></form></html>'
+        )
+
     def mock_get_delete_form(self, alias, status=200):
         responses.add(
             responses.GET,

course_discovery/apps/course_metadata/tests/test_publishers.py

@@ -263,7 +263,10 @@ class ProgramMarketingSitePublisherTests(MarketingSitePublisherTestMixin):
     @mock.patch.object(ProgramMarketingSitePublisher, 'create_node', return_value='node_id')
     @mock.patch.object(ProgramMarketingSitePublisher, 'edit_node', return_value=None)
     @mock.patch.object(ProgramMarketingSitePublisher, 'update_node_alias', return_value=None)
-    def test_publish_obj(self, mock_update_node_alias, mock_edit_node, mock_create_node, *args):  # pylint: disable=unused-argument
+    @mock.patch.object(ProgramMarketingSitePublisher, 'get_and_delete_alias', return_value=None)
+    def test_publish_obj(
+            self, mock_get_and_delete_alias, mock_update_node_alias, mock_edit_node, mock_create_node, *args
+    ):  # pylint: disable=unused-argument
         """
         Verify that the publisher only attempts to publish programs of certain types,
         only attempts an edit when any one of a set of trigger fields is changed,
@@ -287,6 +290,7 @@ class ProgramMarketingSitePublisherTests(MarketingSitePublisherTestMixin):
 
             assert mock_create_node.called
             assert not mock_edit_node.called
+            assert mock_get_and_delete_alias.called
             assert mock_update_node_alias.called
 
             for mocked_method in mocked_methods:
@@ -305,6 +309,7 @@ class ProgramMarketingSitePublisherTests(MarketingSitePublisherTestMixin):
 
             assert not mock_create_node.called
             assert mock_edit_node.called
+            assert mock_get_and_delete_alias.called
             assert mock_update_node_alias.called
 
     @responses.activate
@@ -340,10 +345,14 @@ class ProgramMarketingSitePublisherTests(MarketingSitePublisherTestMixin):
         and deletes an old alias, if one existed, and that appropriate exceptions
         are raised for non-200 status codes.
         """
-        # No previous object is provided. Alias creation should occur, but alias
-        # deletion should not.
+        # No previous object is provided. Create a new node and make sure
+        # title alias created, by default, based on the title is deleted
+        # and a new alias based on marketing slug is created.
         self.mock_api_client()
         self.mock_get_alias_form()
+        self.mock_get_delete_form(self.obj.title)
+        self.mock_delete_alias()
+        self.mock_get_delete_form(self.obj.marketing_slug)
         self.mock_add_alias()
 
         self.publisher.update_node_alias(self.obj, self.node_id, None)
@@ -356,6 +365,9 @@ class ProgramMarketingSitePublisherTests(MarketingSitePublisherTestMixin):
         # alias creation. An exception should be raised.
         self.mock_api_client()
         self.mock_get_alias_form()
+        self.mock_get_delete_form(self.obj.title)
+        self.mock_delete_alias()
+        self.mock_get_delete_form(self.obj.marketing_slug)
         self.mock_add_alias(status=500)
 
         with pytest.raises(AliasCreateError):
@@ -366,6 +378,7 @@ class ProgramMarketingSitePublisherTests(MarketingSitePublisherTestMixin):
         # A previous object is provided, but the marketing slug hasn't changed.
         # Neither alias creation nor alias deletion should occur.
         self.mock_api_client()
+        self.mock_get_delete_form(self.obj.marketing_slug)
 
         self.publisher.update_node_alias(self.obj, self.node_id, self.obj)
 
@@ -376,10 +389,13 @@ class ProgramMarketingSitePublisherTests(MarketingSitePublisherTestMixin):
         previous_obj = ProgramFactory()
 
         self.mock_api_client()
-        self.mock_get_alias_form()
-        self.mock_add_alias()
+        self.mock_get_delete_form(self.obj.marketing_slug)
         self.mock_get_delete_form(previous_obj.marketing_slug)
+        self.mock_delete_alias_form()
         self.mock_delete_alias()
+        self.mock_get_alias_form()
+        self.mock_get_delete_form(self.obj.marketing_slug)
+        self.mock_add_alias()
 
         self.publisher.update_node_alias(self.obj, self.node_id, previous_obj)
 
@@ -391,9 +407,9 @@ class ProgramMarketingSitePublisherTests(MarketingSitePublisherTestMixin):
         # Same scenario, but this time a non-200 status code is returned during
         # alias deletion. An exception should be raised.
         self.mock_api_client()
-        self.mock_get_alias_form()
-        self.mock_add_alias()
+        self.mock_get_delete_form(self.obj.marketing_slug)
         self.mock_get_delete_form(previous_obj.marketing_slug)
+        self.mock_delete_alias_form()
         self.mock_delete_alias(status=500)
 
         with pytest.raises(AliasDeleteError):