Migrate the device plugin to the new resource-layer methods by rtibblesbot · Pull Request #15162 · learningequality/kolibri

rtibblesbot · 2026-08-07T16:10:48Z

Summary

Device reads become retrieve / list, the one write becomes create / update, and the facilities and LOD user lists become useList.
TaskResource is shared with unmigrated plugins, so its four custom methods gain _v2 siblings rather than in-place rewrites.

References

Closes #15060. Recipe: #15057; methods from #15056. getKolibriStudioStatus_v2 landed with #15155, so this branch only points the device call site at it. Acceptance criteria, ticked here because this account cannot write the issue body (403):

No device call site on an old base or cache method — the Task 6 greps return nothing.
Four _v2 siblings; originals kept for their other callers.
Two useList conversions; no useRetrieve candidate exists.
pnpm test-jest --testPathPatterns "plugins/device": 22 suites, 163 tests green.

Reviewer guidance

userPermissions/actions.js:19 — create vs update forks on whether a DevicePermissions row exists, since PATCH 404s on a first grant. Both branches tested.
TaskResource.clearAll_v2 — queue goes in the body; the sibling list action reads the query string, so a params: wrapper would clear every queue.
FacilitiesPage/index.vue:262 — loading && data === null, because CoreTable unmounts rows while dataLoading and a post-sync refetch must not blank the table.
Reads that omitted force now always hit the network, and a 404 used as control flow (mistyped token, wizard/utils.js:16) logs a console group. That one belongs in request.

QA steps

Checked by @radinamatic; all five pass.

Device > Channels: import by token, then by a mistyped token; export, delete, reorder, reload.
Device > Tasks: cancel, clear, restart, then Clear all.
Device > Permissions: grant "can manage content" to a user who never had any, reload; change it, reload.
Device > Facilities: sync a facility — rows stay on screen, no empty flash.
Device > Users (learn-only device): remove a user; the list refreshes.

Screenshots

Page	Screenshot
Facilities — `useList`
LOD users — `useList`
User permissions — the `create` / `update` fork

First grant sent POST …/devicepermissions/ → 201; a change sent PATCH …/<id>/ → 200.

AI usage

Claude Code applied the #15057 recipe and wrote the new addOrUpdateUserPermissions spec. Verified with Jest, prek, and manual QA.

@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

Deviations from the issue spec

DeleteExportChannelsPage.vue keeps an imperative list() instead of useList — @AlexVelezLl asked to leave it, since its channelIsBeingDeleted filter comes from a Vuex module.

	🟡 Waiting for feedback Last updated: 2026-08-11 16:50 UTC

rtibblesbot · 2026-08-07T16:11:06Z

The issue's acceptance criteria, with where each was verified. Recorded here because this account cannot edit the issue body (PATCH /issues/15060 → 403).

No code in device calls the old base methods or cache-management helpers. The three sweep greps from the plan (base methods, TaskResource custom methods, legacy names surviving as jest.mock factory keys) all return nothing across kolibri/plugins/device.
Custom methods handled per the recipe. Five _v2 siblings added — TaskResource.{cancel,clear,restart,clearAll}_v2 and RemoteChannelResource.getKolibriStudioStatus_v2. All five originals remain for the plugins that still call them. Nothing was rewritten in place, because no device-called resource is device-only.
Reactive reads use useRetrieve / useList where possible. Two conversions: useLodDeviceUsers and FacilitiesPage. RearrangeChannelsPage and DeleteExportChannelsPage got a plain list() instead — both mutate the fetched collection after the fetch (optimistic reorder, local row removal), so useList cannot own the data. There are no useRetrieve candidates: every remaining read is a Vuex action or an Options-API method.
Tests that call/mock replaced methods are updated; pnpm test-jest passes for device. 100 suites / 1438 tests green across plugins/(device|setup_wizard|facility|user_profile|user_auth|learn)|apiResources.

Behavioural deltas this accepts

Every read hits the network — the Model/Collection cache is gone. Two sites relied on it (views/ManageContentPage/api.js:53, modules/wizard/apiPeerImport.js:9).
Failures log. request calls logError before re-raising; it bails without an HTTP response, so an offline device stays quiet.
Expected 404s log. Five device paths treat a 404 as control flow — most often a mistyped channel token, which is routine user input. Flagged in the PR body as a possible follow-up on request rather than a special case here.
The permissions PATCH sends both mutable fields instead of a diff. No baseline is passed; the Save button gates the unedited case.
A stuck spinner on the learn-only-device users page is fixed rather than preserved. handleApiError re-throws by default, so a failed fetch never reached loading = false; useFetch swallows it into error and clears the flag.

FacilityUserResource.removeImportedUser and listRemoteFacilityLearners are deliberately untouched — their URLs are hand-registered, so request cannot resolve them, and they already bypass the Model/Collection layer.

github-actions · 2026-08-07T16:11:45Z

npm Package Versions

Warning

The following packages have changed files but no version bump:

Package	Version	Changed files
kolibri	0.18.0	1

If these changes affect published code, consider bumping the version.

github-actions · 2026-08-07T16:36:37Z

