Skip to content

Commit

Permalink
feat: making pathway status non-nullable (#2712)
Browse files Browse the repository at this point in the history
* feat: making pathway status non-nullable

It gets rid of so many problems if we just do a data migration on old
pathway  status, and don't make any assumptions that it might be null or
blank.
  • Loading branch information
deborahgu authored Feb 28, 2025
1 parent d42973e commit c2eb383
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ Course discovery is now allowing pathways to be retired. The `course-discovery`

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`.
The catalog app will now synchronize the `Pathway`'s `status` attribute.
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Generated by Django 4.2.19 on 2025-02-28 16:28

from django.db import migrations, models
from django.db.models import Q


def populate_pathway_status(apps, schema_editor):
Pathway = apps.get_model("catalog", "Pathway")
for pathway in Pathway.objects.filter(Q(status__isnull=True) | Q(status__exact="")):
pathway.status = "published"
pathway.save()


def unpopulate_pathway_status(apps, schema_editor):
"""
We must specify a backwards migration even if it is empty, or else django will raise an
exception if we try to rollback this migration.
We don't want to nullify everything that's published, because we don't know
that's correct.
"""
pass


class Migration(migrations.Migration):

dependencies = [
("catalog", "0016_pathway_status"),
]

operations = [
migrations.RunPython(populate_pathway_status, unpopulate_pathway_status),
migrations.AlterField(
model_name="pathway",
name="status",
field=models.CharField(
choices=[("retired", "retired"), ("unpublished", "unpublished"), ("published", "published")],
default="published",
max_length=24,
),
),
]
10 changes: 6 additions & 4 deletions credentials/apps/catalog/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from credentials.shared.constants import PathwayType

from .data import ProgramStatus
from .data import PathwayStatus, ProgramStatus


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -143,9 +143,11 @@ 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)
status = models.CharField(
max_length=24,
choices=[(tag.value, tag.value) for tag in PathwayStatus],
default=PathwayStatus.PUBLISHED.value,
)

class Meta:
unique_together = (("site", "uuid"),)
Expand Down
2 changes: 1 addition & 1 deletion credentials/apps/catalog/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ def _parse_pathway(self, data):
self.add_item(self.PATHWAY, str(pathway.uuid))

pathway.programs.clear()
if pathway.status in ("", PathwayStatus.PUBLISHED.value):
if pathway.status == PathwayStatus.PUBLISHED.value:
for program_data in data["programs"]:
program = Program.objects.get(site=self.site, uuid=program_data["uuid"])
pathway.programs.add(program)
Expand Down
1 change: 0 additions & 1 deletion credentials/apps/records/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ def test_email_content_complete(self):
(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):
Expand Down
2 changes: 1 addition & 1 deletion credentials/apps/records/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def post(self, request, **kwargs):
Pathway,
id=pathway_id,
programs__uuid=program_uuid,
status__in=(PathwayStatus.PUBLISHED.value, ""),
status=PathwayStatus.PUBLISHED.value,
pathway_type=PathwayType.CREDIT.value,
)
certificate = get_object_or_404(ProgramCertificate, program_uuid=program_uuid, site=request.site)
Expand Down

0 comments on commit c2eb383

Please sign in to comment.