Skip to content

fix: Display complete/reopen actions in milestone notification#4389

Merged
Rockyy174 merged 1 commit into
operately:mainfrom
Rockyy174:fix-milestone-notification
Apr 22, 2026
Merged

fix: Display complete/reopen actions in milestone notification#4389
Rockyy174 merged 1 commit into
operately:mainfrom
Rockyy174:fix-milestone-notification

Conversation

@Rockyy174
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Consider avoiding multiple Repo.preload calls for author and company by preloading nested associations in one call (e.g., Repo.preload(activity, author: :company)) to reduce unnecessary queries.
  • The headline_text/2 function raises on unknown action values; it may be safer to handle nil or unexpected values gracefully (e.g., defaulting to the comment-only headline) to avoid runtime errors when comment_action is missing or malformed.
  • The conditional logic that sets {excerpt_html, excerpt_text} could be made clearer by pattern matching on action (e.g., separate clauses for "none" vs others) instead of using an inline if, which would also avoid creating an unused map just for destructuring.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider avoiding multiple Repo.preload calls for author and company by preloading nested associations in one call (e.g., `Repo.preload(activity, author: :company)`) to reduce unnecessary queries.
- The `headline_text/2` function raises on unknown `action` values; it may be safer to handle `nil` or unexpected values gracefully (e.g., defaulting to the comment-only headline) to avoid runtime errors when `comment_action` is missing or malformed.
- The conditional logic that sets `{excerpt_html, excerpt_text}` could be made clearer by pattern matching on `action` (e.g., separate clauses for `"none"` vs others) instead of using an inline `if`, which would also avoid creating an unused map just for destructuring.

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.

Signed-off-by: Adriano Lazzaretti <lazzaretti136@gmail.com>
@Rockyy174 Rockyy174 merged commit bfea8a7 into operately:main Apr 22, 2026
3 checks passed
@Rockyy174 Rockyy174 deleted the fix-milestone-notification branch April 22, 2026 12:16
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.

1 participant