Skip to content

[WEB-3410] fix: work item permission and validation #6621

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
sriramveeraghanta merged 3 commits into from
Feb 17, 2025
Merged

[WEB-3410] fix: work item permission and validation #6621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb39859
fix: work item permission and validation
anmolsinghbhatia Feb 17, 2025
126c0b6
fix: command palette
anmolsinghbhatia Feb 17, 2025
fa17429
chore: code refactor
anmolsinghbhatia Feb 17, 2025
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
23 changes: 12 additions & 11 deletions web/ce/components/command-palette/modals/issue-level.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,22 @@ import useSWR from "swr";
import { BulkDeleteIssuesModal } from "@/components/core";
import { CreateUpdateIssueModal, DeleteIssueModal } from "@/components/issues";
// constants
import { ISSUE_DETAILS } from "@/constants/fetch-keys";
// hooks
import { useCommandPalette, useUser } from "@/hooks/store";
import { useCommandPalette, useIssueDetail, useUser } from "@/hooks/store";
import { useAppRouter } from "@/hooks/use-app-router";
import { useIssuesStore } from "@/hooks/use-issue-layout-store";
// services
import { IssueService } from "@/services/issue";

// services
const issueService = new IssueService();

export const IssueLevelModals = observer(() => {
// router
const pathname = usePathname();
const { workspaceSlug, projectId, issueId, cycleId, moduleId } = useParams();
const { workspaceSlug, projectId: paramsProjectId, workItem, cycleId, moduleId } = useParams();
const router = useAppRouter();
// store hooks
const { data: currentUser } = useUser();
const {
issues: { removeIssue },
} = useIssuesStore();
const { fetchIssueWithIdentifier } = useIssueDetail();
const {
isCreateIssueModalOpen,
toggleCreateIssueModal,
Expand All @@ -37,13 +32,19 @@ export const IssueLevelModals = observer(() => {
// derived values
const isDraftIssue = pathname?.includes("draft-issues") || false;

const projectIdentifier = workItem?.toString().split("-")[0];
const sequence_id = workItem?.toString().split("-")[1];
Comment on lines +35 to +36
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 17, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Validate workItem split for robust error handling.
Splitting workItem?.toString() by - may cause runtime errors if workItem is undefined or if the string doesn't contain a dash. Add suitable checks or default values.


const { data: issueDetails } = useSWR(
workspaceSlug && projectId && issueId ? ISSUE_DETAILS(issueId as string) : null,
workspaceSlug && projectId && issueId
? () => issueService.retrieve(workspaceSlug as string, projectId as string, issueId as string)
workspaceSlug && workItem ? `ISSUE_DETAIL_${workspaceSlug}_${projectIdentifier}_${sequence_id}` : null,
workspaceSlug && workItem
? () => fetchIssueWithIdentifier(workspaceSlug.toString(), projectIdentifier, sequence_id)
: null
);

const issueId = issueDetails?.id;
const projectId = paramsProjectId ?? issueDetails?.project_id;

return (
<>
<CreateUpdateIssueModal
Expand Down
5 changes: 4 additions & 1 deletion web/core/components/command-palette/actions/issue-actions/actions-list.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ type Props = {
export const CommandPaletteIssueActions: React.FC<Props> = observer((props) => {
const { closePalette, issueDetails, pages, setPages, setPlaceholder, setSearchTerm } = props;
// router
const { workspaceSlug, projectId, issueId } = useParams();
const { workspaceSlug } = useParams();
// hooks
const { updateIssue } = useIssueDetail();
const { toggleCommandPaletteModal, toggleDeleteIssueModal } = useCommandPalette();
const { data: currentUser } = useUser();
// derived values
const issueId = issueDetails?.id;
const projectId = issueDetails?.project_id;

const handleUpdateIssue = async (formData: Partial<TIssue>) => {
if (!workspaceSlug || !projectId || !issueDetails) return;
Expand Down
7 changes: 5 additions & 2 deletions web/core/components/command-palette/actions/issue-actions/change-assignee.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,15 @@ type Props = { closePalette: () => void; issue: TIssue };
export const ChangeIssueAssignee: React.FC<Props> = observer((props) => {
const { closePalette, issue } = props;
// router params
const { workspaceSlug, projectId } = useParams();
const { workspaceSlug } = useParams();
// store
const { updateIssue } = useIssueDetail();
const {
project: { projectMemberIds, getProjectMemberDetails },
project: { getProjectMemberIds, getProjectMemberDetails },
} = useMember();
// derived values
const projectId = issue?.project_id ?? "";
const projectMemberIds = getProjectMemberIds(projectId);

const options =
projectMemberIds
Expand Down
5 changes: 4 additions & 1 deletion web/core/components/command-palette/actions/issue-actions/change-priority.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ type Props = { closePalette: () => void; issue: TIssue };
export const ChangeIssuePriority: React.FC<Props> = observer((props) => {
const { closePalette, issue } = props;
// router params
const { workspaceSlug, projectId } = useParams();
const { workspaceSlug } = useParams();
// store hooks
const { updateIssue } = useIssueDetail();
// derived values
const projectId = issue?.project_id;

const submitChanges = async (formData: Partial<TIssue>) => {
if (!workspaceSlug || !projectId || !issue) return;
Expand Down
7 changes: 5 additions & 2 deletions web/core/components/command-palette/actions/issue-actions/change-state.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ type Props = { closePalette: () => void; issue: TIssue };
export const ChangeIssueState: React.FC<Props> = observer((props) => {
const { closePalette, issue } = props;
// router params
const { workspaceSlug, projectId } = useParams();
const { workspaceSlug } = useParams();
// store hooks
const { updateIssue } = useIssueDetail();
const { projectStates } = useProjectState();
const { getProjectStates } = useProjectState();
// derived values
const projectId = issue?.project_id;
const projectStates = getProjectStates(projectId);
Comment on lines +25 to +26
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 17, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Handle null projectId scenario.
Using issue?.project_id is more reliable, but if issue is undefined, getProjectStates(projectId) might return undefined or throw an error. Consider gracefully falling back or showing a more meaningful warning.


const submitChanges = async (formData: Partial<TIssue>) => {
if (!workspaceSlug || !projectId || !issue) return;
Expand Down
6 changes: 3 additions & 3 deletions web/core/components/command-palette/command-palette.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {

export const CommandPalette: FC = observer(() => {
// router params
const { workspaceSlug, projectId, issueId } = useParams();
const { workspaceSlug, projectId, workItem } = useParams();
// store hooks
const { toggleSidebar } = useAppTheme();
const { setTrackElement } = useEventTracker();
Expand All @@ -51,7 +51,7 @@ export const CommandPalette: FC = observer(() => {
const canPerformProjectAdminActions = allowPermissions([EUserPermissions.ADMIN], EUserPermissionsLevel.PROJECT);

const copyIssueUrlToClipboard = useCallback(() => {
if (!issueId) return;
if (!workItem) return;

const url = new URL(window.location.href);
copyTextToClipboard(url.href)
Expand All @@ -67,7 +67,7 @@ export const CommandPalette: FC = observer(() => {
title: "Some error occurred",
});
});
}, [issueId]);
}, [workItem]);

// auth
const performProjectCreateActions = useCallback(
Expand Down
2 changes: 1 addition & 1 deletion web/core/components/issues/delete-issue-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { useEffect, useState } from "react";
// types
import { PROJECT_ERROR_MESSAGES ,EUserPermissions, EUserPermissionsLevel } from "@plane/constants";
import { PROJECT_ERROR_MESSAGES, EUserPermissions, EUserPermissionsLevel } from "@plane/constants";
import { useTranslation } from "@plane/i18n";
import { TDeDupeIssue, TIssue } from "@plane/types";
// ui
Expand Down
10 changes: 4 additions & 6 deletions web/core/components/issues/issue-detail/issue-activity/activity/actions/label.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ export const IssueLabelActivity: FC<TIssueLabelActivity> = observer((props) => {
const {
activity: { getActivityById },
} = useIssueDetail();
const { projectLabels } = useLabel();
const { getLabelById } = useLabel();

const activity = getActivityById(activityId);
const oldLabelColor = getLabelById(activity?.old_identifier ?? "")?.color;
const newLabelColor = getLabelById(activity?.new_identifier ?? "")?.color;

if (!activity) return <></>;
return (
Expand All @@ -29,11 +31,7 @@ export const IssueLabelActivity: FC<TIssueLabelActivity> = observer((props) => {
{activity.old_value === "" ? `added a new label ` : `removed the label `}
<LabelActivityChip
name={activity.old_value === "" ? activity.new_value : activity.old_value}
color={
activity.old_value === ""
? projectLabels?.find((l) => l.id === activity.new_identifier)?.color
: projectLabels?.find((l) => l.id === activity.old_identifier)?.color
}
color={activity.old_value === "" ? newLabelColor : oldLabelColor}
/>
{showIssue && (activity.old_value === "" ? ` to ` : ` from `)}
{showIssue && <IssueLink activityId={activityId} />}
Expand Down
11 changes: 9 additions & 2 deletions web/core/components/issues/issue-detail/issue-detail-quick-actions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,17 @@ export const IssueDetailQuickActions: FC<Props> = observer((props) => {
};

// auth
const isEditable = allowPermissions([EUserPermissions.ADMIN, EUserPermissions.MEMBER], EUserPermissionsLevel.PROJECT);
const isEditable = allowPermissions(
[EUserPermissions.ADMIN, EUserPermissions.MEMBER],
EUserPermissionsLevel.PROJECT,
workspaceSlug,
projectId
);
const canRestoreIssue = allowPermissions(
[EUserPermissions.ADMIN, EUserPermissions.MEMBER],
EUserPermissionsLevel.PROJECT
EUserPermissionsLevel.PROJECT,
workspaceSlug,
projectId
);
const isArchivingAllowed = !issue?.archived_at && isEditable;
const isInArchivableGroup = !!stateDetails && ARCHIVABLE_STATE_GROUPS.includes(stateDetails?.group);
Expand Down