Skip to content

Migrate user_profile to the new resource-layer methods - #15183

Open
rtibblesbot wants to merge 5 commits into
learningequality:developfrom
rtibblesbot:issue-15064-fba872
Open

Migrate user_profile to the new resource-layer methods#15183
rtibblesbot wants to merge 5 commits into
learningequality:developfrom
rtibblesbot:issue-15064-fba872

Conversation

@rtibblesbot

@rtibblesbot rtibblesbot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reads become retrieve/list, the two profile writes become update, the profile page's user read becomes useRetrieve.
  • NetworkLocationResource is shared with device and learn, so fetchFacilities gains a _v2 sibling rather than an in-place rewrite.

References

Reviewer guidance

  • NetworkLocationResource.js:48 — the _v2 is registered on NetworkLocationResource alone, because request resolves networklocation_facilities_detail for that name only. Confirm no Static/Dynamic caller wants it.
  • actions.js:13update gets no baseline because ProfileEditPage.getUpdates() (:181) already diffs against the loaded user.
  • ProfilePage.spec.js:35 — the resource automock stubs useRetrieve as well, so the spec rebinds the real one off Resource.prototype.

Behaviour deltas:

  • A MergeFacility poll tick overlapping a slow GET coalesces onto it, where legacy Model.fetch queued behind it.
  • Reads that omitted force (useCurrentUser, ProfileEditPage.vue:173) now always hit the network — the Model cache is gone.
  • useCurrentUser no longer leaves isLoading stuck true on a failed fetch.
  • clear_v2 resolves response.data where postDetailEndpoint resolved the whole axios response. All three call sites discard it.
Page Screenshot
Profile — useRetrieve, after a full-name save Profile
Select facility — fetchFacilities_v2 Select facility
  • Saving the full name sent one PATCH /api/auth/facilityuser/<id>/ → 200, same shape as before, read back on reload.
  • The facility list rendered from GET /api/discovery/networklocation_facilities/<id>/ → 200 against a self-registered peer.
  • axe AA is clean on both, bar a pre-existing KSelect placeholder contrast violation on the edit page.
  • The merge flow was not exercised live — it needs a second device holding the same user — so MergeFacility.spec.js is its only cover.

AI usage

Claude Code applied the shared migration recipe and rewrote the two specs whose mocks named the replaced methods. Verified with Jest, prek, and manual QA in a browser against a local server.


@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-12 21:41 UTC

@github-actions github-actions Bot added APP: User Re: User app (sign-in, sign-up, user profile, 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 18:34
@rtibblesbot

rtibblesbot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Build EXE file failed on the chocolatey community feed, not the diff: choco install wget got Failed to fetch results from V2 feedUnable to find package 'Wget', so make webview2 then died on wget: command not found. #15184 hit the same feed error on innosetup 45 minutes later and went green on its next push. The feed answers 200 again; the job needs a re-run, which this account has no rights for.

rtibblesbot and others added 5 commits August 12, 2026 11:42
`NetworkLocationResource` is also imported by `device` and `learn`, so the
legacy `fetchFacilities` body stays in place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The read and the writes move together: `fetchModel` is what marked the model
synced, which is what made `saveModel(..., { exists: true })` PATCH a diff
rather than the whole cached object.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`force: true` has no successor — the cache it bypassed is gone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The spec factory drops `clear` rather than keeping it, so a missed call site
throws instead of passing silently.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rtibblesbot

Copy link
Copy Markdown
Contributor Author

Resolved without intervention: a fresh run on the same SHA (31629087140) built the EXE green. No re-run needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: frontend SIZE: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate user_profile to the new Resource-layer methods

2 participants