Skip to content

[WEB-2449] fix: admin is not able to edit issues in notifications peek overview #5877

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
pushya22 merged 4 commits into from
Oct 29, 2024
Merged

[WEB-2449] fix: admin is not able to edit issues in notifications peek overview #5877

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
208636d
fix backend
sharma01ketan Oct 21, 2024
a24753a
fix missing arguments for allow permissions
sharma01ketan Oct 21, 2024
423b8b1
Revert "fix backend"
sharma01ketan Oct 21, 2024
b82cdaa
Merge branch 'preview' into fix/new-intake-issue
sharma01ketan Oct 29, 2024
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
4 changes: 2 additions & 2 deletions web/core/components/inbox/content/root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ export const InboxContentRoot: FC<TInboxContentRoot> = observer((props) => {
);

const isEditable =
allowPermissions([EUserPermissions.ADMIN], EUserPermissionsLevel.PROJECT) ||
inboxIssue?.issue?.created_by === currentUser?.id;
allowPermissions([EUserPermissions.ADMIN], EUserPermissionsLevel.PROJECT, workspaceSlug, projectId) ||
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 21, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent Usage of allowPermissions Across Codebase

Multiple instances of allowPermissions are still using the old signature without workspaceSlug and projectId parameters. Please update these calls to ensure consistent and secure permission checks.

Examples:

  • web/core/components/project-states/group-item.tsx
  • web/core/components/workspace/sidebar/workspace-menu.tsx
  • web/core/components/workspace/settings/workspace-details.tsx
  • (and others as identified by the verification script)

Additionally, address the following TODO:

  • apiserver/plane/app/permissions/workspace.py: Move the permission logic to Python.
🔗 Analysis chain

Approve: Enhanced permission check with workspace and project context

The addition of workspaceSlug and projectId parameters to the allowPermissions function call improves the granularity of the permission check. This change aligns with the PR objective of fixing admin editing capabilities in the notifications peek overview.

To ensure consistency and catch potential issues, please run the following verification script:

This script will help identify any inconsistencies in the usage of allowPermissions across the codebase and verify the necessary imports.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of allowPermissions function across the codebase

# Test 1: Check for any remaining calls to allowPermissions without workspaceSlug and projectId
echo "Checking for outdated allowPermissions calls:"
rg "allowPermissions\(\[.*\],\s*EUserPermissionsLevel\.[A-Z]+\s*\)" -g '!*.test.*'

# Test 2: Verify the import of necessary constants
echo "Verifying imports:"
rg "import.*EUserPermissions.*EUserPermissionsLevel.*from.*plane-web/constants/user-permissions" -g '!*.test.*'

# Test 3: Check for any TODO or FIXME comments related to permissions
echo "Checking for TODO/FIXME comments related to permissions:"
rg "TODO|FIXME" -g '!*.test.*' | rg -i "permission"

Length of output: 23263

inboxIssue?.created_by === currentUser?.id;

const isGuest = projectPermissionsByWorkspaceSlugAndProjectId(workspaceSlug, projectId) === EUserPermissions.GUEST;
const isOwner = inboxIssue?.issue.created_by === currentUser?.id;
Expand Down