From f677bcd51f88f14f7b4c9e1cfd0245fad4017b5a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:38:26 +0000 Subject: [PATCH 1/2] refactor(roster)!: rename UnitInquiry to UnitPetition The user-facing name has always been "unit petition"; only the internals said "inquiry". This aligns them. The schema change is a RenameModel (ALTER TABLE ... RENAME), so every row is carried over in place. Note that makemigrations autodetects this as a CreateModel/DeleteModel pair instead, which would drop the table and every petition in it -- hence the hand-written 0119. The stored INQ_* choice values become PET_* in 0120, a RunPython over an explicit old <-> new mapping. It only issues UPDATEs, never creates or deletes rows, and leaves any value outside the mapping alone rather than coercing it to a default. Both directions were exercised against a real database: rows migrate backwards to the pre-rename state and forwards again byte-identical, primary keys included. core.UserProfile.email_on_inquiry_complete becomes email_on_petition_complete via RenameField, which is likewise in-place. Renamed throughout: the form, views, URL names, template, factory, admin and local variables. The old /roster/inquiry// URL now redirects to the new one so student bookmarks keep working. BREAKING CHANGE: The Aincrad API changes with it, as agreed. The "accept_inquiries" action is now "accept_petitions", and the init payload's "inquiries" key and its "unlock_inquiry_count" field are now "petitions" and "unlock_petition_count". The consumer needs updating. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012tmVNTck2f5p5pJez64MmT --- aincrad/tests.py | 28 +- aincrad/views.py | 40 +-- ..._userprofile_email_on_petition_complete.py | 17 + core/models.py | 4 +- core/views.py | 2 +- dashboard/templates/dashboard/portal.html | 4 +- otisweb/templates/sidebar.html | 2 +- roster/admin.py | 26 +- roster/factories.py | 8 +- roster/forms.py | 8 +- .../0119_rename_unitinquiry_unitpetition.py | 61 ++++ .../0120_unitpetition_choice_values.py | 53 +++ roster/models.py | 34 +- .../roster/{inquiry.html => petition.html} | 24 +- roster/tests.py | 307 +++++++++--------- roster/urls.py | 11 +- roster/views.py | 116 +++---- 17 files changed, 446 insertions(+), 299 deletions(-) create mode 100644 core/migrations/0072_rename_userprofile_email_on_petition_complete.py create mode 100644 roster/migrations/0119_rename_unitinquiry_unitpetition.py create mode 100644 roster/migrations/0120_unitpetition_choice_values.py rename roster/templates/roster/{inquiry.html => petition.html} (83%) diff --git a/aincrad/tests.py b/aincrad/tests.py index 1d9f6a38..940e8f11 100644 --- a/aincrad/tests.py +++ b/aincrad/tests.py @@ -19,9 +19,9 @@ RegistrationContainerFactory, StudentFactory, StudentRegistrationFactory, - UnitInquiryFactory, + UnitPetitionFactory, ) -from roster.models import ApplyUUID, Invoice, Student, UnitInquiry +from roster.models import ApplyUUID, Invoice, Student, UnitPetition EXAMPLE_PASSWORD = "take just the first 24" TARGET_HASH = sha256(EXAMPLE_PASSWORD.encode("ascii")).hexdigest() @@ -98,14 +98,14 @@ def aincrad_setup(db): PSetFactory.create_batch(4, student=old_alice, status="A") PSetFactory.create_batch(2, student=old_alice, status="P") - UnitInquiryFactory.create_batch( - 5, student=alice, action_type="INQ_ACT_UNLOCK", status="INQ_ACC" + UnitPetitionFactory.create_batch( + 5, student=alice, action_type="PET_ACT_UNLOCK", status="PET_ACC" ) - UnitInquiryFactory.create_batch( - 2, student=alice, action_type="INQ_ACT_DROP", status="INQ_ACC" + UnitPetitionFactory.create_batch( + 2, student=alice, action_type="PET_ACT_DROP", status="PET_ACC" ) - UnitInquiryFactory.create_batch( - 3, student=alice, action_type="INQ_ACT_UNLOCK", status="INQ_NEW" + UnitPetitionFactory.create_batch( + 3, student=alice, action_type="PET_ACT_UNLOCK", status="PET_NEW" ) alice.curriculum.add(submitted_unit) @@ -185,9 +185,9 @@ def test_init(otis, aincrad_setup): else: pytest.fail("Could not find a pset from Bôb B. in aincrad test") - inquiries = out["_children"][1]["inquiries"] - assert len(inquiries) == 3 - assert inquiries[0]["unlock_inquiry_count"] == 8 + petitions = out["_children"][1]["petitions"] + assert len(petitions) == 3 + assert petitions[0]["unlock_petition_count"] == 8 @pytest.mark.django_db @@ -293,17 +293,17 @@ def test_invoice(otis, aincrad_setup): @pytest.mark.django_db @override_settings(API_TARGET_HASH=TARGET_HASH) -def test_accept_inquiries(otis, aincrad_setup): +def test_accept_petitions(otis, aincrad_setup): resp = otis.post_20x( "api", json={ - "action": "accept_inquiries", + "action": "accept_petitions", "token": EXAMPLE_PASSWORD, }, ) assert resp.json()["result"] == "success" assert resp.json()["count"] == 3 - assert not UnitInquiry.objects.filter(status="INQ_NEW").exists() + assert not UnitPetition.objects.filter(status="PET_NEW").exists() @pytest.mark.django_db diff --git a/aincrad/views.py b/aincrad/views.py index 906ae5b3..5f104823 100644 --- a/aincrad/views.py +++ b/aincrad/views.py @@ -33,7 +33,7 @@ Invoice, Student, StudentRegistration, - UnitInquiry, + UnitPetition, ) from suggestions.models import ProblemSuggestion @@ -145,17 +145,17 @@ class JSONData(TypedDict): "student__user__profile__email_on_pset_complete", ) -INQUIRY_VENUEQ_INIT_QUERYSET = UnitInquiry.objects.filter( - status="INQ_NEW", +PETITION_VENUEQ_INIT_QUERYSET = UnitPetition.objects.filter( + status="PET_NEW", student__semester__active=True, student__legit=True, ).annotate( - unlock_inquiry_count=SubqueryCount( - "student__unitinquiry", - filter=Q(action_type="INQ_ACT_UNLOCK"), + unlock_petition_count=SubqueryCount( + "student__unitpetition", + filter=Q(action_type="PET_ACT_UNLOCK"), ), ) -INQUIRY_VENUEQ_INIT_KEYS = ( +PETITION_VENUEQ_INIT_KEYS = ( "action_type", "unit__group__name", "unit__code", @@ -164,14 +164,14 @@ class JSONData(TypedDict): "student__user__email", "explanation", "created_at", - "unlock_inquiry_count", - "student__user__profile__email_on_inquiry_complete", + "unlock_petition_count", + "student__user__profile__email_on_petition_complete", ) -INQUIRY_VENUEQ_AUTO_QUERYSET = UnitInquiry.objects.filter( +PETITION_VENUEQ_AUTO_QUERYSET = UnitPetition.objects.filter( was_auto_processed=True, created_at__gte=timezone.now() + timedelta(days=-2), ) -INQUIRY_VENUEQ_AUTO_KEYS = ( +PETITION_VENUEQ_AUTO_KEYS = ( "action_type", "unit__group__name", "unit__code", @@ -245,12 +245,12 @@ def venueq_handler(action: str, data: JSONData) -> JsonResponse: ), }, { - "_name": "Inquiries", - "inquiries": list( - INQUIRY_VENUEQ_INIT_QUERYSET.values(*INQUIRY_VENUEQ_INIT_KEYS) + "_name": "Petitions", + "petitions": list( + PETITION_VENUEQ_INIT_QUERYSET.values(*PETITION_VENUEQ_INIT_KEYS) ), "reading": list( - INQUIRY_VENUEQ_AUTO_QUERYSET.values(*INQUIRY_VENUEQ_AUTO_KEYS) + PETITION_VENUEQ_AUTO_QUERYSET.values(*PETITION_VENUEQ_AUTO_KEYS) ), }, { @@ -267,14 +267,14 @@ def venueq_handler(action: str, data: JSONData) -> JsonResponse: }, ] return JsonResponse(output_data, status=200) - elif action == "accept_inquiries": + elif action == "accept_petitions": n = 0 - for inquiry in UnitInquiry.objects.filter( - status="INQ_NEW", + for petition in UnitPetition.objects.filter( + status="PET_NEW", student__semester__active=True, student__legit=True, ): - inquiry.run_accept() + petition.run_accept() n += 1 if n > 0: return JsonResponse({"result": "success", "count": n}, status=200) @@ -814,7 +814,7 @@ def api(request: HttpRequest) -> JsonResponse: if action in ( "grade_problem_set", - "accept_inquiries", + "accept_petitions", "mark_suggestion", "triage_job", "init", diff --git a/core/migrations/0072_rename_userprofile_email_on_petition_complete.py b/core/migrations/0072_rename_userprofile_email_on_petition_complete.py new file mode 100644 index 00000000..45073982 --- /dev/null +++ b/core/migrations/0072_rename_userprofile_email_on_petition_complete.py @@ -0,0 +1,17 @@ +# Generated by Django 6.0.8 on 2026-08-25 14:32 + +from django.db import migrations + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0071_alter_semester_first_payment_deadline_and_more"), + ] + + operations = [ + migrations.RenameField( + model_name="userprofile", + old_name="email_on_inquiry_complete", + new_name="email_on_petition_complete", + ), + ] diff --git a/core/models.py b/core/models.py index 194b3241..435b96f2 100644 --- a/core/models.py +++ b/core/models.py @@ -325,7 +325,7 @@ class UserProfile(models.Model): help_text="Receive all-student announcements. If this is set to False, announcements will only appear on OTIS-WEB.", default=True, ) - email_on_inquiry_complete = models.BooleanField( + email_on_petition_complete = models.BooleanField( verbose_name="Receive email on petition processed", help_text="Receive an email when your petition has been processed.", default=False, @@ -357,5 +357,5 @@ def __str__(self) -> str: "email_on_announcement", "email_on_pset_complete", "email_on_suggestion_processed", - "email_on_inquiry_complete", + "email_on_petition_complete", ) diff --git a/core/views.py b/core/views.py index cbe6d13e..65372782 100644 --- a/core/views.py +++ b/core/views.py @@ -289,7 +289,7 @@ class UserProfileUpdateView( model = UserProfile fields = ( "email_on_announcement", - "email_on_inquiry_complete", + "email_on_petition_complete", "email_on_pset_complete", "email_on_suggestion_processed", "show_bars", diff --git a/dashboard/templates/dashboard/portal.html b/dashboard/templates/dashboard/portal.html index 30b34494..5685200d 100644 --- a/dashboard/templates/dashboard/portal.html +++ b/dashboard/templates/dashboard/portal.html @@ -337,7 +337,7 @@

