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

✨ Notify payment via email ⚠️ #5310

Merged
merged 33 commits into from
Feb 9, 2024

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Feb 6, 2024

What do these changes do?

This PR implements a mechanism to notify by email the user when a payment has been acknowledged as successful. In this version, the templates are hard-coded in the code but this will be moved to some external storage (s3 or db) in subsequent PRs.

NOTE that the email notification is implemented is optional i.e. PAYMENTS_EMAIL=null disables this feature.

Highlights

  • ✨ Defined an interface for notifications in the payments service (see services/notifier_acb.py)
    • ♻️ Adapted existing websocket notifier to new interface (see services/notifier_ws.py)
    • ✨ New notifier for emails (see services/notifier_email.py)
      • email sessions (using aiosmtplib)
      • email templates (using Jinja2)

Related issue/s

How to test

  • driving test services/payments/tests/unit/test_services_notifier_email.py

manual tests ⚗️

Using --external-* arguments allows to run some tests against external targets. Some practical

  • testing templates amd email config by sending to a real email:
    pytest [email protected] --external-envfile=.myenv -k test_send_email_workflow --pdb tests/unit
  • validating all repo.config files
    make test-repo-config SEARCH_ROOT=/path/to/ospar-config/deployments

DevOps ⚠️

DevOps Checklist

@pcrespov pcrespov self-assigned this Feb 6, 2024
@pcrespov pcrespov added the a:payments payments service label Feb 6, 2024
@pcrespov pcrespov added this to the This is Sparta! milestone Feb 6, 2024
Copy link

codecov bot commented Feb 6, 2024

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (a284681) 87.2% compared to head (d25c9c1) 87.3%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #5310    +/-   ##
=======================================
  Coverage    87.2%   87.3%            
=======================================
  Files        1317    1320     +3     
  Lines       53935   54073   +138     
  Branches     1174    1174            
=======================================
+ Hits        47085   47209   +124     
- Misses       6599    6613    +14     
  Partials      251     251            
Flag Coverage Δ
integrationtests 65.2% <0.0%> (+<0.1%) ⬆️
unittests 85.1% <88.1%> (+<0.1%) ⬆️

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

Files Coverage Δ
...e/src/simcore_postgres_database/models/products.py 100.0% <100.0%> (ø)
...ore_service_payments/api/rest/_acknowledgements.py 94.5% <100.0%> (ø)
...ents/src/simcore_service_payments/core/settings.py 100.0% <100.0%> (ø)
.../src/simcore_service_payments/services/payments.py 94.2% <100.0%> (ø)
...core_service_payments/services/payments_methods.py 92.1% <100.0%> (ø)
...erver/src/simcore_service_webserver/email/_core.py 82.0% <100.0%> (-0.6%) ⬇️
...ges/settings-library/src/settings_library/email.py 90.0% <66.6%> (-1.9%) ⬇️
.../simcore_service_payments/services/notifier_abc.py 83.3% <83.3%> (ø)
.../simcore_service_payments/db/payment_users_repo.py 81.8% <73.3%> (-2.8%) ⬇️
...imcore_service_payments/services/notifier_email.py 95.3% <95.3%> (ø)
... and 2 more

... and 5 files with indirect coverage changes

@pcrespov pcrespov changed the title WIP: ✨ Is5227/notify user of payment by email WIP: ✨ Is5227/notify user of payment by email (⚠️) Feb 7, 2024
@pcrespov pcrespov changed the title ✨ Is5227/notify user of payment by email (⚠️) ✨ Notify user of payment by email (⚠️) Feb 8, 2024
@pcrespov pcrespov changed the title ✨ Notify user of payment by email (⚠️) ✨ Notify payment via email (⚠️) Feb 8, 2024
@pcrespov pcrespov enabled auto-merge (squash) February 8, 2024 18:10
@pcrespov pcrespov disabled auto-merge February 8, 2024 18:10
@pcrespov pcrespov changed the title ✨ Notify payment via email (⚠️) ✨ Notify payment via email ⚠️ Feb 8, 2024
@pcrespov pcrespov enabled auto-merge (squash) February 8, 2024 18:11
Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

Very very nice! thanks! Just please think about the 2 different DB concept and whether we can still do it easily in the future.

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

I see that this goes into the direction of the notification service.
I would guess at some point well extract all these into a separate service that all other platform services can use.

Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks a lot for the effort.

Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

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

looks good, ack!

Copy link

sonarqubecloud bot commented Feb 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@pcrespov pcrespov disabled auto-merge February 9, 2024 16:11
@pcrespov pcrespov merged commit c80cedd into ITISFoundation:master Feb 9, 2024
55 checks passed
@pcrespov pcrespov deleted the is5227/send-email branch February 9, 2024 16:11
@pcrespov pcrespov modified the milestones: This is Sparta!, Schoggilebe Feb 12, 2024
jsaq007 pushed a commit to jsaq007/osparc-simcore that referenced this pull request Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:payments payments service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants