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)