Build Artifacts

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

Smoke test screenshot

Download screenshot

AlexVelezLl

Thanks, this is looking good, just a few minor changes. After this, could you please add a "Manual QA" section on the PR body following the guidelines on this comment? Just update the description.

AlexVelezLl · 2026-08-10T15:06:45Z

+      // `CoreTable` unmounts its rows while `dataLoading`, so only the first fetch may show it -
+      // a refetch triggered by a completed sync must not blank the rows already on screen.
+      const loadingFacilities = computed(() => loading.value && facilitiesData.value === null);


I don't get this; why is this different from just using loading? Is this fixing a bug in the previous implementation?

Not a bug fix — it preserves the behaviour that was there. loadingFacilities was a data flag initialised true and set false by the beforeMount fetch, then never set back: the refetches (the watched-task handler after a sync completes, and handleClickClearAll) left it false.

useList sets loading for every fetch, and CoreTable renders dataLoading ? null : tbody (packages/kolibri/components/CoreTable.vue:112), so passing loading straight through would unmount the rows on each background refetch — a facility table that blanks itself every time a sync finishes. loading && data === null keeps the loader to the first load.

This page definitely lacks good loading handling, but we won't address that here. thanks.

No change here, then — loadingFacilities keeps the pre-existing first-load-only behaviour.

AlexVelezLl · 2026-08-10T15:23:28Z

Could we introduce useList here smoothly?

Not smoothly. useList owns data, and this page does not own its collection: it filters on the way in (:147) and removes the selected rows locally on delete, restoring them if the task POST fails (:154,168-170).

Converting means the raw response in data, a computed for the filter, and a third ref of optimistically-removed ids to subtract from it — three pieces where there is now one array, and no reactive read gained, since the fetch is a one-shot in beforeMount. RearrangeChannelsPage has the same shape (optimistic reorder plus rollback) and got the same plain list() rename. The two that did convert, FacilitiesPage and useLodDeviceUsers, have collections written only by the fetch itself.

Yeah, and also because channelIsBeingDeleted is coming from a Vuex module, let's keep it as it.

Kept as is — plain list(), with the Vuex channelIsBeingDeleted filter still applied on the way in.

AlexVelezLl · 2026-08-10T15:25:54Z

+async function postJobAction(resource, action, jobId) {
+  const response = await resource.request({
+    method: 'POST',
+    action,
+    routeParams: jobId,
+    data: {},
+  });
+  return response.data;
+}
+


Let's skip these utility methods that only obfuscate the actual implementation; the request method is simple enough to have more wrapper methods on top of it.

Dropped — cancel_v2, clear_v2 and restart_v2 each spell out their own this.request({ ... }) call.

AlexVelezLl · 2026-08-10T17:21:06Z

Let's remove it, I dont think it's adding any value

AlexVelezLl · 2026-08-10T17:21:39Z

+      method: 'POST',
+      action: 'cancel',
+      routeParams: jobId,
+      data: {},


Do we need to specify this empty data? Would it work the same if we didn't?

No — dropped from all three. cancel/clear/restart never read request.data (kolibri/core/tasks/viewsets/tasks.py:288,308,324); the empty body was only mirroring what accessDetailEndpoint happened to send. clearAll_v2 now passes undefined when there is no queue.

AlexVelezLl

Code changes look good, should be good for QA review :)

radinamatic · 2026-08-11T03:35:19Z

Device > Channels: import by token, then by a mistyped token (expect "not found"); export, delete, reorder, reload.

All workflows check out ✔️, replicated the issue while reordering channels. The only unclear guidance is the reload above, does it refer to simply reloading the browser page on Device > Channels? 🤨 Please clarify.
Device > Tasks: cancel a running task, clear a finished one, restart a failed one, then Clear all.

All workflows check out ✔️, no issue when restarting the canceled task, but if you have a suggestion how to simulate/induce a failed one, it would be appreciated.
Device > Permissions: grant "can manage content" to a user who has never had any, reload; change it again, reload.

All workflows check out ✔️. Same question regarding the reload as in the point 1. above: does it refer to simply reloading the browser page on Device > Permissions? 🤨 Please clarify.
Device > Facilities: sync a facility and watch the table — rows stay on screen, no empty flash. ✔️
Device > Users (learn-only device): remove a user; the list refreshes. ✔️

rtibblesbot · 2026-08-11T03:42:28Z

@radinamatic thanks — answers to the three questions:

1 & 3 — reload just means reloading the browser page. This PR removes the Model/Collection cache, so every read now goes to the server; the reload confirms what you see came back from a fresh GET rather than from client-side state.

2 — a failed task needs the work itself to error, easiest by cutting the source mid-import:

Import from a local drive, then unmount/unplug the drive while it copies.
Import from a LAN peer, then stop Kolibri on that peer.
Import from Studio, then disconnect the network — slowest, it retries a few times before giving up.

The reorder problem is #15175, already present on develop since the SortableJS migration. This PR only swaps RearrangeChannelsPage's fetch for DeviceChannelResource.list() — the DragContainer/DragHandle/Draggable wiring is untouched.

