Migrate the facility plugin to the new resource methods by rtibblesbot · Pull Request #15184 · learningequality/kolibri

rtibblesbot · 2026-08-12T16:12:40Z

Summary

Every resource call in facility moves to the new Resource methods; DeletedFacilityUserResource.restoreCollection is rewritten onto request.
The users table reads through useList. Deletes the dead plugin-local PortalResource.
Dropping the cache exposed a backend bug: member_of, related_to__in and user_type OR-ed a multi-valued relation, returning a user once per membership or role. Now a correlated Exists() subquery.
Two behaviour fixes from losing the Model cache's dirty check: a user-type-only save reads the user back rather than sending an empty PATCH, and the settings snapshot is now taken once both halves of a facility settings save are done, not after the login half.
updateClass takes a name rather than an arbitrary updateData, so its payload is already the diff with no cache to compute one.

References

Fixes #15062. Recipe: #15057.

Reviewer guidance

Code review

useFacilityEditor.js:220 — saveFacilityConfig passes settingsCopy as the update baseline, replacing the Model cache's diff. Check no path mutates settings without a matching copySettings().
userManagement/actions.js:62 — a role-only change now retrieves the user; check nothing downstream needed the PATCH response.
useUserManagement.js:73 — check the stale-page 404 redirect cannot loop, now error handling runs in a watch.
facility_user.py:71 — check no caller relied on a row per membership or role.
MoveToTrashModal.vue:90, AssignCoachesSidePanel.vue:178, EnrollLearnersSidePanel.vue:176, ClassCopyModal.vue:178 and RemoveFromClassSidePanel.vue:212 keep list(): their loading ref is also set by a write path, which useList cannot do.
Reads without force: true now hit the network every time.

Manual QA

No UI changes; the facility plugin should keep behaving the same. Both flows need a facility with a user other than yourself: on a fresh install finish the setup wizard, sign in as the super admin, then Facility > Users > New user to create a learner.

Edit that user. Facility > Users → the user's ⋮ → Edit: change full name, username and identifier and save. Return and change only the user type — learner ↔ coach, then super admin on and off — touching no other field. Then ⋮ → Reset password, and ⋮ → Delete. Every change survives a reload; a user-type-only save applies the new type, and the deleted user is out of the table.
Facility settings. Facility > Settings: tick one checkbox, Save changes, reload; then change several at once and save. Create PIN, reload, remove it. Switch the sign-in option between username-and-password, username-only and picture password. Each save persists across a reload — including the checkboxes saved alongside a sign-in change — and nothing you did not edit moves.

AI usage

Used Claude Code to apply the recipe call site by call site and diagnose the duplicate row. Verified with the facility and kolibri-common Jest suites, the auth Python suite, prek, and live QA.

Duplicate row, before and after:

Before	After

Pages exercised live:

Page	Screenshot
Class detail
Deleted users
Data

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

How was this generated?

Ran pre-flight CI checks (lint, format, tests) and verified all pass
Rebased onto the target branch and resolved any conflicts
Reorganized commit history into clean, logical commits
Audited the diff to ensure only issue-relevant files are changed
Built PR body from the repository's PR template with evidence blocks

Committed separately: no commit on this branch introduced the lines these changes touch, so they are a new commit rather than folded into the work they amend:

kolibri/plugins/facility/frontend/modules/userManagement/__tests__/actions.spec.js

	🟡 Waiting for feedback Last updated: 2026-08-17 16:33 UTC

github-actions · 2026-08-12T16:29:43Z

Build Artifacts

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

Smoke test screenshot

Download screenshot

AlexVelezLl

Good job overall, @rtibblesbot! I think the major issue right now is the entire facility object being sent even if we just update a single setting. Beyond that, please add a "Manual QA" section on the PR description following these instructions (and that PR description example), there are several risky areas to be tested here, like the users filters handling, so please carefully describe them.

AlexVelezLl · 2026-08-14T15:40:34Z

-      id: facilityDatasetId.value,
-      data,
-    });
+    await FacilityDatasetResource.update(facilityDatasetId.value, data);


This is no longer doing any diff checking; this is a regression from the old implementation. Please refactor this so that we can send a baseline to the update method.

Fixed — saveFacilityConfig now passes settingsCopy as the update baseline, so only edited settings go on the wire (and an unchanged config sends nothing). settingsCopy is the snapshot last synced with the server: copySettings() runs on mount and after every save. Spec asserts the baseline.

AlexVelezLl · 2026-08-14T15:41:04Z

