Skip to content

Migrate setup_wizard to the new resource layer - #15182

Merged
AlexVelezLl merged 3 commits into
learningequality:developfrom
rtibblesbot:issue-15063-2e6867
Aug 25, 2026
Merged

Migrate setup_wizard to the new resource layer#15182
AlexVelezLl merged 3 commits into
learningequality:developfrom
rtibblesbot:issue-15063-2e6867

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • TaskResource.{cancel,restart,clearAll} call sites now use the _v2 methods.
  • SetupWizardResource and FacilityImportResource are plugin-local, so their four custom methods are rewritten in place onto request rather than paired with a _v2.
  • Three of them now resolve with the response body rather than the response, so LodJoinFacility.vue destructures one level up; facilityadmins already did.
  • Deleted FacilityImportResource.listfacilitylearners — uncalled, and a duplicate of FacilityUserResource.listRemoteFacilityLearners.

References

Closes #15063. Recipe: #15057. _v2 methods: #15162.

Reviewer guidance

Code review

For a developer reading the diff.

  • The rewritten methods route failures through Resource.logError, which accessListEndpoint never called. A wrong password on SelectSuperAdminAccountForm.vue:175 now emits a Request error: group — confirm that is acceptable.
  • No useList conversion: the three TaskResource.list sites are setTimeout polling loops, and useList wraps a single list() with no polling.

QA steps

For a tester installing the build artifacts in a VM. No UI change — every screen below should behave as on develop. The wizard runs once per device, so each row needs a freshly installed VM; rows 2-6 need a second VM on the network running any released Kolibri with a facility on it.

Where What to check
Setting up Kolibri — "Group learning" with a new facility, and separately "On my own" both run through to the end and leave you signed in
Group learning → import a facility from the other VM Cancel mid-import stops it and a restart finishes; after a failed import, Retry and Start over both work
Select super admin, after that import the account dropdown lists the imported facility's admins; a wrong password shows the error and does not advance, the right one continues; finish and sign in as that admin
Select super adminCreate new super admin account the form finishes the wizard, and the account you just made signs in
Learn-only device → Import one or more existing user accounts one user imports, then Import another user account imports two more from the list; each imported user signs in
Learn-only device → Create a new user account for an existing facility (that facility must allow learners to sign up) a username already used there shows the "username already exists" error on the form; a free one creates the account and imports it
Reload the browser midway through a facility import the wizard carries on, with no task from the previous attempt on screen

AI usage

Used Claude Code to apply the shared migration recipe across the plugin and to write the resource spec covering the four rewritten methods. Verified with the setup_wizard Jest suite and prek.


@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

🟡 Waiting for feedback

Last updated: 2026-08-20 15:08 UTC

@github-actions github-actions Bot added APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend SIZE: medium labels Aug 12, 2026
@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

@rtibblesbot
rtibblesbot marked this pull request as ready for review August 12, 2026 16:24
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

The red Android smoke test (API 24) leg is a runner-local flake, not a diff issue: make maestro-install died with curl: (56) fetching the pinned maestro.zip from GitHub releases, before the emulator booted. API 30 and API 35 installed and passed on the same commit in the same run.

@AlexVelezLl AlexVelezLl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good, and smoke tests check out. It's ready for QA review, @rtibblesbot please add a QA steps section similar to the other migration PRs, thanks!

@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Added a QA steps section: seven flows covering the six renamed TaskResource call sites and the four rewritten custom methods — new facility, facility import (cancel/retry/start over), both super-admin branches, both learn-only branches, and a mid-import reload.

Class sweep: checked every open PR in the #15057 migration family for a missing QA-steps section. Three are open — #15163 already had one; #15183 (user_profile) did not, so it now has one too. #15144, #15155, #15162 and #15184 are merged and untouched.

@radinamatic

Copy link
Copy Markdown
Member

Removed the Needs QA label so the PR can be rebased to include changes from #15217 that will unblock the pending QA workflows.

cancel/restart/clearAll are wire-identical to their _v2 pairs; every call
site discards the resolved value, so the envelope-to-body change is invisible.
Both resources are plugin-local, so the recipe's in-place rewrite applies
rather than a _v2 pair. The four methods now resolve with the response body,
which LodJoinFacility destructures one level up.
No caller anywhere in kolibri/ or packages/, and a line-for-line duplicate of
FacilityUserResource.listRemoteFacilityLearners, which ImportIndividualUserForm
already calls. Its removal takes the last client/urls imports with it.
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Rebased onto develop eca38c5, which carries #15217. All three commits replayed without conflict — #15217 is backend-only and this branch is frontend-only, so the diff is unchanged (8 files, all under kolibri/plugins/setup_wizard/frontend/).

Re-verified after the rebase: pnpm test-jest --testPathPatterns setup_wizard 6 suites / 16 tests pass, prek run --from-ref upstream/develop --to-ref HEAD passes, and both acceptance greps (legacy base methods; TaskResource.{cancel,clear,restart,clearAll} without _v2) are still empty — develop landed no new legacy call sites in this plugin.

Class sweep: checked all five open PRs in the #15057 migration family for the same staleness. #15163 and #15183 got the same request and are already on eca38c5. #15206 and #15207 are 48 commits behind but are not in QA and carry no such request, so I left them alone — say the word if you want them rebased too.

QA steps unchanged: none of the seven flows pins a peer Kolibri version, so nothing there was blocked on #15217.

@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Restructured the body: Reviewer guidance now splits into ### Code review and ### QA steps, after @radinamatic asked for that separation on #15183. The seven wizard flows are unchanged in substance, now a Where | What to check table, and the rig is a freshly installed VM rather than a reset KOLIBRI_HOME.


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

@pcenov pcenov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @AlexVelezLl @radinamatic and @rtibblesbot - no regressions observed while manually testing - good to go!

@AlexVelezLl AlexVelezLl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you all!

@AlexVelezLl
AlexVelezLl merged commit f49ba51 into learningequality:develop Aug 25, 2026
95 checks passed
@rtibblesbot
rtibblesbot deleted the issue-15063-2e6867 branch August 25, 2026 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend SIZE: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate setup_wizard to the new Resource-layer methods

4 participants