Discord

Petitions

{% if request.user.is_staff %} - Manage units + Manage units Edit units {% else %} {% if not student.enabled %} @@ -347,7 +347,7 @@

Petitions

{% elif student.newborn %}

Pick units first!

{% else %} - Manage units + Manage units {% if bonus_levels %}

You are also sufficiently high level to diff --git a/otisweb/templates/sidebar.html b/otisweb/templates/sidebar.html index bdeed160..0634ca64 100644 --- a/otisweb/templates/sidebar.html +++ b/otisweb/templates/sidebar.html @@ -131,7 +131,7 @@

Admin

    {% if not student.newborn and semester.active and student.enabled %}
  • - Manage units + Manage units
    • {% endif %} {% if student.semester.social_url %} diff --git a/roster/admin.py b/roster/admin.py index f4174247..928ba872 100644 --- a/roster/admin.py +++ b/roster/admin.py @@ -20,7 +20,7 @@ RegistrationContainer, Student, StudentRegistration, - UnitInquiry, + UnitPetition, ) @@ -407,9 +407,9 @@ class StudentRegistrationAdmin(ImportExportModelAdmin): inlines = (StudentRegistrationStudentInline,) -# INQUIRY -@admin.register(UnitInquiry) -class UnitInquiryAdmin(admin.ModelAdmin): +# PETITION +@admin.register(UnitPetition) +class UnitPetitionAdmin(admin.ModelAdmin): readonly_fields = ( "created_at", "updated_at", @@ -442,22 +442,22 @@ class UnitInquiryAdmin(admin.ModelAdmin): actions = ("hold_petition", "reject_petition", "accept_petition", "reset_petition") - def hold_petition(self, request: HttpRequest, queryset: QuerySet[UnitInquiry]): + def hold_petition(self, request: HttpRequest, queryset: QuerySet[UnitPetition]): del request - queryset.update(status="INQ_HOLD") + queryset.update(status="PET_HOLD") - def reject_petition(self, request: HttpRequest, queryset: QuerySet[UnitInquiry]): + def reject_petition(self, request: HttpRequest, queryset: QuerySet[UnitPetition]): del request - queryset.update(status="INQ_REJ") + queryset.update(status="PET_REJ") - def accept_petition(self, request: HttpRequest, queryset: QuerySet[UnitInquiry]): + def accept_petition(self, request: HttpRequest, queryset: QuerySet[UnitPetition]): del request - for inquiry in queryset: - inquiry.run_accept() + for petition in queryset: + petition.run_accept() - def reset_petition(self, request: HttpRequest, queryset: QuerySet[UnitInquiry]): + def reset_petition(self, request: HttpRequest, queryset: QuerySet[UnitPetition]): del request - queryset.update(status="INQ_NEW") + queryset.update(status="PET_NEW") # REGISTRATION diff --git a/roster/factories.py b/roster/factories.py index 2dec1c16..8b75f95d 100644 --- a/roster/factories.py +++ b/roster/factories.py @@ -11,7 +11,7 @@ RegistrationContainer, Student, StudentRegistration, - UnitInquiry, + UnitPetition, ) @@ -67,11 +67,11 @@ class Meta: preps_taught = 2 -class UnitInquiryFactory(DjangoModelFactory): +class UnitPetitionFactory(DjangoModelFactory): class Meta: - model = UnitInquiry + model = UnitPetition student = SubFactory(StudentFactory) unit = SubFactory(UnitFactory) - action_type = "INQ_ACT_UNLOCK" + action_type = "PET_ACT_UNLOCK" explanation = Faker("sentence") diff --git a/roster/forms.py b/roster/forms.py index d939d92c..2de57452 100644 --- a/roster/forms.py +++ b/roster/forms.py @@ -8,7 +8,7 @@ from core.models import EMAIL_PREFERENCE_FIELDS, Semester, Unit from dashboard.models import PSet -from roster.models import Student, StudentRegistration, UnitInquiry +from roster.models import Student, StudentRegistration, UnitPetition class UnitChoiceBoundField(forms.BoundField): @@ -126,7 +126,7 @@ def __init__(self, *args: Any, **kwargs: Any): ) -class InquiryForm(forms.ModelForm): +class PetitionForm(forms.ModelForm): def __init__(self, *args: Any, **kwargs: Any): student: Student = kwargs.pop("student") super().__init__(*args, **kwargs) @@ -139,7 +139,7 @@ def __init__(self, *args: Any, **kwargs: Any): self.fields["unit"].empty_label = "Search for a unit..." # type: ignore class Meta: - model = UnitInquiry + model = UnitPetition fields = ("unit", "action_type", "explanation") widgets: ClassVar[dict[str, forms.Widget]] = { "explanation": forms.Textarea(attrs={"cols": 40, "rows": 3}), @@ -176,7 +176,7 @@ class DecisionForm(forms.ModelForm): help_text="Receive all-student announcements. If this is set to False, announcements will only appear on OTIS-WEB.", required=False, ) - email_on_inquiry_complete = forms.BooleanField( + email_on_petition_complete = forms.BooleanField( label="Receive email on petition processed", help_text="Receive an email when your petition has been processed.", required=False, diff --git a/roster/migrations/0119_rename_unitinquiry_unitpetition.py b/roster/migrations/0119_rename_unitinquiry_unitpetition.py new file mode 100644 index 00000000..96587cdd --- /dev/null +++ b/roster/migrations/0119_rename_unitinquiry_unitpetition.py @@ -0,0 +1,61 @@ +# Renames UnitInquiry -> UnitPetition. +# +# RenameModel issues an ALTER TABLE ... RENAME, so every row is carried over +# untouched; it must never be replaced by a CreateModel/DeleteModel pair (which +# is what makemigrations autodetects here, since the choices changed at the same +# time). The choice *values* stored in action_type and status are remapped +# separately in 0120, which runs immediately after this one. + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("roster", "0118_applyuuid_applicant_name_applyuuid_memo"), + ] + + operations = [ + migrations.RenameModel( + old_name="UnitInquiry", + new_name="UnitPetition", + ), + migrations.AlterField( + model_name="unitpetition", + name="action_type", + field=models.CharField( + choices=[ + ("PET_ACT_UNLOCK", "Unlock now"), + ("PET_ACT_APPEND", "Add for later"), + ("PET_ACT_DROP", "Drop"), + ("PET_ACT_LOCK", "Lock (Drop + Add for later)"), + ], + help_text="Describe the action you want to make.", + max_length=15, + ), + ), + migrations.AlterField( + model_name="unitpetition", + name="status", + field=models.CharField( + choices=[ + ("PET_ACC", "Accepted"), + ("PET_REJ", "Rejected"), + ("PET_NEW", "Pending"), + ("PET_HOLD", "On hold"), + ("PET_CANC", "Canceled"), + ], + default="PET_NEW", + help_text="The current status of the petition.", + max_length=10, + ), + ), + migrations.AlterField( + model_name="unitpetition", + name="was_auto_processed", + field=models.BooleanField( + default=False, + help_text="Whether the petition was automatically accepted or rejected by auto-criteria.", + verbose_name="Auto", + ), + ), + ] diff --git a/roster/migrations/0120_unitpetition_choice_values.py b/roster/migrations/0120_unitpetition_choice_values.py new file mode 100644 index 00000000..6b317ece --- /dev/null +++ b/roster/migrations/0120_unitpetition_choice_values.py @@ -0,0 +1,53 @@ +# Rewrites the stored INQ_* choice values of UnitPetition to PET_*. +# +# Both directions are plain UPDATEs over an explicit old <-> new mapping: no row +# is created or deleted, and a value outside the mapping is left alone rather +# than being coerced into some default. The two vocabularies are disjoint, so +# re-running a direction is a no-op. + +from django.db import migrations + +RENAMES = { + "action_type": { + "INQ_ACT_UNLOCK": "PET_ACT_UNLOCK", + "INQ_ACT_APPEND": "PET_ACT_APPEND", + "INQ_ACT_DROP": "PET_ACT_DROP", + "INQ_ACT_LOCK": "PET_ACT_LOCK", + }, + "status": { + "INQ_ACC": "PET_ACC", + "INQ_REJ": "PET_REJ", + "INQ_NEW": "PET_NEW", + "INQ_HOLD": "PET_HOLD", + "INQ_CANC": "PET_CANC", + }, +} + + +def remap(apps, reverse: bool): + UnitPetition = apps.get_model("roster", "UnitPetition") + for field, mapping in RENAMES.items(): + for old, new in mapping.items(): + if reverse: + old, new = new, old + UnitPetition.objects.filter(**{field: old}).update(**{field: new}) + + +def forwards(apps, schema_editor): + del schema_editor + remap(apps, reverse=False) + + +def backwards(apps, schema_editor): + del schema_editor + remap(apps, reverse=True) + + +class Migration(migrations.Migration): + dependencies = [ + ("roster", "0119_rename_unitinquiry_unitpetition"), + ] + + operations = [ + migrations.RunPython(forwards, backwards), + ] diff --git a/roster/models.py b/roster/models.py index 633c29ad..14bff45e 100644 --- a/roster/models.py +++ b/roster/models.py @@ -428,7 +428,7 @@ def cleared(self) -> bool: return self.total_owed <= 0 -class UnitInquiry(models.Model): +class UnitPetition(models.Model): unit = models.ForeignKey( Unit, on_delete=models.CASCADE, help_text="The unit being requested." ) @@ -441,23 +441,23 @@ class UnitInquiry(models.Model): action_type = models.CharField( max_length=15, choices=( - ("INQ_ACT_UNLOCK", "Unlock now"), - ("INQ_ACT_APPEND", "Add for later"), - ("INQ_ACT_DROP", "Drop"), - ("INQ_ACT_LOCK", "Lock (Drop + Add for later)"), + ("PET_ACT_UNLOCK", "Unlock now"), + ("PET_ACT_APPEND", "Add for later"), + ("PET_ACT_DROP", "Drop"), + ("PET_ACT_LOCK", "Lock (Drop + Add for later)"), ), help_text="Describe the action you want to make.", ) status = models.CharField( max_length=10, choices=( - ("INQ_ACC", "Accepted"), - ("INQ_REJ", "Rejected"), - ("INQ_NEW", "Pending"), - ("INQ_HOLD", "On hold"), - ("INQ_CANC", "Canceled"), + ("PET_ACC", "Accepted"), + ("PET_REJ", "Rejected"), + ("PET_NEW", "Pending"), + ("PET_HOLD", "On hold"), + ("PET_CANC", "Canceled"), ), - default="INQ_NEW", + default="PET_NEW", help_text="The current status of the petition.", ) explanation = models.TextField( @@ -465,7 +465,7 @@ class UnitInquiry(models.Model): ) was_auto_processed = models.BooleanField( default=False, - help_text="Whether the inquiry was automatically accepted or rejected by auto-criteria.", + help_text="Whether the petition was automatically accepted or rejected by auto-criteria.", verbose_name="Auto", ) @@ -479,19 +479,19 @@ def __str__(self) -> str: def run_accept(self): unit = self.unit - if self.action_type == "INQ_ACT_UNLOCK": + if self.action_type == "PET_ACT_UNLOCK": self.student.curriculum.add(unit) self.student.unlocked_units.add(unit) - elif self.action_type == "INQ_ACT_APPEND": + elif self.action_type == "PET_ACT_APPEND": self.student.curriculum.add(unit) - elif self.action_type == "INQ_ACT_DROP": + elif self.action_type == "PET_ACT_DROP": self.student.curriculum.remove(unit) self.student.unlocked_units.remove(unit) - elif self.action_type == "INQ_ACT_LOCK": + elif self.action_type == "PET_ACT_LOCK": self.student.unlocked_units.remove(unit) else: raise ValueError(f"No action {self.action_type}") - self.status = "INQ_ACC" + self.status = "PET_ACC" self.save() diff --git a/roster/templates/roster/inquiry.html b/roster/templates/roster/petition.html similarity index 83% rename from roster/templates/roster/inquiry.html rename to roster/templates/roster/petition.html index 9e21d81c..def465a1 100644 --- a/roster/templates/roster/inquiry.html +++ b/roster/templates/roster/petition.html @@ -59,29 +59,29 @@

    Current petitions

    - {% for inquiry in inquiries %} + {% for petition in petitions %} - {{ inquiry.created_at|date:"M d" }} + {{ petition.created_at|date:"M d" }} - {{ inquiry.unit }} + {{ petition.unit }} - {{ inquiry.get_action_type_display }} + {{ petition.get_action_type_display }} - {% if inquiry.status == "INQ_NEW" %} + {% if petition.status == "PET_NEW" %} Pending - {% elif inquiry.status == "INQ_ACC" %} + {% elif petition.status == "PET_ACC" %} Accepted - {% elif inquiry.status == "INQ_REJ" %} + {% elif petition.status == "PET_REJ" %} Rejected - {% elif inquiry.status == "INQ_CANC" %} + {% elif petition.status == "PET_CANC" %} Canceled - {% elif inquiry.status == "INQ_HOLD" %} + {% elif petition.status == "PET_HOLD" %} On hold {% endif %} - {% if inquiry.status == "INQ_NEW" %} -
    + {% if petition.status == "PET_NEW" %} + {% csrf_token %}