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

🐛 fix wallet_update when only extra_settings requested #2612

Merged

Conversation

ff137
Copy link
Contributor

@ff137 ff137 commented Nov 17, 2023

Minor bug fix: wallet_update has a condition:

    if all(
        v is None for v in (wallet_webhook_urls, wallet_dispatch_type, label, image_url)
    ):
        raise web.HTTPBadRequest(reason="At least one parameter is required.")

This ignores the newly added extra_settings, and so trying to update a wallet with only extra_settings in the request raises a faulty Bad Request: "At least one parameter is required."

This PR simply adds extra_settings to the list of possible parameters.

Edit: I moved the default value of extra_settings to be set in the get_extra_settings_dict_per_tenant call. If extra_settings defaults to {} immediately, then the v is None check for extra_settings is never True.

@ff137 ff137 force-pushed the fix/update_wallet_w_extra_settings branch from 28f0db3 to f082fb5 Compare November 17, 2023 10:15
@ff137 ff137 force-pushed the fix/update_wallet_w_extra_settings branch from f082fb5 to c9fff58 Compare November 17, 2023 10:27
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
No Duplication information No Duplication information

@dbluhm dbluhm merged commit 8a04943 into openwallet-foundation:main Nov 17, 2023
9 checks passed
@ff137 ff137 deleted the fix/update_wallet_w_extra_settings branch July 4, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants