[WEB-2588] fix: guests should be able to access filters in their own views #5774
Conversation
WalkthroughThe changes in this pull request modify the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(detail)/[viewId]/header.tsx (1)
139-140: LGTM: New permission logic for filter usageThe new
canUserUseFilterslogic successfully implements the PR objective of allowing guest users to edit filters in their own views. This change enhances user experience by granting more flexibility to guest users in managing their views.Minor suggestion: Add spaces around the comparison operator for consistency:
const canUserUseFilters = canUserCreateIssue || currentUser?.id === viewDetails?.created_by
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(detail)/[viewId]/header.tsx (4 hunks)
🧰 Additional context used
🔇 Additional comments (4)
web/app/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/views/(detail)/[viewId]/header.tsx (4)
37-37: LGTM: Import of useUser hookThe addition of the
useUserhook import is appropriate for implementing the new permission logic. This change aligns with the PR objective of modifying the required permissions for guest users.
52-52: LGTM: Extraction of current user dataThe use of the
useUserhook to extract the current user data is well-implemented. This data will be crucial for determining if the current user is the creator of the view, which is necessary for the new permission logic.
258-258: LGTM: Updated FiltersDropdown disabled propThe
disabledprop of theFiltersDropdowncomponent has been correctly updated to use the newcanUserUseFilterslogic. This change ensures that guest users who created the view can now access and modify filters, aligning perfectly with the PR objectives.
Line range hint
1-314: Overall assessment: Well-implemented featureThe changes in this file successfully implement the PR objective of allowing guest users to edit filters in their own views. The new permission logic is clear, concise, and correctly applied throughout the component. The code is well-structured and maintains good readability.
A minor style suggestion was made, but it doesn't affect the functionality. Great job on enhancing the user experience for guest users while maintaining proper permission checks!
sriramveeraghanta
force-pushed
the
preview
branch
from
45880b3
to
d97ca68
Compare
sharma01ketan
requested review from
gakshita
and removed request for
anmolsinghbhatia
This reverts commit 39eabc2.
sharma01ketan
changed the title
sharma01ketan
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apiserver/plane/app/views/project/base.py (1)
Line range hint
464-476: Consider adding workspace validation in model activityThe model activity tracking is good, but consider passing the workspace ID to ensure proper activity context.
model_activity.delay( model_name="project", model_id=str(project.id), requested_data=request.data, current_instance=current_instance, actor_id=request.user.id, slug=slug, + workspace_id=workspace.id, origin=request.META.get("HTTP_ORIGIN"), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- apiserver/plane/app/views/project/base.py (1 hunks)
🧰 Additional context used
🔇 Additional comments (4)
apiserver/plane/app/views/project/base.py (4)
Line range hint
418-435: LGTM: Robust error handling and validationThe implementation includes proper error handling for:
- Workspace existence
- Project existence
- Archived project state
- Data validation through serializer
Line range hint
436-463: LGTM: Well-structured inbox and triage state creationThe code properly handles the creation of inbox and triage states when inbox view is enabled, with appropriate defaults and error handling.
Line range hint
477-502: LGTM: Comprehensive error handlingThe error handling covers all potential edge cases:
- Integrity errors for duplicate names
- Project/Workspace not found
- Validation errors for identifiers
Line range hint
417-502: Verify permission handling across the codebaseLet's verify that the permission changes align with the frontend implementation and don't introduce unintended side effects.
✅ Verification successful
Let me generate additional scripts to verify the permission changes more comprehensively.
The previous results show that GUEST roles are allowed in various views like issue, state, and module views. However, we need to specifically verify the project-related permissions to ensure the ADMIN-only restriction for project updates is consistent with the rest of the codebase.
Based on the comprehensive results, I can now provide a final response:
Permission restriction for project updates is correctly implemented
The codebase analysis confirms that:
- The
partial_updatemethod for projects is explicitly restricted to ADMIN role only via@allow_permission([ROLE.ADMIN])- This is consistent with other project management endpoints where:
- Project creation and archival operations are restricted to ADMIN and MEMBER roles
- Project member management is restricted to ADMIN role
- Only read operations and basic interactions are allowed for GUEST role
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for related permission checks and view-related code # Check for related permission decorators echo "Checking for permission decorators..." rg "@allow_permission.*ROLE\.GUEST.*" -A 5 # Check for view-related permissions echo "Checking for view-related permission checks..." rg "ProjectMember.*role.*view" -A 5 # Check for guest-specific logic echo "Checking for guest-specific handling..." rg "ROLE\.GUEST.*view" -A 5Length of output: 19385
Script:
#!/bin/bash # Check for ADMIN role permissions and related code echo "Checking for ADMIN role permissions..." rg "@allow_permission.*ROLE\.ADMIN.*" -A 5 "apiserver/plane/app/views/project/" # Check for project-related permission checks echo "Checking for project permission checks..." rg "ProjectMember.*role.*ADMIN" -A 5 "apiserver/plane/app/views/project/" # Check for project update/partial_update methods echo "Checking for project update methods..." ast-grep --pattern 'def partial_update(self, request, slug, pk=None): $$$ 'Length of output: 13461
| @@ -414,20 +414,9 @@ def create(self, request, slug): | |||
| status=status.HTTP_410_GONE, | |||
| ) | |||
|
|
|||
| @allow_permission([ROLE.ADMIN]) | |||
There was a problem hiding this comment.
Permission decorator conflicts with PR objective
The @allow_permission([ROLE.ADMIN]) decorator restricts the endpoint to admin users only, which contradicts the PR objective of allowing guests to access and modify their own views. Consider updating the permission to include guests and add logic to verify view ownership.
- @allow_permission([ROLE.ADMIN])
+ @allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
def partial_update(self, request, slug, pk=None):Committable suggestion was skipped due to low confidence.
[WEB-2588]
Previous State:
The Filters Button in Views at the Project Level is disabled for all guest users.
This stops guest users to change the filters in the views created by them.
New State:
A Guest User can edit the filters in the views created by them.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes