-
Notifications
You must be signed in to change notification settings - Fork 32
♻️✅ invitations service: small refactoring and cleanup #7945
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
♻️✅ invitations service: small refactoring and cleanup #7945
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7945 +/- ##
==========================================
- Coverage 87.83% 85.10% -2.74%
==========================================
Files 1848 704 -1144
Lines 71253 32964 -38289
Branches 1250 176 -1074
==========================================
- Hits 62586 28053 -34533
+ Misses 8305 4853 -3452
+ Partials 362 58 -304
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
f58dbfb to
fd847b9
Compare
3fb8f79 to
696a661
Compare
|
|
@mergify queue |
🛑 The pull request has been removed from the queue
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the invitations service to add support for an extra_credits_in_usd field, cleans up the CLI I/O to use Typer's echo/secho, and expands test coverage with new scenarios around invalid invitation codes.
- Introduces
extra_credits_in_usdacross events, API, fixtures, and CLI. - Replaces raw
printcalls withtyper.echo/typer.sechoand proper exit codes in the CLI. - Adds a new unit test for handling invalid invitation codes and asserts default log level in settings.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| services/web/server/.../wallets/_events.py | Added keyword-only marker (*) for the new extra_credits_in_usd parameter |
| services/invitations/tests/unit/test_core_settings.py | Asserts default LOG_LEVEL equals "INFO" |
| services/invitations/tests/unit/conftest.py | Introduces extra_credits_in_usd fixture and populates invitation_data |
| services/invitations/tests/unit/test_cli.py | Refactors CLI option building, adds extra_credits_in_usd, and tests invalid code |
| services/invitations/tests/unit/api/test_api_invitations.py | Validates extra_credits_in_usd in API inputs and outputs |
| services/invitations/src/simcore_service_invitations/cli.py | Imports os, adds CLI option for extra credits, and switches to typer.echo/secho |
Comments suppressed due to low confidence (2)
services/invitations/src/simcore_service_invitations/cli.py:159
- [nitpick] The error message is generic. Consider making it more descriptive, e.g.,
typer.secho("Invalid invitation code", fg=typer.colors.RED, bold=True, err=True)to clarify the failure.
typer.secho("Invalid code", fg=typer.colors.RED, bold=True, err=True)
services/invitations/tests/unit/test_cli.py:65
- [nitpick] Concatenating
other_optionsdirectly can introduce extra spaces when options are empty. Consider usingother_options.strip()or building the CLI args as a list and joining to ensure consistent spacing.
f"invite {invitation_data.guest} --issuer={invitation_data.issuer} {other_options}",
|
This pull request has been removed from the queue for the following reason: The pull request can't be updated. You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again. |



What do these changes do?
This PR includes some cleanup and increase of test-coverage undertaken while debugging #7932. The latter was a bug detected in the front-end and solved in #7950
Related issue/s
How to test
Dev-ops