The device plugin migrates to the new resource methods, but TaskResource is shared with plugins that have not. The _v2 methods keep the originals' signatures and return response.data; the originals stay until every consumer has moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

fetchModel/fetchCollection become retrieve/list, and the force flag goes with the cache it controlled. Two comments explaining cache behaviour are deleted: RemoteChannelResource.retrieve returns a single object either way, and a token lookup can no longer return a stale model. RearrangeChannelsPage and DeleteExportChannelsPage get a plain list() rename rather than useList - both mutate the fetched collection after the fetch, so useList cannot own the data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The Studio status call also stops destructuring - getKolibriStudioStatus_v2 resolves with the body rather than the whole axios response. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

saveModel chose POST or PATCH from cache state: a user with no DevicePermissions row had a 404 recorded against the model and got a POST. DevicePermissionsViewSet is a plain ModelViewSet over a OneToOne primary key, so an unconditional update would 404 on a first grant. The store already carries the discriminator - showUserPermissionsPage substitutes a row without a `user` key on 404 - so the fork reads that rather than catching a 404 and retrying. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Both hand-rolled the loading flag and the fetch state that useList owns. FacilitiesPage writes its collection in exactly one place, and useLodDeviceUsers now derives `kind` in a computed rather than writing it onto the fetched objects. A failed LOD user fetch used to leave the page spinning forever - handleApiError re-throws by default, so the catch never reached its loading reset. useFetch resolves on failure instead, and the error watcher passes shouldThrow: false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

radinamatic · 2026-08-12T01:08:34Z

Tested the remaining scenarios of resuming and restarting failed import tasks, no regressions found, we are good to go!

radinamatic

LGTM! 💯 🚀

AlexVelezLl

lgtm!

github-actions Bot added APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) DEV: frontend labels Aug 7, 2026

github-actions Bot added the SIZE: medium label Aug 7, 2026

rtibblesbot force-pushed the issue-15060-ad4e59 branch from 37d1901 to 47513c5 Compare August 7, 2026 16:21

rtibblesbot force-pushed the issue-15060-ad4e59 branch 4 times, most recently from 50e4f40 to 0542543 Compare August 7, 2026 18:26

rtibblesbot marked this pull request as ready for review August 8, 2026 01:36

rtibblesbot force-pushed the issue-15060-ad4e59 branch from 0542543 to dd091c5 Compare August 8, 2026 02:52

AlexVelezLl requested changes Aug 10, 2026

View reviewed changes

rtibblesbot force-pushed the issue-15060-ad4e59 branch from dd091c5 to 65d9261 Compare August 10, 2026 16:18

github-actions Bot added SIZE: large and removed SIZE: medium labels Aug 10, 2026

rtibblesbot force-pushed the issue-15060-ad4e59 branch from 65d9261 to eb9b73b Compare August 10, 2026 16:37

AlexVelezLl requested changes Aug 10, 2026

View reviewed changes

rtibblesbot force-pushed the issue-15060-ad4e59 branch from eb9b73b to a44ef3b Compare August 10, 2026 17:27

github-actions Bot added SIZE: medium and removed SIZE: large labels Aug 10, 2026

rtibblesbot force-pushed the issue-15060-ad4e59 branch from a44ef3b to c60cb64 Compare August 10, 2026 18:07

AlexVelezLl reviewed Aug 10, 2026

View reviewed changes

AlexVelezLl requested review from pcenov and radinamatic August 10, 2026 21:58

radinamatic mentioned this pull request Aug 11, 2026

Drag and drop not working on Rearrange Channels page #15175

Closed

3 tasks

rtibbles assigned AlexVelezLl Aug 11, 2026

rtibblesbot and others added 5 commits August 11, 2026 08:40

Point the device task-queue calls at the _v2 methods

3a658cd

The Studio status call also stops destructuring - getKolibriStudioStatus_v2 resolves with the body rather than the whole axios response. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rtibblesbot force-pushed the issue-15060-ad4e59 branch from c60cb64 to fe2988a Compare August 11, 2026 15:42

radinamatic approved these changes Aug 12, 2026

View reviewed changes

AlexVelezLl approved these changes Aug 12, 2026

View reviewed changes

AlexVelezLl merged commit 095250b into learningequality:develop Aug 12, 2026
84 checks passed

rtibblesbot deleted the issue-15060-ad4e59 branch August 12, 2026 13:48

This was referenced Aug 12, 2026

Migrate setup_wizard to the new resource layer #15182

Open

Migrate user_profile to the new resource-layer methods #15183

Open

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

References

Reviewer guidance

QA steps

Screenshots

AI usage

Deviations from the issue spec

🟡 Waiting for feedback

Uh oh!

rtibblesbot commented Aug 7, 2026

Behavioural deltas this accepts

Uh oh!

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

Uh oh!

npm Package Versions

Uh oh!

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

Uh oh!

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

radinamatic commented Aug 11, 2026

Uh oh!

rtibblesbot commented Aug 11, 2026

Uh oh!

radinamatic commented Aug 12, 2026

Uh oh!

radinamatic left a comment

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

rtibblesbot commented Aug 7, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading