Skip to content

fix: notify goal description mentions#4555

Merged
mancinux merged 1 commit into
mainfrom
codex/1219-description-mention-notifications
May 27, 2026
Merged

fix: notify goal description mentions#4555
mancinux merged 1 commit into
mainfrom
codex/1219-description-mention-notifications

Conversation

@mancinux

Copy link
Copy Markdown
Collaborator

resolves #1219

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In GoalCreated.dispatch/1, consider guarding against goal.description being nil (e.g., by defaulting to an empty rich text structure) before calling RichContent.find_mentioned_ids/2 to avoid edge-case crashes when a goal is created without a description.
  • For load_preloaded_resource(:goal, ids), you may want to normalize g.description (e.g., ensure it is non-nil or in a consistent rich-text shape) so the direct mention classifier doesn't have to handle nil or unexpected formats differently per resource type.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `GoalCreated.dispatch/1`, consider guarding against `goal.description` being `nil` (e.g., by defaulting to an empty rich text structure) before calling `RichContent.find_mentioned_ids/2` to avoid edge-case crashes when a goal is created without a description.
- For `load_preloaded_resource(:goal, ids)`, you may want to normalize `g.description` (e.g., ensure it is non-nil or in a consistent rich-text shape) so the direct mention classifier doesn't have to handle `nil` or unexpected formats differently per resource type.

## Individual Comments

### Comment 1
<location path="app/lib/operately/activities/notifications/goal_created.ex" line_range="8" />
<code_context>
     goal = Operately.Goals.get_goal!(goal_id)

-    people = [goal.champion_id, goal.reviewer_id]
+    mentioned_ids = RichContent.find_mentioned_ids(goal.description, :decode_ids)
+    people = [goal.champion_id, goal.reviewer_id | mentioned_ids]

</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against nil goal.description when passing it into RichContent.find_mentioned_ids.

If `goal.description` can be `nil` (e.g. for API-created or legacy goals), this call can raise. Please normalize to a safe default (e.g. `goal.description || ""` or a guard clause) before calling `RichContent.find_mentioned_ids/2`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread app/lib/operately/activities/notifications/goal_created.ex Outdated
@mancinux
mancinux force-pushed the codex/1219-description-mention-notifications branch from 5f3fdb0 to f437a1d Compare May 27, 2026 13:31
Signed-off-by: Luca Mancini <lmancini@operately.com>
@mancinux
mancinux force-pushed the codex/1219-description-mention-notifications branch from 2500c0f to 4c79834 Compare May 27, 2026 14:42
@mancinux
mancinux merged commit 20d592c into main May 27, 2026
4 checks passed
@mancinux
mancinux deleted the codex/1219-description-mention-notifications branch May 27, 2026 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trigger a notification when mentioning someone in a project/goal/milestone/task description

1 participant