refactor(users): account view route-driven via SurfaceTabBar — homogeneous chrome with Org/Admin#4185
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR refactors the ChangesUser Account View Refactoring into Routed Child Pages
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4185 +/- ##
=======================================
Coverage 99.55% 99.55%
=======================================
Files 31 31
Lines 1136 1136
Branches 328 328
=======================================
Hits 1131 1131
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Refactors the Users / Account surface to use the same route-driven “chrome” pattern as Organization/Admin (PageHeader + CoreSurfaceTabBar + router-view), by extracting the Profile and Organizations panels into dedicated child routes under /users.
Changes:
- Convert
UserViewinto a layout-only view that rendersCoreSurfaceTabBarand arouter-view. - Add
/users/profileand/users/organizationschild routes (with bare/usersredirecting to profile). - Introduce
config.users.tabsand expand unit tests to cover the new routing + extracted views.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/users/views/user.view.vue | Converts Account into a shared layout chrome (header + surface tab bar + router-view). |
| src/modules/users/views/user.profile.view.vue | New route view for Profile content, including delete-account danger zone. |
| src/modules/users/views/user.organizations.view.vue | New route view for Organizations list + leave dialog. |
| src/modules/users/router/users.router.js | Adds /users child routes for route-driven tabs and CASL meta. |
| src/modules/users/config/users.development.config.js | Adds users.tabs descriptors consumed by CoreSurfaceTabBar. |
| src/modules/users/tests/users.router.unit.tests.js | Adds assertions for Account child route structure/meta. |
| src/modules/users/tests/users.config.unit.tests.js | Adds test coverage for config.users.tabs shape/order. |
| src/modules/users/tests/user.view.unit.tests.js | Rewrites tests to validate the new layout shape and props to CoreSurfaceTabBar. |
| src/modules/users/tests/user.profile.view.unit.tests.js | Adds unit tests for the extracted profile route view behaviors. |
| src/modules/users/tests/user.organizations.view.unit.tests.js | Adds unit tests for the extracted organizations route view behaviors. |
| <v-list v-if="organizations && organizations.length" lines="two" class="pa-0 bg-transparent"> | ||
| <template v-for="(org, i) in organizations" :key="org.id || org._id"> | ||
| <v-list-item | ||
| :to="org.role === 'owner' || org.role === 'admin' ? `/users/organizations/${org.id || org._id}/general` : undefined" |
Comment on lines
+2
to
+7
|
|
||
| describe('users config – users.tabs', () => { | ||
| test('users.tabs has Profile + Organizations descriptors in order', async () => { | ||
| // Import via the generated config index so any future merges are covered. | ||
| const config = (await import('../../../config/index.js')).default; | ||
| expect(config.users.tabs).toEqual([ |
| this.deleteConfirmInput = ''; | ||
| } | ||
| userCan() { | ||
| return (action, subject) => (ability ? ability.can(action, subject) : true); |
Comment on lines
+116
to
+120
| * @desc Fetch organizations on component creation so the list is populated | ||
| * immediately without waiting for the parent layout's watcher. | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async created() { |
Comment on lines
+116
to
+117
| * @desc Fetch organizations on component creation so the list is populated | ||
| * immediately without waiting for the parent layout's watcher. |
Moves Profile form + danger zone (Delete Account) out of user.view.vue into user.profile.view.vue. The delete dialog is scoped to this view. Unit tests cover render, dialog state, deleteAccount success + error paths.
Moves the org list + Leave dialog out of user.view.vue into user.organizations.view.vue. The leave dialog stays scoped to this view. fetchOrganizations is called in created() for standalone hydration. Unit tests cover render, dialog state, and leaveOrg redirect behaviour.
…rganizations Adds users.tabs descriptor array (Profile + Organizations) to the module config and converts the Account route to a layout parent with three children: bare '' → redirect to Account Profile, 'profile' (Account Profile), 'organizations' (Account Organizations). beforeEnter guard for legacy ?tab=subscriptions preserved intact.
…uter-view Converts user.view.vue from a monolithic slot-driven PageTabs host to a thin layout (PageHeader + CoreSurfaceTabBar + <router-view>), homogeneous with OrganizationDetailComponent and AdminLayout. All inline tab content (profile form, danger zone, org list, dialogs) now lives in child views. PageTabs is now an orphan in devkit — flagged for a follow-up Theta PR. Tests rewritten to assert layout shape and absence of removed concerns.
PierreBrisorgueil
force-pushed
the
refactor/account-route-driven-tabs
branch
from
c0e962f to
9b1cd4c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/organizations/tests/organizations.domainJoin.e2e.tests.js`:
- Around line 304-305: Remove the brittle page.waitForLoadState('networkidle')
call after page.goto('/users/organizations') and instead wait deterministically
for the UI to be ready (for example use page.waitForSelector or waitForResponse)
that matches the target list item you already assert; update the test around
page.goto and the subsequent visibility assertion so it waits explicitly for the
organization list item selector or the specific API response used to populate
that list before performing the visibility assertion.
In `@src/modules/users/router/users.router.js`:
- Around line 60-70: The anonymous lazy-load component factories (the arrow
functions assigned to the component property in users.router.js for
'user.profile.view' and 'user.organizations.view') lack JSDoc; add a JSDoc block
immediately above each component property describing the function in one line
and including a `@returns` tag indicating it returns a Promise resolving to the
Vue component (e.g., "`@returns` {Promise<*>} Promise resolving to the
component"). There are no params so omit `@param` blocks; ensure the JSDoc is
placed directly above the component: () => import('...') entries so the
linter/documentation picks it up.
In `@src/modules/users/tests/user.organizations.view.unit.tests.js`:
- Around line 37-44: Add a JSDoc header for the sharedMocks function describing
its input and output: document the optional parameter $router (type: object with
push function, default vi.fn()) using `@param` and describe the returned mock
object shape (properties $router, $route, config with api and vuetify) using
`@returns`; place the comment directly above the sharedMocks definition so tools
and linters recognize it.
In `@src/modules/users/tests/users.router.unit.tests.js`:
- Around line 125-126: Add a one-line JSDoc description for the helper function
getAccountRoute so the header has a short description plus the existing
`@returns`; update the comment block above getAccountRoute to include a
single-line summary like "Return the users route record." (keep the existing
`@returns` annotation as-is and do not add `@param` since the function takes no
arguments).
In `@src/modules/users/views/user.organizations.view.vue`:
- Around line 1-171: The view file name violates the module naming convention —
rename the file from user.organizations.view.vue to users.organizations.view.vue
and update all references to it (imports, route definitions, lazy-loaded view
paths, and any index or export that points to the old filename); ensure the
component name UserOrganizationsView remains unchanged and verify routes (e.g.,
any router lazy import using the old path) now point to
"users.organizations.view.vue" so builds and runtime imports resolve correctly.
- Line 75: The file directly imports the optional organizations store
(useOrganizationsStore) which breaks module boundaries; remove the direct import
and instead obtain organization data via a neutral boundary (e.g., accept the
organizations composable or data as a prop, use a provided/injected token, or
call a generic optional-store accessor that does not reference the organizations
module name). Replace all references to useOrganizationsStore in this component
with the injected/prop/generic accessor (or a feature-flagged factory) so the
users core module no longer imports the organizations module symbol directly
(update user.organizations.view.vue to read from the neutral API you choose).
- Around line 92-97: Add a JSDoc header above the data() method describing its
purpose (one-line), include an `@returns` tag describing the returned object shape
(leaveDialog: boolean, orgToLeave: null|Object) and an `@param` tag for Vue's
props/context if applicable (or indicate none) to satisfy function documentation
rules; place the comment directly above the data() function declaration in the
user.organizations.view.vue file so the linter recognizes the documentation for
data().
In `@src/modules/users/views/user.profile.view.vue`:
- Around line 70-75: The data() component method lacks a JSDoc header; add a
JSDoc block immediately above the data() function with a one-line description,
include `@returns` {Object} describing the returned reactive state object and list
the returned properties (confirmDeleteAccount, deleteConfirmInput) in the
description, and include `@param` entries if you later add parameters (currently
none) so the data() method meets the function-doc requirements.
In `@src/modules/users/views/user.view.vue`:
- Around line 86-89: Add a JSDoc header above the async handler(loggedIn) method
describing its purpose in one line, include an `@param` {boolean} loggedIn
explaining the flag, and add an `@returns` {Promise<void>} (since the function is
async) describing that it resolves after optionally fetching organizations via
organizationsStore.fetchOrganizations(); ensure the JSDoc follows the project's
format and is placed immediately above the handler definition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bb88037c-c308-4bf3-9be0-61798375fec9
📒 Files selected for processing (11)
src/modules/organizations/tests/organizations.domainJoin.e2e.tests.jssrc/modules/users/config/users.development.config.jssrc/modules/users/router/users.router.jssrc/modules/users/tests/user.organizations.view.unit.tests.jssrc/modules/users/tests/user.profile.view.unit.tests.jssrc/modules/users/tests/user.view.unit.tests.jssrc/modules/users/tests/users.config.unit.tests.jssrc/modules/users/tests/users.router.unit.tests.jssrc/modules/users/views/user.organizations.view.vuesrc/modules/users/views/user.profile.view.vuesrc/modules/users/views/user.view.vue
Comment on lines
+304
to
+305
| await page.goto('/users/organizations'); | ||
| await page.waitForLoadState('networkidle'); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Avoid networkidle here; prefer deterministic readiness checks.
networkidle can introduce E2E flakiness when background requests are present. Since you already assert the target list item visibility, remove networkidle and rely on explicit element/API completion conditions instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/organizations/tests/organizations.domainJoin.e2e.tests.js` around
lines 304 - 305, Remove the brittle page.waitForLoadState('networkidle') call
after page.goto('/users/organizations') and instead wait deterministically for
the UI to be ready (for example use page.waitForSelector or waitForResponse)
that matches the target list item you already assert; update the test around
page.goto and the subsequent visibility assertion so it waits explicitly for the
organization list item selector or the specific API response used to populate
that list before performing the visibility assertion.
Comment on lines
+60
to
+70
| component: () => import('../views/user.profile.view.vue'), | ||
| meta: { | ||
| display: false, | ||
| action: 'read', | ||
| subject: 'User', | ||
| }, | ||
| }, | ||
| { | ||
| path: 'organizations', | ||
| name: 'Account Organizations', | ||
| component: () => import('../views/user.organizations.view.vue'), |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc for the new lazy-load route component factories.
Both new function literals (() => import(...)) are added without JSDoc, which breaks the repo’s JS/Vue function documentation requirement.
As per coding guidelines, “Every new or modified function must have a JSDoc header with one-line description, @param for each argument, and @returns for any non-void return value (always include @returns for async functions)”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/users/router/users.router.js` around lines 60 - 70, The anonymous
lazy-load component factories (the arrow functions assigned to the component
property in users.router.js for 'user.profile.view' and
'user.organizations.view') lack JSDoc; add a JSDoc block immediately above each
component property describing the function in one line and including a `@returns`
tag indicating it returns a Promise resolving to the Vue component (e.g.,
"`@returns` {Promise<*>} Promise resolving to the component"). There are no params
so omit `@param` blocks; ensure the JSDoc is placed directly above the component:
() => import('...') entries so the linter/documentation picks it up.
Comment on lines
+37
to
+44
| const sharedMocks = ($router = { push: vi.fn() }) => ({ | ||
| $router, | ||
| $route: { path: '/users/organizations' }, | ||
| config: { | ||
| api: { protocol: 'http', host: 'localhost', port: '3000', base: 'api' }, | ||
| vuetify: { theme: { rounded: 'rounded-lg', flat: true } }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document sharedMocks with JSDoc.
sharedMocks is a standalone function and needs a JSDoc header (@param, @returns) under the current repo rule set.
As per coding guidelines, every function must have JSDoc header with @param and @returns annotations.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/users/tests/user.organizations.view.unit.tests.js` around lines
37 - 44, Add a JSDoc header for the sharedMocks function describing its input
and output: document the optional parameter $router (type: object with push
function, default vi.fn()) using `@param` and describe the returned mock object
shape (properties $router, $route, config with api and vuetify) using `@returns`;
place the comment directly above the sharedMocks definition so tools and linters
recognize it.
Comment on lines
+125
to
+126
| /** @returns {import('vue-router').RouteRecordRaw|undefined} */ | ||
| const getAccountRoute = () => usersRoutes.find((r) => r.path === '/users'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Complete JSDoc for getAccountRoute with a one-line description.
This helper has only @returns; the required short description line is missing.
As per coding guidelines, “Every new or modified function must have a JSDoc header with one-line description, @param for each argument, and @returns for any non-void return value”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/users/tests/users.router.unit.tests.js` around lines 125 - 126,
Add a one-line JSDoc description for the helper function getAccountRoute so the
header has a short description plus the existing `@returns`; update the comment
block above getAccountRoute to include a single-line summary like "Return the
users route record." (keep the existing `@returns` annotation as-is and do not add
`@param` since the function takes no arguments).
Comment on lines
+1
to
+171
| <template> | ||
| <v-container fluid> | ||
| <v-list v-if="organizations && organizations.length" lines="two" class="pa-0 bg-transparent"> | ||
| <template v-for="(org, i) in organizations" :key="org.id || org._id"> | ||
| <v-list-item | ||
| :to="org.role === 'owner' || org.role === 'admin' ? `/users/organizations/${org.id || org._id}/general` : undefined" | ||
| :class="config.vuetify.theme.rounded" | ||
| class="pa-4" | ||
| > | ||
| <template #prepend> | ||
| <orgAvatarComponent :org="org" :size="40" class="mr-4" /> | ||
| </template> | ||
| <v-list-item-title class="text-body-large font-weight-medium">{{ org.name }}</v-list-item-title> | ||
| <v-list-item-subtitle v-if="org.description" class="text-body-small">{{ org.description }}</v-list-item-subtitle> | ||
| <template #append> | ||
| <div class="d-flex align-center ga-2"> | ||
| <v-chip v-if="org.role" size="small" :color="roleColor(org.role)" variant="tonal" class="text-capitalize">{{ org.role }}</v-chip> | ||
| <v-chip v-if="isActiveOrg(org)" size="small" color="success" variant="flat">Active</v-chip> | ||
| <v-btn | ||
| v-if="org.role !== 'owner'" | ||
| color="error" | ||
| variant="text" | ||
| size="small" | ||
| class="text-none" | ||
| @click.stop.prevent="confirmLeave(org)" | ||
| >Leave</v-btn> | ||
| <v-icon | ||
| v-if="org.role === 'owner' || org.role === 'admin'" | ||
| icon="fa-solid fa-chevron-right" | ||
| size="small" | ||
| color="medium-emphasis" | ||
| ></v-icon> | ||
| </div> | ||
| </template> | ||
| </v-list-item> | ||
| <v-divider v-if="i < organizations.length - 1"></v-divider> | ||
| </template> | ||
| </v-list> | ||
| <v-btn | ||
| color="primary" | ||
| variant="tonal" | ||
| :class="config.vuetify.theme.rounded" | ||
| class="text-none text-body-medium mt-4" | ||
| to="/users/organizations/create" | ||
| block | ||
| data-test="users-orgs-new" | ||
| > | ||
| <v-icon icon="fa-solid fa-plus" size="small" class="mr-2"></v-icon> | ||
| New Organization | ||
| </v-btn> | ||
| <div v-if="!organizations || !organizations.length" class="text-center text-medium-emphasis pa-8"> | ||
| <v-icon icon="fa-solid fa-building" size="x-large" class="mb-4 text-medium-emphasis"></v-icon> | ||
| <p class="text-body-medium">No organizations yet.</p> | ||
| </div> | ||
|
|
||
| <!-- Leave organization dialog --> | ||
| <v-dialog v-model="leaveDialog" max-width="440"> | ||
| <v-card :class="config.vuetify.theme.rounded" class="pa-4"> | ||
| <v-card-title class="text-title-large font-weight-medium">Leave Organization</v-card-title> | ||
| <v-card-text class="text-body-medium"> | ||
| Are you sure you want to leave {{ orgToLeave?.name }}? You will lose access to all resources in this organization. | ||
| </v-card-text> | ||
| <v-card-actions> | ||
| <v-spacer></v-spacer> | ||
| <v-btn variant="text" class="text-none text-body-medium" @click="leaveDialog = false">Cancel</v-btn> | ||
| <v-btn color="error" variant="flat" :class="config.vuetify.theme.rounded" class="text-none text-body-medium" @click="leaveOrg">Leave</v-btn> | ||
| </v-card-actions> | ||
| </v-card> | ||
| </v-dialog> | ||
| </v-container> | ||
| </template> | ||
|
|
||
| <script> | ||
| import { useAuthStore } from '../../auth/stores/auth.store'; | ||
| import { useOrganizationsStore } from '../../organizations/stores/organizations.store'; | ||
| import roleColor from '../../../lib/helpers/roleColor'; | ||
| import orgAvatarComponent from '../../core/components/org.avatar.component.vue'; | ||
|
|
||
| export default { | ||
| name: 'UserOrganizationsView', | ||
| components: { orgAvatarComponent }, | ||
| /** | ||
| * @desc Wires auth and organizations stores for computed properties and methods. | ||
| * @returns {{ authStore: Object, organizationsStore: Object }} | ||
| */ | ||
| setup() { | ||
| return { | ||
| authStore: useAuthStore(), | ||
| organizationsStore: useOrganizationsStore(), | ||
| }; | ||
| }, | ||
| data() { | ||
| return { | ||
| leaveDialog: false, | ||
| orgToLeave: null, | ||
| }; | ||
| }, | ||
| computed: { | ||
| /** | ||
| * @desc Organizations the current user belongs to. | ||
| * @returns {Array} | ||
| */ | ||
| organizations() { | ||
| return this.organizationsStore.organizations; | ||
| }, | ||
| /** | ||
| * @desc The ID of the user's current active organization. | ||
| * @returns {string|undefined} | ||
| */ | ||
| currentOrganizationId() { | ||
| const id = this.authStore.user?.currentOrganization; | ||
| return id?._id || id?.id || id; | ||
| }, | ||
| }, | ||
| /** | ||
| * @desc Fetch organizations on component creation so the list is populated | ||
| * immediately without waiting for the parent layout's watcher. | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async created() { | ||
| try { | ||
| await this.organizationsStore.fetchOrganizations(); | ||
| } catch { | ||
| // interceptor handles snackbar | ||
| } | ||
| }, | ||
| methods: { | ||
| roleColor, | ||
| /** | ||
| * @desc Check whether the given org is the user's active organization. | ||
| * @param {Object} org - Organization object. | ||
| * @returns {boolean} | ||
| */ | ||
| isActiveOrg(org) { | ||
| return (org.id || org._id) === this.currentOrganizationId; | ||
| }, | ||
| /** | ||
| * @desc Open the Leave confirmation dialog for a specific organization. | ||
| * @param {Object} org - Organization object the user wants to leave. | ||
| * @returns {void} | ||
| */ | ||
| confirmLeave(org) { | ||
| this.orgToLeave = org; | ||
| this.leaveDialog = true; | ||
| }, | ||
| /** | ||
| * @desc Leave the pending organization, refresh abilities, and redirect to | ||
| * `/organization-required` when no orgs remain or switch to the first | ||
| * remaining org when currentOrganization becomes null. | ||
| * @returns {Promise<void>} | ||
| */ | ||
| async leaveOrg() { | ||
| try { | ||
| await this.organizationsStore.leaveOrganization(this.orgToLeave.id || this.orgToLeave._id); | ||
| this.leaveDialog = false; | ||
| this.orgToLeave = null; | ||
| await this.authStore.refreshAbilities(); | ||
| if (this.organizationsStore.organizations.length === 0) { | ||
| this.$router.push('/organization-required'); | ||
| } else if (!this.organizationsStore.currentOrganization) { | ||
| await this.organizationsStore.switchOrganization( | ||
| this.organizationsStore.organizations[0].id || this.organizationsStore.organizations[0]._id, | ||
| ); | ||
| } | ||
| } catch { | ||
| this.leaveDialog = false; | ||
| } | ||
| }, | ||
| }, | ||
| }; | ||
| </script> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Rename the view file to match the enforced module view naming convention.
user.organizations.view.vue does not match {module}.{name}.view.vue for the users module. Please rename it (for example, users.organizations.view.vue) and update imports/routes accordingly.
As per coding guidelines, src/modules/**/*.view.vue: Use naming convention {module}.{name}.view.vue for Vue view files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/users/views/user.organizations.view.vue` around lines 1 - 171,
The view file name violates the module naming convention — rename the file from
user.organizations.view.vue to users.organizations.view.vue and update all
references to it (imports, route definitions, lazy-loaded view paths, and any
index or export that points to the old filename); ensure the component name
UserOrganizationsView remains unchanged and verify routes (e.g., any router lazy
import using the old path) now point to "users.organizations.view.vue" so builds
and runtime imports resolve correctly.
|
|
||
| <script> | ||
| import { useAuthStore } from '../../auth/stores/auth.store'; | ||
| import { useOrganizationsStore } from '../../organizations/stores/organizations.store'; |
There was a problem hiding this comment.
Remove direct organizations module coupling from users core module.
src/modules/users/** should not directly import optional-module symbols. Line 75 introduces a hard dependency on organizations, which breaks the module boundary contract and makes users non-optional-safe.
As per coding guidelines, src/modules/{core,auth,users,home,app}/**/*.{js,ts,vue} must not reference optional module names in imports/store references.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/users/views/user.organizations.view.vue` at line 75, The file
directly imports the optional organizations store (useOrganizationsStore) which
breaks module boundaries; remove the direct import and instead obtain
organization data via a neutral boundary (e.g., accept the organizations
composable or data as a prop, use a provided/injected token, or call a generic
optional-store accessor that does not reference the organizations module name).
Replace all references to useOrganizationsStore in this component with the
injected/prop/generic accessor (or a feature-flagged factory) so the users core
module no longer imports the organizations module symbol directly (update
user.organizations.view.vue to read from the neutral API you choose).
Comment on lines
+92
to
+97
| data() { | ||
| return { | ||
| leaveDialog: false, | ||
| orgToLeave: null, | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc for data() to satisfy function documentation rules.
data() is a new function without a JSDoc header.
Suggested patch
+ /**
+ * `@desc` Local state for leave-organization dialog workflow.
+ * `@returns` {{ leaveDialog: boolean, orgToLeave: Object|null }}
+ */
data() {
return {
leaveDialog: false,
orgToLeave: null,
};
},As per coding guidelines, every new or modified function must have a JSDoc header with one-line description, @param for each argument, and @returns for non-void returns.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| data() { | |
| return { | |
| leaveDialog: false, | |
| orgToLeave: null, | |
| }; | |
| }, | |
| /** | |
| * `@desc` Local state for leave-organization dialog workflow. | |
| * `@returns` {{ leaveDialog: boolean, orgToLeave: Object|null }} | |
| */ | |
| data() { | |
| return { | |
| leaveDialog: false, | |
| orgToLeave: null, | |
| }; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/users/views/user.organizations.view.vue` around lines 92 - 97,
Add a JSDoc header above the data() method describing its purpose (one-line),
include an `@returns` tag describing the returned object shape (leaveDialog:
boolean, orgToLeave: null|Object) and an `@param` tag for Vue's props/context if
applicable (or indicate none) to satisfy function documentation rules; place the
comment directly above the data() function declaration in the
user.organizations.view.vue file so the linter recognizes the documentation for
data().
Comment on lines
+70
to
+75
| data() { | ||
| return { | ||
| confirmDeleteAccount: false, | ||
| deleteConfirmInput: '', | ||
| }; | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add JSDoc to data() to meet function-doc requirements.
data() is a new function but currently undocumented.
As per coding guidelines, “Every new or modified function must have a JSDoc header with one-line description, @param for each argument, and @returns for any non-void return value”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/users/views/user.profile.view.vue` around lines 70 - 75, The
data() component method lacks a JSDoc header; add a JSDoc block immediately
above the data() function with a one-line description, include `@returns` {Object}
describing the returned reactive state object and list the returned properties
(confirmDeleteAccount, deleteConfirmInput) in the description, and include
`@param` entries if you later add parameters (currently none) so the data() method
meets the function-doc requirements.
Comment on lines
86
to
89
| async handler(loggedIn) { | ||
| if (!loggedIn) return; | ||
| await this.organizationsStore.fetchOrganizations().catch(() => {}); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document the watcher handler signature with full JSDoc.
The new async handler(loggedIn) function is missing required @param and @returns documentation.
As per coding guidelines, “Every new or modified function must have a JSDoc header with one-line description, @param for each argument, and @returns for any non-void return value (always include @returns for async functions)”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/modules/users/views/user.view.vue` around lines 86 - 89, Add a JSDoc
header above the async handler(loggedIn) method describing its purpose in one
line, include an `@param` {boolean} loggedIn explaining the flag, and add an
`@returns` {Promise<void>} (since the function is async) describing that it
resolves after optionally fetching organizations via
organizationsStore.fetchOrganizations(); ensure the JSDoc follows the project's
format and is placed immediately above the handler definition.
…rejecting members)
PierreBrisorgueil
dismissed
coderabbitai[bot]’s stale review
Addressed via subsequent fixes (CASL guard removed, watcher added). CI + E2E now green.
This was referenced May 21, 2026
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Converts Account from inline slot-driven
<PageTabs>to route-driven<CoreSurfaceTabBar>+ router-view. Profile and Organizations each become their own child route (/users/profile,/users/organizations). Result: Account / Organization / Admin all share the same chrome — PageHeader + SurfaceTabBar + container padding.Resolves user feedback item 4 (2026-05-20): 'l'apparence des onglets est différentes et placés différemment dans admin/account/organization on a bien un seul et même composant et la même structure de page'.
Files (~4 commits)
user.profile.view.vue+user.profile.view.unit.tests.jsuser.organizations.view.vue+user.organizations.view.unit.tests.jsusers.development.config.js(users.tabs) + router children +users.config.unit.tests.js+users.router.unit.tests.js(extended)user.view.vueto layout-only +user.view.unit.tests.jsrewrittenArchitecture
<PageTabs>is now orphan in devkit; cleanup scoped to a follow-up Theta PR.Test summary
Refs: docs/superpowers/plans/2026-05-20-trawl-ui-feedback-batch-v2.md PR Gamma.
Summary by CodeRabbit
New Features
Tests