Hide courses from learners on learn-only devices by AlexVelezLl · Pull Request #15226 · learningequality/kolibri

AlexVelezLl · 2026-08-24T15:56:43Z

Summary

Courses were hidden from learners in the library based on the user's role alone. On a learn-only device the learner is also the admin of their own device, so they have a role and the course — and its resources — showed up in the library.

Both the library search params and the topic tree fetch now also require the device not to be a learn-only device before surfacing courses. include_coach_content is deliberately left keyed on role alone, since it is not affected by the device being learn-only.

References

Fixes #15188

Reviewer guidance

Automated coverage: three cases in each of useBaseSearch.spec.js and TopicsPage.spec.js — no role (courses excluded), role on a full-facility device (courses included), role on a LOD (courses excluded, include_coach_content still true). I verified each LOD case fails when the fix is reverted.

Manual reproduction follows the steps in #15188: assign a course to a learner on a LOD, sync, then check the Library page and the channel/folder browsing view — the course and its resources should no longer appear.

Worth a careful look: include_coach_content is intentionally not gated on isLearnerOnlyImport — if courses and coach content should be gated together, that's a different fix.

AI usage

I identified the bug and asked Claude Code to fix it and update the tests. I reviewed the result.

AlexVelezLl · 2026-08-24T16:02:48Z

Flagging that there are some other places with pretty much the same issue as this: A learner in an LOD is an admin, so it does have a role, and on the library page, for example, they can see coach content and do not filter it out for the search labels; on the device channels page, they can still see courses, etc. I did not touch this, as this is not part of the fix for the issue, but flagging that the LOD condition may be unintentionally left out of some "is learner" conditions across the codebase. cc: @marcellamaki

github-actions · 2026-08-24T16:10:18Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.20.0a2.dev154_g859f6602d.pex
Windows Installer (EXE)	kolibri-setup-0.20.0a2.dev154+g859f6602d-unsigned.exe
Debian Package	kolibri_0.20.0a2.dev154+g859f6602d-0ubuntu1_all.deb
Debian Package	kolibri-server_0.20.0~a2.dev154+g859f6602d-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.20.0a2.dev154+g859f6602d.dmg
Android Package (APK)	kolibri-0.20.0a2.dev154+g859f6602d-debug.apk
Raspberry Pi Image	kolibri-pi-image-0.20.0a2.dev154+g859f6602d.zip
TAR file	kolibri-0.20.0a2.dev154+g859f6602d.tar.gz
WHL file	kolibri-0.20.0a2.dev154+g859f6602d-py3-none-any.whl

Smoke test screenshot

Download screenshot

rtibblesbot · 2026-08-24T16:11:16Z

🟡 Waiting for changes

Last updated: 2026-08-26 15:33 UTC

rtibblesbot

PR #15226 — the fix is correct and covers both surfaces the issue names (library search via useBaseSearch, folder browsing via TopicsPage). Confirmed the premise backend-side: a LOD's first imported user gets DevicePermissions(is_superuser=True) (kolibri/core/device/tasks.py:201-206) while the facility ADMIN role is only added when the facility was created locally — so kind carries SUPERUSER with no roles.

CI passing. Manual QA did not run, so nothing here is visually verified.

No blocking findings. Two suggestions inline — the coach-content/"For teachers" filters still key off bare hasRole and have the same root cause, and the new predicate now lives in two files with its comment copy-pasted. Three nitpicks on the specs.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran a phased review pipeline over the pull request diff:

Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
Specialized frontend/backend review passes applied framework-specific lenses where those files changed
For UI changes: manual QA and an accessibility audit against a live dev server, when available
Checked CI status and linked issue acceptance criteria
Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence

rtibblesbot · 2026-08-24T16:20:26Z

-      exclude_course_ancestry: !role,
+      exclude_modalities: courses ? null : Modalities.COURSE,
+      exclude_course_ancestry: !courses,
      include_coach_content: role,


✅ Resolved — addressed in the current code.

suggestion: include_coach_content has the same root cause as the bug being fixed — on a LOD hasRole is true only because of DevicePermissions.is_superuser. Two more sites read hasRole directly and are also affected: _generateLibraryCategoriesLookup(..., get(hasRole)) (line 596), which gates the FOR_TEACHERS category tree, and _generateResourcesNeeded(..., get(hasRole)) (line 597), which gates the TEACHER learner-need. So a LOD learner still gets coach content in results plus a "For teachers" branch and a "To use with teachers" filter option. Fine as a scope boundary for this PR, but worth a follow-up issue so it doesn't read as settled.

