refactor(users): make Users Management fully independent frontend-only module#33
refactor(users): make Users Management fully independent frontend-only module#33rachid-hammami wants to merge 11 commits into
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (7)
-
src/app/(dashboard)/users/_components/UsersTable.tsx, line 71-83 (link)logic: Update is applied optimistically but rollback logic on error reverts to stale state, not the previous snapshot.
-
src/app/(dashboard)/users/_components/UserActions.tsx, line 48-51 (link)logic: Validation accepts any email containing
@, allowing invalid formats likeuser@or@domain.com. -
src/app/(dashboard)/users/_components/UserActions.tsx, line 64-69 (link)style: Using browser
confirm()instead of a proper dialog component creates inconsistent UX and isn't customizable to match the design system.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
-
src/lib/api.ts, line 9-31 (link)logic: All fetch errors (network, timeout, non-200 status) trigger mock mode fallback, masking real errors that should be surfaced to users. Should server errors (500, 403, etc.) also trigger demo mode, or should they show an error to the user?
-
src/app/(dashboard)/users/_components/UsersTable.tsx, line 150-152 (link)style:
Add Userbutton doesn't have any click handler - consider implementing or removing until functionality is ready. -
src/app/(dashboard)/users/_components/UserDropdown.tsx, line 51-53 (link)style: Toast fires even when selecting the current role (no actual change made).
-
src/app/(dashboard)/users/_components/UsersTable.tsx, line 98-103 (link)style: Suspend action always sets status to
Suspendedregardless of current state - if user is already suspended, still shows the warning toast.
17 files reviewed, 7 comments
|
Thanks for reviewing this PR. Let me know if you’d like me to make any changes |
|
Thanks for the PR. The idea of this dashboard is to provide screens and basic functionality that users can build on top of as per their needs. Adding api.ts & checking backend is not needed here. Static data is enough for this use case. Some components and dependencies were installed even though they already exist, so those can be removed. The most important point is that this work is based on an old fork. Many things have changed since then. Please pull the latest changes from main and then add the Users module inside the dashboard, not outside of it. After that, you can start resolving the Greptile comments. |
rachid-hammami
force-pushed
the
feature/users-management
branch
from
e027784 to
d433e68
Compare
| } | ||
|
|
||
|
|
||
| export function hasAnyPermission( |
There was a problem hiding this comment.
Null-safe permission check
hasAnyPermission indexes ROLE_PERMISSIONS[user.roleId] without optional chaining. If ROLE_PERMISSIONS is missing an entry for a RoleId (e.g., future role added to types.ts but not added to role-permissions.ts), this will throw at runtime. Consider mirroring hasPermission’s null-safe access (ROLE_PERMISSIONS[user.roleId]?.includes(...) ?? false).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/rbac/guards.ts
Line: 17:20
Comment:
**Null-safe permission check**
`hasAnyPermission` indexes `ROLE_PERMISSIONS[user.roleId]` without optional chaining. If `ROLE_PERMISSIONS` is missing an entry for a `RoleId` (e.g., future role added to `types.ts` but not added to `role-permissions.ts`), this will throw at runtime. Consider mirroring `hasPermission`’s null-safe access (`ROLE_PERMISSIONS[user.roleId]?.includes(...) ?? false`).
How can I resolve this? If you propose a fix, please make it concise.| } | ||
|
|
||
| export default function UsersTable({ | ||
| columns, | ||
| data, |
There was a problem hiding this comment.
Unused RBACUser import
import type { RBACUser } from "@/lib/rbac" is unused in this file, which will fail tsc/lint if noUnusedLocals is enabled. Please remove it to keep the build green.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/users/_components/UsersTable.tsx
Line: 37:41
Comment:
**Unused RBACUser import**
`import type { RBACUser } from "@/lib/rbac"` is unused in this file, which will fail `tsc`/lint if `noUnusedLocals` is enabled. Please remove it to keep the build green.
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (2)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/(main)/dashboard/_components/SearchBar.tsx
Line: 1:1
Comment:
**Empty file committed**
This file is 0 bytes (`e69de29`) and exports nothing, which is easy to accidentally import and then hit runtime/compile errors. If it’s not used, it should be removed from the PR; if it is meant to exist, it needs an actual implementation/export.
How can I resolve this? If you propose a fix, please make it concise.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! Prompt To Fix With AIThis is a comment left during a code review.
Path: package.json
Line: 19:23
Comment:
**Malformed package.json formatting**
The `"@tanstack/react-table"` dependency line is flush-left (no indentation) while the surrounding entries are indented. Although JSON still parses, this is likely to fail formatting checks (Biome) and makes diffs noisy. Please reformat `package.json` to keep consistent indentation.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise. |
| role: string | ||
| status: "Active" | "Disabled" |
There was a problem hiding this comment.
Type mismatch: status: "Active" | "Disabled" here conflicts with src/app/(dashboard)/users/_data/users.ts which defines status: "active" | "pending" | "suspended". This will cause type errors if both are used together.
| role: string | |
| status: "Active" | "Disabled" | |
| status: "active" | "pending" | "suspended" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/demo-users.ts
Line: 5:6
Comment:
Type mismatch: `status: "Active" | "Disabled"` here conflicts with `src/app/(dashboard)/users/_data/users.ts` which defines `status: "active" | "pending" | "suspended"`. This will cause type errors if both are used together.
```suggestion
status: "active" | "pending" | "suspended"
```
How can I resolve this? If you propose a fix, please make it concise.| export async function patchUser(id: number, payload: Record<string, any>) { | ||
| try { | ||
| const res = await fetch(`/api/users/${id}`, { | ||
| method: "PATCH", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(payload), | ||
| }) | ||
|
|
||
| // Si le backend répond, on retourne la vraie donnée | ||
| if (res.ok) { | ||
| const data = await res.json() | ||
| console.info("✅ Backend response:", data) | ||
| return data | ||
| } | ||
|
|
||
| // Sinon on passe en mode "mock" | ||
| throw new Error("Backend not available") | ||
| } catch (error) { | ||
| console.warn("⚠️ No backend detected — using mock mode.") | ||
| // Simule un retour identique à ce qu'enverrait une vraie API | ||
| await new Promise((r) => setTimeout(r, 400)) // petit délai simulé | ||
| return { id, ...payload } | ||
| } | ||
| } |
There was a problem hiding this comment.
patchUser is defined but never imported or used anywhere in the codebase.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/api.ts
Line: 9:32
Comment:
`patchUser` is defined but never imported or used anywhere in the codebase.
How can I resolve this? If you propose a fix, please make it concise.
| import { RolePermissions } from "./_components/RolePermissions" | ||
|
|
||
| import { Button } from "@/components/ui/button" | ||
| import type { Role, RoleStatus } from "@/types/role" |
There was a problem hiding this comment.
Import path @/types/role is incorrect - no src/types/ directory exists. Should be @/lib/role.
| import type { Role, RoleStatus } from "@/types/role" | |
| import type { Role, RoleStatus } from "@/lib/role" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/page.tsx
Line: 11:11
Comment:
Import path `@/types/role` is incorrect - no `src/types/` directory exists. Should be `@/lib/role`.
```suggestion
import type { Role, RoleStatus } from "@/lib/role"
```
How can I resolve this? If you propose a fix, please make it concise.| TooltipTrigger, | ||
| } from "@/components/ui/tooltip" | ||
|
|
||
| import type { Role } from "@/types/role" |
There was a problem hiding this comment.
Import path @/types/role is incorrect - no src/types/ directory exists. Should be @/lib/role.
| import type { Role } from "@/types/role" | |
| import type { Role } from "@/lib/role" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/RoleActions.tsx
Line: 28:28
Comment:
Import path `@/types/role` is incorrect - no `src/types/` directory exists. Should be `@/lib/role`.
```suggestion
import type { Role } from "@/lib/role"
```
How can I resolve this? If you propose a fix, please make it concise.| import { RoleActions } from "./RoleActions" | ||
| import { RoleBadge } from "./RoleBadge" | ||
|
|
||
| import type { Role } from "@/types/role" |
There was a problem hiding this comment.
Import path @/types/role is incorrect - no src/types/ directory exists. Should be @/lib/role.
| import type { Role } from "@/types/role" | |
| import type { Role } from "@/lib/role" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/columns.tsx
Line: 8:8
Comment:
Import path `@/types/role` is incorrect - no `src/types/` directory exists. Should be `@/lib/role`.
```suggestion
import type { Role } from "@/lib/role"
```
How can I resolve this? If you propose a fix, please make it concise.| SelectValue, | ||
| } from "@/components/ui/select" | ||
|
|
||
| import type { Role } from "@/types/role" |
There was a problem hiding this comment.
Import path @/types/role is incorrect - no src/types/ directory exists. Should be @/lib/role.
| import type { Role } from "@/types/role" | |
| import type { Role } from "@/lib/role" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/RolesTable.tsx
Line: 19:19
Comment:
Import path `@/types/role` is incorrect - no `src/types/` directory exists. Should be `@/lib/role`.
```suggestion
import type { Role } from "@/lib/role"
```
How can I resolve this? If you propose a fix, please make it concise.| FormMessage, | ||
| } from "@/components/ui/form"; | ||
|
|
||
| import type { Role } from "@/types/role"; |
There was a problem hiding this comment.
Import path @/types/role is incorrect - no src/types/ directory exists. Should be @/lib/role.
| import type { Role } from "@/types/role"; | |
| import type { Role } from "@/lib/role"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/EditRoleDialog.tsx
Line: 28:28
Comment:
Import path `@/types/role` is incorrect - no `src/types/` directory exists. Should be `@/lib/role`.
```suggestion
import type { Role } from "@/lib/role";
```
How can I resolve this? If you propose a fix, please make it concise.| @@ -0,0 +1,10 @@ | |||
| export type RoleStatus = 'Active' | 'Inactive' | |||
There was a problem hiding this comment.
RoleStatus is defined as 'Active' | 'Inactive' but the code in roles/page.tsx:84 and roles/_data/roles.ts uses "Disabled" instead of "Inactive".
| export type RoleStatus = 'Active' | 'Inactive' | |
| export type RoleStatus = 'Active' | 'Disabled' |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/role.ts
Line: 1:1
Comment:
`RoleStatus` is defined as `'Active' | 'Inactive'` but the code in `roles/page.tsx:84` and `roles/_data/roles.ts` uses `"Disabled"` instead of `"Inactive"`.
```suggestion
export type RoleStatus = 'Active' | 'Disabled'
```
How can I resolve this? If you propose a fix, please make it concise.| @@ -0,0 +1,14 @@ | |||
| import { demoUsers } from "@/lib/demo-users" | |||
There was a problem hiding this comment.
Type mismatch: imports demoUsers (with status: "Active" | "Disabled") but UsersTable expects User type from _data/users.ts (with status: "active" | "pending" | "suspended"). This will cause type errors.
| import { demoUsers } from "@/lib/demo-users" | |
| import { users } from "./_data/users" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/users/page.tsx
Line: 1:1
Comment:
Type mismatch: imports `demoUsers` (with `status: "Active" | "Disabled"`) but `UsersTable` expects `User` type from `_data/users.ts` (with `status: "active" | "pending" | "suspended"`). This will cause type errors.
```suggestion
import { users } from "./_data/users"
```
How can I resolve this? If you propose a fix, please make it concise.| <h1 className="text-2xl font-bold">Users</h1> | ||
| </div> | ||
|
|
||
| <UsersTable data={demoUsers} /> |
There was a problem hiding this comment.
Should use users instead of demoUsers to match the User type defined in _data/users.ts.
| <UsersTable data={demoUsers} /> | |
| <UsersTable data={users} /> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/users/page.tsx
Line: 11:11
Comment:
Should use `users` instead of `demoUsers` to match the `User` type defined in `_data/users.ts`.
```suggestion
<UsersTable data={users} />
```
How can I resolve this? If you propose a fix, please make it concise.| export type User = { | ||
| id: number | ||
| name: string | ||
| email: string | ||
| role: string | ||
| status: "Active" | "Disabled" | ||
| } |
There was a problem hiding this comment.
This file is never used in the codebase (only users.ts is used), and its User type conflicts with the one in users/_data/users.ts. Consider removing this file or renaming the type to DemoUser to avoid confusion.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/demo-users.ts
Line: 1:7
Comment:
This file is never used in the codebase (only `users.ts` is used), and its `User` type conflicts with the one in `users/_data/users.ts`. Consider removing this file or renaming the type to `DemoUser` to avoid confusion.
How can I resolve this? If you propose a fix, please make it concise.
|
The Users Management module has been refactored to use static demo data only and is now fully frontend-only, in accordance with the project structure and recommendations. The module is fully independent and aligned with the latest main branch. Please let me know if any further adjustments are needed. |
rachid-hammami
changed the title
|
Please resolve the Greptile comments. Also, new modules should open inside the dashboard layout itself, not navigate to a separate page. Keep everything within the existing shell. Make sure the design stays consistent with the current dashboard UI in terms of spacing, typography, card styles, and overall layout. |
rachid-hammami
force-pushed
the
feature/users-management
branch
from
f87d918 to
1994da3
Compare
| @@ -0,0 +1,14 @@ | |||
| import { demoUsers } from "@/lib/demo-users" | |||
There was a problem hiding this comment.
Type mismatch: demoUsers has status: "Active" | "Disabled" but UsersTable expects User type from _data/users.ts with status: "active" | "pending" | "suspended". This will cause runtime type errors.
| import { demoUsers } from "@/lib/demo-users" | |
| import { users } from "./_data/users" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/users/page.tsx
Line: 1
Comment:
Type mismatch: `demoUsers` has `status: "Active" | "Disabled"` but `UsersTable` expects `User` type from `_data/users.ts` with `status: "active" | "pending" | "suspended"`. This will cause runtime type errors.
```suggestion
import { users } from "./_data/users"
```
How can I resolve this? If you propose a fix, please make it concise.| <h1 className="text-2xl font-bold">Users</h1> | ||
| </div> | ||
|
|
||
| <UsersTable data={demoUsers} /> |
There was a problem hiding this comment.
| <UsersTable data={demoUsers} /> | |
| <UsersTable data={users} /> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/users/page.tsx
Line: 11
Comment:
```suggestion
<UsersTable data={users} />
```
How can I resolve this? If you propose a fix, please make it concise.| import { RolePermissions } from "./_components/RolePermissions" | ||
|
|
||
| import { Button } from "@/components/ui/button" | ||
| import type { Role, RoleStatus } from "@/types/role" |
There was a problem hiding this comment.
Import path @/types/role doesn't exist - there's no src/types/ directory.
| import type { Role, RoleStatus } from "@/types/role" | |
| import type { Role, RoleStatus } from "@/lib/role" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/page.tsx
Line: 11
Comment:
Import path `@/types/role` doesn't exist - there's no `src/types/` directory.
```suggestion
import type { Role, RoleStatus } from "@/lib/role"
```
How can I resolve this? If you propose a fix, please make it concise.| SelectValue, | ||
| } from "@/components/ui/select" | ||
|
|
||
| import type { Role } from "@/types/role" |
There was a problem hiding this comment.
Import path @/types/role doesn't exist - there's no src/types/ directory.
| import type { Role } from "@/types/role" | |
| import type { Role } from "@/lib/role" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/RolesTable.tsx
Line: 19
Comment:
Import path `@/types/role` doesn't exist - there's no `src/types/` directory.
```suggestion
import type { Role } from "@/lib/role"
```
How can I resolve this? If you propose a fix, please make it concise.| import { RoleActions } from "./RoleActions" | ||
| import { RoleBadge } from "./RoleBadge" | ||
|
|
||
| import type { Role } from "@/types/role" |
There was a problem hiding this comment.
Import path @/types/role doesn't exist - there's no src/types/ directory.
| import type { Role } from "@/types/role" | |
| import type { Role } from "@/lib/role" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/columns.tsx
Line: 8
Comment:
Import path `@/types/role` doesn't exist - there's no `src/types/` directory.
```suggestion
import type { Role } from "@/lib/role"
```
How can I resolve this? If you propose a fix, please make it concise.| TooltipTrigger, | ||
| } from "@/components/ui/tooltip" | ||
|
|
||
| import type { Role } from "@/types/role" |
There was a problem hiding this comment.
Import path @/types/role doesn't exist - there's no src/types/ directory.
| import type { Role } from "@/types/role" | |
| import type { Role } from "@/lib/role" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/RoleActions.tsx
Line: 28
Comment:
Import path `@/types/role` doesn't exist - there's no `src/types/` directory.
```suggestion
import type { Role } from "@/lib/role"
```
How can I resolve this? If you propose a fix, please make it concise.| @@ -0,0 +1,78 @@ | |||
| "use client"; | |||
|
|
|||
| import type { Role } from "@/types/role"; | |||
There was a problem hiding this comment.
Import path @/types/role doesn't exist - there's no src/types/ directory.
| import type { Role } from "@/types/role"; | |
| import type { Role } from "@/lib/role"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/RolePermissions.tsx
Line: 3
Comment:
Import path `@/types/role` doesn't exist - there's no `src/types/` directory.
```suggestion
import type { Role } from "@/lib/role";
```
How can I resolve this? If you propose a fix, please make it concise.| FormMessage, | ||
| } from "@/components/ui/form"; | ||
|
|
||
| import type { Role } from "@/types/role"; |
There was a problem hiding this comment.
Import path @/types/role doesn't exist - there's no src/types/ directory.
| import type { Role } from "@/types/role"; | |
| import type { Role } from "@/lib/role"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(dashboard)/roles/_components/EditRoleDialog.tsx
Line: 28
Comment:
Import path `@/types/role` doesn't exist - there's no `src/types/` directory.
```suggestion
import type { Role } from "@/lib/role";
```
How can I resolve this? If you propose a fix, please make it concise.
rachid-hammami
force-pushed
the
feature/users-management
branch
from
1994da3 to
07ad57c
Compare
| "use client" | ||
|
|
||
| import { useState } from "react" | ||
| import { Input } from "@/components/ui/input" | ||
|
|
||
| interface SearchBarProps { | ||
| onSearch: (query: string) => void | ||
| } | ||
|
|
||
| export default function SearchBar({ onSearch }: SearchBarProps) { | ||
| const [query, setQuery] = useState("") | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const value = e.target.value | ||
| setQuery(value) | ||
| onSearch(value) | ||
| } | ||
|
|
||
| return ( | ||
| <div className="mb-4 w-full sm:w-1/2"> | ||
| <Input | ||
| type="text" | ||
| placeholder="Search users..." | ||
| value={query} | ||
| onChange={handleChange} | ||
| className="w-full" | ||
| /> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
Never imported or used anywhere. UsersTable implements its own search input inline at line 129-134.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(main)/dashboard/users/_components/SearchBar.tsx
Line: 1-30
Comment:
Never imported or used anywhere. UsersTable implements its own search input inline at line 129-134.
How can I resolve this? If you propose a fix, please make it concise.
| name: string | ||
| email: string | ||
| role: string | ||
| status: "Active" | "Disabled" |
There was a problem hiding this comment.
Type conflict: status is "Active" | "Disabled" (capitalized) here, but _data/users.ts:9 defines it as "active" | "pending" | "suspended" (lowercase). This mismatch causes rendering issues in UserBadge and UserDropdown components which expect the capitalized version.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/demo-users.ts
Line: 6
Comment:
Type conflict: status is "Active" | "Disabled" (capitalized) here, but `_data/users.ts:9` defines it as "active" | "pending" | "suspended" (lowercase). This mismatch causes rendering issues in `UserBadge` and `UserDropdown` components which expect the capitalized version.
How can I resolve this? If you propose a fix, please make it concise.| "use client" | ||
|
|
||
| import { useState } from "react" | ||
| import { Input } from "@/components/ui/input" | ||
|
|
||
| interface SearchBarProps { | ||
| onSearch: (query: string) => void | ||
| } | ||
|
|
||
| export default function SearchBar({ onSearch }: SearchBarProps) { | ||
| const [query, setQuery] = useState("") | ||
|
|
||
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const value = e.target.value | ||
| setQuery(value) | ||
| onSearch(value) | ||
| } | ||
|
|
||
| return ( | ||
| <div className="mb-4 w-full sm:w-1/2"> | ||
| <Input | ||
| type="text" | ||
| placeholder="Search users..." | ||
| value={query} | ||
| onChange={handleChange} | ||
| className="w-full" | ||
| /> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
This component is never imported or used. UsersTable implements its own inline search at lines 129-134. Consider removing this unused file.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/(main)/dashboard/users/_components/SearchBar.tsx
Line: 1-30
Comment:
This component is never imported or used. `UsersTable` implements its own inline search at lines 129-134. Consider removing this unused file.
How can I resolve this? If you propose a fix, please make it concise.
| name: string | ||
| email: string | ||
| role: string | ||
| status: "Active" | "Disabled" |
There was a problem hiding this comment.
Type conflict: status uses "Active" | "Disabled" (capitalized) but _data/users.ts:9 defines it as "active" | "pending" | "suspended" (lowercase). This mismatch causes rendering issues in UserBadge and UserDropdown
| status: "Active" | "Disabled" | |
| status: "active" | "pending" | "suspended" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/demo-users.ts
Line: 6
Comment:
Type conflict: status uses `"Active" | "Disabled"` (capitalized) but `_data/users.ts:9` defines it as `"active" | "pending" | "suspended"` (lowercase). This mismatch causes rendering issues in `UserBadge` and `UserDropdown`
```suggestion
status: "active" | "pending" | "suspended"
```
How can I resolve this? If you propose a fix, please make it concise.| export async function patchUser(id: number, payload: Record<string, any>) { | ||
| try { | ||
| const res = await fetch(`/api/users/${id}`, { | ||
| method: "PATCH", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(payload), | ||
| }) | ||
|
|
||
| // Si le backend répond, on retourne la vraie donnée | ||
| if (res.ok) { | ||
| const data = await res.json() | ||
| console.info("✅ Backend response:", data) | ||
| return data | ||
| } | ||
|
|
||
| // Sinon on passe en mode "mock" | ||
| throw new Error("Backend not available") | ||
| } catch (error) { | ||
| console.warn("⚠️ No backend detected — using mock mode.") | ||
| // Simule un retour identique à ce qu'enverrait une vraie API | ||
| await new Promise((r) => setTimeout(r, 400)) // petit délai simulé | ||
| return { id, ...payload } | ||
| } | ||
| } |
There was a problem hiding this comment.
PR claims to remove backend detection logic, but patchUser still includes try/catch with backend fallback. This contradicts the goal of making it "fully frontend-only"
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/api.ts
Line: 9-32
Comment:
PR claims to remove backend detection logic, but `patchUser` still includes try/catch with backend fallback. This contradicts the goal of making it "fully frontend-only"
How can I resolve this? If you propose a fix, please make it concise.
Additional Comments (2)
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/(main)/dashboard/users/_components/SearchBar.tsx
Line: 1-31
Comment:
Unused component - `UsersTable` implements its own inline search at lines 132-136. Remove this file
How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/demo-users.ts
Line: 1-46
Comment:
This entire file is redundant - the canonical data source is `_data/users.ts`. Having two conflicting User type definitions causes type errors throughout the codebase
How can I resolve this? If you propose a fix, please make it concise. |
|
All review comments have been addressed. The module is now fully isolated, strictly typed, and aligned with the current dashboard architecture. Happy to adjust further if needed. |
|
Here as well, please improve the overall design since it feels too plain. Try using the Interface Design skill: https://www.ui-skills.com/skills/interface-design/. Also, make sure it opens within the dashboard layout, not as a separate page. |
rachid-hammami
force-pushed
the
feature/users-management
branch
from
11503a3 to
d8793ee
Compare
|
Hi! I addressed the review feedback and fixed the missing meta issue, so CRUD actions are now fully functional. Let me know if anything else needs adjustment 🙂 |
…démo, documentation et intégration du tableau des utilisateurs
…dundant layout, fix package.json formatting
rachid-hammami
force-pushed
the
feature/users-management
branch
from
36646ff to
3391c8c
Compare
2 participants
Summary
This PR refactors the Users Management module into a fully frontend-only, self-contained feature based on static demo data.
The goal is to remove backend detection, RBAC coupling, and environment-based logic to ensure architectural clarity and independence from external systems.
What Changed
Architecture
api.tsusagestrict: true)Data
src/app/(main)/dashboard/users/_data/users.tsCleanup
SearchBarAssignRoleDialogusers/layout.tsxToasterconfiguration to avoid app-wide side effectspackage.jsonformatting issueType Safety
Row<User>typing inUserActionsanyResult
The Users module is now:
Notes
Review Status
Greptile Summary
This PR successfully refactors the Users Management module into a fully self-contained, frontend-only feature. It replaces the previous backend-detection / RBAC-coupled approach with a clean static dataset, properly typed components, and wired CRUD handlers via TanStack Table's
metaAPI.Key changes:
_data/users.tswith canonicalUser,UserRole, andUserStatustypes (lowercase, strict)UsersTablenow passes all mutation handlers (onEdit,onDelete,onSuspend,onUpdate) throughuseReactTable'smeta, resolving the previously reported disconnected CRUD issueUserBadgecorrectly handles all threeUserStatusvalues with distinct colour classesUserActionsuses properRow<User>typing andChangeEvent<HTMLInputElement>— strict-mode compliant/dashboard/coming-soonto/dashboard/userspage.tsxdisplay hardcoded numbers (124 total / 98 active / 6 pending) that bear no relation to the 5 static users in_data/users.ts, and will not update when rows are mutated in the tableConfidence Score: 4/5
Safe to merge after fixing the misleading hardcoded stats in the page header
All previously reported critical issues (disconnected meta handlers, type mismatches, implicit any, broken imports) have been resolved. One new P2 issue remains: the stats cards show fabricated numbers (124/98/6) that contradict the 5-user static dataset and are permanently frozen regardless of table mutations. This is minor enough that it doesn't block a demo module, but it's visible to every user of the page and trivial to fix.
src/app/(main)/dashboard/users/page.tsx — stats cards show hardcoded numbers disconnected from actual data
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["_data/users.ts\n(5 static users + types)"] -->|"initial state"| B["UsersTable (client)\nuseState users, search, sorting"] B -->|"meta: onEdit/onDelete\nonSuspend/onUpdate"| C["useReactTable\n(TanStack Table)"] C -->|"columns def"| D["columns.tsx"] D --> E["UserDropdown\n(role + status)"] D --> F["UserBadge\n(status display)"] D --> G["UserActions\n(edit dialog / delete / suspend)"] E -->|"onUpdate(id, field, value)"| B G -->|"onEdit / onDelete / onSuspend"| B H["page.tsx (server)\nHardcoded stats 124/98/6"] -.->|"NOT derived from data"| A B -->|rendered in| HComments Outside Diff (3)
package-lock.json, line 1 (link)package-lock.jsonemptied — breaks reproducible installsThe file has been reduced to an empty blob (all 10,354 lines deleted). Without a valid lock file,
npm ciwill fail outright, andnpm installwill resolve packages non-deterministically — different developers and CI environments may end up with different dependency versions. This is a reliability and security risk (a silently upgraded transitive dependency could introduce a breaking change or vulnerability).Please restore the lock file by running
npm installlocally and committing the regeneratedpackage-lock.json.Prompt To Fix With AI
package-lock.json, line 1 (link)package-lock.jsonemptied — breaksnpm ciThe file went from 10,354 lines of valid lockfile JSON to an empty file (git blob
e69de29). While this removes the lockfile noise from the diff, an emptypackage-lock.jsonis not valid JSON and will causenpm cito fail immediately in any CI pipeline or fresh-clone workflow that uses it for reproducible installs.If the intent was to stop tracking the lockfile, the correct approach is to add it to
.gitignore(and delete it from the repo viagit rm --cached package-lock.json), not to replace it with an empty file. If the intent was to regenerate it, the regenerated lockfile should be committed instead.Prompt To Fix With AI
package-lock.json, line 1 (link)Lock file entirely deleted — reproducible builds broken
package-lock.jsonhas been replaced with an empty file (the resulting blobe69de29is git's empty-file hash). Without a lock filenpm ciwill fail outright, andnpm installwill resolve every semver range to whatever version is current at install time. This breaks reproducible builds for all contributors and CI pipelines.The file should be regenerated from the current
package.jsonvianpm installso that exact dependency versions are pinned again, and the resulting file committed.Prompt To Fix All With AI
Reviews (16): Last reviewed commit: "fix(users): wire CRUD actions via table ..." | Re-trigger Greptile