Repository navigation
refactor: convert the suggested-schedule alerts to TypeScript - #2092
Merged
brian-smith-tcril merged 1 commit intoSep 22, 2026
Merged
Conversation
brian-smith-tcril
added this pull request to stack #2080
September 21, 2026 21:31
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2092 +/- ##
==========================================
+ Coverage 93.89% 93.92% +0.03%
==========================================
Files 365 365
Lines 5895 5908 +13
Branches 1406 1407 +1
==========================================
+ Hits 5535 5549 +14
+ Misses 346 345 -1
Partials 14 14 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
brian-smith-tcril
marked this pull request as ready for review
September 21, 2026 22:03
brian-smith-tcril
force-pushed
the
bsmith/suggested-schedule-alerts-typescript
branch
from
September 21, 2026 22:03
d221640 to
f2a1b57
Compare
`ShiftDatesAlert` and `UpgradeToShiftDatesAlert` become `.tsx`, so their
props are a `Props` interface instead of `propTypes` + `defaultProps` —
the destructuring-default shape `UnitButton` uses. Structural only: both
still read `useModel(model, courseId)`, and nothing else changes.
`ShiftDatesAlert` passes `courseId` to two query keys and to the
reset-deadlines mutation, all of which want a `string` while `useParams()`
yields `string | undefined`. Rather than assert it three times, `!courseId`
joins the early return the component already has, narrowing it for the rest
of the body. The guard cannot fire today — `useModel` returns `{}` for a
missing id, so the destructure above it throws first — but it is what lets
the conversion drop the assertions.
Peeled out of #2083, which gives both components new props; doing the
conversion first keeps that layer's diff to the data-source change.
Part of #1946 (Stage 1). Closes #2091.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
brian-smith-tcril
force-pushed
the
bsmith/suggested-schedule-alerts-typescript
branch
from
September 21, 2026 22:15
f2a1b57 to
4d28cf9
Compare
brian-smith-tcril
deleted the
bsmith/suggested-schedule-alerts-typescript
branch
September 22, 2026 20:46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Convert the two suggested-schedule alerts to TypeScript. No user-facing change — structural only: both components still read
useModel(model, courseId)and render the same markup. Peeled out of #2083, which gives both of them new props, so that layer edits aPropsinterface instead of growingpropTypesthat TypeScript would delete one layer later — the peel convention #2065 used ahead of #2014. Part of the Redux → React Query migration (#1946, Stage 1) and the model-store dissolution (#1977). Closes #2091. Stacked above #2082.What changed
ShiftDatesAlert.jsx→ShiftDatesAlert.tsxandUpgradeToShiftDatesAlert.jsx→UpgradeToShiftDatesAlert.tsx; git detects both as renames, so the diff reads as edits rather than delete-plus-add.propTypes+defaultProps→ aPropsinterface with destructuring defaults — the shapeUnitButtonuses (refactor: de-class UnitButton and convert it to TypeScript #2065), rather thanReact.FC+defaultProps.ShiftDatesAlertpassescourseIdto two query keys and to the reset-deadlines mutation, all of which want astring, whileuseParams()yieldsstring | undefined. Rather than assert it three times,!courseIdjoins the early return the component already has, which narrows it for the rest of the body including both closures.Testing
npm run types,npm run lintand the full suite (109 suites, 1112 passed, 3 skipped) are green.Manually verified on tutor dev against a self-paced course with a seeded missed deadline, on this layer alone with #2083 not applied: both banners render on the dates and outline tabs, the reset posts and the dates shift, and neither renders where it shouldn't. Five of the eight checks were run by hand — the two upgrade-variant checks need an audit learner with missed gated content, which that course cannot produce (
content_type_gating_enabled: false), and the per-tab research event rests onapiHooks.test.tsx, which asserts the exact request body.Decisions
Working notes for this layer, kept out of the tree:
Full decision log
Decisions — suggested-schedule alerts to TypeScript (#2091)
Peel below #2083 (layer A of the model-store dissolution, #1977). Structural
only — the data-source change stays in #2083.
Peeled out rather than folded into Read the dates and outline tab data from their queries, not useModel #2083. Read the dates and outline tab data from their queries, not useModel #2083 gives both alerts new
props, so without this layer that diff would edit
propTypesentriesTypeScript deletes one layer later. Same reasoning as refactor: de-class UnitButton and convert it to TypeScript #2065, peeled out of
Convert bookmarking to React Query + de-class UnitButton #2014: structural change first, conversion second, each reviewable on its
own.
Scope is the two components that gain props.
UpgradeToCompleteAlert.jsxkeeps the props it has today — Read the dates and outline tab data from their queries, not useModel #2083 onlychanges where it reads its data — so it stays
.jsx. Converting the wholedirectory for symmetry would be scope the conversion doesn't need.
Propsinterface with destructuring defaults, notReact.FC+defaultProps. MatchesUnitButton.tsx(refactor: de-class UnitButton and convert it to TypeScript #2065), the epic's most recentTypeScript conversion.
Section.tsxandSequenceLink.tsxuseReact.FC<Props>, but that predates this effort anddefaultPropsonfunction components is on its way out.
courseIdis narrowed, not asserted.ShiftDatesAlertfeeds it tocourseHomeQueryKeys.datesTab/outlineTaband to the reset-deadlinesmutation, all typed
string, whileuseParams()yieldsstring | undefined. Adding!courseIdto the early return the componentalready has narrows it for the whole body below, including the
refreshTabDataandonClickclosures, so all threecourseId!go away.The repo's other
courseId!uses all sit inapiHooks.tsnext toenabled: !!courseId, where the query cannot run with an undefined id;a component-level assertion has nothing backing it.
The guard is unreachable, and that is understood. With an undefined
courseId,useModelreturns{}and thedatesBannerInfodestructureabove the guard throws first, so
!courseIdcannot fire in this layer. Itbecomes reachable only once Read the dates and outline tab data from their queries, not useModel #2083 passes that data in as a prop, and even
then only off a course route — where it renders nothing instead of a button
that would POST
course_key: undefinedand invalidate cache keys containingundefined. Accepted as the cost of dropping the assertions.No test changes. Neither component has its own test file; both are
covered through
DatesTab.test.jsxandOutlineTab.test.jsx, which staygreen unchanged.
Manual testing
Checklist
Manual testing — suggested-schedule alerts to TypeScript (#2091)
In-browser verification against a live backend (tutor local). This layer claims
no user-visible change at all:
ShiftDatesAlertandUpgradeToShiftDatesAlertmove from
.jsxto.tsxand frompropTypesto aPropsinterface, stillreading
useModel(model, courseId)and rendering the same markup. So the test isthat both banners still appear under the same conditions and their buttons still
do the same thing.
Nothing here exercises the
!courseIdguard added toShiftDatesAlert— it isunreachable in this layer (
useModelreturns{}for a missing id, so thedatesBannerInfodestructure above it throws first), and both components onlyrender under
/course/:courseId/....Setup — producing a missed deadline (tutor local)
Both banners are driven by Personalized Learner Schedules. The suggested
schedule is not a field you set: it is derived from the learner's
Schedule.start_dateplus relative due dates on the course's gradedsubsections. So the lever is "make relative dates exist, then backdate the
schedule". These steps were used for the dates-tab (#1984) and outline-tab
(#1991) passes and are known to work:
Self-paced course with a graded subsection — Studio → Settings →
Schedule & Details → Course Pacing → Self-Paced, and at least one subsection
set to Grade as Homework. PLS only schedules graded subsections. The
course used previously was
course-v1:OpenedX+PD1+PD1_1astutorsuper;DemoX on its own was not sufficient, because a graded subsection still
has no relative due date until one is written to edx-when (step 5).
Relative dates flag on — LMS admin
/admin/waffle/flag/, createcourse_experience.relative_dateswith Everyone = Yes (per-coursealternative:
/admin/waffle_utils/waffleflagcourseoverridemodel/, overridechoice On). Definitive check, in
tutor local exec lms ./manage.py lms shell:Course end date in the future —
hasEndedsuppresses both banners.Enrollment mode picks which banner you get — verified, or a course with
content-type gating off, gives the plain "Shift due dates"; audit plus gated
content the learner missed gives "Upgrade to shift due dates".
Arm it — one LMS-shell script gives the graded subsection a relative due
date and backdates the schedule so that date is in the past:
tutor dev run lms ./manage.py lms shell # or `tutor local ...`, per your setupGet the
Schedulerow matching both the username and this exact courseid — editing the wrong one is the easy mistake, and the dates payload's
enrollment-date block tells you whether it took.
Do not re-publish the course afterwards. A publish re-syncs edx-when from
the block's own fields, which have no due date, wiping the relative date.
Re-arm between runs — clicking "Shift due dates" resets
start_datetonow, which is the feature working, so the banner disappears and you cannot
immediately retest. The edx-when date persists, so re-arming is just the
backdate:
tutor dev run lms ./manage.py lms shell -c "from datetime import timedelta; from django.utils import timezone; from opaque_keys.edx.keys import CourseKey; from openedx.core.djangoapps.schedules.models import Schedule; ck = CourseKey.from_string('course-v1:OpenedX+PD1+PD1_1'); s = Schedule.objects.get(enrollment__user__username='tutorsuper', enrollment__course_id=ck); s.start_date = timezone.now() - timedelta(days=30); s.save()"The backend is shared, so one re-arm covers whichever frontend you point at —
useful for taking a master baseline and then re-checking on the branch.
Read it off the API, not the screen: DevTools → Network →
/api/course_home/dates/<courseId>→dates_banner_info(
missed_deadlines,missed_gated_content,content_type_gating_enabled). Ifcourse_date_blocksholds only an "Enrollment Date" entry, the course has nograded assignments and there is nothing to miss — backdating won't help.
The two banners are mutually exclusive on
missedGatedContent:ShiftDatesAlert("shift dates")missedDeadlines && !missedGatedContent && !hasEndedUpgradeToShiftDatesAlert("upgrade to shift")contentTypeGatingEnabled && missedDeadlines && missedGatedContent && verifiedUpgradeLink && !hasEndedSo the shift-dates banner needs an enrolled learner past a due date in a
self-paced course; the upgrade variant additionally needs content-type gating,
a verified mode with an upgrade link, and gated content the learner has missed.
Both render on two pages — the outline tab (
/home) and the dates tab(
/dates) — which is the reason these components took amodelprop at all, soeach check is worth doing on both.
nvm use && npm run dev, then sign in as the enrolled learner.Verify by hand
missed deadline: the warning alert appears above the timeline with the
"missed deadlines" heading and a "Shift dates" button.
/home:same alert, above the start/resume card.
appears and the due dates have moved forward.
/api/course_experience/v1/reset_course_deadlinescarriesresearch_event_data.location=dates-tabfrom the dates tab andoutline-tabfrom the outline tab, the one thing themodelprop stillfeeds. Not checked by hand; see Results.
with missed gated content), on both
/homeand/dates.(
edx.bi.ecommerce.upsell_links_clicked,linkName=course_home_upgrade_shift_datesfrom the outline tab,dates_upgradefrom the dates tab) and the browser navigates to theverified upgrade link.
self-paced course with no missed deadlines: no banner on either page.
Results
Env: tutor dev,
course-v1:OpenedX+PD1+PD1_1, usertutorsuper, branchbsmith/suggested-schedule-alerts-typescript@d221640d— the TypeScript layeron its own, with #2083 not yet applied, so both alerts were still reading
useModel(model, courseId).Five of the eight checks passed, matching the behaviour seen before the
conversion. No divergence found. On the reset itself, what was observed is the
toast appearing and the dates having moved forward — confirmed by navigating
between pages and refreshing, not by watching the request.
The per-tab research event was not checked by hand. It is covered
automatically:
apiHooks.test.tsxasserts the exact request body,{"course_key":"course-1","research_event_data":{"location":"dates-tab"}}, sothe
model→location: '<model>-tab'mapping is pinned by a test. What thattest does not cover is each tab passing the right literal, and those are
unchanged by this layer —
model="dates"inDatesTab.jsxandmodel="outline"in
OutlineTab.jsx, both visible in the diff as untouched.The two upgrade-to-shift checks were not run: that variant needs an audit
learner with missed gated content, and PD1 reports
content_type_gating_enabled: false, somissed_gated_contentnever becomestrue there and the banner cannot render.
UpgradeToShiftDatesAlert's conversionrests on its rendering being exercised by
DatesTab.test.jsx/OutlineTab.test.jsxand on the layer being structural — same markup, samegate, only
propTypes→Props.Not applicable to this layer: the in-unit banner (
vert_module.html, untouchedby this work). The "Shift due dates" CTA rendered under a past-due problem is
broken upstream and does nothing on any branch — openedx/xblocks-core#305.
🤖 Generated with Claude Code