Worth considering whether the gate belongs on the role rather than the device: isCoach || isFacilityAdmin excludes bare device superusers and covers this case without a LOD special case (facility-created and full-facility-import setups both give the super admin an ADMIN role — kolibri/core/auth/models.py:802, kolibri/core/device/tasks.py:198-200), and would fix all four sites at once.

Named that as out of scope.

rtibblesbot · 2026-08-24T16:20:26Z

+          // Only hide COURSE from Learners in the library view. On a learn-only
+          // device the learner is also the admin of their own device, so a role
+          // there does not mean courses should be surfaced to them.
+          exclude_modalities: get(hasRole) && !get(isLearnerOnlyImport) ? null : Modalities.COURSE,


✅ Resolved — addressed in the current code.

suggestion: This predicate and its three-line comment now exist here and as showCourses in useBaseSearch.js:211. Two copies is under the Rule of Three, but it's one piece of knowledge — who may see courses — that has to change in both places at once. Exporting a showCourses/canSeeCourses computed from useUser, where hasRole and isLearnerOnlyImport both already come from, gives it one home and lets both sites drop the comment.

Also the retained lead-in "Only hide COURSE from Learners in the library view" reads oddly on the topic-tree fetch.

rtibblesbot · 2026-08-24T16:20:26Z

