Skip to content

payments service: notification of payments to the front-end via socketio #5057

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

Merged
merged 23 commits into from
Nov 29, 2023

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Nov 21, 2023

What do these changes do?

This PR implements socketio link between the payments and the webserver services. This link is used to notify payment completion to the front-end.

image

NOTE: This PR adds the functionality but the two services are still not connected. This will be done in a separate PR !

  • payments/gateway:
    • 📝 Adds openapi specs for gateway in gateway/openapi.json
    • 🔨 Includes scripts in an example implementation that can be used for development. NOTE that this is installed with the payments package and therefore available in the $PATH after installation. i.e. this will create a gateway
        cd services/payments
        make install-dev
        example_payment_gateway.py run
  • 🎨 webserver's wallets/_payments_handlers.py: Improves notification with a small delay in payment w/ payment-method ack which is directly performed at the web-servers handler (remember that this type of payment is done in one call and not using the init-prompt-ack flow)

Related issue/s

How to test

  • Driving test services/payments/tests/unit/test_services_socketio.py

Dev Checklist

DevOps

None

@pcrespov pcrespov self-assigned this Nov 21, 2023
@pcrespov pcrespov added this to the 7peaks milestone Nov 21, 2023
@pcrespov pcrespov added the a:payments payments service label Nov 21, 2023
Copy link

codecov bot commented Nov 21, 2023

Codecov Report

Merging #5057 (453d955) into master (482c544) will increase coverage by 6.9%.
The diff coverage is 51.2%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5057      +/-   ##
=========================================
+ Coverage    79.9%   86.8%    +6.9%     
=========================================
  Files        1226     669     -557     
  Lines       50098   31971   -18127     
  Branches     1124     577     -547     
=========================================
- Hits        40051   27767   -12284     
+ Misses       9807    4072    -5735     
+ Partials      240     132     -108     
Flag Coverage Δ
integrationtests 60.9% <83.3%> (-4.0%) ⬇️
unittests 86.2% <51.2%> (+7.6%) ⬆️

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

Files Coverage Δ
packages/service-library/src/servicelib/utils.py 82.3% <100.0%> (ø)
...ice_webserver/notifications/_rabbitmq_consumers.py 90.8% <100.0%> (+0.1%) ⬆️
...rc/simcore_service_webserver/payments/_socketio.py 100.0% <100.0%> (+41.6%) ⬆️
...simcore_service_webserver/projects/projects_api.py 87.0% <100.0%> (+39.5%) ⬆️
...rc/simcore_service_webserver/socketio/_handlers.py 85.1% <100.0%> (+3.9%) ⬆️
...src/simcore_service_webserver/socketio/messages.py 94.2% <100.0%> (-0.6%) ⬇️
...r/src/simcore_service_webserver/socketio/server.py 100.0% <100.0%> (ø)
...re_service_webserver/wallets/_payments_handlers.py 100.0% <100.0%> (+41.6%) ⬆️
...s/service-library/src/servicelib/socketio_utils.py 0.0% <0.0%> (ø)

... and 836 files with indirect coverage changes

@pcrespov pcrespov changed the title WIP: Is4657/enh one time payment payments service notifies front-end via socketio Nov 29, 2023
@pcrespov pcrespov force-pushed the is4657/enh-one-time-payment branch from 1fc2968 to 3552945 Compare November 29, 2023 11:57
@pcrespov pcrespov marked this pull request as ready for review November 29, 2023 11:57
@pcrespov pcrespov changed the title payments service notifies front-end via socketio payments service: notification of payments to the front-end via socketio Nov 29, 2023
@pcrespov pcrespov changed the title payments service: notification of payments to the front-end via socketio ✨🐛 payments service: notification of payments to the front-end via socketio Nov 29, 2023
@pcrespov pcrespov changed the title ✨🐛 payments service: notification of payments to the front-end via socketio payments service: notification of payments to the front-end via socketio Nov 29, 2023
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

very nice!

@pcrespov pcrespov force-pushed the is4657/enh-one-time-payment branch from 69aef6f to 6874686 Compare November 29, 2023 16:29
@pcrespov pcrespov enabled auto-merge (squash) November 29, 2023 16:43
Copy link
Collaborator

@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.

cool! thanks

@pcrespov pcrespov force-pushed the is4657/enh-one-time-payment branch from 966190b to 453d955 Compare November 29, 2023 16:56
Copy link

codeclimate bot commented Nov 29, 2023

Code Climate has analyzed commit 453d955 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link

Kudos, SonarCloud Quality Gate passed!    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.4% 0.4% Duplication

@pcrespov pcrespov merged commit 70efe70 into ITISFoundation:master Nov 29, 2023
@pcrespov pcrespov deleted the is4657/enh-one-time-payment branch November 29, 2023 18:41
teardown = asyncio.Event()

server = TestServer(aiohttp_app, port=aiohttp_unused_port())
t = asyncio.create_task(_lifespan(server, setup, teardown), name="server-lifespan")
Copy link
Contributor

Choose a reason for hiding this comment

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

@pcrespov sorry, I didn't manage to review before the PR got in. But I really like this and I think it would be very useful to have this somewhere in one of the packages. I guess one could do this with any of the test servers we are using, right?

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.

5 participants