pyronear / pyro-api

Alert Management API for wildfire prevention, detection & monitoring. Built with FastAPI & PostgreSQL
Apache License 2.0
21 stars 9 forks source link

feat: send notifications when an alert is received #272

Closed blenzi closed 1 year ago

blenzi commented 1 year ago

With this PR, a notification message is sent when the first alert in each event is created. The notification is sent via telegram (or another type, to be implemented if needed) to the recipients subscribed and associated to a group, and logged in the db.

ghost commented 1 year ago

Nice of you to open a PR :pray: When you're ready and want to get it reviewed, post a comment in this Pull Request with this message: /quack review

codecov[bot] commented 1 year ago

Codecov Report

Merging #272 (63c43cb) into main (c10ff1c) will decrease coverage by 4.65%. The diff coverage is 98.13%.

:exclamation: Current head 63c43cb differs from pull request most recent head e314baf. Consider uploading reports for the commit e314baf to get more accurate results

@@             Coverage Diff             @@
##              main     #272      +/-   ##
===========================================
- Coverage   100.00%   95.35%   -4.65%     
===========================================
  Files            3       66      +63     
  Lines           74     1571    +1497     
===========================================
+ Hits            74     1498    +1424     
- Misses           0       73      +73     
Flag Coverage Δ
client 100.00% <ø> (ø)
unittests 95.12% <98.13%> (?)

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

Files Changed Coverage Δ
src/app/models/notification.py 94.11% <94.11%> (ø)
src/app/models/recipient.py 95.23% <95.23%> (ø)
src/app/api/endpoints/notifications.py 96.29% <96.29%> (ø)
src/app/api/crud/alerts.py 100.00% <100.00%> (ø)
src/app/api/endpoints/alerts.py 98.63% <100.00%> (ø)
src/app/api/endpoints/recipients.py 100.00% <100.00%> (ø)
src/app/config.py 97.14% <100.00%> (ø)
src/app/db/tables.py 100.00% <100.00%> (ø)
src/app/main.py 76.92% <100.00%> (ø)
src/app/models/__init__.py 100.00% <100.00%> (ø)
... and 6 more

... and 47 files with indirect coverage changes

blenzi commented 1 year ago

/quack review

blenzi commented 1 year ago

Once it is deployed, we can close #263

frgfm commented 1 year ago

Sorry I wasn't available to review here! FYI, this is typically something we designed webhooks for :sweat_smile:

Meaning the webhooks would be a content (Telegram msg + recipient) and a destination (Telegram API). And in that regard, I would argue we don't need the notifications table (we could get the timestamp of an alert or add a "notification_sent_at" column) and I think the recipient table could be put in the webhook if we do this correctly.

Not a problem, but perhaps it would be helpful to consider that for the tables. The more we have, the bigger the DB, the slower the API & the tests, and the more we have to maintained :upside_down_face:

Happy to discuss this a bit later!