-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Check that page is a positive integer (Fixes #344) #401
Conversation
It isn't obvious to me where I would write a test for this. Can someone give a pointer? |
@nigelbabu Since this bug fix is in a controller, it'd be a frontend test that you would have to write. Frontend tests are in |
@@ -282,7 +282,9 @@ def custom(self): | |||
try: | |||
page = int(request.params.get('page', 1)) | |||
except ValueError: | |||
abort(400, ('"page" parameter must be an integer')) | |||
abort(400, ('"page" parameter must be a positive integer')) |
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.
should the error be translatable?
abort(400, _('"page" parameter must be a positive integer'))
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.
None of the errors in feed.py seem translated. Do you want me to tackle that in this pull request or move it to a different one?
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.
The only existing one I see is 'group not found' you may as well fix that here
I'll merge this for now but might change the validation in #473. |
Check that page is a positive integer (Fixes #344)
No description provided.