From 3835d5540bbc56574d83e655c933eac0233b6f6b Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Thu, 16 Jul 2026 15:06:03 +0200 Subject: [PATCH] fix(auth,organizations): surface failures in org-create and email-verification flows - organization.create.view.vue: the create() catch only logged to the console, leaving the user with no feedback on failure. Surface err.response.data.message (with fallbacks) via a v-alert, mirroring the pattern applied to organizationSetup.component.vue (f92003d5). - verifyEmail.view.vue: the refreshAbilities() -> token() swap (#4431) dropped the "stay on page if the refresh fails" bail-out, since token() never throws. After the soft-refresh, bail out (no redirecting UI, no navigation) unless the store actually has a populated user, mirroring the isLoggedIn/!user check app.router.js already uses to detect a stale/failed refresh. Closes #4447 Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup --- ERRORS.md | 1 + .../tests/auth.verifyEmail.view.unit.tests.js | 22 +++++ src/modules/auth/views/verifyEmail.view.vue | 9 ++ .../organization.create.view.unit.tests.js | 84 +++++++++++++++++++ .../views/organization.create.view.vue | 18 +++- 5 files changed, 133 insertions(+), 1 deletion(-) diff --git a/ERRORS.md b/ERRORS.md index 0691202b7..3b8426d3c 100644 --- a/ERRORS.md +++ b/ERRORS.md @@ -23,3 +23,4 @@ Use this file as a compact memory of recurring AI mistakes. - [2026-03-15] pr scope: batching multiple unrelated fixes in one PR -> one fix = one PR to isolate blast radius and reduce iteration loops - [2026-05-09] update-stack: adding `import './modules/foo/styles/x.css'` to src/main.js downstream -> moved to project view (`src/modules/{project}/views/{project}.view.vue`). Stack-managed entry; --theirs wipes silently. See pierreb-devkit/Vue#4093. - [2026-06-15] npm audit: treating the 3 advisories (brace-expansion+ip-address moderate, picomatch high) as a runtime exposure -> all are transitive devDependencies (`npm audit --omit=dev` = 0); track upgrades, do not panic-patch the runtime bundle. +- [2026-07-16] auth: swapping a throwing store action (`refreshAbilities()`) for a silently-swallowing one (`token()`, never throws) without preserving the caller's bail-out -> after an `await token()` soft-refresh, check store state (e.g. `!authStore.user`) the way sibling guards do (`app.router.js`'s `isLoggedIn && !user`) before proceeding with UI that assumes the refresh succeeded. See #4447 (verifyEmail.view.vue), follow-up to #4431. diff --git a/src/modules/auth/tests/auth.verifyEmail.view.unit.tests.js b/src/modules/auth/tests/auth.verifyEmail.view.unit.tests.js index 5990b0623..04e62b970 100644 --- a/src/modules/auth/tests/auth.verifyEmail.view.unit.tests.js +++ b/src/modules/auth/tests/auth.verifyEmail.view.unit.tests.js @@ -167,5 +167,27 @@ describe('auth.verifyEmail.view', () => { expect(wrapper.vm.success).toBe(true); expect(wrapper.vm.$router.push).not.toHaveBeenCalled(); }); + + // #4447 — the refreshAbilities() -> token() swap (#4431) dropped the + // "stay on page if the refresh fails" bail-out, since token() never + // throws. A failed soft-refresh leaves the store without a populated + // user; the view must stay on the verified-success page instead of + // showing "Redirecting..." and navigating on stale state. + it('stays on page when the post-verification soft-refresh fails to populate a user', async () => { + storeMock.isLoggedIn = true; + storeMock.user = null; // token() swallowed a failure, left no user + storeMock.serverConfig = { organizations: { enabled: true } }; + verifyEmailMock.mockResolvedValueOnce({ message: 'Email verified' }); + + const wrapper = mountView(); + await wrapper.vm.$nextTick(); + await vi.dynamicImportSettled(); + + expect(tokenMock).toHaveBeenCalledTimes(1); + expect(wrapper.vm.success).toBe(true); + expect(wrapper.vm.redirecting).toBe(false); + expect(wrapper.vm.$router.push).not.toHaveBeenCalled(); + expect(wrapper.text()).toContain('Your email has been verified successfully. You can now sign in.'); + }); }); }); diff --git a/src/modules/auth/views/verifyEmail.view.vue b/src/modules/auth/views/verifyEmail.view.vue index f080f336d..64c187bd4 100644 --- a/src/modules/auth/views/verifyEmail.view.vue +++ b/src/modules/auth/views/verifyEmail.view.vue @@ -97,6 +97,7 @@ export default { * @desc Redirect the user after successful email verification based on auth state. * - Logged in + no org + orgs enabled → /organization-required * - Logged in + has org → home route + * - Logged in but soft-refresh fails (no user in store) → stay on page * - Not logged in → stay on page with sign-in link * @param {Object} authStore - The auth store instance. * @returns {Promise} @@ -107,6 +108,14 @@ export default { // refreshAbilities() signs out + rethrows on failure, which would eject // the user who just verified their email. await authStore.token(); + if (!authStore.user) { + // token() swallows failures internally, so a failed soft-refresh + // leaves the store without a populated user (same isLoggedIn + + // !user signal the app.router.js guard checks). Stay on this page + // with the verified-success message instead of showing + // "Redirecting..." and navigating on stale state. + return; + } this.redirecting = true; const serverConfig = authStore.serverConfig || (await authStore.fetchServerConfig()); if (!authStore.user?.currentOrganization && serverConfig?.organizations?.enabled) { diff --git a/src/modules/organizations/tests/organization.create.view.unit.tests.js b/src/modules/organizations/tests/organization.create.view.unit.tests.js index 05cc4bda6..39e8fc269 100644 --- a/src/modules/organizations/tests/organization.create.view.unit.tests.js +++ b/src/modules/organizations/tests/organization.create.view.unit.tests.js @@ -90,3 +90,87 @@ describe('organization.create.view — first-org redirect (#4422)', () => { expect(push).toHaveBeenCalledWith('/tasks'); }); }); + +// #4447 — the catch block only logged to the console, leaving the user with +// no feedback on a failed create. Surface the backend message via the error +// alert, mirroring organizationSetup.component.vue (f92003d5). +describe('organization.create.view — error surfacing (#4447)', () => { + beforeEach(() => { + setActivePinia(createPinia()); + push.mockReset(); + tokenMock.mockReset().mockResolvedValue(); + createOrganizationMock.mockReset().mockResolvedValue({ id: 'org-9' }); + authStoreMock.user = { id: 'u1' }; + }); + + it('sets error from response data message on createOrganization failure and resets loading', async () => { + const apiError = { response: { data: { message: 'An organization with this name already exists' } } }; + createOrganizationMock.mockRejectedValueOnce(apiError); + + const wrapper = mountView(); + wrapper.vm.name = 'Acme'; + await wrapper.vm.create(); + await flushPromises(); + + expect(wrapper.vm.error).toBe('An organization with this name already exists'); + expect(wrapper.vm.loading).toBe(false); + expect(push).not.toHaveBeenCalled(); + }); + + it('renders the error alert with the message in the DOM', async () => { + const apiError = { response: { data: { message: 'An organization with this name already exists' } } }; + createOrganizationMock.mockRejectedValueOnce(apiError); + + const wrapper = mountView(); + wrapper.vm.name = 'Acme'; + await wrapper.vm.create(); + await flushPromises(); + await wrapper.vm.$nextTick(); + + expect(wrapper.text()).toContain('An organization with this name already exists'); + }); + + it('falls back to err.message when a create failure has no response data message', async () => { + createOrganizationMock.mockRejectedValueOnce(new Error('Network error')); + + const wrapper = mountView(); + wrapper.vm.name = 'Acme'; + await wrapper.vm.create(); + await flushPromises(); + + expect(wrapper.vm.error).toBe('Network error'); + }); + + it('falls back to a generic message when the error has no message', async () => { + createOrganizationMock.mockRejectedValueOnce({}); + + const wrapper = mountView(); + wrapper.vm.name = 'Acme'; + await wrapper.vm.create(); + await flushPromises(); + + expect(wrapper.vm.error).toBe('Could not create organization. Please try again.'); + }); + + it('clears a stale error at the start of a new create attempt', async () => { + const wrapper = mountView(); + wrapper.vm.error = 'Could not create organization. Please try again.'; + + createOrganizationMock.mockResolvedValueOnce({ id: 'org-9' }); + wrapper.vm.name = 'Acme'; + await wrapper.vm.create(); + await flushPromises(); + + expect(wrapper.vm.error).toBeNull(); + }); + + it('leaves no error on a successful create', async () => { + const wrapper = mountView(); + wrapper.vm.name = 'Acme'; + await wrapper.vm.create(); + await flushPromises(); + + expect(wrapper.vm.error).toBeNull(); + expect(push).toHaveBeenCalled(); + }); +}); diff --git a/src/modules/organizations/views/organization.create.view.vue b/src/modules/organizations/views/organization.create.view.vue index aaff4f68b..9fe3a69c0 100644 --- a/src/modules/organizations/views/organization.create.view.vue +++ b/src/modules/organizations/views/organization.create.view.vue @@ -5,6 +5,20 @@

Organization Details

+ + + + {{ error }} + + (!!v && !!v.trim()) || 'Required' }, @@ -75,6 +90,7 @@ export default { const form = await this.$refs.form.validate(); if (form.valid) { this.loading = true; + this.error = null; const organizationsStore = useOrganizationsStore(); const authStore = useAuthStore(); // No current org (a first org, or a management user who just deleted their @@ -98,7 +114,7 @@ export default { ); } } catch (err) { - console.error(err); + this.error = err?.response?.data?.message || err?.message || 'Could not create organization. Please try again.'; } finally { this.loading = false; }