[WEB-5346]: refactor notifications to separate modules based on entities #8092
+1,004
−663
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…imports in notification_task
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
Contributor
WalkthroughThe changes introduce a refactored notification system that replaces a legacy, multi-branch notification pipeline with a centralized, handler-based architecture. A new Changes
Sequence Diagram(s)sequenceDiagram
participant IssueActivityTask as issue_activities_task
participant NotificationTask as process_issue_notifications<br/>(Celery Task)
participant IssueHandler as IssueNotificationHandler
participant DBModels as DB Models<br/>(Issue, Notification, etc.)
IssueActivityTask->>NotificationTask: Call with activities_data,<br/>actor_id, etc.
Note over NotificationTask: Create NotificationContext<br/>from arguments
NotificationTask->>IssueHandler: Instantiate with context
IssueHandler->>DBModels: load_entity, load_project,<br/>load_workspace, load_actor
IssueHandler->>IssueHandler: parse_activities +<br/>normalize_activity_data
IssueHandler->>IssueHandler: get_subscribers +<br/>process_entity_mentions
IssueHandler->>DBModels: create_in_app_notification,<br/>create_email_notification
IssueHandler->>NotificationTask: Return NotificationPayload
NotificationTask->>NotificationTask: Extract notification counts
NotificationTask->>IssueActivityTask: Return success dict with counts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…nd update imports
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the issue notification system by introducing an abstract base notification handler architecture. The changes replace a large monolithic notification function (~600 lines) with a modular, object-oriented design that separates concerns and improves code reusability.
Key Changes:
- Introduced
BaseNotificationHandlerabstract class for entity-agnostic notification logic - Implemented
IssueNotificationHandlerwith issue-specific notification behavior - Refactored the
process_issue_notificationsCelery task to use the new handler architecture
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/api/plane/utils/notifications/base.py | New abstract base class defining the notification handler interface and common logic |
| apps/api/plane/utils/notifications/issue.py | Issue-specific implementation of the notification handler with mention processing and email preferences |
| apps/api/plane/utils/notifications/init.py | Exports the notification handler classes and context |
| apps/api/plane/bgtasks/notification_task.py | Simplified Celery task that delegates to the new handler architecture |
| apps/api/plane/bgtasks/issue_activities_task.py | Updated to call the new notification task function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1582
to
1594
| process_issue_notifications( | ||
| issue_id=issue_id, | ||
| actor_id=actor_id, | ||
| project_id=project_id, | ||
| subscriber=subscriber, | ||
| issue_activities_created=json.dumps( | ||
| actor_id=actor_id, | ||
| activities_data=json.dumps( | ||
| IssueActivitySerializer(issue_activities_created, many=True).data, | ||
| cls=DjangoJSONEncoder, | ||
| ), | ||
| requested_data=requested_data, | ||
| current_instance=current_instance, | ||
| subscriber=subscriber, | ||
| notification_type=type, | ||
| ) |
There was a problem hiding this comment.
The function process_issue_notifications is missing the .delay() call which is required for Celery async task execution. Without .delay(), the task will execute synchronously, blocking the main thread.
Change:
process_issue_notifications(To:
process_issue_notifications.delay(
Comment on lines
+436
to
+450
| def build_notification_data(self, activity: ActivityData) -> Dict[str, Any]: | ||
| """Build notification data dictionary (can be overridden for entity-specific data)""" | ||
| return { | ||
| self.ENTITY_NAME: self.get_entity_data(), | ||
| f"{self.ENTITY_NAME}_activity": { | ||
| "id": str(activity.id), | ||
| "verb": str(activity.verb), | ||
| "field": str(activity.field), | ||
| "actor": str(activity.actor_id), | ||
| "new_value": str(activity.new_value), | ||
| "old_value": str(activity.old_value), | ||
| "old_identifier": str(activity.old_identifier) if activity.old_identifier else None, | ||
| "new_identifier": str(activity.new_identifier) if activity.new_identifier else None, | ||
| }, | ||
| } |
There was a problem hiding this comment.
The base class calls self.get_entity_data() on line 439 in build_notification_data, but this method is not defined as an abstract method in the base class. This will cause an AttributeError at runtime.
You should add get_entity_data as an abstract method in BaseNotificationHandler:
@abstractmethod
def get_entity_data(self) -> Dict[str, Any]:
"""Get entity data for notification payload"""
pass| subscriber_data[f"{self.ENTITY_NAME}_id"] = self.context.entity_id | ||
|
|
||
| self.SUBSCRIBER_MODEL.objects.get_or_create(**subscriber_data) | ||
| except Exception: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/api/plane/bgtasks/issue_activities_task.py(2 hunks)apps/api/plane/bgtasks/notification_task.py(1 hunks)apps/api/plane/utils/notifications/__init__.py(1 hunks)apps/api/plane/utils/notifications/base.py(1 hunks)apps/api/plane/utils/notifications/workitem.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
apps/api/plane/utils/notifications/__init__.py (2)
apps/api/plane/utils/notifications/base.py (1)
NotificationContext(50-61)apps/api/plane/utils/notifications/workitem.py (1)
WorkItemNotificationHandler(28-403)
apps/api/plane/bgtasks/notification_task.py (3)
apps/api/plane/db/models/project.py (1)
Project(65-159)apps/api/plane/utils/notifications/base.py (3)
NotificationContext(50-61)parse_activities(114-121)process(136-190)apps/api/plane/utils/exception_logger.py (1)
log_exception(9-20)
apps/api/plane/bgtasks/issue_activities_task.py (1)
apps/api/plane/bgtasks/notification_task.py (1)
process_issue_notifications(12-59)
apps/api/plane/utils/notifications/base.py (4)
apps/api/plane/db/models/notification.py (2)
EmailNotificationLog(97-125)UserNotificationPreference(57-94)apps/api/plane/db/models/user.py (1)
User(38-164)apps/api/plane/db/models/project.py (1)
Project(65-159)apps/api/plane/db/models/workspace.py (1)
Workspace(115-178)
apps/api/plane/utils/notifications/workitem.py (4)
apps/api/plane/db/models/issue.py (5)
Issue(104-250)IssueSubscriber(543-566)IssueMention(318-337)IssueAssignee(340-363)IssueComment(445-479)apps/api/plane/db/models/project.py (1)
ProjectMember(192-236)apps/api/plane/db/models/notification.py (1)
UserNotificationPreference(57-94)apps/api/plane/utils/notifications/base.py (5)
BaseNotificationHandler(90-542)SubscriberData(84-87)ActivityData(14-46)process_mentions(295-329)extract_mentions(276-293)
Comment on lines
+1582
to
1594
| process_issue_notifications( | ||
| issue_id=issue_id, | ||
| actor_id=actor_id, | ||
| project_id=project_id, | ||
| subscriber=subscriber, | ||
| issue_activities_created=json.dumps( | ||
| actor_id=actor_id, | ||
| activities_data=json.dumps( | ||
| IssueActivitySerializer(issue_activities_created, many=True).data, | ||
| cls=DjangoJSONEncoder, | ||
| ), | ||
| requested_data=requested_data, | ||
| current_instance=current_instance, | ||
| subscriber=subscriber, | ||
| notification_type=type, | ||
| ) |
Contributor
There was a problem hiding this comment.
Restore async task import after handler rename.
from plane.utils.notifications import IssueNotificationHandler now resolves to nothing because the package only exports NotificationContext and WorkItemNotificationHandler. The import here will raise ImportError, and the task never runs. Pick one: either re-export IssueNotificationHandler from the package, or switch this call site to import the new class (e.g. from plane.utils.notifications import WorkItemNotificationHandler as IssueNotificationHandler). Until then, every notification-triggering activity fails immediately.
🤖 Prompt for AI Agents
In apps/api/plane/bgtasks/issue_activities_task.py around lines 1582 to 1594,
the code assumes IssueNotificationHandler is exported from
plane.utils.notifications but that symbol no longer exists (package now exposes
NotificationContext and WorkItemNotificationHandler), causing ImportError and
preventing the task from running; update the import at the top of this file to
either re-export the old name or directly import the new handler (e.g. from
plane.utils.notifications import WorkItemNotificationHandler as
IssueNotificationHandler) and adjust any references accordingly so the task uses
the available WorkItemNotificationHandler (or re-export IssueNotificationHandler
in the package if you prefer keeping the old name).
Comment on lines
+8
to
+47
| from plane.utils.notifications import IssueNotificationHandler, NotificationContext | ||
| from plane.utils.exception_logger import log_exception | ||
|
|
||
| @shared_task | ||
| def notifications( | ||
| type, | ||
| def process_issue_notifications( | ||
| issue_id, | ||
| project_id, | ||
| actor_id, | ||
| subscriber, | ||
| issue_activities_created, | ||
| requested_data, | ||
| current_instance, | ||
| activities_data, | ||
| requested_data=None, | ||
| current_instance=None, | ||
| subscriber=False, | ||
| notification_type="" | ||
| ): | ||
| """ | ||
| Process notifications for issue activities. | ||
| """ | ||
| try: | ||
| issue_activities_created = ( | ||
| json.loads(issue_activities_created) if issue_activities_created is not None else None | ||
| # Let the handler normalize and parse activities | ||
| activities = IssueNotificationHandler.parse_activities(activities_data) | ||
|
|
||
| project = Project.objects.get(pk=project_id) | ||
| workspace_id = project.workspace_id | ||
|
|
||
| # Create context | ||
| context = NotificationContext( | ||
| entity_id=issue_id, | ||
| project_id=project_id, | ||
| workspace_id=workspace_id, | ||
| actor_id=actor_id, | ||
| activities=activities, | ||
| requested_data=requested_data, | ||
| current_instance=current_instance, | ||
| subscriber=subscriber, | ||
| notification_type=notification_type | ||
| ) | ||
| if type not in [ | ||
| "cycle.activity.created", | ||
| "cycle.activity.deleted", | ||
| "module.activity.created", | ||
| "module.activity.deleted", | ||
| "issue_reaction.activity.created", | ||
| "issue_reaction.activity.deleted", | ||
| "comment_reaction.activity.created", | ||
| "comment_reaction.activity.deleted", | ||
| "issue_vote.activity.created", | ||
| "issue_vote.activity.deleted", | ||
| "issue_draft.activity.created", | ||
| "issue_draft.activity.updated", | ||
| "issue_draft.activity.deleted", | ||
| ]: | ||
| # Create Notifications | ||
| bulk_notifications = [] | ||
| bulk_email_logs = [] | ||
|
|
||
| """ | ||
| Mention Tasks | ||
| 1. Perform Diffing and Extract the mentions, that mention notification needs to be sent | ||
| 2. From the latest set of mentions, extract the users which are not a subscribers & make them subscribers | ||
| """ | ||
|
|
||
| # get the list of active project members | ||
| project_members = ProjectMember.objects.filter(project_id=project_id, is_active=True).values_list( | ||
| "member_id", flat=True | ||
| ) | ||
|
|
||
| # Get new mentions from the newer instance | ||
| new_mentions = get_new_mentions(requested_instance=requested_data, current_instance=current_instance) | ||
| new_mentions = list(set(new_mentions) & {str(member) for member in project_members}) | ||
| removed_mention = get_removed_mentions(requested_instance=requested_data, current_instance=current_instance) | ||
|
|
||
| comment_mentions = [] | ||
| all_comment_mentions = [] | ||
|
|
||
| # Get New Subscribers from the mentions of the newer instance | ||
| requested_mentions = extract_mentions(issue_instance=requested_data) | ||
| mention_subscribers = extract_mentions_as_subscribers( | ||
| project_id=project_id, issue_id=issue_id, mentions=requested_mentions | ||
| ) | ||
|
|
||
| for issue_activity in issue_activities_created: | ||
| issue_comment = issue_activity.get("issue_comment") | ||
| issue_comment_new_value = issue_activity.get("new_value") | ||
| issue_comment_old_value = issue_activity.get("old_value") | ||
| if issue_comment is not None: | ||
| # TODO: Maybe save the comment mentions, so that in future, we can filter out the issues based on comment mentions as well. | ||
|
|
||
| all_comment_mentions = all_comment_mentions + extract_comment_mentions(issue_comment_new_value) | ||
|
|
||
| new_comment_mentions = get_new_comment_mentions( | ||
| old_value=issue_comment_old_value, | ||
| new_value=issue_comment_new_value, | ||
| ) | ||
| comment_mentions = comment_mentions + new_comment_mentions | ||
| comment_mentions = [ | ||
| mention for mention in comment_mentions if UUID(mention) in set(project_members) | ||
| ] | ||
|
|
||
| comment_mention_subscribers = extract_mentions_as_subscribers( | ||
| project_id=project_id, issue_id=issue_id, mentions=all_comment_mentions | ||
| ) | ||
| """ | ||
| We will not send subscription activity notification to the below mentioned user sets | ||
| - Those who have been newly mentioned in the issue description, we will send mention notification to them. | ||
| - When the activity is a comment_created and there exist a mention in the comment, | ||
| then we have to send the "mention_in_comment" notification | ||
| - When the activity is a comment_updated and there exist a mention change, | ||
| then also we have to send the "mention_in_comment" notification | ||
| """ | ||
|
|
||
| # --------------------------------------------------------------------------------------------------------- | ||
| issue_subscribers = list( | ||
| IssueSubscriber.objects.filter( | ||
| project_id=project_id, | ||
| issue_id=issue_id, | ||
| subscriber__in=Subquery(project_members), | ||
| ) | ||
| .exclude(subscriber_id__in=list(new_mentions + comment_mentions + [actor_id])) | ||
| .values_list("subscriber", flat=True) | ||
| ) | ||
|
|
||
| issue = Issue.objects.filter(pk=issue_id).first() | ||
|
|
||
| if subscriber: | ||
| # add the user to issue subscriber | ||
| try: | ||
| _ = IssueSubscriber.objects.get_or_create( | ||
| project_id=project_id, issue_id=issue_id, subscriber_id=actor_id | ||
| ) | ||
| except Exception: | ||
| pass | ||
|
|
||
| project = Project.objects.get(pk=project_id) | ||
|
|
||
| issue_assignees = IssueAssignee.objects.filter( | ||
| issue_id=issue_id, | ||
| project_id=project_id, | ||
| assignee__in=Subquery(project_members), | ||
| ).values_list("assignee", flat=True) | ||
|
|
||
| issue_subscribers = list(set(issue_subscribers) - {uuid.UUID(actor_id)}) | ||
|
|
||
| for subscriber in issue_subscribers: | ||
| if issue.created_by_id and issue.created_by_id == subscriber: | ||
| sender = "in_app:issue_activities:created" | ||
| elif subscriber in issue_assignees and issue.created_by_id not in issue_assignees: | ||
| sender = "in_app:issue_activities:assigned" | ||
| else: | ||
| sender = "in_app:issue_activities:subscribed" | ||
|
|
||
| preference = UserNotificationPreference.objects.get(user_id=subscriber) | ||
|
|
||
| for issue_activity in issue_activities_created: | ||
| # If activity done in blocking then blocked by email should not go | ||
| if issue_activity.get("issue_detail").get("id") != issue_id: | ||
| continue | ||
|
|
||
| # Do not send notification for description update | ||
| if issue_activity.get("field") == "description": | ||
| continue | ||
|
|
||
| # Check if the value should be sent or not | ||
| send_email = False | ||
| if issue_activity.get("field") == "state" and preference.state_change: | ||
| send_email = True | ||
| elif ( | ||
| issue_activity.get("field") == "state" | ||
| and preference.issue_completed | ||
| and State.objects.filter( | ||
| project_id=project_id, | ||
| pk=issue_activity.get("new_identifier"), | ||
| group="completed", | ||
| ).exists() | ||
| ): | ||
| send_email = True | ||
| elif issue_activity.get("field") == "comment" and preference.comment: | ||
| send_email = True | ||
| elif preference.property_change: | ||
| send_email = True | ||
| else: | ||
| send_email = False | ||
|
|
||
| # If activity is of issue comment fetch the comment | ||
| issue_comment = ( | ||
| IssueComment.objects.filter( | ||
| id=issue_activity.get("issue_comment"), | ||
| issue_id=issue_id, | ||
| project_id=project_id, | ||
| workspace_id=project.workspace_id, | ||
| ).first() | ||
| if issue_activity.get("issue_comment") | ||
| else None | ||
| ) | ||
|
|
||
| # Create in app notification | ||
| bulk_notifications.append( | ||
| Notification( | ||
| workspace=project.workspace, | ||
| sender=sender, | ||
| triggered_by_id=actor_id, | ||
| receiver_id=subscriber, | ||
| entity_identifier=issue_id, | ||
| entity_name="issue", | ||
| project=project, | ||
| title=issue_activity.get("comment"), | ||
| data={ | ||
| "issue": { | ||
| "id": str(issue_id), | ||
| "name": str(issue.name), | ||
| "identifier": str(issue.project.identifier), | ||
| "sequence_id": issue.sequence_id, | ||
| "state_name": issue.state.name, | ||
| "state_group": issue.state.group, | ||
| }, | ||
| "issue_activity": { | ||
| "id": str(issue_activity.get("id")), | ||
| "verb": str(issue_activity.get("verb")), | ||
| "field": str(issue_activity.get("field")), | ||
| "actor": str(issue_activity.get("actor_id")), | ||
| "new_value": str(issue_activity.get("new_value")), | ||
| "old_value": str(issue_activity.get("old_value")), | ||
| "issue_comment": str( | ||
| issue_comment.comment_stripped if issue_comment is not None else "" | ||
| ), | ||
| "old_identifier": ( | ||
| str(issue_activity.get("old_identifier")) | ||
| if issue_activity.get("old_identifier") | ||
| else None | ||
| ), | ||
| "new_identifier": ( | ||
| str(issue_activity.get("new_identifier")) | ||
| if issue_activity.get("new_identifier") | ||
| else None | ||
| ), | ||
| }, | ||
| }, | ||
| ) | ||
| ) | ||
| # Create email notification | ||
| if send_email: | ||
| bulk_email_logs.append( | ||
| EmailNotificationLog( | ||
| triggered_by_id=actor_id, | ||
| receiver_id=subscriber, | ||
| entity_identifier=issue_id, | ||
| entity_name="issue", | ||
| data={ | ||
| "issue": { | ||
| "id": str(issue_id), | ||
| "name": str(issue.name), | ||
| "identifier": str(issue.project.identifier), | ||
| "project_id": str(issue.project.id), | ||
| "workspace_slug": str(issue.project.workspace.slug), | ||
| "sequence_id": issue.sequence_id, | ||
| "state_name": issue.state.name, | ||
| "state_group": issue.state.group, | ||
| }, | ||
| "issue_activity": { | ||
| "id": str(issue_activity.get("id")), | ||
| "verb": str(issue_activity.get("verb")), | ||
| "field": str(issue_activity.get("field")), | ||
| "actor": str(issue_activity.get("actor_id")), | ||
| "new_value": str(issue_activity.get("new_value")), | ||
| "old_value": str(issue_activity.get("old_value")), | ||
| "issue_comment": str( | ||
| issue_comment.comment_stripped if issue_comment is not None else "" | ||
| ), | ||
| "old_identifier": ( | ||
| str(issue_activity.get("old_identifier")) | ||
| if issue_activity.get("old_identifier") | ||
| else None | ||
| ), | ||
| "new_identifier": ( | ||
| str(issue_activity.get("new_identifier")) | ||
| if issue_activity.get("new_identifier") | ||
| else None | ||
| ), | ||
| "activity_time": issue_activity.get("created_at"), | ||
| }, | ||
| }, | ||
| ) | ||
| ) | ||
|
|
||
| # -------------------------------------------------------------------------------------------------------- # | ||
|
|
||
| # Add Mentioned as Issue Subscribers | ||
| IssueSubscriber.objects.bulk_create( | ||
| mention_subscribers + comment_mention_subscribers, | ||
| batch_size=100, | ||
| ignore_conflicts=True, | ||
| ) | ||
|
|
||
| last_activity = IssueActivity.objects.filter(issue_id=issue_id).order_by("-created_at").first() | ||
|
|
||
| actor = User.objects.get(pk=actor_id) | ||
|
|
||
| for mention_id in comment_mentions: | ||
| if mention_id != actor_id: | ||
| preference = UserNotificationPreference.objects.get(user_id=mention_id) | ||
| for issue_activity in issue_activities_created: | ||
| notification = create_mention_notification( | ||
| project=project, | ||
| issue=issue, | ||
| notification_comment=f"{actor.display_name} has mentioned you in a comment in issue {issue.name}", # noqa: E501 | ||
| actor_id=actor_id, | ||
| mention_id=mention_id, | ||
| issue_id=issue_id, | ||
| activity=issue_activity, | ||
| ) | ||
|
|
||
| # check for email notifications | ||
| if preference.mention: | ||
| bulk_email_logs.append( | ||
| EmailNotificationLog( | ||
| triggered_by_id=actor_id, | ||
| receiver_id=mention_id, | ||
| entity_identifier=issue_id, | ||
| entity_name="issue", | ||
| data={ | ||
| "issue": { | ||
| "id": str(issue_id), | ||
| "name": str(issue.name), | ||
| "identifier": str(issue.project.identifier), | ||
| "sequence_id": issue.sequence_id, | ||
| "state_name": issue.state.name, | ||
| "state_group": issue.state.group, | ||
| "project_id": str(issue.project.id), | ||
| "workspace_slug": str(issue.project.workspace.slug), | ||
| }, | ||
| "issue_activity": { | ||
| "id": str(issue_activity.get("id")), | ||
| "verb": str(issue_activity.get("verb")), | ||
| "field": str("mention"), | ||
| "actor": str(issue_activity.get("actor_id")), | ||
| "new_value": str(issue_activity.get("new_value")), | ||
| "old_value": str(issue_activity.get("old_value")), | ||
| "old_identifier": ( | ||
| str(issue_activity.get("old_identifier")) | ||
| if issue_activity.get("old_identifier") | ||
| else None | ||
| ), | ||
| "new_identifier": ( | ||
| str(issue_activity.get("new_identifier")) | ||
| if issue_activity.get("new_identifier") | ||
| else None | ||
| ), | ||
| "activity_time": issue_activity.get("created_at"), | ||
| }, | ||
| }, | ||
| ) | ||
| ) | ||
| bulk_notifications.append(notification) | ||
|
|
||
| for mention_id in new_mentions: | ||
| if mention_id != actor_id: | ||
| preference = UserNotificationPreference.objects.get(user_id=mention_id) | ||
| if ( | ||
| last_activity is not None | ||
| and last_activity.field == "description" | ||
| and actor_id == str(last_activity.actor_id) | ||
| ): | ||
| bulk_notifications.append( | ||
| Notification( | ||
| workspace=project.workspace, | ||
| sender="in_app:issue_activities:mentioned", | ||
| triggered_by_id=actor_id, | ||
| receiver_id=mention_id, | ||
| entity_identifier=issue_id, | ||
| entity_name="issue", | ||
| project=project, | ||
| message=f"You have been mentioned in the issue {issue.name}", | ||
| data={ | ||
| "issue": { | ||
| "id": str(issue_id), | ||
| "name": str(issue.name), | ||
| "identifier": str(issue.project.identifier), | ||
| "sequence_id": issue.sequence_id, | ||
| "state_name": issue.state.name, | ||
| "state_group": issue.state.group, | ||
| "project_id": str(issue.project.id), | ||
| "workspace_slug": str(issue.project.workspace.slug), | ||
| }, | ||
| "issue_activity": { | ||
| "id": str(last_activity.id), | ||
| "verb": str(last_activity.verb), | ||
| "field": str(last_activity.field), | ||
| "actor": str(last_activity.actor_id), | ||
| "new_value": str(last_activity.new_value), | ||
| "old_value": str(last_activity.old_value), | ||
| "old_identifier": ( | ||
| str(issue_activity.get("old_identifier")) | ||
| if issue_activity.get("old_identifier") | ||
| else None | ||
| ), | ||
| "new_identifier": ( | ||
| str(issue_activity.get("new_identifier")) | ||
| if issue_activity.get("new_identifier") | ||
| else None | ||
| ), | ||
| }, | ||
| }, | ||
| ) | ||
| ) | ||
| if preference.mention: | ||
| bulk_email_logs.append( | ||
| EmailNotificationLog( | ||
| triggered_by_id=actor_id, | ||
| receiver_id=subscriber, | ||
| entity_identifier=issue_id, | ||
| entity_name="issue", | ||
| data={ | ||
| "issue": { | ||
| "id": str(issue_id), | ||
| "name": str(issue.name), | ||
| "identifier": str(issue.project.identifier), | ||
| "sequence_id": issue.sequence_id, | ||
| "state_name": issue.state.name, | ||
| "state_group": issue.state.group, | ||
| }, | ||
| "issue_activity": { | ||
| "id": str(last_activity.id), | ||
| "verb": str(last_activity.verb), | ||
| "field": "mention", | ||
| "actor": str(last_activity.actor_id), | ||
| "new_value": str(last_activity.new_value), | ||
| "old_value": str(last_activity.old_value), | ||
| "old_identifier": ( | ||
| str(issue_activity.get("old_identifier")) | ||
| if issue_activity.get("old_identifier") | ||
| else None | ||
| ), | ||
| "new_identifier": ( | ||
| str(issue_activity.get("new_identifier")) | ||
| if issue_activity.get("new_identifier") | ||
| else None | ||
| ), | ||
| "activity_time": str(last_activity.created_at), | ||
| }, | ||
| }, | ||
| ) | ||
| ) | ||
| else: | ||
| for issue_activity in issue_activities_created: | ||
| notification = create_mention_notification( | ||
| project=project, | ||
| issue=issue, | ||
| notification_comment=f"You have been mentioned in the issue {issue.name}", | ||
| actor_id=actor_id, | ||
| mention_id=mention_id, | ||
| issue_id=issue_id, | ||
| activity=issue_activity, | ||
| ) | ||
| if preference.mention: | ||
| bulk_email_logs.append( | ||
| EmailNotificationLog( | ||
| triggered_by_id=actor_id, | ||
| receiver_id=subscriber, | ||
| entity_identifier=issue_id, | ||
| entity_name="issue", | ||
| data={ | ||
| "issue": { | ||
| "id": str(issue_id), | ||
| "name": str(issue.name), | ||
| "identifier": str(issue.project.identifier), | ||
| "sequence_id": issue.sequence_id, | ||
| "state_name": issue.state.name, | ||
| "state_group": issue.state.group, | ||
| }, | ||
| "issue_activity": { | ||
| "id": str(issue_activity.get("id")), | ||
| "verb": str(issue_activity.get("verb")), | ||
| "field": str("mention"), | ||
| "actor": str(issue_activity.get("actor_id")), | ||
| "new_value": str(issue_activity.get("new_value")), | ||
| "old_value": str(issue_activity.get("old_value")), | ||
| "old_identifier": ( | ||
| str(issue_activity.get("old_identifier")) | ||
| if issue_activity.get("old_identifier") | ||
| else None | ||
| ), | ||
| "new_identifier": ( | ||
| str(issue_activity.get("new_identifier")) | ||
| if issue_activity.get("new_identifier") | ||
| else None | ||
| ), | ||
| "activity_time": issue_activity.get("created_at"), | ||
| }, | ||
| }, | ||
| ) | ||
| ) | ||
| bulk_notifications.append(notification) | ||
|
|
||
| # save new mentions for the particular issue and remove the mentions that has been deleted from the description # noqa: E501 | ||
| update_mentions_for_issue( | ||
| issue=issue, | ||
| project=project, | ||
| new_mentions=new_mentions, | ||
| removed_mention=removed_mention, | ||
| ) | ||
| # Bulk create notifications | ||
| Notification.objects.bulk_create(bulk_notifications, batch_size=100) | ||
| EmailNotificationLog.objects.bulk_create(bulk_email_logs, batch_size=100, ignore_conflicts=True) | ||
| return | ||
|
|
||
| # Process notifications | ||
| handler = IssueNotificationHandler(context) | ||
| payload = handler.process() |
Contributor
There was a problem hiding this comment.
Fix handler lookup for description mentions.
WorkItemNotificationHandler.process_entity_mentions() populates all_mentions for descriptions from the old description (current_instance). When a user introduces a new mention, it never shows up in all_mentions, so the subsequent create_subscribers(...) call skips it. Result: the mention recipient never becomes a subscriber and misses follow-up updates. Build all_mentions from the updated description (requested_data) instead (or union old/new) so new mentions are actually subscribed.
🤖 Prompt for AI Agents
In apps/api/plane/bgtasks/notification_task.py around lines 8 to 47, the
notification handler currently builds mention lists from the old description
(current_instance) so newly added mentions in requested_data are ignored; update
the mention-collection logic
(WorkItemNotificationHandler.process_entity_mentions) to derive description text
from the updated payload (context.requested_data) when present, or take the
union of mentions from both current_instance and requested_data, and then pass
that combined set into create_subscribers(...) so newly introduced mentions are
included as subscribers.
Comment on lines
+1
to
+7
| from .base import NotificationContext | ||
| from .workitem import WorkItemNotificationHandler | ||
|
|
||
| __all__ = [ | ||
| "NotificationContext", | ||
| "WorkItemNotificationHandler", | ||
| ] No newline at end of file |
Contributor
There was a problem hiding this comment.
Export the backward-compatible handler name.
Downstream code still imports IssueNotificationHandler, but this initializer only exposes WorkItemNotificationHandler. Without an alias/re-export (e.g. IssueNotificationHandler = WorkItemNotificationHandler and add it to __all__), those imports fail at runtime. Please restore the old symbol until callers are updated.
🤖 Prompt for AI Agents
In apps/api/plane/utils/notifications/__init__.py around lines 1 to 7,
downstream code still imports the old name IssueNotificationHandler but the
module only exports WorkItemNotificationHandler; add a backward-compatible alias
by assigning IssueNotificationHandler = WorkItemNotificationHandler and include
"IssueNotificationHandler" in the __all__ list so existing imports continue to
work until callers are updated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Type of Change
Test Scenarios
References
WEB-5346
Summary by CodeRabbit
Refactor
Bug Fixes