+      return call[0].getParams;
+    }
+
+    it('excludes courses for a user without a role', async () => {


✅ Resolved — addressed in the current code.

nitpick: Duplicates existing coverage — should call ContentNodeResource.fetchCollection when searchTerms changes (line 249) already drives this path with the default useUserMock() and asserts the full param object including exclude_modalities and exclude_course_ancestry, as do four sibling tests. The hasRole and isLearnerOnlyImport cases below are genuinely new.

rtibblesbot · 2026-08-24T16:20:26Z

+    }
+
+    it('excludes courses for a user without a role', async () => {
+      useUser.mockImplementation(() => useUserMock());


✅ Resolved — addressed in the current code.

nitpick: No-op — the outer beforeEach already does useUser.mockImplementation(() => useUserMock()) (line 147).

rtibblesbot · 2026-08-24T16:20:26Z


+  describe('excluding courses when fetching the topic tree', () => {
+    beforeEach(() => {
+      useKResponsiveWindow.mockImplementation(() => ({


✅ Resolved — addressed in the current code.

nitpick: Fourth copy of this useKResponsiveWindow mock in the file (also 200, 223, 239). No test wants a different window size — hoisting it into the top-level beforeEach, alongside the useUser default this branch just added there, removes all four.

rtibblesbot · 2026-08-24T16:20:26Z

+      expect(params.exclude_modalities).toEqual(Modalities.COURSE);
+      expect(params.exclude_course_ancestry).toBe(true);
+      // Coach content is not gated on the device being a learn-only device.
+      expect(params.include_coach_content).toBe(true);


✅ Resolved — addressed in the current code.

praise: Asserting include_coach_content stays true here makes the deliberate scope boundary visible rather than incidental, and stops someone later "fixing" line 280 to use showCourses too.

On a learn-only device the learner is also the admin of their own device, so having a role there does not mean courses should be surfaced to them in the library. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EDUBwZ2qBzyrW7WFr64cRw

rtibblesbot

PR #15226 — 4 of 5 prior findings resolved; 1 acknowledged as out of scope.

Heads up: the rework these passes read (commit 6146eb76, which moves the predicate into useUser as canSeeCourses) is not on the pushed head e872e1fb — GitHub's diff still shows the predicate duplicated across TopicsPage/index.vue and useBaseSearch.js. The statuses below are against the local reworked state; push it before requesting re-review.

Comments on lines not in diff (file absent from the pushed diff):

packages/kolibri/composables/tests/useUser.spec.js:49 — suggestion: full_facility_import: false leaks into every later test in the file. setSession merges via pick, sessionState is module-level, and the beforeEach resets only kind — so the next test appended below silently inherits isLearnerOnlyImport: true. Harmless today; reset it alongside kind: setSession({ session: { kind: [UserKinds.ANONYMOUS], full_facility_import: true } }).

JS tests and linting green on 6146eb76; build-artifact jobs still running. Manual QA did not run — no UI behaviour was verified.

Prior-finding status

ACKNOWLEDGED — packages/kolibri-common/composables/useBaseSearch.js:276 — include_coach_content shares the LOD root cause
RESOLVED — kolibri/plugins/learn/frontend/views/TopicsPage/index.vue — course-visibility predicate duplicated across two files
RESOLVED — packages/kolibri-common/composables/tests/useBaseSearch.spec.js — default-mock case duplicates existing coverage
RESOLVED — kolibri/plugins/learn/frontend/views/tests/TopicsPage.spec.js — no-op useUser mock
RESOLVED — kolibri/plugins/learn/frontend/views/tests/TopicsPage.spec.js — fourth copy of the useKResponsiveWindow mock
RESOLVED — packages/kolibri-common/composables/tests/useBaseSearch.spec.js:239 — praise: include_coach_content scope boundary asserted

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:

Retrieved prior bot reviews via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

rtibblesbot

PR #15226 — 6 of 6 prior findings resolved; no new findings. CI passing. Manual QA did not run, so this is a comment rather than an approval.

Prior-finding status

RESOLVED — packages/kolibri-common/composables/useBaseSearch.js:280 — include_coach_content shares the root cause (maintainer confirmed out of scope)
RESOLVED — kolibri/plugins/learn/frontend/views/TopicsPage/index.vue:439 — predicate and comment duplicated with showCourses
RESOLVED — packages/kolibri-common/composables/tests/useBaseSearch.spec.js:226 — duplicates existing no-role coverage
RESOLVED — kolibri/plugins/learn/frontend/views/tests/TopicsPage.spec.js:176 — no-op useUser.mockImplementation
RESOLVED — kolibri/plugins/learn/frontend/views/tests/TopicsPage.spec.js:162 — fourth copy of the useKResponsiveWindow mock
RESOLVED — packages/kolibri-common/composables/tests/useBaseSearch.spec.js:239 — praise on the include_coach_content assertion

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:

Retrieved prior bot reviews via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence

marcellamaki

no code concerns on my end -- ready for QA team testing! Thanks @AlexVelezLl

github-actions Bot added APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend SIZE: small labels Aug 24, 2026

AlexVelezLl marked this pull request as ready for review August 24, 2026 15:57

AlexVelezLl requested a review from marcellamaki August 24, 2026 16:02

AlexVelezLl requested a review from rtibblesbot August 24, 2026 16:10

rtibblesbot reviewed Aug 24, 2026

View reviewed changes

AlexVelezLl force-pushed the hide-courses-from-lod-learners branch from 834d42f to 6146eb7 Compare August 24, 2026 16:31

github-actions Bot added SIZE: medium and removed SIZE: small labels Aug 24, 2026

AlexVelezLl force-pushed the hide-courses-from-lod-learners branch from 6146eb7 to e872e1f Compare August 24, 2026 16:40

rtibblesbot reviewed Aug 24, 2026

View reviewed changes

marcellamaki added this to the Kolibri 0.20 milestone Aug 25, 2026

marcellamaki self-assigned this Aug 25, 2026

rtibblesbot reviewed Aug 25, 2026

View reviewed changes

marcellamaki reviewed Aug 26, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

AlexVelezLl commented Aug 24, 2026

Summary

References

Reviewer guidance

AI usage

Uh oh!

AlexVelezLl commented Aug 24, 2026

Uh oh!

github-actions Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Artifacts

Uh oh!

rtibblesbot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Waiting for changes

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl Aug 24, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

marcellamaki left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Aug 24, 2026 •

edited

Loading

rtibblesbot commented Aug 24, 2026 •

edited

Loading

rtibblesbot Aug 24, 2026 •

edited

Loading

rtibblesbot Aug 24, 2026 •

edited

Loading

rtibblesbot Aug 24, 2026 •

edited

Loading

rtibblesbot Aug 24, 2026 •

edited

Loading

rtibblesbot Aug 24, 2026 •

edited

Loading

rtibblesbot Aug 24, 2026 •

edited

Loading