-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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.
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
[WEB-2711]fix: guest mentions and assignees #6315
Conversation
WalkthroughThis pull request introduces modifications across multiple files to enhance user mention and search functionality. The changes primarily focus on improving the context-aware retrieval of user details, particularly in relation to project and issue IDs. The modifications span both backend and frontend components, introducing a more robust approach to filtering and displaying user mentions, with an emphasis on project-specific member details and search capabilities. Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (9)
web/core/components/project/member-select.tsx (2)
13-13
: Clarify the usage ofEUserPermissions
.
The import forEUserPermissions
is fine, but ensure that any future role checks remain consistent with all user-permission logic across the codebase.
32-36
: Prevent role-based filtering issues.
You’re correctly excluding guests by returning early ifisGuest
. Confirm there’s no edge case where guest data might be needed.web/core/components/project/member-list.tsx (1)
39-41
: Refactor repeated calls
Here you createconst memberDetails
from the mappedsearchedMembers
. Consider memoizing or handling the logic once to avoid repeated calls togetProjectMemberDetails
. This might improve performance for large lists.web/core/store/member/project-member.store.ts (2)
39-39
: Use descriptive JSDoc for newly added parameterSince
getProjectMemberDetails
now accepts aprojectId
, it would be helpful to add/update JSDoc to describe how and why this parameter is used. This will improve clarity for other contributors.
113-116
: Remove or refineconsole.log
statementLeaving
console.log
in computed functions could unnecessarily clutter the console in production. Consider removing or guarding this log statement behind an environment or debug flag.- console.log({ projectMember });
web/core/components/issues/issue-detail/issue-activity/comments/comment-card.tsx (2)
25-25
: Document newly addedissueId
prop usageWhile the new
issueId
prop is self-explanatory, consider adding a short comment or JSDoc to highlight its importance for linking comments to specific issues, aiding future maintenance.
149-149
: Properly retrieve or validate issue_idPassing
issue_id={issueId}
is correct. On the receiving side, ensure it appropriately handles null or undefined scenarios in caseissueId
was not provided.apiserver/plane/app/views/search/base.py (2)
281-287
: Use placeholders for improved readabilityThe chained filters are valid. For complex queries, consider incrementally breaking them into more verbose logic or add short comments to clarify each filter's purpose.
325-335
: Include consistent else-block documentationThe else-block filters out any user whose role is not >10. Consider clarifying or documenting the rationale in code comments to avoid confusion about permission constraints or potential expansions in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apiserver/plane/app/views/search/base.py
(2 hunks)packages/types/src/search.d.ts
(1 hunks)web/core/components/command-palette/actions/issue-actions/change-assignee.tsx
(2 hunks)web/core/components/dropdowns/member/member-options.tsx
(4 hunks)web/core/components/editor/embeds/mentions/user.tsx
(2 hunks)web/core/components/editor/lite-text-editor/lite-text-editor.tsx
(3 hunks)web/core/components/issues/description-input.tsx
(1 hunks)web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx
(1 hunks)web/core/components/issues/issue-detail/issue-activity/comments/comment-card.tsx
(3 hunks)web/core/components/issues/issue-detail/issue-activity/comments/comment-create.tsx
(1 hunks)web/core/components/issues/issue-detail/issue-activity/comments/root.tsx
(1 hunks)web/core/components/issues/issue-modal/base.tsx
(1 hunks)web/core/components/project/member-list.tsx
(4 hunks)web/core/components/project/member-select.tsx
(3 hunks)web/core/components/project/settings/member-columns.tsx
(1 hunks)web/core/store/member/project-member.store.ts
(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/core/components/issues/issue-modal/base.tsx
🔇 Additional comments (27)
web/core/components/issues/issue-detail/issue-activity/comments/root.tsx (1)
39-39
: Prop injection looks good.
Passing theissueId
prop toIssueCommentCard
ensures each comment can reference the correct issue context. No issues found with this addition.packages/types/src/search.d.ts (1)
77-77
: Optional property addition is consistent.
Theissue_id?: string;
property cleanly extends the request payload for issue-related filtering. This complements the updated components that now handle and passissue_id
.web/core/components/project/member-select.tsx (3)
5-5
: Ensure correct usage ofnext/navigation
.
The import ofuseParams
fromnext/navigation
is a good approach, but verify that you only use it in Client Components. This file is marked"use client"
, so it’s valid.
23-24
: ValidateprojectId
usage.
TheuseParams
hook retrieves strings, so confirm thatprojectId.toString()
is safe. IfprojectId
can ever beundefined
, gracefully handle that to avoid runtime errors.
56-56
: Check default selection logic.
Retrieving member details withgetProjectMemberDetails(value, projectId.toString())
might fail ifvalue
is absent. Consider a fallback or conditional check to handle empty selection gracefully.web/core/components/command-palette/actions/issue-actions/change-assignee.tsx (2)
36-63
: Robust handling for missingprojectId
.
In lines 38–39, you gracefully skip ifprojectId
is undefined, which prevents lookups from crashing. This is a good defensive approach.
86-97
: Prevent rendering undefined options.
The filter checkoption && <Command.Item ...>
is good practice to avoid runtime errors. The overall item rendering logic looks clean.web/core/components/issues/issue-detail/issue-activity/activity-comment-root.tsx (1)
47-47
: PassissueId
parameter effectively
The addition ofissueId
prop toIssueCommentCard
aligns with the PR objective to identify comments by their specific issue context. This implementation looks correct, given the usage inIssueCommentCard
.web/core/components/editor/embeds/mentions/user.tsx (2)
22-23
: Check fallback for missingprojectId
WhenprojectId
is absent in the route, the code gracefully defaults tonull
; however, ensure there's no scenario that expects a validprojectId
and fails silently. Consider documenting or logging a warning if it’s expected to always have some project context.
49-49
: Restrict guest mentions if appropriate
Integration withgetProjectMemberDetails
for role-based checks is consistent with the PR objective to restrict guest mentions. Verify that there's additional coverage or a higher-level check ensuring that guests are not able to mention other users if they lack permission.web/core/components/project/member-list.tsx (3)
5-5
: Import usage is correct
ImportinguseParams
from"next/navigation"
is correct under Next.js 13+ standards. Continue ensuring consistent usage across the codebase.
18-19
: Ensure consistent naming
ExtractingprojectId
here is straightforward. Just ensure that other references throughout the file (like the variable in lines 39-41) are consistent in naming and logic.
30-30
: Handle null or guest user
When callinggetProjectMemberDetails(userId, projectId.toString())
, confirm that the function gracefully handles guests by returning a falsy member property if the user is indeed a guest. Ensure that the UI doesn't inadvertently reveal guests in lists if restricted by project policy.web/core/components/issues/issue-detail/issue-activity/comments/comment-create.tsx (1)
98-98
: Contextualizing comments byissue_id
Passing theissue_id
prop toLiteTextEditor
is aligned with the goal of restricting guest mentions to the relevant issue. Ensure that the editor’s mention search logic respects the user’s role and doesn’t expose the mention of other guests.web/core/components/editor/lite-text-editor/lite-text-editor.tsx (2)
33-33
: Introducing optionalissue_id
prop appears valid.
Allowing an optionalissue_id
aligns with the new requirement for context-specific mentions. Looks good.
42-42
: Verify usage ofissue_id
in search.
Passing theissue_id
tosearchEntity
ensures mentions are contextualized by the issue. Confirm that upstream or downstream calls (e.g., other references toLiteTextEditor
) supply a valid string when necessary.Also applies to: 63-63
web/core/components/issues/description-input.tsx (1)
128-128
: Confirmissue_id
inclusion insearchMentionCallback
.
AddingissueId
strengthens mention filtering. Ensure that it’s non-null in contexts where issue-based search is required.web/core/components/dropdowns/member/member-options.tsx (5)
20-20
: ImportedEUserPermissions
used effectively.
The import is relevant for filtering guests correctly.
43-43
: Expanded destructuring forgetProjectMemberDetails
.
Retrieving project member info is consistent with the newly required filtering logic.
82-104
: Exclude guest users from dropdown.
Filtering out guest users (isGuest
) ensures correct user selection based on roles. Confirm that logic properly handles scenarios with multiple roles or no role defined.
107-107
:filteredOptions
respects empty queries before filtering.
Implementation is sound. Ifquery
is empty, no user filtering is applied. This is a sensible approach for auto-suggest.
138-158
: Robust conditional rendering for search results.
Rendering conditions are appropriately handled to avoid null references. Good job safeguarding the UI from undefined states.web/core/components/project/settings/member-columns.tsx (1)
115-115
: Project-specific role retrieval is correct.
UsinggetProjectMemberDetails(currentUser.id, projectId)
ensures accurate role checks within the current project context. This is a proper fix to avoid mismatches with workspace-level roles.web/core/store/member/project-member.store.ts (2)
184-184
: Ensure robust error handling or fallbackWhen
memberDetails
isnull
, the code properly throws an error. Ensure any upstream code callingupdateMember
includes try-catch or.catch
logic to handle this situation gracefully in the UI or logs.
215-215
: Double-check concurrencyThe
removeMemberFromProject
method fetchesmemberDetails
and then deletes it. If there's a possible concurrency scenario where another process modifies the membership map in parallel, ensure these changes are atomic or well-synchronized to avoid inconsistent states.web/core/components/issues/issue-detail/issue-activity/comments/comment-card.tsx (1)
38-38
: Verify usage alignment with other componentsYou're passing
issueId
to the comment card here. Confirm that all other references to this props match the correct type (string) and usage.apiserver/plane/app/views/search/base.py (1)
309-324
: Refine logic for user mention with issue-based filteringBy combining
role__gt=10
ormember_id=issue_created_by
, guest users appear to be excluded except the issue creator. Confirm if other specialized roles need to be included or if there's a scenario for broader permissions.
Description
Updated mentions to restrict guest users from being mentioned in issues not created by them.
Updated member drop-downs to exclude guests from being assigned to issues, modules and cycles.
Type of Change
References
WEB-2711
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates