From d42973ea83fbac93a17b37442e6abe86d83d348f Mon Sep 17 00:00:00 2001 From: Deborah Kaplan Date: Thu, 27 Feb 2025 13:36:18 -0500 Subject: [PATCH] feat: adding pathway status (#2710) * feat: adding pathway status course-discovery is adding the concept of retired pathways, which it will expose with a status field through the API. We still want to ingest retired and unpublished pathways, because we don't want to break any of our existing foreign key relationships. However, we don't want to expose them to either the Learner Record MFE or the program dashboard. FIXES: APER-3929 --- credentials/apps/catalog/admin.py | 15 ++++++++++++--- credentials/apps/catalog/data.py | 6 ++++++ .../docs/decisions/0002-pathway-status.rst | 14 ++++++++++++++ .../catalog/migrations/0016_pathway_status.py | 18 ++++++++++++++++++ credentials/apps/catalog/models.py | 4 ++++ credentials/apps/catalog/tests/factories.py | 2 ++ credentials/apps/catalog/tests/test_utils.py | 3 +++ credentials/apps/catalog/utils.py | 15 ++++++++++----- credentials/apps/records/api.py | 7 ++++++- credentials/apps/records/tests/test_views.py | 16 ++++++++++++++++ credentials/apps/records/views.py | 2 ++ 11 files changed, 93 insertions(+), 9 deletions(-) create mode 100644 credentials/apps/catalog/docs/decisions/0002-pathway-status.rst create mode 100644 credentials/apps/catalog/migrations/0016_pathway_status.py diff --git a/credentials/apps/catalog/admin.py b/credentials/apps/catalog/admin.py index 6d5e641c1..31c9e7e6b 100644 --- a/credentials/apps/catalog/admin.py +++ b/credentials/apps/catalog/admin.py @@ -38,9 +38,18 @@ class ProgramAdmin(admin.ModelAdmin): @admin.register(Pathway) class PathwayAdmin(admin.ModelAdmin): - list_display = ("name", "org_name", "pathway_type", "email", "uuid") - list_filter = ("site",) - readonly_fields = ("name", "org_name", "pathway_type", "email", "uuid", "site", "programs") + list_display = ("name", "org_name", "pathway_type", "status", "email", "uuid") + list_filter = ("site", "status") + readonly_fields = ( + "name", + "org_name", + "pathway_type", + "email", + "uuid", + "site", + "programs", + "status", + ) search_fields = ("name", "uuid") diff --git a/credentials/apps/catalog/data.py b/credentials/apps/catalog/data.py index c6ffa2599..d43ee8fd3 100644 --- a/credentials/apps/catalog/data.py +++ b/credentials/apps/catalog/data.py @@ -42,3 +42,9 @@ class ProgramStatus(Enum): RETIRED = "retired" DELETED = "deleted" UNPUBLISHED = "unpublished" + + +class PathwayStatus(Enum): + RETIRED = "retired" + UNPUBLISHED = "unpublished" + PUBLISHED = "published" diff --git a/credentials/apps/catalog/docs/decisions/0002-pathway-status.rst b/credentials/apps/catalog/docs/decisions/0002-pathway-status.rst new file mode 100644 index 000000000..ea9f8e9cc --- /dev/null +++ b/credentials/apps/catalog/docs/decisions/0002-pathway-status.rst @@ -0,0 +1,14 @@ +Pathway Status +============== + +Status +------ +Accepted + +Background +---------- +Course discovery is now allowing pathways to be retired. The `course-discovery` API will continue to expose retired pathways, and it will rely on consumers to expose or process retired pathways as appropriate. + +Decision +-------- +The catalog app will now synchronize the `Pathway`'s `status` attribute. In the absence of a populated `status` attribute, a pathway will be considered to have the status of `published`. diff --git a/credentials/apps/catalog/migrations/0016_pathway_status.py b/credentials/apps/catalog/migrations/0016_pathway_status.py new file mode 100644 index 000000000..34e3e5ef6 --- /dev/null +++ b/credentials/apps/catalog/migrations/0016_pathway_status.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.19 on 2025-02-25 20:14 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("catalog", "0015_auto_20201130_2041"), + ] + + operations = [ + migrations.AddField( + model_name="pathway", + name="status", + field=models.CharField(max_length=24, null=True), + ), + ] diff --git a/credentials/apps/catalog/models.py b/credentials/apps/catalog/models.py index bc39c4311..8e29a6d01 100644 --- a/credentials/apps/catalog/models.py +++ b/credentials/apps/catalog/models.py @@ -143,6 +143,10 @@ class Pathway(TimeStampedModel): email = models.EmailField() programs = SortedManyToManyField(Program, related_name="pathways") + # We're not migration-creating a status of all the old Pathways, + # we're just treating them as inherently 'published'. + status = models.CharField(max_length=24, null=True, blank=False) + class Meta: unique_together = (("site", "uuid"),) diff --git a/credentials/apps/catalog/tests/factories.py b/credentials/apps/catalog/tests/factories.py index 93fe6d219..a486bc552 100644 --- a/credentials/apps/catalog/tests/factories.py +++ b/credentials/apps/catalog/tests/factories.py @@ -10,6 +10,7 @@ from pytz import UTC from slugify import slugify +from credentials.apps.catalog.data import PathwayStatus from credentials.apps.catalog.models import Course, CourseRun, Organization, Pathway, Program from credentials.apps.core.tests.factories import SiteFactory @@ -90,6 +91,7 @@ class Meta: name = FuzzyText(prefix="Test Pathway ") org_name = FuzzyText() email = factory.Faker("safe_email") + status = PathwayStatus.PUBLISHED.value @factory.post_generation def programs(self, create, extracted): diff --git a/credentials/apps/catalog/tests/test_utils.py b/credentials/apps/catalog/tests/test_utils.py index ccb594373..f621e2427 100644 --- a/credentials/apps/catalog/tests/test_utils.py +++ b/credentials/apps/catalog/tests/test_utils.py @@ -52,6 +52,7 @@ class SynchronizerTests(TestCase): "name": "First Pathway", "org_name": "First Org Name", "email": "test@example.com", + "status": "published", "programs": [FIRST_PROGRAM], "pathway_type": PathwayType.INDUSTRY.value, } # Check type is industry since type defaults to credit @@ -150,6 +151,7 @@ def test_fetch_data_create(self): # Check pathway pathway = Pathway.objects.all().first() assert str(pathway.uuid) == self.FIRST_PATHWAY["uuid"] + assert pathway.status == self.FIRST_PATHWAY["status"] assert list(pathway.programs.all()) == [program] def test_fetch_data_update(self): @@ -202,6 +204,7 @@ def test_fetch_data_update(self): # Check pathway still exists and has updated program pathway = Pathway.objects.all().first() assert str(pathway.uuid) == self.FIRST_PATHWAY["uuid"] + assert pathway.status == self.FIRST_PATHWAY["status"] assert list(pathway.programs.all()) == [program] def test_remove_obsolete_data(self): diff --git a/credentials/apps/catalog/utils.py b/credentials/apps/catalog/utils.py index db8c80b9b..d1decfd67 100644 --- a/credentials/apps/catalog/utils.py +++ b/credentials/apps/catalog/utils.py @@ -5,6 +5,7 @@ from django.db import transaction +from credentials.apps.catalog.data import PathwayStatus from credentials.apps.catalog.models import Course, CourseRun, Organization, Pathway, Program @@ -315,9 +316,11 @@ def _parse_course_run(self, course, data): @transaction.atomic def _parse_pathway(self, data): """ - Creates or updates a pathway and links it to connected programs + Creates or updates a pathway and links it to connected programs. - Assumes that the associated programs were parsed before this is run. + * Assumes that the associated programs were parsed before this is run. + * Always re-creates the foreign keys between Pathway and Program on modification. + If the Pathway is retired or unpublished, no relationship is created Arguments: data (dict): The pathway data pulled from the API @@ -332,6 +335,7 @@ def _parse_pathway(self, data): "name": data["name"], "email": data["email"], "org_name": data["org_name"], + "status": data["status"], "pathway_type": data["pathway_type"], }, ) @@ -339,8 +343,9 @@ def _parse_pathway(self, data): self.add_item(self.PATHWAY, str(pathway.uuid)) pathway.programs.clear() - for program_data in data["programs"]: - program = Program.objects.get(site=self.site, uuid=program_data["uuid"]) - pathway.programs.add(program) + if pathway.status in ("", PathwayStatus.PUBLISHED.value): + for program_data in data["programs"]: + program = Program.objects.get(site=self.site, uuid=program_data["uuid"]) + pathway.programs.add(program) return pathway diff --git a/credentials/apps/records/api.py b/credentials/apps/records/api.py index a20315907..cb9d265ca 100644 --- a/credentials/apps/records/api.py +++ b/credentials/apps/records/api.py @@ -100,7 +100,12 @@ def _get_transformed_pathway_data(program, user): program_pathways_set = frozenset(program_pathways) user_credit_pathways = ( - UserCreditPathway.objects.select_related("pathway").filter(user=user, pathway__in=program_pathways_set).all() + UserCreditPathway.objects.select_related("pathway") + .filter( + user=user, + pathway__in=program_pathways_set, + ) + .all() ) # maps a learner's pathway status to a pathway user_credit_pathways_dict = {user_pathway.pathway: user_pathway.status for user_pathway in user_credit_pathways} diff --git a/credentials/apps/records/tests/test_views.py b/credentials/apps/records/tests/test_views.py index 61595d30f..b97863aea 100644 --- a/credentials/apps/records/tests/test_views.py +++ b/credentials/apps/records/tests/test_views.py @@ -17,6 +17,7 @@ from django.test.utils import override_settings from django.urls import reverse +from credentials.apps.catalog.data import PathwayStatus from credentials.apps.catalog.tests.factories import ( CourseFactory, CourseRunFactory, @@ -105,6 +106,7 @@ def test_pcr_already_exists(self): self.assertEqual(url1, url2) +@ddt.ddt @override_settings(EMAIL_BACKEND="django.core.mail.backends.locmem.EmailBackend") class ProgramSendTests(SiteMixin, TestCase): USERNAME = "test-user" @@ -208,6 +210,20 @@ def test_email_content_complete(self): self.assertEqual(self.user.email, email.reply_to[0]) self.assertListEqual([self.pathway.email], email.to) + @ddt.data( + (PathwayStatus.PUBLISHED.value, 200), + (PathwayStatus.UNPUBLISHED.value, 404), + (PathwayStatus.RETIRED.value, 404), + ("", 200), + ) + @ddt.unpack + def test_pathway_must_be_published(self, pathway_status, http_status): + """Verify a pathway only sends if its status is published or empty""" + self.pathway.status = pathway_status + self.pathway.save() + response = self.post() + self.assertEqual(response.status_code, http_status) + def test_email_content_incomplete(self): """Verify an email is actually sent""" self.user_credential.delete() diff --git a/credentials/apps/records/views.py b/credentials/apps/records/views.py index af3c6861f..67fe9c01d 100644 --- a/credentials/apps/records/views.py +++ b/credentials/apps/records/views.py @@ -20,6 +20,7 @@ from edx_ace import Recipient, ace from segment.analytics.client import Client as SegmentClient +from credentials.apps.catalog.data import PathwayStatus from credentials.apps.catalog.models import Pathway, Program from credentials.apps.core.api import get_user_by_username from credentials.apps.core.views import ThemeViewMixin @@ -150,6 +151,7 @@ def post(self, request, **kwargs): Pathway, id=pathway_id, programs__uuid=program_uuid, + status__in=(PathwayStatus.PUBLISHED.value, ""), pathway_type=PathwayType.CREDIT.value, ) certificate = get_object_or_404(ProgramCertificate, program_uuid=program_uuid, site=request.site)