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 #1804 - Bad request on failed redemption #1822

Merged
merged 3 commits into from
Oct 24, 2022

Conversation

florian-str
Copy link
Collaborator

Fix for issue #1804

@richardebeling richardebeling linked an issue Oct 17, 2022 that may be closed by this pull request
Copy link
Member

@richardebeling richardebeling left a comment

Choose a reason for hiding this comment

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

Thanks a lot! The view code looks very good. For the tests, we think we can use the status argument on all requests that we issue, so we only have the expected error in one line instead of two lines.

evap/rewards/tests/test_views.py Outdated Show resolved Hide resolved
evap/rewards/tests/test_views.py Outdated Show resolved Hide resolved
@niklasmohrin
Copy link
Member

About the linter error: In the original PR where we noticed this issue, we moved the code into a new function. As this PR is still open, I am fine with ignoring the warning for this PR as #1790 will fix it again.

To silence the warning, add a line with # pylint: disable=too-many-locals at the top of the function.

Copy link
Member

@richardebeling richardebeling left a comment

Choose a reason for hiding this comment

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

Our linter (pylint) performs a static analysis of the code without any call-chain inference. We thus have to disable the check in the scope where it fails otherwise (because there are too many locals according to our rules). If you move the # pylint: disable=too-many-locals comment into the rewards/views.py index view, as shown in the change suggestions below, it should work.

Don't worry about the typescript tests, that failure was unrelated to your changes. We will have to fix that outside of this PR.

evap/rewards/tests/test_views.py Outdated Show resolved Hide resolved
evap/rewards/tests/test_views.py Outdated Show resolved Hide resolved
evap/rewards/views.py Outdated Show resolved Hide resolved
@niklasmohrin niklasmohrin merged commit 17075cd into e-valuation:main Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Response status code of failed redemption is 200
3 participants