-
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
[PE-93] regression: issue mentions and subscribers logic #6265
Conversation
WalkthroughThe pull request introduces modifications to the mention extraction and filtering logic in the Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (1)
apiserver/plane/bgtasks/notification_task.py (1)
143-145
: Consolidate repetitive mention extraction logic
The logic mirrors the one in lines 127-129. To adhere to the DRY principle, consider consolidating repeated mention-extraction code (in “extract_mentions” and “extract_comment_mentions”) into a single helper function, potentially parameterized for different HTML sources.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apiserver/plane/bgtasks/notification_task.py
(3 hunks)
🔇 Additional comments (2)
apiserver/plane/bgtasks/notification_task.py (2)
127-129
: Use of "entity_name" for user mention extraction
This correctly aligns the parsing logic with the updated attribute (“entity_name”: “user_mention”). Confirm that the front-end consistently uses the same “entity_name” to avoid missed or malformed mentions.
Consider the following script to locate any remaining “mention-component” references that might still rely on the old "target" attribute:
✅ Verification successful
Attribute "entity_name" with "user_mention" is consistently used across the codebase
The verification shows consistent usage of the "entity_name" attribute with "user_mention" value across both frontend and backend:
- Frontend editor components set this attribute in
web/core/hooks/use-editor-mention.tsx
- Frontend parsing logic in
web/core/hooks/use-parse-editor-content.ts
expects this attribute - Backend notification tasks in
apiserver/plane/bgtasks/notification_task.py
use the same attribute pattern - Editor package components in
packages/editor
maintain the same convention
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for mention-component usage with target attribute
rg -A 2 $'<mention-component[^>]*target="users"'
Length of output: 49
Script:
#!/bin/bash
# Search for mention-component usage in the codebase
rg -i "mention-component" -A 3
Length of output: 5264
Script:
#!/bin/bash
# Search for files containing mention-related code
fd -e js -e jsx -e ts -e tsx -e py . | xargs rg -l "mention"
Length of output: 4274
Script:
#!/bin/bash
# Search for entity_name attribute usage
rg "entity_name.*user_mention" -A 2
Length of output: 1384
261-263
: Efficient membership filtering approach
Using a set intersection here is concise and clear. Verify that “new_mentions” and “project_members” are consistently typed (e.g., both are strings or both are UUIDs). Mixing string-based identifiers with UUID objects could lead to empty intersections.
✅ Verification successful
Let me gather more information about the types and usage of new_mentions
and project_members
to verify the consistency.
From the results, I can see that:
project_members
is obtained from Django's ORM usingvalues_list("member_id", flat=True)
, which returns a list of member IDsnew_mentions
is constructed fromextract_mentions()
function and contains string values- Before the intersection,
project_members
values are explicitly converted to strings:{str(member) for member in project_members}
Let me check the extract_mentions
function to confirm the type of mentions it returns.
From the results, I can see the extract_mentions
function that parses HTML content to extract user mentions, and it appears to work with user IDs. Let me check one more thing to verify how these mentions are processed.
Types are properly handled in the mention intersection operation
The code is correctly handling type consistency in the set intersection operation:
extract_mentions()
returns a list of entity identifiers extracted from HTML mentionsproject_members
contains member IDs from the database- Before the intersection, there's an explicit conversion of project members to strings:
{str(member) for member in project_members}
- The intersection operation
set(new_mentions) & {str(member) for member in project_members}
ensures both sets contain string values
The type conversion is properly handled, preventing any potential empty intersections due to type mismatches.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Let's first look at how new_mentions is constructed and used
rg -B 5 -A 2 "new_mentions\s*=" apiserver/plane/bgtasks/notification_task.py
# Also check how project_members is defined and used
rg -B 5 -A 2 "project_members\s*=" apiserver/plane/bgtasks/notification_task.py
# Look for any type hints or docstrings that might indicate types
ast-grep --pattern 'def $FUNC($PARAMS):
$$$
new_mentions = $$$
$$$'
Length of output: 2067
Script:
#!/bin/bash
# Look for extract_mentions function definition and implementation
rg -B 2 -A 10 "def extract_mentions" apiserver/plane/bgtasks/notification_task.py
# Also check if there are any type hints in the file
rg "from typing" apiserver/plane/bgtasks/notification_task.py
Length of output: 1229
Script:
#!/bin/bash
# Look for the complete extract_mentions function implementation to see what it returns
rg -B 2 -A 20 "def extract_mentions" apiserver/plane/bgtasks/notification_task.py
# Also check for any mention-related type conversions
rg -A 2 "mention" apiserver/plane/bgtasks/notification_task.py | grep -E "str|uuid"
Length of output: 2541
Description
This PR fixes the logic to update issue mentions and subscribers.
Type of Change
Summary by CodeRabbit
New Features
Bug Fixes