| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ 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.
|
Sorry, something went wrong.
`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>
There was a problem hiding this comment.
👍🏼
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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 a Props interface instead of growing propTypes that 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
Testing
npm run types, npm run lint and 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 on apiHooks.test.tsx, which asserts the exact request body.
Decisions
Working notes for this layer, kept out of the tree:
Full decision logDecisions — 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 propTypes entries
TypeScript 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.jsx keeps the props it has today — Read the dates and outline tab data from their queries, not useModel #2083 only
changes where it reads its data — so it stays .jsx. Converting the whole
directory for symmetry would be scope the conversion doesn't need.
Props interface with destructuring defaults, not React.FC +
defaultProps. Matches UnitButton.tsx (refactor: de-class UnitButton and convert it to TypeScript #2065), the epic's most recent
TypeScript conversion. Section.tsx and SequenceLink.tsx use
React.FC<Props>, but that predates this effort and defaultProps on
function components is on its way out.
courseId is narrowed, not asserted. ShiftDatesAlert feeds it to
courseHomeQueryKeys.datesTab/outlineTab and to the reset-deadlines
mutation, all typed string, while useParams() yields
string | undefined. Adding !courseId to the early return the component
already has narrows it for the whole body below, including the
refreshTabData and onClick closures, so all three courseId! go away.
The repo's other courseId! uses all sit in apiHooks.ts next to
enabled: !!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, useModel returns {} and the datesBannerInfo destructure
above the guard throws first, so !courseId cannot fire in this layer. It
becomes 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: undefined and invalidate cache keys containing
undefined. Accepted as the cost of dropping the assertions.
No test changes. Neither component has its own test file; both are
covered through DatesTab.test.jsx and OutlineTab.test.jsx, which stay
green unchanged.
Manual testing
ChecklistManual 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: ShiftDatesAlert and UpgradeToShiftDatesAlert
move from .jsx to .tsx and from propTypes to a Props interface, still
reading useModel(model, courseId) and rendering the same markup. So the test is
that both banners still appear under the same conditions and their buttons still
do the same thing.
Nothing here exercises the !courseId guard added to ShiftDatesAlert — it is
unreachable in this layer (useModel returns {} for a missing id, so the
datesBannerInfo destructure above it throws first), and both components only
render 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_date plus relative due dates on the course's graded
subsections. 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_1 as tutorsuper;
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/, create
course_experience.relative_dates with Everyone = Yes (per-course
alternative: /admin/waffle_utils/waffleflagcourseoverridemodel/, override
choice On). Definitive check, in tutor local exec lms ./manage.py lms shell:
Course end date in the future — hasEnded suppresses 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 Schedule row matching both the username and this exact course
id — 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_date to
now, 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). If
course_date_blocks holds only an "Enrollment Date" entry, the course has no
graded assignments and there is nothing to miss — backdating won't help.
The two banners are mutually exclusive on missedGatedContent:
So 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 a model prop at all, so
each 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.
same alert, above the start/resume card.
appears and the due dates have moved forward.
/api/course_experience/v1/reset_course_deadlines carries
research_event_data.location = dates-tab from the dates tab and
outline-tab from the outline tab, the one thing the model prop still
feeds. Not checked by hand; see Results.
with missed gated content), on both /home and /dates.
(edx.bi.ecommerce.upsell_links_clicked, linkName =
course_home_upgrade_shift_dates from the outline tab,
dates_upgrade from the dates tab) and the browser navigates to the
verified upgrade link.
self-paced course with no missed deadlines: no banner on either page.
Results
Env: tutor dev, course-v1:OpenedX+PD1+PD1_1, user tutorsuper, branch
bsmith/suggested-schedule-alerts-typescript @ d221640d — the TypeScript layer
on 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.tsx asserts the exact request body,
{"course_key":"course-1","research_event_data":{"location":"dates-tab"}}, so
the model → location: '<model>-tab' mapping is pinned by a test. What that
test does not cover is each tab passing the right literal, and those are
unchanged by this layer — model="dates" in DatesTab.jsx and model="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, so missed_gated_content never becomes
true there and the banner cannot render. UpgradeToShiftDatesAlert's conversion
rests on its rendering being exercised by DatesTab.test.jsx /
OutlineTab.test.jsx and on the layer being structural — same markup, same
gate, only propTypes → Props.
Not applicable to this layer: the in-unit banner (vert_module.html, untouched
by 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