Skip to content

Conversation

@mwtrew
Copy link
Contributor

@mwtrew mwtrew commented Nov 3, 2025

Creates new RTKQ endpoints for user creation and updates. This replaces the createUser and updateUser async thunks. Most of the logic from there has been moved into the onQueryStarted lifecycle method of their respective
queries. The email change confirmation logic was moved out of there and into the My Account component, seeing as that's the only place it is used.

Components that used createUser/updateUser now use the generated RTKQ hook instead, and use its error handling instead of generalErrors. I have also added ExigentErrors to surface these where needed.

This affects a few different flows:

  • Registration
  • Updating details via My Account
  • Updating details via Required Account Information or User Context Reconfirmation modals
  • Consenting to LLM question usage

Registration and LLM questions have E2E test coverage. If the reviewer feels strongly about adding automated tests for the update flows we can do that, and that is probably sensible, but we don't currently have much in those areas so it would be a non-trivial amount of effort. Short of that I have manually tested thoroughly.

@codecov
Copy link

codecov bot commented Nov 3, 2025

Codecov Report

❌ Patch coverage is 30.63063% with 77 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.69%. Comparing base (19a2d80) to head (69f3889).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
src/app/state/slices/api/userApi.ts 11.11% 32 Missing ⚠️
...pp/components/elements/modals/EmailChangeModal.tsx 37.50% 10 Missing ⚠️
src/app/components/pages/MyAccount.tsx 35.71% 9 Missing ⚠️
...lements/modals/RequiredAccountInformationModal.tsx 22.22% 7 Missing ⚠️
...elements/modals/UserContextReconfirmationModal.tsx 64.28% 5 Missing ⚠️
...rc/app/components/pages/RegistrationSetDetails.tsx 44.44% 5 Missing ⚠️
...nents/navigation/LLMFreeTextQuestionInfoBanner.tsx 20.00% 4 Missing ⚠️
...pp/components/pages/RegistrationSetPreferences.tsx 33.33% 4 Missing ⚠️
src/IsaacAppTypes.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1816      +/-   ##
==========================================
+ Coverage   41.67%   41.69%   +0.01%     
==========================================
  Files         543      544       +1     
  Lines       23829    23835       +6     
  Branches     7867     7894      +27     
==========================================
+ Hits         9930     9937       +7     
- Misses      13255    13855     +600     
+ Partials      644       43     -601     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mwtrew mwtrew marked this pull request as ready for review November 4, 2025 12:08
Copy link
Contributor

@axlewin axlewin left a comment

Choose a reason for hiding this comment

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

LGTM, this all works well.

In terms of automated tests for the update flows: considering the time it would take to add them (in addition to the time already spent manually verifying everything works), my suggestion would be that it's not worth blocking these changes on. @barna-isaac might feel more strongly about this, though?

@barna-isaac
Copy link
Contributor

@axlewin: thanks for asking <3. I'd love to increase test coverage, but I certainly wouldn't block merging over this.

@axlewin axlewin merged commit 8074a02 into main Nov 5, 2025
10 checks passed
@axlewin axlewin deleted the move-account-create-update-to-rtk branch November 5, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants