Exclude Courses in Search, Resume, Library & plugin_data flag for courses by nucleogenesis · Pull Request #14132 · learningequality/kolibri

nucleogenesis · 2026-02-04T22:02:30Z

Summary

Adds a helper function to content/api.py get_course_ids() that returns a flat list of the IDs for every course on the device - then it caches that data against the ContentCacheKey
Adds exclude_course_ancestry BooleanFilter field to the ContentNodeFilter allowing us to pass a query param exclude_course_ancestry: true to exclude all resources that are within a Course
Uses previously added exclude_modalities filter to filter out the Courses themselves in Search
Exclude all Course descendants from the "recent/resume" recommendations

Potential concerns

The demo channel only has courses so when you open it up it's just an empty channel, but there is no messaging to say as much -- should we a) steal a "No resources available" string or similar and put it in the empty channel page?, b) Hide all channels that exclusively have Courses as direct chidlren from the channels list?, c) other?
I called the plugin_data flag courses_exist for consistency as this is what it is called in the Coach plugin (just noting I didn't call it courses_active like the issue said to).

References

Closes #14108

Reviewer guidance

Code

I'm feeling like I should add a test for the exclude_course_ancestry maybe?
Is any of the other added code worth putting under test?

Performance

It would be good to get a sense of how quickly/slowly the filtering query is when excluding descendants. Open the /profile to see Silk's outputs and do some investigation on how long calls to get recommendations and searches.

QA Testing

You will need to have the courses channel imported and to go onto base release-v0.19.x and interact with some of the courses' descendants (ie, take a couple of the exercises about hummingbirds). This gives your user progress on the course-descended resource so you can see how it's hidden in this PR.

After that, checkout this PR and:

Try searching for the resources that you saw previously and see that you cannot find anything from the courses
See that the resources you made progress on last time aren't shown on the Home or Library pages
In the console do document.querySelector('template[data-plugin="kolibri.plugins.learn.app"]').content.textContent.includes("courses_exist") to see that the key is set
Run JSON.parse(document.querySelector('template[data-plugin="kolibri.plugins.learn.app"]').content.textContent).courses_exist to see the value of it - when you have Course content it should be true, otherwise, false.

kolibri/plugins/learn/frontend/views/TopicsPage/index.vue

github-actions · 2026-02-04T22:38:27Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.19.4.dev0_git.20260320200141.pex
Windows Installer (EXE)	kolibri-0.19.4.dev0+git.20260320200141-windows-setup-unsigned.exe
Debian Package	kolibri_0.19.4.dev0+git.20260320200141-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.19.4.dev0+git.20260320200141.dmg
Android Package (APK)	kolibri-0.19.4.dev0+git.20260320200141-0.1.7-debug.apk
Raspberry Pi Image	kolibri-pi-image-0.19.4.dev0+git.20260320200141.zip
TAR file	kolibri-0.19.4.dev0+git.20260320200141.tar.gz
WHL file	kolibri-0.19.4.dev0+git.20260320200141-py2.py3-none-any.whl

Smoke test screenshot

Download screenshot

rtibbles

Blocking:

Missing exclude_course_ancestry in initial resume fetch and hydration endpoint
Cache bug in get_course_ids when there are no courses on the device
value parameter not checked in filter_exclude_course_ancestry

Suggestions:

Stale docstring on the filter method

This PR review was initially generated by Claude Code and then reviewed, supplemented, and edited by @rtibbles.

rtibbles · 2026-02-06T03:00:33Z

kolibri/plugins/learn/frontend/composables/useLearnerResources.js


 export function setResumableContentNodes(nodes, more = null) {
+  if (more) {
+    more = { ...more, exclude_course_ancestry: true };


blocking: The exclude_course_ancestry: true param is only injected into the more pagination object here, but the initial fetch at line 280 uses { resume: true, max_results: 12, ordering: '-last_interacted' } without it. This means the first page of resume results is fetched without course ancestry exclusion — only subsequent pages get the filter.

This is a workaround because the initial resume call is not getting it properly set on it.

The same issue exists in the server-side hydration endpoint (LearnHomePageHydrationView in viewsets.py ~line 349) which also calls serialize_list with {"resume": True, "max_results": 12, "ordering": "-last_interacted"} and no exclude_course_ancestry.

Suggestion: add exclude_course_ancestry: true to the initial params in fetchResumableContentNodes() at line 280, and to the hydration view's params dict here: https://github.com/learningequality/kolibri/blob/release-v0.19.x/kolibri/plugins/learn/viewsets.py#L356

rtibbles · 2026-02-06T03:00:33Z

kolibri/core/content/api.py

+def get_course_ids():
+    cache_key = "COURSES_IDS_{}".format(get_cache_key())
+    cached_data = cache.get(cache_key)
+    if cached_data:


blocking: if cached_data: is falsy for an empty queryset (no courses on device). This means when there are no courses, the DB query will be re-executed on every call instead of returning the cached empty result, defeating the cache.

Consider using a sentinel pattern or checking is not None:

cached_data = cache.get(cache_key) if cached_data is not None: return cached_data

rtibbles · 2026-02-06T03:00:33Z

kolibri/core/content/api.py

        model = models.ContentNode
        fields = contentnode_filter_fields

+    def filter_exclude_course_ancestry(self, queryset, name, value):


blocking: The value parameter is not checked. Since this is a BooleanFilter, passing exclude_course_ancestry=false will still trigger the exclusion. The filter should be conditional on value:

def filter_exclude_course_ancestry(self, queryset, name, value): if not value: return queryset ...

Currently the filter is always called with true from the frontend, but since this is a public API filter, it should handle false correctly.

kolibri/core/content/api.py

rtibbles · 2026-02-06T03:07:15Z

kolibri/core/content/api.py

            identifier=identifier, baseurl=baseurl, keyword=keyword, language=language
        )
        try:
-


Did this line offend thee?

rtibblesbot

Clean implementation of course exclusion from Library search, channel browsing, and resume recommendations. The two-filter approach (exclude_modalities for course nodes, exclude_course_ancestry for descendants) is well-designed, giving each call site the right level of filtering.

CI passing across all Python versions and builds. No new i18n strings added (appropriate for release-v0.19.x target). UI changes are logic-only (no template/style changes), so no visual verification needed.

suggestion: get_course_ids() caches a queryset — consider materializing to list() for predictable caching behavior
suggestion: Backend test for filter_exclude_course_ancestry (as the author noted)
nitpick: Unrelated filter_exclude_content_ids guard mixed in

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

How was this generated?

Reviewed the pull request diff checking for:

Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
Architecture: duplicated concerns, minimal interfaces, composition over inheritance
Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
Checked CI status and linked issue acceptance criteria
For UI changes: inspected screenshots for layout, visual completeness, and consistency

kolibri/core/content/api.py

packages/kolibri-common/composables/useBaseSearch.js

rtibblesbot · 2026-02-20T21:56:55Z

kolibri/plugins/learn/frontend/composables/useLearnerResources.js


 export function setResumableContentNodes(nodes, more = null) {
+  if (more) {
+    more = { ...more, exclude_course_ancestry: true };


suggestion: Minor: the more object comes from the API and already reflects the server's pagination state. Injecting exclude_course_ancestry: true here ensures it's present for fetchMoreResumableContentNodes, which is correct — but this coupling means the client is responsible for remembering to add the filter to paginated requests. If the server's more response already included this param (since the original request had it), this client-side injection wouldn't be needed. Worth verifying whether the API's more object already carries forward the original query params.

Note this is the same thing I flagged previously here: #14132 (comment) we shouldn't have to inject this here, because it should already have come from the API request.

rtibblesbot

Delta review — 1 prior finding resolved (get_course_ids now materializes to list()).

CI: Morango Integration Tests still in progress; all other checks passing.

Prior unresolved:

[suggestion] Backend test for filter_exclude_course_ancestry — UNRESOLVED (author acknowledged this would be good to add; still missing)
[nitpick] Unrelated filter_exclude_content_ids guard — UNRESOLVED (defensive fix is fine but unrelated to this PR)

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

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

nucleogenesis · 2026-02-26T22:00:09Z

Thanks for the review, taking a look now.

rtibblesbot

Delta review — 2 prior finding(s) resolved (backend tests added; list() materialization already done).

CI passing. No new i18n strings. UI changes are logic-only.

Prior unresolved:

[nitpick] Unrelated filter_exclude_content_ids guard — UNRESOLVED (still mixed in; minor)

New finding:

[suggestion] get_course_ids() missing available=True filter — see inline comment

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

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNRESOLVED, PARTIALLY_ADDRESSED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

rtibblesbot · 2026-02-27T00:03:23Z

kolibri/core/content/api.py

+    if cached_data is not None:
+        return cached_data
+    updated_data = list(
+        ContentNode.objects.filter(modality=modalities.COURSE).values_list(


suggestion: get_course_ids() queries all course nodes regardless of availability, but courses_exist in kolibri_plugin.py:64 filters available=True. If an unavailable course node exists (e.g. partial import/removal), its available descendants would still be excluded from search and resume results — even though the course itself isn't accessible to learners.

Consider adding available=True:

ContentNode.objects.filter(available=True, modality=modalities.COURSE).values_list(...)

This aligns the filtering with the courses_exist flag and avoids excluding descendants when the course itself isn't active on the device.

If an unavailable course node exists (e.g. partial import/removal), its available descendants would still be excluded from search and resume results — even though the course itself isn't accessible to learners.

I reckon this is desired anyhow - am I misunderstanding @rtibbles ?

That's a fair point — if the course was partially imported, you'd still want its descendants excluded from search/resume since they belong to an incomplete course. Withdrawing this suggestion.

It's probably still worth filtering, because a topic is only unavailable if all its descendants are unavailable. So we can filter by available is True because we are generally filtering by available True anyway, so we would still be excluding all its descendants, and gives us one fewer courses to filter against here.

Good point — thanks for clarifying. So the available=True filter is worthwhile as a minor optimization since unavailable topics already imply all descendants are unavailable. Glad the suggestion holds up after all.

Pushed it up thanks Tibbles'

rtibblesbot

Delta review — 3 prior finding(s) resolved.

CI: Build Assets check in progress; prior checks were passing.

Prior unresolved:

[nitpick] Unrelated filter_exclude_content_ids guard — UNADDRESSED (still mixed in; too minor to keep raising)

No new findings. Implementation is clean — the two-filter approach, caching, tests, and consistent application across search/resume/channel browsing all look solid. Approving.

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

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

rtibbles

I think it would be prudent to get some manual QA on this just to be sure!

pcenov · 2026-03-06T12:43:43Z

Hi @nucleogenesis - I confirm everything mentioned in the QA guidance except that the courses which I had assigned to the learner (and had marked some of the resources as completed) are still visible at Home > Recent courses and Home > Classes > class > Your courses. When I click on the course card I am getting a 500 error:

not.excluded.mp4

logs.zip

Also flagging, as it's not completely clear that as a superadmin user I am still seeing the Courses tab and can access it at Coach > Courses but at the same time if I go to the Library and search for the course I am not seeing it. Shouldn't the courses be hidden only for users of type Learner?

nucleogenesis · 2026-03-11T16:26:42Z

@pcenov I've pushed changes that only hides courses from Learners in the Learn library

However:

When I click on the course card I am getting a 500 error:

This might have been due to my needing to rebase the PR to pull in other changes - I tried to replicate the 500 error and the assigned course opened up as expected.

(cc @rtibbles)

pcenov · 2026-03-12T09:08:30Z

Thanks @nucleogenesis - I confirm that a learner who had an assigned course can see the course card in the Recent courses section and can access the course content. Also such a learner can't see any courses at the Library page:

learner.with.progress.mp4

Unfortunately for some reason users of type coach, admin and super admin still can't see the courses at the Library page:

admin.mp4

nucleogenesis · 2026-03-12T17:42:01Z

@pcenov sorry for the oversight - I only fixed the search part 😓

I've pushed an update to also only exclude course content for learners in the Library.

pcenov · 2026-03-13T11:32:24Z

Thanks @nucleogenesis - I confirm that the courses are now visible at the Library page for coaches, admins and super admins and not visible to learners.

Noticed that if "Explore without account" is visible a user who's not signed in can see the courses at the Library. I guess there's no harm in that, right?

explore.mp4

nucleogenesis · 2026-03-17T15:45:02Z

@pcenov I've pushed an update to exclude courses from not-logged-in users as well - not an urgent item to QA but would good to get another quick pass at your convenience.

rtibbles

:just-one-more-thing;

rtibbles · 2026-03-17T16:21:22Z

packages/kolibri-common/composables/useBaseSearch.js


  function createBaseSearchGetParams() {
    const getParams = {
+      exclude_modalities: get(isLearner) ? Modalities.COURSE : null,


This and below are still using isLearner.

rtibbles

Just one question

rtibbles · 2026-03-20T02:18:49Z

packages/kolibri-common/composables/useBaseSearch.js

+        get(isAdmin) || get(isCoach) || get(isSuperuser) ? null : Modalities.COURSE,
+      exclude_course_ancestry: !(get(isAdmin) || get(isCoach) || get(isSuperuser)),
      include_coach_content: get(isAdmin) || get(isCoach) || get(isSuperuser),
-      baseurl: get(baseurl),


Why was this changed?

Claude noticed that the previous implementation would result in there being baseurl: undefined in the params due to the test outputs.

I think Claude noticed the baseurl: undefined in the diff and figured it was related to the test failures or maybe just saw "undefined value in params is bad" but the change made sense to me to avoid sending undefined values.

Right, but undefined params don't get sent, so this is fine.

pcenov

Thanks Jacob, LGTM!

rtibbles

Code changes check out, and most importantly manual QA is approving!

Add a cached lookup of course IDs and a new boolean filter that excludes content nodes descended from Course-modality nodes. Includes backend tests for both the true and false cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add courses_exist flag to plugin_data. Pass exclude_course_ancestry=True when fetching resumable resources in the hydration view so learners do not see course-internal content outside of courses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add exclude_modalities and exclude_course_ancestry params to search requests for non-admin/coach/superuser roles. Hide Course-modality nodes from the topics page for learners. Update useBaseSearch tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Pass exclude_course_ancestry when fetching resumable content nodes. Add course-related computed properties and helpers to useLearnerResources. Update tests for fetchMoreResumableContentNodes and related behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

rtibbles

Everything all in place after the history rewrite!

nucleogenesis requested review from AlexVelezLl and rtibbles and removed request for rtibbles February 4, 2026 22:02

github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: Learn Re: Learn App (content, quizzes, lessons, etc.) DEV: frontend SIZE: small labels Feb 4, 2026

nucleogenesis commented Feb 4, 2026

View reviewed changes

kolibri/plugins/learn/frontend/views/TopicsPage/index.vue Show resolved Hide resolved

nucleogenesis force-pushed the 14108--excluded-courses branch from c2510f5 to 6c68b96 Compare February 4, 2026 22:29

rtibbles self-assigned this Feb 5, 2026

nucleogenesis force-pushed the 14108--excluded-courses branch 3 times, most recently from e56a431 to 09fe9d0 Compare February 5, 2026 21:38

rtibbles requested changes Feb 6, 2026

View reviewed changes

github-actions bot added the SIZE: medium label Feb 11, 2026

nucleogenesis requested a review from rtibbles February 11, 2026 22:35

nucleogenesis requested a review from rtibblesbot February 20, 2026 21:51

rtibblesbot reviewed Feb 20, 2026

View reviewed changes

nucleogenesis force-pushed the 14108--excluded-courses branch 2 times, most recently from dcf4ee4 to 71b7f03 Compare February 20, 2026 22:38

rtibblesbot reviewed Feb 20, 2026

View reviewed changes

rtibblesbot reviewed Feb 27, 2026

View reviewed changes

rtibblesbot approved these changes Mar 2, 2026

View reviewed changes

rtibbles reviewed Mar 6, 2026

View reviewed changes

nucleogenesis force-pushed the 14108--excluded-courses branch from 489f877 to 8a9892e Compare March 6, 2026 20:07

nucleogenesis requested a review from pcenov March 11, 2026 16:23

pcenov assigned rtibbles and unassigned rtibbles Mar 13, 2026

nucleogenesis requested a review from rtibbles March 17, 2026 15:43

rtibbles requested changes Mar 17, 2026

View reviewed changes

nucleogenesis requested a review from rtibbles March 19, 2026 01:39

rtibbles reviewed Mar 20, 2026

View reviewed changes

pcenov approved these changes Mar 20, 2026

View reviewed changes

nucleogenesis force-pushed the 14108--excluded-courses branch from f313b71 to 3e8f41b Compare March 20, 2026 17:14

rtibbles approved these changes Mar 20, 2026

View reviewed changes

nucleogenesis and others added 4 commits March 20, 2026 12:59

nucleogenesis force-pushed the 14108--excluded-courses branch from 3e8f41b to be5d0b6 Compare March 20, 2026 20:01

rtibbles approved these changes Mar 20, 2026

View reviewed changes

rtibbles merged commit ad4a71d into learningequality:release-v0.19.x Mar 20, 2026
55 checks passed

rtibbles mentioned this pull request Mar 20, 2026

Exclude courses from Library discovery #14108

Closed

8 tasks

Conversation

nucleogenesis commented Feb 4, 2026

Summary

Potential concerns

References

Reviewer guidance

Code

Performance

QA Testing

Uh oh!

Uh oh!

github-actions bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rtibbles left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

nucleogenesis commented Feb 26, 2026

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

rtibbles left a comment

Choose a reason for hiding this comment

Uh oh!

pcenov commented Mar 6, 2026

Uh oh!

nucleogenesis commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pcenov commented Mar 12, 2026

Uh oh!

nucleogenesis commented Mar 12, 2026

Uh oh!

pcenov commented Mar 13, 2026

Uh oh!

nucleogenesis commented Mar 17, 2026

Uh oh!

rtibbles left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 4, 2026 •

edited

Loading

nucleogenesis commented Mar 11, 2026 •

edited

Loading