Skip to content

Add alert-specific URL and improve Slack notification message#566

Merged
MateoLostanlen merged 9 commits intomainfrom
update_slack
Apr 15, 2026
Merged

Add alert-specific URL and improve Slack notification message#566
MateoLostanlen merged 9 commits intomainfrom
update_slack

Conversation

@MateoLostanlen
Copy link
Copy Markdown
Member

@MateoLostanlen MateoLostanlen commented Apr 15, 2026

  • Include alert ID in platform URL (e.g. /alert/38187) when available
  • Shorten S3 image URL using Slack mrkdwn link format
  • Fix French accents in notification text
  • Fix image alt text placeholder
  • Replace bare platform URL with an action-verb CTA: "Visualiser l'alerte en détail sur la plateforme Pyronear"
  • Drop Slack image block — presigned/internal S3 URLs triggered invalid_blocks errors
  • Make platform URL configurable via new PLATFORM_URL env var (default https://platform.pyronear.org); wired into docker-compose.yml and docker-compose.dev.yml
  • Remove unused url parameter from slack_client.notify and its caller
  • Log the failing payload alongside Slack error responses to ease future debugging
Screenshot 2026-04-15 at 12 27 23

- Include alert ID in platform URL (e.g. /alert/38187) when available
- Shorten S3 image URL using Slack mrkdwn link format
- Fix French accents in notification text
- Fix image alt text placeholder
@MateoLostanlen MateoLostanlen requested a review from fe51 April 15, 2026 05:56
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.15%. Comparing base (b71cc17) to head (20e9bd5).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/app/services/slack.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #566      +/-   ##
==========================================
+ Coverage   88.07%   88.15%   +0.07%     
==========================================
  Files          51       51              
  Lines        2147     2152       +5     
==========================================
+ Hits         1891     1897       +6     
+ Misses        256      255       -1     
Flag Coverage Δ
backend 88.17% <92.85%> (+0.07%) ⬆️
client 87.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fe51
Copy link
Copy Markdown
Member

fe51 commented Apr 15, 2026

Hi @MateoLostanlen, thanks for the PR.

Few questions before reweing

Have you tested on a dedicated test slack channel ?

Also, could you add a screenshot of how it renders on slack ?

When I have tested in #547 it was looking like below (unfortunately the image is no more available in my example)

image

fe51
fe51 previously approved these changes Apr 15, 2026
Copy link
Copy Markdown
Member

@fe51 fe51 left a comment

Choose a reason for hiding this comment

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

Hello @MateoLostanlen ! Thank for this PR and discussion about it ! Good to me

Copy link
Copy Markdown
Member

@fe51 fe51 left a comment

Choose a reason for hiding this comment

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

Why do you have the fallback for the platform URL in two places: in the compose file and in the config file? Shouldn't it just be one or the other?

@fe51 fe51 dismissed their stale review April 15, 2026 12:40

Why do you have the fallback for the URL in two places: in the compose file and in the config file? Shouldn't it just be one or the other?

fe51
fe51 previously approved these changes Apr 15, 2026
Copy link
Copy Markdown
Member

@fe51 fe51 left a comment

Choose a reason for hiding this comment

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

Thanks for the udpates, all good now !

@MateoLostanlen MateoLostanlen merged commit 3ddb18d into main Apr 15, 2026
24 checks passed
@MateoLostanlen MateoLostanlen deleted the update_slack branch April 15, 2026 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants