Skip to content

Commit

Permalink
feat: add url slug automation
Browse files Browse the repository at this point in the history
To make a consistent SEO experience, url slug generation strategy
is modified.It will introduce its hook right after the course is
transitioned into its Review.

PROD-3395
  • Loading branch information
uzairr committed Jun 28, 2023
1 parent 56182b6 commit e7a52de
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 5 deletions.
26 changes: 25 additions & 1 deletion course_discovery/apps/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
import functools
import logging
import math
import re
from urllib.parse import parse_qsl, urlencode, urljoin

from django.conf import settings
from django.core.files.base import ContentFile
from django.db.models.fields.related import ManyToManyField
from django.utils.translation import gettext as _
Expand All @@ -13,11 +15,33 @@

from course_discovery.apps.core.api_client.lms import LMSAPIClient
from course_discovery.apps.core.utils import serialize_datetime
from course_discovery.apps.course_metadata.models import CourseRun
from course_discovery.apps.course_metadata.constants import SUBDIRECTORY_SLUG_FORMAT_REGEX
from course_discovery.apps.course_metadata.models import Course, CourseRun
from course_discovery.apps.course_metadata.utils import get_slug_for_course

logger = logging.getLogger(__name__)


def set_subdirectory_slug_for_course(course_run):
"""
Sets the active url slug for draft and non-draft courses if the current
slug is not validated as per the new format.
"""
draft_course = Course.everything.get(key=course_run.course.key, draft=True)
is_slug_in_subdirectory_format = bool(re.match(SUBDIRECTORY_SLUG_FORMAT_REGEX, draft_course.active_url_slug))
if not is_slug_in_subdirectory_format and draft_course.product_source.slug == settings.DEFAULT_PRODUCT_SOURCE_SLUG:
slug, error = get_slug_for_course(draft_course)
if slug:
draft_course.set_active_url_slug(slug)
if draft_course.official_version:
draft_course.official_version.set_active_url_slug(slug)
else:
raise Exception( # pylint: disable=broad-exception-raised
f"Slug generation Failed: unable to set active url slug for course: {draft_course.key} "
f"with error {error}"
)