-    id,
-    data: updateData,
-  }).then(
+  return ClassroomResource.update(id, updateData).then(


This is only being used to edit the class name, right? Could you rather have updateData specify which specific fields it allows? So that we can better justify why we don't do any diff checking here.

Right, rename is the only use. The action now takes { id, name } and sends { name } — the one field it can change, so the payload is the diff by construction. ClassRenameModal updated to match.

The Model/Collection cache goes with them, so every read that did not pass `force: true` now hits the network. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The resource is imported only by facility, so the recipe's in-place rewrite applies rather than a `_v2` sibling. It now resolves with the payload instead of the axios response, and failures are logged by `request`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Four behaviour changes come with useFetch: `dataLoading` clears on failure rather than spinning forever, `usersCount`/`totalPages` normalise to 0 instead of undefined, a superseded fetch's response is discarded, and failure handling runs from a watcher a tick later instead of a catch. `fetchClasses` stays a plain `list()` — `useUsersFilters` dereferences the `classes` ref before any fetch runs, and `useFetch` initialises `data` to null. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

It duplicates packages/kolibri-common/apiResources/PortalResource.js and has no importers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

member_of, related_to__in and user_type joined memberships and roles, returning a user once per matching row - a learner in two classes appeared twice. The legacy Collection cache deduplicated by id client-side, hiding it until the resource migration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Legacy saveModel dirty-checked and skipped the request when the payload was empty; update() always PATCHes. A user-type change with no edited details sent an empty PATCH purely to obtain roles for updateFacilityLevelRoles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`saveConfig` saves login settings and then the rest of the config. `saveFacilityLoginSettings` re-snapshotted `settingsCopy` as it finished, so the config diff that follows saw no changes and `update` sent no request — every edited setting was silently dropped behind a "saved" snackbar. `saveFacilityConfig` already takes the snapshot once both halves are saved. Dropping the early one also fixes the failure path, where `undoSettingsChange` restored the edited state rather than the original. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

AlexVelezLl

Code changes look good, ready for QA!

rtibblesbot · 2026-08-14T20:37:27Z

Holding off on changes while this PR is in 🧪 Needs QA — the branch will not move while it is being tested. Questions still get answered.

Change the board status when QA is done and I'll pick up any outstanding review feedback, CI failures and conflicts on the next cycle.

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

How was this generated?

Ran pre-flight CI checks (lint, format, tests) and verified all pass
Rebased onto the target branch and resolved any conflicts
Reorganized commit history into clean, logical commits
Audited the diff to ensure only issue-relevant files are changed
Built PR body from the repository's PR template with evidence blocks

radinamatic · 2026-08-17T15:40:14Z

All the facility features listed in the QA guidance perform as expected: editing user details, deleting accounts, changing facility settings & PIN - everything persists across reloads, and no regressions or console errors are observed.

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

Holding off on changes while this PR is in 🧪 Needs QA — the branch will not move while it is being tested. Questions still get answered.

Change the board status when QA is done and I'll pick up any outstanding review feedback, CI failures and conflicts on the next cycle.

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

How was this generated?

Ran pre-flight CI checks (lint, format, tests) and verified all pass
Rebased onto the target branch and resolved any conflicts
Reorganized commit history into clean, logical commits
Audited the diff to ensure only issue-relevant files are changed
Built PR body from the repository's PR template with evidence blocks

rtibblesbot · 2026-08-17T16:17:28Z

Thanks @radinamatic. The branch is still ae36409 — nothing has been pushed since QA started, so what you tested is what is here.

Nothing outstanding on my side: all of @AlexVelezLl's review comments have replies and were confirmed. CI is green (the one pending check is the hourly contributor-check workflow, unrelated).

Merge is blocked only by the standing CHANGES_REQUESTED review — it needs a re-approval from @AlexVelezLl to clear.

Move this off 🧪 Needs QA and I'll pick up anything that lands afterwards.

radinamatic

Delay in approval due to GH glitches...

LGTM! 💯 🚀

AlexVelezLl

lgtm! Thanks @radinamatic!

github-actions Bot added DEV: backend Python, databases, networking, filesystem... APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) DEV: frontend SIZE: large labels Aug 12, 2026

rtibblesbot force-pushed the issue-15062-fbe9fc branch 2 times, most recently from 39f2b74 to 7f68609 Compare August 12, 2026 17:11

rtibblesbot mentioned this pull request Aug 12, 2026

Migrate user_profile to the new resource-layer methods #15183

Open

rtibbles assigned AlexVelezLl Aug 12, 2026

rtibblesbot marked this pull request as ready for review August 12, 2026 21:11

rtibblesbot force-pushed the issue-15062-fbe9fc branch from 7f68609 to e08ef88 Compare August 12, 2026 21:21

AlexVelezLl requested changes Aug 14, 2026

View reviewed changes

rtibblesbot force-pushed the issue-15062-fbe9fc branch 2 times, most recently from 9e061cb to 7ee2391 Compare August 14, 2026 16:19

AlexVelezLl reviewed Aug 14, 2026

View reviewed changes

Comment thread kolibri/plugins/facility/frontend/composables/useFacilityEditor.js Outdated

rtibblesbot and others added 8 commits August 14, 2026 13:07

Move facility resource calls onto the new Resource methods

514c830

The Model/Collection cache goes with them, so every read that did not pass `force: true` now hits the network. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Delete the unused plugin-local PortalResource

c3d6507

It duplicates packages/kolibri-common/apiResources/PortalResource.js and has no importers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Apply review-cycle changes with no owning commit

ae36409

rtibblesbot force-pushed the issue-15062-fbe9fc branch from fec356f to ae36409 Compare August 14, 2026 20:09

AlexVelezLl reviewed Aug 14, 2026

View reviewed changes

AlexVelezLl requested review from pcenov and radinamatic August 14, 2026 20:36

radinamatic approved these changes Aug 17, 2026

View reviewed changes

AlexVelezLl approved these changes Aug 17, 2026

View reviewed changes

AlexVelezLl merged commit af3d48c into learningequality:develop Aug 17, 2026
130 checks passed

rtibblesbot deleted the issue-15062-fbe9fc branch August 17, 2026 17:10

rtibblesbot mentioned this pull request Aug 17, 2026

Migrate setup_wizard to the new resource layer #15182

Open

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

References

Reviewer guidance

Code review

Manual QA

AI usage

🟡 Waiting for feedback

Uh oh!

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

Uh oh!

Build Artifacts

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexVelezLl Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

rtibblesbot commented Aug 14, 2026

Uh oh!

radinamatic commented Aug 17, 2026

Uh oh!

rtibblesbot commented Aug 17, 2026

Uh oh!

rtibblesbot commented Aug 17, 2026

Uh oh!

radinamatic left a comment

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl left a comment • 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rtibblesbot commented Aug 12, 2026 •

edited

Loading

github-actions Bot commented Aug 12, 2026 •

edited

Loading

AlexVelezLl left a comment •

edited

Loading