Repository navigation
refactor!: read the access-expiration masquerade banner from the tab queries, not useModel(tab) - #2153
Merged
brian-smith-tcril merged 1 commit intoOct 5, 2026
Conversation
brian-smith-tcril
added this pull request to stack #2141
October 2, 2026 10:47
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2153 +/- ##
=======================================
Coverage 95.59% 95.59%
=======================================
Files 374 374
Lines 6081 6087 +6
Branches 1451 1500 +49
=======================================
+ Hits 5813 5819 +6
Misses 258 258
Partials 10 10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
brian-smith-tcril
force-pushed
the
bsmith/access-expiration-banner-query-read
branch
from
October 2, 2026 12:27
a644af0 to
f7641e7
Compare
brian-smith-tcril
marked this pull request as ready for review
October 2, 2026 12:46
This was referenced Oct 2, 2026
brian-smith-tcril
force-pushed
the
bsmith/access-expiration-banner-query-read
branch
from
October 5, 2026 17:33
f7641e7 to
3c2be4c
Compare
…queries, not useModel(tab)
`useAccessExpirationMasqueradeBanner(courseId, tab)` reads the outline and
progress queries with `{ enabled: false }` and picks `accessExpiration` from
a lookup keyed by `tab`, in place of `useModel(tab, courseId)`, the last
`useModel` reader in the app. Only those two course-home payloads carry
`access_expiration` (openedx-platform's `OutlineTabSerializer` and
`ProgressTabSerializer`; the dates serializer has no such field), so they
are the only two pages the banner has ever rendered on; every other slug
yields `undefined` where `useModel` yielded `{}`. `targetUserId` for the
progress key comes from `useParams()`, as it does in `ProgressTab`, so the
read matches the owner's key on `/progress/:targetUserId` and is `undefined`
elsewhere. `CourseHomeProgress` gains `accessExpiration`.
With the reader gone, `useDatesTabData`, `useOutlineTabData` and
`useProgressTabData` stop mirroring their results into the model store;
the progress hook keeps `meta: { logStatusAs: { 404: 'silent' } }`. No
query opts into the bridge any more, which leaves #1977 as deletion only.
No behaviour or request change: the store entries were these queries'
results, written by the bridge before observers re-rendered, and the two
new observers sit on keys the page either owns or never fetches. The
courseware metadata carries the same `masquerading_expired_course` flag,
so a uniform read from `TabPage`'s `courseStatus` would have shown the
banner in courseware for the first time; the tab-keyed lookup keeps it off
there, pinned by an `InstructorToolbar` case.
Tests: the owners' existing banner cases run bridge-free unchanged; added a
progress case on `/progress/10/`, a staff variant of each owner's
once-per-load request count, and two seeded `InstructorToolbar` cases.
Closes #1999. Part of #1946 (layer E of #1977).
BREAKING CHANGE: nothing writes the `dates`, `outline` or `progress` models
into the model store any more, so `useModel('dates', courseId)`,
`useModel('outline', courseId)` and `useModel('progress', courseId)` return
`{}`. Read the tab data from `useDatesTabData(courseId, { enabled: false }).data`,
`useOutlineTabData(courseId, { enabled: false }).data` and
`useProgressTabData(courseId, targetUserId, { enabled: false }).data`.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
brian-smith-tcril
force-pushed
the
bsmith/access-expiration-banner-query-read
branch
from
October 5, 2026 17:39
3c2be4c to
f990b59
Compare
brian-smith-tcril
deleted the
bsmith/access-expiration-banner-query-read
branch
October 5, 2026 17:47
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
useAccessExpirationMasqueradeBanner(courseId, tab)reads the outline and progress queries with{ enabled: false }and picksaccessExpirationfrom a lookup keyed bytab, in place ofuseModel(tab, courseId), the lastuseModelreader in the app. Only those two course-home payloads carryaccess_expiration(openedx-platform'sOutlineTabSerializerandProgressTabSerializer; the dates serializer has no such field), so they are the only two pages the banner has ever rendered on, and every other slug yieldsundefinedwhereuseModelyielded{}.targetUserIdfor the progress key comes fromuseParams(), as inProgressTab. With the reader gone, the three tab-data hooks stop mirroring into the model store, so no query opts into the bridge any more and #1977 becomes deletion only. No behaviour or request change. Part of the Redux → React Query migration (#1946, Stage 1), layer E of #1977; stacked above #2152, the TypeScript peel that made this a two-line read swap. Closes #1999.What changed
alerts/access-expiration-alert/hooks.ts(decisions 1–2, 8):useOutlineTabData(courseId, { enabled: false })anduseProgressTabData(courseId, targetUserId, { enabled: false }), a two-entry lookup keyed bytab,targetUserIdfromuseParams(); theuseModelimport goes.course-home/data/apiHooks.ts(decision 5): themetabridge entries onuseDatesTabDataanduseOutlineTabDataand theirTransitional (#1999)comments removed;useProgressTabDatakeepsmeta: { logStatusAs: { 404: 'silent' } }.course-home/data/api.ts(decision 6):CourseHomeProgress.accessExpiration?: CourseHomeAccessExpiration | null, the type imported fromcourseHomeOutline.ts.InstructorToolbar,LoadedTabPage,TabPage(decision 1), the model store, the bridge,queryClient.ts,store.ts(all Dissolve the model-store normalized cache #1977).Testing
npm run typesandnpm run lintclean; full suite 122 suites, 1200 passed, 0 skipped.git grep useModelover non-test source outside the store and bridge is empty;git grep "modelType:"finds onlysetupTest.js's seeds. The owners' existing banner cases (OutlineTab.test.jsx› Access Expiration Alert,ProgressTab.test.jsx› Access expiration masquerade banner) now run axios → query → hook with no bridge between and needed no edit. Added (decision 7): a progress banner case on/progress/10/; a staff variant of each owner's once-per-load request count (outline once and progress never on the outline page, progress once and outline never on the progress page); and twoInstructorToolbarcases on a seeded query client, the banner from the outline query ontab="outline"and no banner ontab="courseware"even with the flag seeded in both the courseware metadata and the outline. TwouseModelcases inmodel-store/hooks.test.tsx(decision 9) keep the hook covered now that nothing in the app calls it, as #2140 did foruseModels.Decisions
Full decision log
Decisions — source the access-expiration masquerade banner from the tab queries, not
useModel(tab)(#1999)Layer E of the model-store dissolution (#1977), the last
useModelreader.Layer of the running stack #2141 above #2150 (#2130). Settled with the user
2026-10-02.
The hook reads the owner queries itself, selected by tab.
useAccessExpirationMasqueradeBanner(courseId, tab)readsuseOutlineTabData(courseId, { enabled: false }).dataanduseProgressTabData(courseId, targetUserId, { enabled: false }).dataandpicks
accessExpirationfrom a lookup keyed bytabwith two entries,outlineandprogress. Those are the only two course-home payloads thatcarry
access_expiration(openedx-platformOutlineTabSerializerandProgressTabSerializer; the dates serializer has no such field), so theyare the only two pages the banner has ever rendered on. Every other slug
(
dates,discussion,lti_live,courseware) yieldsundefinedwhereuseModelyielded{}; no page changes. This continues the shared-hookreader pattern of
useEnrollmentAlert/useLogistrationAlert(Read the dates and outline tab data from their queries, not useModel #2083) andthe per-tab gating of the sibling
useCourseStartMasqueradeBanner.Threading the value down from
TabPage'scourseStatuswas rejected: thecourseware page's tab-data query (the courseware metadata) carries the same
masquerading_expired_courseflag, so a uniform read would show the bannerin courseware for the first time,
CourseExitpasses two tab-data querieswith no single answer, and the hook would stop sourcing the data it is
named for. Moving
access_expirationonto the course metadata endpoint isa backend change outside Stage 1.
InstructorToolbar,LoadedTabPageandTabPageare untouched.targetUserIdcomes fromuseParams()inside the hook. The progressquery is keyed on
(courseId, targetUserId), andProgressTab.jsxtakesboth from
useParams(), so reading the same param in the hook makes theenabled: falsekey match the owner's by construction: the learner's id on/progress/:targetUserId,undefinedeverywhere else, which is also whatthe progress tab passes on its own route.
useContextId(
src/data/hooks.ts, Tear down the courseware Redux slice + replace useContextId #1976) is the existing shared data hook that reads aroute param this way. react-router 6's
useParamsreturns{}when noroute matched, so renders without a router (the bare toolbar tests) keep
working. Threading the id as a prop through
TabPage,LoadedTabPageandInstructorToolbarwas rejected: three component edits to carry a valuethe hook can read itself.
The TypeScript conversion of
alerts/access-expiration-alertis its ownpeel layer below this one, not part of it. The peel renames
hooks.js/index.jsto.ts, typescourseIdandtab, and types the reads asthey are today, with no runtime change and the existing tests untouched,
the shape Convert Unit, UnitSuspense, UnitTitleSlot and BookmarkButton to TypeScript #2125 used below the
unitsread conversion (Read units and sequences from the courseware queries, not useModel #2088). This layerthen shows only the read swap and the
metaremovals on an already typedfile, instead of a rename-plus-rewrite that buries one behavioural change
in a whole-file diff and carries a
refactor!:footer beside an unrelatedrename. Converting inside this layer (the Clean up SidebarContextProvider for React Query and convert it to TypeScript #2111 shape) was considered and
passed over for that reason. Stack order: refactor: type the course-home API module on the functions that produce its results #2150 ← peel ← Source the access-expiration masquerade banner from the tab query, not useModel(tab) #1999.
useAccessExpirationAlertandAccessExpirationAlert.jsxstay. Thelearner-facing alert in the same folder has had no in-repo caller since
feat: Remove upsell banner on course home #499 (2021-06) and REV-2132 (2021-07), and feat: add page banner to masquerade (AA-877) #606 (2021-08) left it behind
when it replaced the masquerade alert with the toolbar banner. Deleting it
in the peel was proposed and rejected: the folder's
indexexports it,plugins bundled into the app can import host modules through
@src, andthe only remedy for a plugin that did would be to copy the hook, the
component and four message ids into itself. Every earlier removal footer
in this migration (refactor!: read sequences from the courseware queries, not useModel #2134, refactor!: read sections and coursewareMeta from the courseware queries, not useModel #2140, refactor!: read the redirects and container from the courseware queries, not the model store #2144) asked for a one-line read swap;
that ask is of a different order and is not the migration's to make. The
peel types the hook's six parameters and leaves the component
.jsx,like
AccessExpirationMasqueradeBanner.jsx. Removal, if wanted, is astandalone cleanup proposal with its own notice, outside Convert Learning from redux to Context + react-query #1946.
The three bridge
metaentries go; the progress hook keepslogStatusAs.useDatesTabDataanduseOutlineTabDatalose theirmetaand theTransitional (#1999)comments;useProgressTabData'sbecomes
meta: { logStatusAs: { 404: 'silent' } }, since that key isqueryClient.ts's error-logging switch (refactor: convert the courseware metadata fetch to React Query #2023), not the bridge's. Withthem gone no query opts into the bridge:
git grep "modelType:"overnon-test source finds only
setupTest.js's seeds, which F removes alongwith the bridge, the
onSuccesswiring andqueryClient.test.ts's onebridge case.
CourseHomeProgress.accessExpiration?: CourseHomeAccessExpiration | null.The progress serializer sends the same dict the outline does, so the type
is imported from
courseHomeOutline.tsrather than redeclared or moved.Optional, like every other field of that interface (its 401/403 branches
return
{}); the outline's stays required, matching its interface.Four test cases, plus the owners' existing ones unchanged. The outline
and progress suites' Access Expiration cases now run axios → query →
hook with no bridge between and needed no edit; they remain the proof of
the real path. Added: a progress case on
/progress/10/, so the hook'suseParams()key matches the owner's when the param is present; a staffvariant of each owner's once-per-load request count (outline requested
once and progress never on the outline page; progress once and outline
never on the progress page), since the non-staff counts never mounted the
toolbar's two
enabled: falseobservers; and twoInstructorToolbarcases on a seeded query client, the banner from the outline query on
tab="outline", and no banner ontab="courseware"even with the flagseeded in both the courseware metadata and the outline, which pins the
finding that a
courseStatusread would have changed behaviour.refactor!:with the refactor!: read the redirects and container from the courseware queries, not the model store #2144-shaped footer; no comment on the lookup.The
dates,outlineandprogressmodels stop being written, so aplugin
useModelof any of them returns{}; the footer names the threequery hooks to read instead. Two drafts of a comment on the lookup (which
payloads carry
access_expiration, and that the courseware page has nevershown the banner) were cut by the user: the two keys say which tabs, and
the reason there are two is this log's (entry 1), not the code's.
Two
useModelcases for codecov's project check. Every patch line wascovered, but project coverage dipped: with its last app reader gone,
useModel's selector ingeneric/model-store/hooks.jsis reached by notest, two lines and a branch. refactor!: read sections and coursewareMeta from the courseware queries, not useModel #2140 met the same thing for
useModelsandadded
model-store/hooks.test.tsx; this layer adds auseModeldescribeto that file in the same shape (a model in the store,
{}for a missingid and for a missing type). The hook is deleted in Dissolve the model-store normalized cache #1977 with the rest of
the store, so the cases live for one layer.
Manual testing
Manual testing — source the access-expiration masquerade banner from the tab queries (#1999)
In-browser verification against a live backend (tutor local). PR #2153. Run by
the user 2026-10-02 with the three temporary fetcher edits below, then reverted.
What changed:
useAccessExpirationMasqueradeBannerreads the outline andprogress queries (non-fetching, keyed by the toolbar's
tab) instead of theuseModel(tab, courseId)mirror, and the three tab-data queries stop writingthe model store. The banner should render exactly where it did (Course and
Progress tabs, while masquerading as a learner whose access has expired) and
nowhere else; no request count changes.
The bugs this layer could introduce.
key now includes
targetUserIdfrom the route; a mismatch with the owner'skey reads an empty cache entry and hides the banner on
/progress/<id>.same flag; the tab-keyed lookup must keep ignoring it.
enabled: falseobservers mount with thetoolbar on every tab; neither may fetch.
Setup
The banner needs
access_expiration.masquerading_expired_course: truein theoutline and progress payloads. Producing that for real is the platform's FBE
access-duration feature (config row, verified mode, backdated audit enrollment,
specific-student masquerade; appendix below), which this layer does not touch.
Fake it in the fetchers instead, after the real request so the Network
counts stay real; remove all three edits afterwards (
git diffempty).src/course-home/data/api.ts,getOutlineTabData: afterconst { data, headers } = tabData;add
data.access_expiration = expiredAccess;.src/course-home/data/api.ts,getProgressTabData: beforeconst camelCasedData = camelCaseObject(data);add
data.access_expiration = expiredAccess;.src/courseware/data/api.js,getCourseMetadata: beforereturn normalizeCoursewareMeta(metadata);add
metadata.data.access_expiration = expiredAccess;(for the "must not render in courseware" check).Then:
needed: the flag is in the payload.
course_home|courseware/course, Console open.Checks
Where the banner renders
/progress/<any learner id>/: same banner (the new keyed read; the progress request URL changes, and the hook must read that entry).Where it must not render
Request count (unchanged)
course_home/outline/<id>1,course_home/progress/0.course_home/progress/<id>1,course_home/outline/0.course_home/dates/<id>1, no outline or progress request.Console
Appendix: producing the flag for real
Only if a real-data pass is ever wanted.
get_user_course_expiration_date(
openedx/features/course_duration_limits/access.py) returns a date when allhold:
CourseDurationLimitConfigenabled with Enabled as of before theenrollment (
/admin/course_duration_limits/coursedurationlimitconfig/); averifiedmode on the course (/admin/course_modes/coursemode/, expireddeadline fine); the learner enrolled as
audit; andmax(enrollment.created, course.start)plus the expected duration in the past, so backdateCourseEnrollment.createda year in the LMS shell and the course start inStudio.
masquerading_expired_courseis true only for a specific-studentmasquerade (
is_masquerading_as_specific_student), not "Learner". Diagnosefrom
/api/course_home/outline/<courseId>as the learner.🤖 Generated with Claude Code