-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
ref(slack): Compact Slack issue alert message layout #105994
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
ref(slack): Compact Slack issue alert message layout #105994
Conversation
17702fa to
5f79552
Compare
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.
This test is very fragile, but would be a much bigger initiative to change for now, so I just resolved the indices. We were rendering the action_text block ('issue resolved by X') before the context on tags/counts -- when in reality it should be replacing the action block which is underneath it.
That correction, caused this rewrite :/
Add tests for the slack-compact-alerts feature flag behavior: - Verify AI summary displays as "Initial Guess:" context block - Verify suggested assignees appear in compact format Also fix nested quote syntax error in get_issue_summary_text(). Refs ECO-1315 Co-Authored-By: Claude <[email protected]>
Store the feature flag result in an instance variable at the start of build() instead of checking it 8 times throughout the method. Also add a basic test for the compact layout without AI summary dependencies. Refs ECO-1315 Co-Authored-By: Claude <[email protected]>
The compact alerts refactor moved the action_text block from before the context block to after it. Update test assertions to use the correct block indices (blocks[2] -> blocks[3] for action_text). Co-Authored-By: Claude <[email protected]>
Add validation to prevent appending an empty context block when both get_context() returns empty and there are no suggested assignees. This mirrors the guard logic in the non-compact code path and prevents potential Slack API validation errors. Co-Authored-By: Claude <[email protected]>
Extract compact/non-compact block building logic into separate helper methods for better maintainability and testability. Add MAX_SUGGESTED_ASSIGNEES constant to limit displayed assignees to 3 in Slack messages. Addresses PR review feedback: - Truncate suggested assignees list to prevent overly long messages - Split compact mode logic into _build_context_blocks and _build_trailing_blocks Co-Authored-By: Claude <[email protected]>
Rename method to follow the build_ prefix convention used by other block-building methods in the class. Move method near related build_description_block and build_pre_footer_context_block methods. Co-Authored-By: Claude <[email protected]>
1a34698 to
2861669
Compare
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.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Fix build_description_block to pass the stripped text variable instead of the original description_text, ensuring consistent whitespace handling. Strip context_text in build_group_context_block before empty check to prevent whitespace-only context blocks from being rendered. Co-Authored-By: Claude <[email protected]>
Rename method to plural form and return a list of blocks instead of a single optional block. This allows the method to return multiple context blocks (e.g., both suggested assignees and suspect commit) in the non-compact mode. Co-Authored-By: Claude <[email protected]>
Adds a slack-compact-alerts flag for the new alert layout. The compact layout won't affect most customers, key differences are - the AI summary is moved - title will not vary by your access to Seer - suggested assignee is moved to the context stats - suspect commit removed <img width="652" height="454" alt="image" src="https://github.com/user-attachments/assets/24ea61b5-3f76-4555-a9fd-7ab49ca6bb43" /> Hard to display all the differences without setting them up each feature and preventing race conditions (i.e. alert fires before suggested assignee is determined), so we can verify with tests and in production via the flag. --------- Co-authored-by: Claude <[email protected]>
Adds a slack-compact-alerts flag for the new alert layout. The compact layout won't affect most customers, key differences are
Hard to display all the differences without setting them up each feature and preventing race conditions (i.e. alert fires before suggested assignee is determined), so we can verify with tests and in production via the flag.