search

fabric8-services / fabric8-auth

Identity and Access Management for fabric8 services
https://auth.openshift.io/api/status
Apache License 2.0
14 stars 26 forks source link

Sync call to notification service when notifying inactive users #842

Closed alexeykazakov closed 5 years ago

alexeykazakov commented 5 years ago

We should call notification service synchronously from deactivation notification worker. Otherwise we will mark the user as successfully notified even if we got an error from the notification service.

codecov[bot] commented 5 years ago

Codecov Report

Merging #842 into master will decrease coverage by <.01%. The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #842      +/-   ##
==========================================
- Coverage   78.01%   78.01%   -0.01%     
==========================================
  Files          99       99              
  Lines        9334     9341       +7     
==========================================
+ Hits         7282     7287       +5     
- Misses       1510     1511       +1     
- Partials      542      543       +1
Impacted Files Coverage Δ
authentication/account/service/user.go 74.41% <100%> (ø) :arrow_up:
notification/service/notification.go 97.84% <71.42%> (-2.16%) :arrow_down:

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 8b6a604...a8070c4. Read the comment docs.

nurali-techie commented 5 years ago

@alexeykazakov just for future ref: notification_service /api/notify returns HTTP 202 Accepted on successful notification request and later send notification email asynchronously. So notification will be always async only. The consumer of notification_service (here auth_service) should send notification synchronously only which this PR does. Check here for notification seq_flow: https://github.com/fabric8-services/fabric8-notification#notification-api