Skip to content
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

Improve new campaign slack notification #681

Merged
merged 4 commits into from
May 3, 2021

Conversation

mininao
Copy link
Member

@mininao mininao commented Apr 30, 2021

Problème

  • Les notifications slack de nouvelle campagne contenaient des infos inutiles, et ne permettaient pas d'accéder facilement à l'interface d'administration
  • Il est possible que certaines addressent de centres n'aient pas été géocodés, sans qu'on le sache

Solution

  • Clarifier les notifications slack
  • J'en profite pour utiliser l'API "blocks" qui doit remplacer l'api "attachements" côté slack
Before After
image image

@mininao mininao requested review from mathieuripert and carsso April 30, 2021 00:11
@codecov
Copy link

codecov bot commented Apr 30, 2021

Codecov Report

Merging #681 (ad6428d) into master (43b0ed8) will decrease coverage by 0.18%.
The diff coverage is 7.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #681      +/-   ##
==========================================
- Coverage   50.27%   50.08%   -0.19%     
==========================================
  Files          73       73              
  Lines        1665     1673       +8     
==========================================
+ Hits          837      838       +1     
- Misses        828      835       +7     
Impacted Files Coverage Δ
app/jobs/slack_notifier_job.rb 0.00% <0.00%> (ø)
app/jobs/push_new_campaign_to_slack_job.rb 36.84% <9.09%> (-13.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43b0ed8...ad6428d. Read the comment docs.

@mininao
Copy link
Member Author

mininao commented Apr 30, 2021

Tests look broken, I'll see about that tomorrow

@MatthieuGariel
Copy link
Collaborator

Est-ce qu'on pourrait rajouter l'id campagne, peut-être dans l'en-tête ? "La campagne #139 a été créée par" ?

Et enlever ce "d" à adresse 😬😉 ?

@mininao
Copy link
Member Author

mininao commented Apr 30, 2021

Yes j'ajoute l'id.
Et oui pour le d, décidément j'y arrive pas hahaha

@mininao mininao marked this pull request as draft May 2, 2021 09:58
@mininao mininao force-pushed the maxence/better-slack-capaign-alert branch from d033ee6 to 4e281f8 Compare May 2, 2021 15:58
@mininao mininao marked this pull request as ready for review May 2, 2021 16:27
@mininao
Copy link
Member Author

mininao commented May 2, 2021

Ready to review :)

@sonarqubecloud
Copy link

sonarqubecloud bot commented May 3, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mathieuripert mathieuripert merged commit 4b999ca into master May 3, 2021
@mathieuripert mathieuripert deleted the maxence/better-slack-capaign-alert branch May 3, 2021 11:42
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.

3 participants