def cast2int(value, name):
"""
Attempt to cast the provided value to an integer.
Expand Down
52 changes: 49 additions & 3 deletions course_discovery/apps/api/v1/tests/test_views/test_course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import responses
from django.contrib.auth.models import Group
from django.db.models.functions import Lower
from django.test import override_settings
from edx_toggles.toggles.testutils import override_waffle_switch
from freezegun import freeze_time
from rest_framework.reverse import reverse
from rest_framework.test import APIRequestFactory
Expand All @@ -21,22 +23,26 @@
from course_discovery.apps.course_metadata.models import CourseRun, CourseRunType, Seat, SeatType
from course_discovery.apps.course_metadata.tests.factories import (
CourseEditorFactory, CourseFactory, CourseRunFactory, CourseRunTypeFactory, CourseTypeFactory, OrganizationFactory,
PersonFactory, ProgramFactory, SeatFactory, SourceFactory, TrackFactory
PersonFactory, ProgramFactory, SeatFactory, SourceFactory, SubjectFactory, TrackFactory
)
from course_discovery.apps.course_metadata.toggles import IS_SUBDIRECTORY_SLUG_FORMAT_ENABLED
from course_discovery.apps.course_metadata.utils import is_valid_slug_format
from course_discovery.apps.ietf_language_tags.models import LanguageTag
from course_discovery.apps.publisher.tests.factories import OrganizationExtensionFactory

LOGGER_NAME = 'course_discovery.apps.api.utils'


@ddt.ddt
class CourseRunViewSetTests(SerializationMixin, ElasticsearchTestMixin, OAuth2Mixin, APITestCase):
def setUp(self):
super().setUp()
self.user = UserFactory(is_staff=True)
self.product_source = SourceFactory(name='test source')
self.product_source = SourceFactory(name='test source', slug='edx')
self.client.force_authenticate(self.user)
self.course_run = CourseRunFactory(course__partner=self.partner)
self.course_run_2 = CourseRunFactory(course__key='Test+Course', course__partner=self.partner)
self.draft_course = CourseFactory(partner=self.partner, draft=True)
self.draft_course = CourseFactory(partner=self.partner, draft=True, product_source=self.product_source)
self.draft_course_run = CourseRunFactory(course=self.draft_course, draft=True)
self.draft_course_run.course.authoring_organizations.add(OrganizationFactory(key='course-id'))
self.course_run_type = CourseRunTypeFactory(tracks=[TrackFactory()])
Expand Down Expand Up @@ -1279,3 +1285,43 @@ def test_change_status_and_ofac_info(self, patch_transaction):
response = self.client.patch(url, patch_transaction['body'], format='json')

assert response.status_code == patch_transaction['status_code']

@override_settings(DEFAULT_PRODUCT_SOURCE_SLUG='edx')
@ddt.data(
(IS_SUBDIRECTORY_SLUG_FORMAT_ENABLED, True),
(IS_SUBDIRECTORY_SLUG_FORMAT_ENABLED, False),
)
@ddt.unpack
def test_url_restructuring_with_feature_flag_state(self, flag, state):
"""
Tests url slug restructuring must work under its relevant feature flag
"""
self.mock_patch_to_studio(self.draft_course_run.key)
course = self.draft_course_run.course
course.subjects.add(SubjectFactory(name='Subject1'))

self.draft_course_run.status = CourseRunStatus.Unpublished
self.draft_course_run.save()
url = reverse('api:v1:course_run-detail', kwargs={'key': self.draft_course_run.key})
body = {
'course': self.draft_course_run.course.key, # required, so we need for a put
'start': self.draft_course_run.start, # required, so we need for a put
'end': self.draft_course_run.end, # required, so we need for a put
'run_type': str(self.draft_course_run.type.uuid), # required, so we need for a put
'draft': False,
}

with override_waffle_switch(flag, active=state):
if state:
response = self.client.put(url, body, format='json')
draft_course_run = CourseRun.everything.get(key=self.draft_course_run.key, draft=True)
assert draft_course_run.course.active_url_slug.startswith('learn/')
else:
response = self.client.put(url, body, format='json')
draft_course_run = CourseRun.everything.get(key=self.draft_course_run.key, draft=True)
assert not draft_course_run.course.active_url_slug.startswith('learn/')

assert is_valid_slug_format(draft_course_run.course.active_url_slug)

assert response.status_code == 200, f"Status {response.status_code}: {response.content}"
assert draft_course_run.status == CourseRunStatus.LegalReview
10 changes: 9 additions & 1 deletion course_discovery/apps/api/v1/views/course_runs.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging

from django.conf import settings
from django.db import models, transaction
from django.db.models.functions import Lower
from django.http.response import Http404
Expand All @@ -18,13 +19,16 @@
from course_discovery.apps.api.pagination import ProxiedPagination
from course_discovery.apps.api.permissions import IsCourseRunEditorOrDjangoOrReadOnly
from course_discovery.apps.api.serializers import MetadataWithRelatedChoices
from course_discovery.apps.api.utils import StudioAPI, get_query_param, reviewable_data_has_changed
from course_discovery.apps.api.utils import (
StudioAPI, get_query_param, reviewable_data_has_changed, set_subdirectory_slug_for_course
)
from course_discovery.apps.api.v1.exceptions import EditableAndQUnsupported
from course_discovery.apps.core.utils import SearchQuerySetWrapper
from course_discovery.apps.course_metadata.choices import CourseRunStatus
from course_discovery.apps.course_metadata.constants import COURSE_RUN_ID_REGEX
from course_discovery.apps.course_metadata.exceptions import EcommerceSiteAPIClientException
from course_discovery.apps.course_metadata.models import Course, CourseEditor, CourseRun
from course_discovery.apps.course_metadata.toggles import IS_SUBDIRECTORY_SLUG_FORMAT_ENABLED
from course_discovery.apps.course_metadata.utils import ensure_draft_world
from course_discovery.apps.publisher.utils import is_publisher_user

Expand Down Expand Up @@ -275,6 +279,7 @@ def _update_course_run(self, course_run, draft, changed, serializer, request, pr
save_kwargs = {}
# If changes are made after review and before publish, revert status to unpublished.
# Unless we're just switching the status
product_source = course_run.course.product_source
non_exempt_update = changed and course_run.status == CourseRunStatus.Reviewed
if non_exempt_update:
save_kwargs['status'] = CourseRunStatus.Unpublished
Expand All @@ -286,6 +291,9 @@ def _update_course_run(self, course_run, draft, changed, serializer, request, pr
# back into legal review if a non exempt field was changed (expected_program_name and expected_program_type)
if not draft and (course_run.status == CourseRunStatus.Unpublished or non_exempt_update):
save_kwargs['status'] = CourseRunStatus.LegalReview
if IS_SUBDIRECTORY_SLUG_FORMAT_ENABLED.is_enabled() and \
product_source.slug == settings.DEFAULT_PRODUCT_SOURCE_SLUG:
set_subdirectory_slug_for_course(course_run)

course_run = serializer.save(**save_kwargs)

Expand Down

0 comments on commit e7a52de

Please sign in to comment.