Skip to content

[WEB-2055] fix: enable admins to change role of other admins and add missing observers #5236

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.

Sign up for GitHub

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

Merged
SatishGandham merged 1 commit into from
Jul 29, 2024
Merged

[WEB-2055] fix: enable admins to change role of other admins and add missing observers #5236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 60 additions & 47 deletions web/core/components/project/settings/member-columns.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { observer } from "mobx-react";
import Link from "next/link";
import { Controller, useForm } from "react-hook-form";
import { Trash2 } from "lucide-react";
Expand All @@ -6,7 +7,7 @@ import { IUser, IWorkspaceMember } from "@plane/types";
import { CustomSelect, PopoverMenu, TOAST_TYPE, setToast } from "@plane/ui";
import { EUserProjectRoles } from "@/constants/project";
import { EUserWorkspaceRoles, ROLE } from "@/constants/workspace";
import { useMember } from "@/hooks/store";
import { useMember, useUser } from "@/hooks/store";

export interface RowData {
member: IWorkspaceMember;
Expand Down Expand Up @@ -80,62 +81,74 @@ export const NameColumn: React.FC<NameProps> = (props) => {
);
};

export const AccountTypeColumn: React.FC<AccountTypeProps> = (props) => {
export const AccountTypeColumn: React.FC<AccountTypeProps> = observer((props) => {
const { rowData, currentProjectRole, projectId, workspaceSlug } = props;
// form info
const {
control,
formState: { errors },
} = useForm();
// store hooks
const {
project: { updateMember },
} = useMember();
return rowData.role === EUserWorkspaceRoles.ADMIN || currentProjectRole !== EUserProjectRoles.ADMIN ? (
<div className="w-32 flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
) : (
<Controller
name="role"
control={control}
rules={{ required: "Role is required." }}
render={({ field: { value } }) => (
<CustomSelect
value={value}
onChange={(value: EUserProjectRoles) => {
if (!workspaceSlug) return;
const { data: currentUser } = useUser();

updateMember(workspaceSlug.toString(), projectId.toString(), rowData.member.id, {
role: value as unknown as EUserProjectRoles, // Cast value to unknown first, then to EUserWorkspaceRoles
}).catch((err) => {
console.log(err, "err");
const error = err.error;
const errorString = Array.isArray(error) ? error[0] : error;
// derived values
const isCurrentUser = currentUser?.id === rowData.member.id;
const isAdminRole = currentProjectRole === EUserProjectRoles.ADMIN;
const isRoleEditable = isCurrentUser && isAdminRole;

setToast({
type: TOAST_TYPE.ERROR,
title: "Error!",
message: errorString ?? "An error occurred while updating member role. Please try again.",
});
});
}}
label={
<div className="flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
}
buttonClassName={`!px-0 !justify-start hover:bg-custom-background-100 ${errors.role ? "border-red-500" : "border-none"}`}
className="rounded-md p-0 w-32"
optionsClassName="w-full"
input
>
{Object.keys(ROLE).map((item) => (
<CustomSelect.Option key={item} value={item as unknown as EUserProjectRoles}>
{ROLE[item as unknown as keyof typeof ROLE]}
</CustomSelect.Option>
))}
</CustomSelect>
return (
<>
{isRoleEditable ? (
<div className="w-32 flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
) : (
<Controller
name="role"
control={control}
rules={{ required: "Role is required." }}
render={({ field: { value } }) => (
<CustomSelect
value={value}
onChange={(value: EUserProjectRoles) => {
if (!workspaceSlug) return;

updateMember(workspaceSlug.toString(), projectId.toString(), rowData.member.id, {
role: value as unknown as EUserProjectRoles, // Cast value to unknown first, then to EUserWorkspaceRoles
}).catch((err) => {
console.log(err, "err");
const error = err.error;
const errorString = Array.isArray(error) ? error[0] : error;

setToast({
type: TOAST_TYPE.ERROR,
title: "Error!",
message: errorString ?? "An error occurred while updating member role. Please try again.",
});
});
}}
label={
<div className="flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
}
buttonClassName={`!px-0 !justify-start hover:bg-custom-background-100 ${errors.role ? "border-red-500" : "border-none"}`}
className="rounded-md p-0 w-32"
optionsClassName="w-full"
input
>
{Object.keys(ROLE).map((item) => (
<CustomSelect.Option key={item} value={item as unknown as EUserProjectRoles}>
{ROLE[item as unknown as keyof typeof ROLE]}
</CustomSelect.Option>
))}
</CustomSelect>
)}
/>
Comment on lines +109 to +150
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Role selection logic is well-implemented but improve error handling.

The role selection logic using Controller and CustomSelect is well-implemented. However, consider improving error handling by logging errors using a logging library instead of console.log.

- console.log(err, "err");
+ // Consider using a logging library for better error tracking
+ logger.error("Error updating member role", err);
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
<Controller
name="role"
control={control}
rules={{ required: "Role is required." }}
render={({ field: { value } }) => (
<CustomSelect
value={value}
onChange={(value: EUserProjectRoles) => {
if (!workspaceSlug) return;
updateMember(workspaceSlug.toString(), projectId.toString(), rowData.member.id, {
role: value as unknown as EUserProjectRoles, // Cast value to unknown first, then to EUserWorkspaceRoles
}).catch((err) => {
console.log(err, "err");
const error = err.error;
const errorString = Array.isArray(error) ? error[0] : error;
setToast({
type: TOAST_TYPE.ERROR,
title: "Error!",
message: errorString ?? "An error occurred while updating member role. Please try again.",
});
});
}}
label={
<div className="flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
}
buttonClassName={`!px-0 !justify-start hover:bg-custom-background-100 ${errors.role ? "border-red-500" : "border-none"}`}
className="rounded-md p-0 w-32"
optionsClassName="w-full"
input
>
{Object.keys(ROLE).map((item) => (
<CustomSelect.Option key={item} value={item as unknown as EUserProjectRoles}>
{ROLE[item as unknown as keyof typeof ROLE]}
</CustomSelect.Option>
))}
</CustomSelect>
)}
/>
}).catch((err) => {
// Consider using a logging library for better error tracking
logger.error("Error updating member role", err);
const error = err.error;
const errorString = Array.isArray(error) ? error[0] : error;
setToast({
type: TOAST_TYPE.ERROR,
title: "Error!",
message: errorString ?? "An error occurred while updating member role. Please try again.",
});
});

)}
/>
</>
);
};
});
109 changes: 61 additions & 48 deletions web/core/components/workspace/settings/member-columns.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { observer } from "mobx-react";
import Link from "next/link";
import { Controller, useForm } from "react-hook-form";
import { Trash2 } from "lucide-react";
Expand All @@ -6,7 +7,7 @@ import { IUser, IWorkspaceMember } from "@plane/types";
import { CustomSelect, PopoverMenu, TOAST_TYPE, setToast } from "@plane/ui";
import { EUserProjectRoles } from "@/constants/project";
import { EUserWorkspaceRoles, ROLE } from "@/constants/workspace";
import { useMember } from "@/hooks/store";
import { useMember, useUser } from "@/hooks/store";

export interface RowData {
member: IWorkspaceMember;
Expand Down Expand Up @@ -79,63 +80,75 @@ export const NameColumn: React.FC<NameProps> = (props) => {
);
};

export const AccountTypeColumn: React.FC<AccountTypeProps> = (props) => {
export const AccountTypeColumn: React.FC<AccountTypeProps> = observer((props) => {
const { rowData, currentWorkspaceRole, workspaceSlug } = props;
// form info
const {
control,
formState: { errors },
} = useForm();
// store hooks
const {
workspace: { updateMember },
} = useMember();
return rowData.role === EUserWorkspaceRoles.ADMIN || currentWorkspaceRole !== EUserWorkspaceRoles.ADMIN ? (
<div className="w-32 flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
) : (
<Controller
name="role"
control={control}
rules={{ required: "Role is required." }}
render={({ field: { value } }) => (
<CustomSelect
value={value}
onChange={(value: EUserProjectRoles) => {
console.log({ value, workspaceSlug }, "onChange");
if (!workspaceSlug) return;
const { data: currentUser } = useUser();

updateMember(workspaceSlug.toString(), rowData.member.id, {
role: value as unknown as EUserWorkspaceRoles, // Cast value to unknown first, then to EUserWorkspaceRoles
}).catch((err) => {
console.log(err, "err");
const error = err.error;
const errorString = Array.isArray(error) ? error[0] : error;
// derived values
const isCurrentUser = currentUser?.id === rowData.member.id;
const isAdminRole = currentWorkspaceRole === EUserWorkspaceRoles.ADMIN;
const isRoleEditable = isCurrentUser && isAdminRole;

setToast({
type: TOAST_TYPE.ERROR,
title: "Error!",
message: errorString ?? "An error occurred while updating member role. Please try again.",
});
});
}}
label={
<div className="flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
}
buttonClassName={`!px-0 !justify-start hover:bg-custom-background-100 ${errors.role ? "border-red-500" : "border-none"}`}
className="rounded-md p-0 w-32"
optionsClassName="w-full"
input
>
{Object.keys(ROLE).map((item) => (
<CustomSelect.Option key={item} value={item as unknown as EUserProjectRoles}>
{ROLE[item as unknown as keyof typeof ROLE]}
</CustomSelect.Option>
))}
</CustomSelect>
return (
<>
{isRoleEditable ? (
<div className="w-32 flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
) : (
<Controller
name="role"
control={control}
rules={{ required: "Role is required." }}
render={({ field: { value } }) => (
<CustomSelect
value={value}
onChange={(value: EUserProjectRoles) => {
console.log({ value, workspaceSlug }, "onChange");
if (!workspaceSlug) return;

updateMember(workspaceSlug.toString(), rowData.member.id, {
role: value as unknown as EUserWorkspaceRoles, // Cast value to unknown first, then to EUserWorkspaceRoles
}).catch((err) => {
console.log(err, "err");
const error = err.error;
const errorString = Array.isArray(error) ? error[0] : error;

setToast({
type: TOAST_TYPE.ERROR,
title: "Error!",
message: errorString ?? "An error occurred while updating member role. Please try again.",
});
});
}}
label={
<div className="flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
}
buttonClassName={`!px-0 !justify-start hover:bg-custom-background-100 ${errors.role ? "border-red-500" : "border-none"}`}
className="rounded-md p-0 w-32"
optionsClassName="w-full"
input
>
{Object.keys(ROLE).map((item) => (
<CustomSelect.Option key={item} value={item as unknown as EUserProjectRoles}>
{ROLE[item as unknown as keyof typeof ROLE]}
</CustomSelect.Option>
))}
</CustomSelect>
)}
/>
Comment on lines +108 to +150
Copy link
Contributor

@coderabbitai coderabbitai bot Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Role selection logic is well-implemented but improve error handling.

The role selection logic using Controller and CustomSelect is well-implemented. However, consider improving error handling by logging errors using a logging library instead of console.log.

- console.log(err, "err");
+ // Consider using a logging library for better error tracking
+ logger.error("Error updating member role", err);
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
<Controller
name="role"
control={control}
rules={{ required: "Role is required." }}
render={({ field: { value } }) => (
<CustomSelect
value={value}
onChange={(value: EUserProjectRoles) => {
console.log({ value, workspaceSlug }, "onChange");
if (!workspaceSlug) return;
updateMember(workspaceSlug.toString(), rowData.member.id, {
role: value as unknown as EUserWorkspaceRoles, // Cast value to unknown first, then to EUserWorkspaceRoles
}).catch((err) => {
console.log(err, "err");
const error = err.error;
const errorString = Array.isArray(error) ? error[0] : error;
setToast({
type: TOAST_TYPE.ERROR,
title: "Error!",
message: errorString ?? "An error occurred while updating member role. Please try again.",
});
});
}}
label={
<div className="flex ">
<span>{ROLE[rowData.role as keyof typeof ROLE]}</span>
</div>
}
buttonClassName={`!px-0 !justify-start hover:bg-custom-background-100 ${errors.role ? "border-red-500" : "border-none"}`}
className="rounded-md p-0 w-32"
optionsClassName="w-full"
input
>
{Object.keys(ROLE).map((item) => (
<CustomSelect.Option key={item} value={item as unknown as EUserProjectRoles}>
{ROLE[item as unknown as keyof typeof ROLE]}
</CustomSelect.Option>
))}
</CustomSelect>
)}
/>
}).catch((err) => {
// Consider using a logging library for better error tracking
logger.error("Error updating member role", err);
const error = err.error;
const errorString = Array.isArray(error) ? error[0] : error;
setToast({
type: TOAST_TYPE.ERROR,
title: "Error!",
message: errorString ?? "An error occurred while updating member role. Please try again.",
});
});

)}
/>
</>
);
};
});