Skip to content

ELE-673 slack summary remove attention required section#811

Merged
IDoneShaveIt merged 3 commits intomasterfrom
ele-673-slack-summary-remove-the-action-required
Apr 23, 2023
Merged

ELE-673 slack summary remove attention required section#811
IDoneShaveIt merged 3 commits intomasterfrom
ele-673-slack-summary-remove-the-action-required

Conversation

@IDoneShaveIt
Copy link
Copy Markdown
Contributor

Removed the attention required section from the report summary message.
Reformatted the alert structure to make it look good without it.
image
image

@linear
Copy link
Copy Markdown

linear bot commented Apr 18, 2023

ELE-673 Slack summary | Remove the "action required" section

We want to do this after getting some feedback from customers.

The users we talked to don't use this section and some even find it noisy.

The alerts are those who should get people attention therefore it make more sense that they will do the tagging and notify users.

The summary is meant to provide a summary of the report and not call to action.

image.png

@github-actions
Copy link
Copy Markdown
Contributor

👋 @IDoneShaveIt
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

assert "The amount of results exceeded Slack" in attachments_as_string


def test_owners_tags_and_subscribers_of_passed_tests_are_filtered_out(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This test is no longer relevant because we removed the tags / owners / subscribers from the summary

# Within attachments limitation
message_builder = SlackReportSummaryMessageBuilder()
passed_tests_from_fixture = [x for x in test_results_summary if x.status == "pass"]
message_builder._add_preview_to_slack_alert(passed_tests_from_fixture)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not needed for the test

@IDoneShaveIt IDoneShaveIt requested a review from RoiTabach April 18, 2023 09:40
formatted_tags = [
tag if tag.startswith(TAG_PREFIX) else f"{TAG_PREFIX}{tag}"
for tag in test.tags
def _add_preview_to_slack_alert(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No new code, just moved sections from the title to the preview

Copy link
Copy Markdown
Contributor

@RoiTabach RoiTabach left a comment

Choose a reason for hiding this comment

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

Small docs question, otherwise go for merge

@IDoneShaveIt IDoneShaveIt merged commit 0294133 into master Apr 23, 2023
@IDoneShaveIt IDoneShaveIt deleted the ele-673-slack-summary-remove-the-action-required branch April 23, 2023 10:56
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.

2 participants