fix: workspace roles for settings and members button #6574
Conversation
WalkthroughThe change updates the conditional rendering logic in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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
🧹 Nitpick comments (1)
web/core/components/workspace/sidebar/dropdown-item.tsx (1)
88-108: LGTM! Security improvement by restricting admin features.The change correctly implements the principle of least privilege by restricting settings and member invitation access to workspace admins only. This is a security improvement over the previous implementation that allowed any role above guest to access these features.
Consider documenting this permission requirement in your security documentation to help maintain consistency across the application.
🧰 Tools
🪛 Biome (1.9.4)
[error] 87-108: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/core/components/workspace/sidebar/dropdown-item.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
prateekshourya29
requested review from
sriramveeraghanta and
pablohashescobar
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apiserver/plane/app/serializers/workspace.py(0 hunks)apiserver/plane/app/views/workspace/base.py(2 hunks)packages/types/src/workspace.d.ts(0 hunks)web/app/layout.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- apiserver/plane/app/serializers/workspace.py
- packages/types/src/workspace.d.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (3)
web/app/layout.tsx (1)
69-73: Consider a more targeted cache-busting strategy.While preventing caching ensures fresh workspace data (including roles), adding a timestamp on every render is aggressive and could impact performance. Consider these alternatives:
- Use cache-control headers on the API response instead
- Implement selective cache invalidation when roles change
- Add the timestamp parameter only when workspace data actually changes
Example implementation using cache-control headers in the API:
// In the API endpoint response.headers['Cache-Control'] = 'max-age=300, must-revalidate'; // Cache for 5 minutesLet's check if this endpoint is called frequently:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for references to the workspaces API endpoint rg -A 3 "api/users/me/workspaces" --type ts --type tsxLength of output: 82
I'll run another shell script that searches for the workspace API endpoint across both TS and TSX files without relying on pre-defined file types. This should give us a clearer picture of where and how frequently the endpoint is referenced.
🏁 Script executed:
#!/bin/bash rg -g '*.ts' -g '*.tsx' "api\/users\/me\/workspaces" -A 3Length of output: 155
Optimize Cache-Busting Strategy for Workspace API
The current implementation in
web/app/layout.tsxforces a new data fetch on every render by appending a timestamp (v=${Date.now()}) to the workspaces API URL. While this guarantees fresh data (useful for reflecting role changes immediately), it also aggressively bypasses browser caching, potentially increasing network requests and server load.Recommendations:
- Leverage Cache-Control Headers: Configure the API endpoint to return appropriate cache-control headers (e.g., a short TTL with revalidation) to allow controlled caching.
- Selective Cache Invalidation: Instead of appending a timestamp on every render, consider updating the query parameter only when a meaningful change is detected (e.g., when a user's role or workspace data is modified).
- Reevaluate Preload Necessity: Verify if preloading with a non-cached response is essential. If the workspace information doesn't change every second, a less aggressive cache-busting method might suffice.
The verification process (searching for additional references within the codebase) shows that this preload link appears to be the only direct usage of the workspaces API endpoint. Given this and the potential performance implications, the review comment's suggestion to optimize the cache-busting strategy is well-founded.
apiserver/plane/app/views/workspace/base.py (2)
191-191: LGTM! Efficient use of annotations.The code efficiently combines role and total_members annotations in a single query using Django's annotation system.
50-50: LGTM! Proper permission controls in place.The code maintains proper security through:
- Base permission class for the viewset
- Role-based permission decorators for specific actions
- Proper filtering of querysets based on user membership
Also applies to: 147-147, 151-151, 155-155
Description
This PR fixes the permission for settings and invite button in workspace switcher
Summary by CodeRabbit
New Features
Bug Fixes
Chores
total_issuesfield from the workspace serializer and interface, streamlining data handling.