Revert: refactor project member page" #8476
Conversation
This reverts commit c97e418.
📝 WalkthroughWalkthroughThis PR reorganizes workspace settings components by consolidating right-sidebar functionality under workspace instead of generic settings, removes unused member activity tracking components and methods, refactors WorkspaceMemberStore to use CoreRootStore, and integrates workspace member activity mutations into member management operations across multiple components. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Pull request overview
This PR reverts PR #8464, which refactored the project member page. The revert restores the previous architecture where workspace and project member activity mutations are handled at the component level rather than within the store methods.
Key changes:
- Removes centralized
mutateProjectMembersActivityand workspace member activity tracking from stores - Restores component-level mutation calls after member operations (add, remove, update roles)
- Reverts sidebar component organization from unified
SettingsRightSidebarback to separate workspace/project sidebars
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
apps/web/core/store/member/workspace/workspace-member.store.ts |
Reverts to CoreRootStore dependency, removes rootStore reference and centralized activity mutations |
apps/web/core/store/member/project/base-project-member.store.ts |
Removes abstract mutateProjectMembersActivity method and its calls from member operations |
apps/web/core/constants/fetch-keys.ts |
Restores parameter names without underscore prefix and removes PROJECT_MEMBER_ACTIVITY key |
apps/web/core/components/workspace/settings/members-list-item.tsx |
Adds back component-level mutateWorkspaceMembersActivity call after member removal |
apps/web/core/components/workspace/settings/member-columns.tsx |
Adds back activity mutation after role update |
apps/web/core/components/workspace/settings/invitations-list-item.tsx |
Adds back activity mutation after invitation deletion |
apps/web/core/components/project/member-list.tsx |
Removes ProjectMembersActivityButton and related imports, removes button size prop |
apps/web/ce/store/member/project-member.store.ts |
Removes no-op mutateProjectMembersActivity implementation |
apps/web/ce/components/workspace/right-sidebar/root.tsx |
Creates new WorkspaceSettingsRightSidebar stub component |
apps/web/ce/components/workspace/right-sidebar/index.ts |
New export file for workspace right sidebar |
apps/web/ce/components/settings/right-sidebar.tsx |
Removes unified SettingsRightSidebar component |
apps/web/ce/components/projects/members/members-activity-button.tsx |
Removes ProjectMembersActivityButton stub component |
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx |
Removes SettingsRightSidebar usage and observer wrapper |
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsx |
Adds back activity mutation after workspace invitation |
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsx |
Updates to use WorkspaceSettingsRightSidebar instead of SettingsRightSidebar |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| data: Partial<IWorkspaceMemberInvitation> | ||
| ) => { | ||
| const originalMemberInvitations = [...(this.workspaceMemberInvitations?.[workspaceSlug] ?? [])]; // in case of error, we will revert back to original members | ||
| const originalMemberInvitations = [...this.workspaceMemberInvitations?.[workspaceSlug]]; // in case of error, we will revert back to original members |
There was a problem hiding this comment.
The spread operator on line 320 may throw a runtime error if this.workspaceMemberInvitations?.[workspaceSlug] is undefined. The optional chaining will return undefined, and spreading undefined will cause an error. Consider using the nullish coalescing operator to provide a fallback empty array.
| const originalMemberInvitations = [...this.workspaceMemberInvitations?.[workspaceSlug]]; // in case of error, we will revert back to original members | |
| const originalMemberInvitations = [...(this.workspaceMemberInvitations?.[workspaceSlug] ?? [])]; // in case of error, we will revert back to original members |
| inviteMembersToWorkspace = async (workspaceSlug: string, data: IWorkspaceBulkInviteFormData) => { | ||
| await this.workspaceService.inviteWorkspace(workspaceSlug, data); | ||
| const response = await this.workspaceService.inviteWorkspace(workspaceSlug, data); | ||
| await this.fetchWorkspaceMemberInvitations(workspaceSlug); | ||
| void this.rootStore.workspaceRoot.mutateWorkspaceMembersActivity(workspaceSlug); | ||
| return response; | ||
| }; |
There was a problem hiding this comment.
The interface declares that inviteMembersToWorkspace returns Promise<void>, but the implementation now returns response. The return type in the interface should be updated to match the implementation, or the implementation should be changed to not return the response.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/store/member/workspace/workspace-member.store.ts (1)
50-50: Interface return type mismatch.The interface declares
inviteMembersToWorkspacereturningPromise<void>(line 50), but the implementation now returnsresponse(line 306). Update the interface to reflect the actual return type.🔎 Proposed fix for interface
- inviteMembersToWorkspace: (workspaceSlug: string, data: IWorkspaceBulkInviteFormData) => Promise<void>; + inviteMembersToWorkspace: (workspaceSlug: string, data: IWorkspaceBulkInviteFormData) => Promise<IWorkspaceMemberInvitation[]>;Note: Adjust the return type based on what
this.workspaceService.inviteWorkspaceactually returns.Also applies to: 303-307
🧹 Nitpick comments (3)
apps/web/core/components/workspace/settings/invitations-list-item.tsx (1)
62-62: Consider adding.toString()for type consistency.On line 56,
workspaceSlug.toString()is used when callingdeleteMemberInvitation. For consistency and type safety, consider doing the same here sinceworkspaceSlugfromuseParams()can bestring | string[].🔎 Proposed fix
- void mutateWorkspaceMembersActivity(workspaceSlug); + void mutateWorkspaceMembersActivity(workspaceSlug.toString());apps/web/core/constants/fetch-keys.ts (2)
157-158: Consider removing the unusedworkspaceSlugparameter entirely.The
_workspaceSlugprefix correctly indicates this parameter is unused, but since the function body doesn't reference it and similar project-level functions (lines 163-200) don't include this parameter, consider removing it entirely for a cleaner API.If the parameter must be retained for backward compatibility with existing callers, the current underscore prefix is appropriate.
🔎 Proposed simplification
-export const PROJECT_DETAILS = (workspaceSlug: string, projectId: string) => +export const PROJECT_DETAILS = (projectId: string) => `PROJECT_DETAILS_${projectId.toString().toUpperCase()}`;
160-161: Consider removing the unusedworkspaceSlugparameter entirely.Same as line 157-158: the
_workspaceSlugparameter is unused in the function body. Consider removing it entirely unless required for backward compatibility.🔎 Proposed simplification
-export const PROJECT_ME_INFORMATION = (workspaceSlug: string, projectId: string) => +export const PROJECT_ME_INFORMATION = (projectId: string) => `PROJECT_ME_INFORMATION_${projectId.toString().toUpperCase()}`;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsxapps/web/ce/components/projects/members/members-activity-button.tsxapps/web/ce/components/settings/right-sidebar.tsxapps/web/ce/components/workspace/right-sidebar/index.tsapps/web/ce/components/workspace/right-sidebar/root.tsxapps/web/ce/store/member/project-member.store.tsapps/web/core/components/project/member-list.tsxapps/web/core/components/workspace/settings/invitations-list-item.tsxapps/web/core/components/workspace/settings/member-columns.tsxapps/web/core/components/workspace/settings/members-list-item.tsxapps/web/core/constants/fetch-keys.tsapps/web/core/store/member/project/base-project-member.store.tsapps/web/core/store/member/workspace/workspace-member.store.ts
💤 Files with no reviewable changes (4)
- apps/web/ce/components/settings/right-sidebar.tsx
- apps/web/core/components/project/member-list.tsx
- apps/web/ce/components/projects/members/members-activity-button.tsx
- apps/web/ce/store/member/project-member.store.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsxapps/web/ce/components/workspace/right-sidebar/root.tsxapps/web/core/constants/fetch-keys.tsapps/web/ce/components/workspace/right-sidebar/index.tsapps/web/core/components/workspace/settings/members-list-item.tsxapps/web/core/store/member/project/base-project-member.store.tsapps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsxapps/web/core/components/workspace/settings/member-columns.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsxapps/web/core/components/workspace/settings/invitations-list-item.tsxapps/web/core/store/member/workspace/workspace-member.store.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Enable TypeScript strict mode and ensure all files are fully typed
Files:
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsxapps/web/ce/components/workspace/right-sidebar/root.tsxapps/web/core/constants/fetch-keys.tsapps/web/ce/components/workspace/right-sidebar/index.tsapps/web/core/components/workspace/settings/members-list-item.tsxapps/web/core/store/member/project/base-project-member.store.tsapps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsxapps/web/core/components/workspace/settings/member-columns.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsxapps/web/core/components/workspace/settings/invitations-list-item.tsxapps/web/core/store/member/workspace/workspace-member.store.ts
**/*.{js,jsx,ts,tsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier with Tailwind plugin for code formatting, run
pnpm fix:format
Files:
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsxapps/web/ce/components/workspace/right-sidebar/root.tsxapps/web/core/constants/fetch-keys.tsapps/web/ce/components/workspace/right-sidebar/index.tsapps/web/core/components/workspace/settings/members-list-item.tsxapps/web/core/store/member/project/base-project-member.store.tsapps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsxapps/web/core/components/workspace/settings/member-columns.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsxapps/web/core/components/workspace/settings/invitations-list-item.tsxapps/web/core/store/member/workspace/workspace-member.store.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,jsx,ts,tsx}: Use ESLint with shared config across packages, adhering to max warnings limits per package
Use camelCase for variable and function names, PascalCase for components and types
Use try-catch with proper error types and log errors appropriately
Files:
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsxapps/web/ce/components/workspace/right-sidebar/root.tsxapps/web/core/constants/fetch-keys.tsapps/web/ce/components/workspace/right-sidebar/index.tsapps/web/core/components/workspace/settings/members-list-item.tsxapps/web/core/store/member/project/base-project-member.store.tsapps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsxapps/web/core/components/workspace/settings/member-columns.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsxapps/web/core/components/workspace/settings/invitations-list-item.tsxapps/web/core/store/member/workspace/workspace-member.store.ts
🧠 Learnings (8)
📚 Learning: 2025-09-02T08:14:49.260Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7697
File: apps/web/app/(all)/[workspaceSlug]/(projects)/header.tsx:12-13
Timestamp: 2025-09-02T08:14:49.260Z
Learning: The star-us-link.tsx file in apps/web/app/(all)/[workspaceSlug]/(projects)/ already has "use client" directive at the top, making it a proper Client Component for hook usage.
Applied to files:
apps/web/ce/components/workspace/right-sidebar/root.tsx
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to **/package.json : Use `workspace:*` for internal packages and `catalog:` for external dependencies in imports
Applied to files:
apps/web/core/constants/fetch-keys.ts
📚 Learning: 2025-05-28T09:53:44.635Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: All role enums in this codebase (EUserPermissions, EUserWorkspaceRoles, EUserProjectRoles) use the same consistent numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. None of these enums have a value of 0, so truthy checks work correctly with these enum values.
Applied to files:
apps/web/core/constants/fetch-keys.ts
📚 Learning: 2025-05-28T09:53:44.635Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7094
File: web/core/store/user/base-permissions.store.ts:196-201
Timestamp: 2025-05-28T09:53:44.635Z
Learning: The `EUserPermissions` enum in this codebase uses numeric values: ADMIN = 20, MEMBER = 15, GUEST = 5. All roles in this enum are always numbers, not string values.
Applied to files:
apps/web/core/constants/fetch-keys.ts
📚 Learning: 2025-06-16T07:23:39.497Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7214
File: web/core/store/issue/helpers/base-issues.store.ts:117-117
Timestamp: 2025-06-16T07:23:39.497Z
Learning: In the updateIssueDates method of BaseIssuesStore (web/core/store/issue/helpers/base-issues.store.ts), the projectId parameter is intentionally made optional to support override implementations in subclasses. The base implementation requires projectId and includes an early return check, but making it optional allows derived classes to override the method with different parameter requirements.
Applied to files:
apps/web/core/store/member/project/base-project-member.store.ts
📚 Learning: 2025-12-12T15:20:36.542Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T15:20:36.542Z
Learning: Applies to packages/shared-state/**/*.{ts,tsx} : Maintain MobX stores in `packages/shared-state` using reactive patterns
Applied to files:
apps/web/core/store/member/project/base-project-member.store.tsapps/web/core/store/member/workspace/workspace-member.store.ts
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsxapps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsx
🧬 Code graph analysis (8)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsx (2)
apps/admin/core/hooks/store/use-workspace.tsx (1)
useWorkspace(6-10)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/constants/fetch-keys.ts (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/components/workspace/settings/members-list-item.tsx (2)
apps/admin/core/hooks/store/use-workspace.tsx (1)
useWorkspace(6-10)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/store/member/project/base-project-member.store.ts (1)
apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsx (2)
apps/web/ce/components/workspace/right-sidebar/root.tsx (1)
WorkspaceSettingsRightSidebar(5-10)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/components/workspace/settings/member-columns.tsx (2)
apps/admin/core/hooks/store/use-workspace.tsx (1)
useWorkspace(6-10)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/components/workspace/settings/invitations-list-item.tsx (2)
apps/admin/core/hooks/store/use-workspace.tsx (1)
useWorkspace(6-10)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
apps/web/core/store/member/workspace/workspace-member.store.ts (1)
apps/web/core/store/member/index.ts (1)
IMemberRootStore(13-22)
🪛 Biome (2.1.2)
apps/web/core/store/member/workspace/workspace-member.store.ts
[error] 320-320: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Build packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (10)
apps/web/ce/components/workspace/right-sidebar/index.ts (1)
1-1: LGTM!Standard barrel export pattern for the new workspace right-sidebar module.
apps/web/core/components/workspace/settings/members-list-item.tsx (1)
35-35: LGTM!The pattern of calling
mutateWorkspaceMembersActivityafter member removal is consistent with other components in this PR. Usingvoidto explicitly discard the promise is appropriate for fire-and-forget operations.Also applies to: 61-62
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/members/page.tsx (1)
43-43: LGTM!The integration of
mutateWorkspaceMembersActivityfollows the established pattern in this PR. Calling it after successful invitation ensures the workspace member activity state stays synchronized.Also applies to: 55-56
apps/web/ce/components/workspace/right-sidebar/root.tsx (1)
5-10: Placeholder component noted.This CE placeholder returns an empty fragment. The
observerwrapper and eslint-disable comment indicate this is intentional scaffolding, likely for enterprise edition to extend.apps/web/app/(all)/[workspaceSlug]/(settings)/settings/(workspace)/layout.tsx (1)
12-12: LGTM!The component replacement from
SettingsRightSidebartoWorkspaceSettingsRightSidebaris correctly applied, with the import path and prop passing updated appropriately.Also applies to: 51-51
apps/web/core/components/workspace/settings/member-columns.tsx (1)
19-19: LGTM!The
mutateWorkspaceMembersActivityintegration follows the consistent pattern established across this PR. Triggering the mutation after a successful role update ensures the activity state remains synchronized.Also applies to: 124-124, 159-159
apps/web/app/(all)/[workspaceSlug]/(settings)/settings/projects/[projectId]/layout.tsx (2)
13-33: Observer pattern correctly implemented.The observer wrapper is properly applied at the default export level, ensuring the component will reactively update when observable store data changes. The component structure follows Next.js App Router conventions with correct use of the
paramsprop for layout.tsx files.
13-33: Export pattern change is safe. No files in the codebase import the named exportProjectDetailSettingsLayout, so the change from named export to default export poses no risk to dependent code.apps/web/core/store/member/project/base-project-member.store.ts (1)
432-436: LGTM! Proper async state update pattern.The change correctly awaits the service call before updating state within
runInAction, ensuring that member removal only occurs after successful deletion from the backend. This prevents state inconsistency if the API call fails.apps/web/core/constants/fetch-keys.ts (1)
157-161: The removal ofPROJECT_MEMBER_ACTIVITYis safe. No remaining references to this constant exist in the codebase.
| update(this.projectRoot.projectMap, [projectId, "members"], (memberIds) => | ||
| uniq([...memberIds, ...data.members.map((m) => m.member_id)]) | ||
| ); | ||
| this.projectRoot.projectMap[projectId].members = this.projectRoot.projectMap?.[projectId]?.members?.concat( | ||
| data.members.map((m) => m.member_id) | ||
| ); | ||
| void this.mutateProjectMembersActivity(workspaceSlug, projectId); | ||
|
|
There was a problem hiding this comment.
Critical: Duplicate and conflicting member updates outside runInAction.
Lines 318-320 use update() with uniq() to deduplicate members, but lines 321-323 immediately overwrite this with a direct mutation that:
- Executes outside the
runInActionblock (which ends at line 317), violating MobX patterns - Doesn't deduplicate, potentially introducing duplicate member IDs
- Directly mutates observable state without proper action wrapping
- Makes the first update (lines 318-320) redundant
🔎 Proposed fix: Remove the redundant direct mutation
update(this.projectRoot.projectMap, [projectId, "members"], (memberIds) =>
uniq([...memberIds, ...data.members.map((m) => m.member_id)])
);
- this.projectRoot.projectMap[projectId].members = this.projectRoot.projectMap?.[projectId]?.members?.concat(
- data.members.map((m) => m.member_id)
- );
return response;Alternatively, if you need both updates for some reason, wrap the second one in runInAction:
update(this.projectRoot.projectMap, [projectId, "members"], (memberIds) =>
uniq([...memberIds, ...data.members.map((m) => m.member_id)])
);
- this.projectRoot.projectMap[projectId].members = this.projectRoot.projectMap?.[projectId]?.members?.concat(
- data.members.map((m) => m.member_id)
- );
+ runInAction(() => {
+ set(this.projectRoot.projectMap, [projectId, "members"],
+ uniq([...(this.projectRoot.projectMap?.[projectId]?.members ?? []), ...data.members.map((m) => m.member_id)])
+ );
+ });
return response;However, the first solution (removing lines 321-323) is preferred since the update() call already handles the mutation correctly.
🤖 Prompt for AI Agents
In apps/web/core/store/member/project/base-project-member.store.ts around lines
318 to 324, there is a redundant direct mutation that overwrites the prior
update: remove the direct assignment block (lines 321-323) that does
this.projectRoot.projectMap[projectId].members = ... since update(...) with
uniq(...) already performs the correct, deduplicated mutation inside the
runInAction; if you must keep a second assignment for some reason, instead wrap
it in runInAction and apply uniq to its result to avoid duplicates and avoid
mutating observables outside an action.
| data: Partial<IWorkspaceMemberInvitation> | ||
| ) => { | ||
| const originalMemberInvitations = [...(this.workspaceMemberInvitations?.[workspaceSlug] ?? [])]; // in case of error, we will revert back to original members | ||
| const originalMemberInvitations = [...this.workspaceMemberInvitations?.[workspaceSlug]]; // in case of error, we will revert back to original members |
There was a problem hiding this comment.
Unsafe optional chaining can throw TypeError.
If this.workspaceMemberInvitations?.[workspaceSlug] short-circuits to undefined, the spread operator will throw a TypeError: undefined is not iterable. The static analysis tool correctly flagged this.
🔎 Proposed fix
- const originalMemberInvitations = [...this.workspaceMemberInvitations?.[workspaceSlug]]; // in case of error, we will revert back to original members
+ const originalMemberInvitations = [...(this.workspaceMemberInvitations?.[workspaceSlug] ?? [])]; // in case of error, we will revert back to original members🧰 Tools
🪛 Biome (2.1.2)
[error] 320-320: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
🤖 Prompt for AI Agents
In apps/web/core/store/member/workspace/workspace-member.store.ts around line
320, the spread uses this.workspaceMemberInvitations?.[workspaceSlug] which can
be undefined and cause "undefined is not iterable"; change the expression to
default to an empty array before spreading (e.g., use nullish coalescing or
logical OR) so the spread always receives an iterable and preserves the original
invitations safely.
sriramveeraghanta
deleted the
revert-8464-chore-project-members-enchancements
branch
Reverts #8464
Summary by CodeRabbit
Release Notes
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.