-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Revert: refactor project member page" #8476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export * from "./root"; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import { observer } from "mobx-react"; | ||
|
|
||
| type TWorkspaceSettingsRightSidebarProps = { workspaceSlug: string }; | ||
|
|
||
| export const WorkspaceSettingsRightSidebar = observer(function WorkspaceSettingsRightSidebar( | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| props: TWorkspaceSettingsRightSidebarProps | ||
| ) { | ||
| return <></>; | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,14 +4,14 @@ import { computedFn } from "mobx-utils"; | |||||
| // types | ||||||
| import type { EUserPermissions } from "@plane/constants"; | ||||||
| import type { IWorkspaceBulkInviteFormData, IWorkspaceMember, IWorkspaceMemberInvitation } from "@plane/types"; | ||||||
| // plane-web imports | ||||||
| import type { RootStore } from "@/plane-web/store/root.store"; | ||||||
| // plane-web constants | ||||||
| // services | ||||||
| import { WorkspaceService } from "@/plane-web/services"; | ||||||
| // types | ||||||
| import type { IRouterStore } from "@/store/router.store"; | ||||||
| import type { IUserStore } from "@/store/user"; | ||||||
| // store | ||||||
| import type { CoreRootStore } from "../../root.store"; | ||||||
| import type { IMemberRootStore } from "../index.ts"; | ||||||
| import type { IWorkspaceMemberFiltersStore } from "./workspace-member-filters.store"; | ||||||
| import { WorkspaceMemberFiltersStore } from "./workspace-member-filters.store"; | ||||||
|
|
@@ -69,11 +69,10 @@ export class WorkspaceMemberStore implements IWorkspaceMemberStore { | |||||
| routerStore: IRouterStore; | ||||||
| userStore: IUserStore; | ||||||
| memberRoot: IMemberRootStore; | ||||||
| rootStore: RootStore; | ||||||
| // services | ||||||
| workspaceService; | ||||||
|
|
||||||
| constructor(_memberRoot: IMemberRootStore, _rootStore: RootStore) { | ||||||
| constructor(_memberRoot: IMemberRootStore, _rootStore: CoreRootStore) { | ||||||
| makeObservable(this, { | ||||||
| // observables | ||||||
| workspaceMemberMap: observable, | ||||||
|
|
@@ -96,7 +95,6 @@ export class WorkspaceMemberStore implements IWorkspaceMemberStore { | |||||
| this.routerStore = _rootStore.router; | ||||||
| this.userStore = _rootStore.user; | ||||||
| this.memberRoot = _memberRoot; | ||||||
| this.rootStore = _rootStore; | ||||||
| // services | ||||||
| this.workspaceService = new WorkspaceService(); | ||||||
| } | ||||||
|
|
@@ -261,7 +259,6 @@ export class WorkspaceMemberStore implements IWorkspaceMemberStore { | |||||
| set(this.workspaceMemberMap, [workspaceSlug, userId, "role"], data.role); | ||||||
| }); | ||||||
| await this.workspaceService.updateWorkspaceMember(workspaceSlug, memberDetails.id, data); | ||||||
| void this.rootStore.workspaceRoot.mutateWorkspaceMembersActivity(workspaceSlug); | ||||||
| } catch (error) { | ||||||
| // revert back to original members in case of error | ||||||
| runInAction(() => { | ||||||
|
|
@@ -279,11 +276,11 @@ export class WorkspaceMemberStore implements IWorkspaceMemberStore { | |||||
| removeMemberFromWorkspace = async (workspaceSlug: string, userId: string) => { | ||||||
| const memberDetails = this.getWorkspaceMemberDetails(userId); | ||||||
| if (!memberDetails) throw new Error("Member not found"); | ||||||
| await this.workspaceService.deleteWorkspaceMember(workspaceSlug, memberDetails?.id); | ||||||
| runInAction(() => { | ||||||
| set(this.workspaceMemberMap, [workspaceSlug, userId, "is_active"], false); | ||||||
| await this.workspaceService.deleteWorkspaceMember(workspaceSlug, memberDetails?.id).then(() => { | ||||||
| runInAction(() => { | ||||||
| set(this.workspaceMemberMap, [workspaceSlug, userId, "is_active"], false); | ||||||
| }); | ||||||
| }); | ||||||
| void this.rootStore.workspaceRoot.mutateWorkspaceMembersActivity(workspaceSlug); | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
|
|
@@ -304,9 +301,9 @@ export class WorkspaceMemberStore implements IWorkspaceMemberStore { | |||||
| * @param data | ||||||
| */ | ||||||
| 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; | ||||||
| }; | ||||||
|
Comment on lines
303
to
307
|
||||||
|
|
||||||
| /** | ||||||
|
|
@@ -320,7 +317,7 @@ export class WorkspaceMemberStore implements IWorkspaceMemberStore { | |||||
| invitationId: string, | ||||||
| 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 | ||||||
|
||||||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Duplicate and conflicting member updates outside
runInAction.Lines 318-320 use
update()withuniq()to deduplicate members, but lines 321-323 immediately overwrite this with a direct mutation that:runInActionblock (which ends at line 317), violating MobX patterns🔎 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