packages/frontend/src/components/common/base/BaseSnackbar.vue (2)

13-13: Consider retaining helpful comments for maintainability.

The removed comment explained the countdown step calculation logic, which could be valuable for future maintenance. Consider keeping such explanatory comments, especially for mathematical calculations.

---

21-21: Consider retaining helpful comments for maintainability.

The removed comment documented the interval update frequency (100ms), which is useful context for understanding the countdown behavior.

packages/frontend/src/main.ts (1)

43-43: Consider selective retry configuration for different query types.

The global retry: false setting improves error handling control but disables retries for all queries. Consider if some queries (like fetching project data) might benefit from retries while authentication-related queries should fail fast.

You could configure retry behavior per query type:

// In individual services
useQuery({
  queryKey: ['projects'],
  queryFn: fetchProjects,
  retry: 3, // Override global setting for this query
})

packages/frontend/src/composables/useTryCatch.ts (2)

6-11: Consider making error messages configurable.

The error message is hardcoded to a generic message. Consider allowing custom error messages for better user experience.

-function showErrorSnackbar() {
+function showErrorSnackbar(message?: string) {
   showSnackbar({
-    message: 'Something went wrong, please try again.',
+    message: message || 'Something went wrong, please try again.',
     color: 'error',
   });
 }

---

13-19: Enhance function handling for better flexibility.

The current implementation has limitations: it doesn't handle return values, async functions, or provide error details for debugging.

Consider this more flexible implementation:

-function tryCatch(fn: () => unknown) {
+function tryCatch<T>(fn: () => T, errorMessage?: string): T | undefined {
   try {
-    fn();
+    return fn();
   } catch (error) {
+    console.error('tryCatch error:', error);
-    showErrorSnackbar();
+    showErrorSnackbar(errorMessage);
+    return undefined;
   }
 }
+
+async function tryCatchAsync<T>(fn: () => Promise<T>, errorMessage?: string): Promise<T | undefined> {
+  try {
+    return await fn();
+  } catch (error) {
+    console.error('tryCatchAsync error:', error);
+    showErrorSnackbar(errorMessage);
+    return undefined;
+  }
+}

packages/shared/src/lib/auth/index.ts (1)

34-43: Well-designed AccessControl type with good flexibility.

The type definition properly models access control permissions with:

• Clear permission levels using existing enums
• Flexible resource targeting via optional properties
• Proper metadata tracking (id, createdAt)
• Integration with existing shared types

However, consider adding validation to ensure at least one resource is specified:

 export type AccessControl = {
   id: number;
   permissionLevel: PermissionLevel;
   user: User;
   project?: Project;
   workspace?: Workspace;
   space?: Space;
   list?: List;
   createdAt: string;
-};
+} & (
+  | { project: Project; workspace?: never; space?: never; list?: never }
+  | { workspace: Workspace; project?: never; space?: never; list?: never }
+  | { space: Space; project?: never; workspace?: never; list?: never }
+  | { list: List; project?: never; workspace?: never; space?: never }
+);

This ensures exactly one resource type is specified while maintaining type safety.

packages/frontend/src/services/useAccessControlService.ts (1)

14-19: Consider enhancing query configuration for better caching behavior.

The service implementation is correct and follows established patterns. However, consider these improvements:

1. Query key specificity: The current key ['accessControl'] is global. Consider making it user-specific like ['accessControl', { userId }] if multiple users' permissions might be cached.

2. Stale time configuration: Adding a staleTime property would improve performance by reducing unnecessary refetches, similar to other services in the codebase.

  function useGetUserPermissions() {
    return useQuery({
-     queryKey: ['accessControl'],
+     queryKey: ['accessControl', 'me'],
      queryFn: getUserPermissions,
+     staleTime: 5 * 60 * 1000, // 5 minutes, consistent with other services
    });
  }

packages/frontend/src/services/useProjectUsersService.ts (1)

38-48: Clarify parameter naming for better code readability.

The delete function parameter projectUserId could be misleading. Based on the RESTful endpoint pattern /projects/${projectId}/users/${projectUserId}, this appears to be the ProjectUser entity ID, but the name suggests it's a User ID.

Consider renaming for clarity:

  async function deleteProjectUser({
    projectId,
-   projectUserId,
+   userId, // or projectUserEntityId if it's the ProjectUser.id
  }: {
    projectId: number;
-   projectUserId: number;
+   userId: number; // or projectUserEntityId: number
  }): Promise<void> {
-   return sendRequest(`/projects/${projectId}/users/${projectUserId}`, {
+   return sendRequest(`/projects/${projectId}/users/${userId}`, {
      method: 'DELETE',
    });
  }

packages/frontend/src/components/common/base/BaseSnackbarWrapper.vue (2)

11-11: Consider extracting closingSnackbars to the snackbar store for better state management.

Since this closing state is integral to the snackbar lifecycle, it would be more appropriate to manage it within the snackbar store itself rather than as local component state. This would centralize all snackbar-related state management.

---

37-40: Extract inline styles to a computed property for better maintainability.

The inline style object could be extracted to a computed property, especially if you plan to add more dynamic styles in the future.

+const getSnackbarStyle = (index: number) => ({
+  bottom: `${snackbarOffsets[index]}px`,
+  transition: 'all ease 0.2s',
+});

 <base-snackbar
   v-if="!closingSnackbars.has(snackbar.id)"
   :color="snackbar.options.color"
   :timeout="snackbar.options.timeout ?? 4000"
-  :style="{
-    bottom: `${snackbarOffsets[index]}px`,
-    transition: 'all ease 0.2s',
-  }"
+  :style="getSnackbarStyle(index)"
   @close="handleSnackbarClose(snackbar)"
 >

packages/frontend/src/stores/access.control.ts (1)

31-36: Consider returning PermissionLevel.NONE instead of null for consistency.

Since PermissionLevel.NONE exists in the enum, returning it instead of null would provide better type consistency and avoid null checks in consuming code.

 function getPermissionLevel(
   type: 'project' | 'workspace' | 'space' | 'list',
   id: number
-): PermissionLevel | null {
-  return permissionsMap.value[type]?.[id]?.permissionLevel ?? null;
+): PermissionLevel {
+  return permissionsMap.value[type]?.[id]?.permissionLevel ?? PermissionLevel.NONE;
 }

packages/frontend/src/components/common/settings/ProjectMembersSettings.vue (1)

40-50: Consider adding more member management actions to the menu.

The menu currently only has a delete action. You might want to add actions like changing roles, viewing member details, etc.

Do you want me to suggest additional menu items that could enhance the member management functionality?

packages/backend/src/app/common/auth/services/access.control.service.ts (3)

186-199: Sequential recursion may slow bulk grants

for ... of combined with await runs the DB write serially for every child item; granting hundreds of descendants could become noticeably slow.

-        for (const child of children) {
-            for (const item of child.items) {
-                await this.grantPermission(user, child.type, item.id, permissionLevel);
-            }
-        }
+        // Execute grants in parallel on each recursion layer
+        for (const child of children) {
+            await Promise.all(
+                child.items.map((item) =>
+                    this.grantPermission(user, child.type, item.id, permissionLevel),
+                ),
+            );
+        }

Using Promise.all preserves the recursive structure while significantly improving throughput.

---

217-226: Same performance caveat for revocations

Revocation mirrors the grant logic and inherits the same serial-execution penalty. Consider parallelising with Promise.all, analogous to the previous suggestion.

---

264-273: Eager-loading all relations could be heavy

findAllForUser fetches every access-control row together with project, workspace, space, and list relations. In large organisations this may:

• Load thousands of rows into memory.
• Cause N+1 queries if relations aren’t properly joined.

Consider adding pagination or allowing the caller to specify which relations it needs:

-    async findAllForUser(userId: number) {
-        return this.accessControlRepository.find({
-            where: { user: { id: userId } },
-            relations: ["project", "workspace", "space", "list"],
-        });
-    }
+    async findAllForUser(
+        userId: number,
+        relations: ("project" | "workspace" | "space" | "list")[] = [],
+    ) {
+        return this.accessControlRepository.find({
+            where: { user: { id: userId } },
+            relations,
+        });
+    }

This keeps the method flexible and avoids unnecessary load on the DB.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro