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.

Sign up for GitHub

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

Jump to bottom

abort(301) is not a valid choice #1120

Closed
karlcow opened this issue Jul 7, 2016 · 4 comments
Closed

abort(301) is not a valid choice #1120

karlcow opened this issue Jul 7, 2016 · 4 comments

Comments

@karlcow
Copy link
Member

karlcow commented Jul 7, 2016

@miketaylr @deepthivenkat

    # Note that 'needstriage' here is primarily used on the homepage.
    # For paginated results on the /issues page,
    # see /issues/search/needstriage.
    # We abort with 301 here because the new endpoint has
    # been replaced with needstriage.
    elif issue_category == 'new':
        abort(301)

I have seen that in the code today :) It doesn't seem right. :)
Let's see.

→ git log --oneline | head -3
99efc78 Merge pull request #1114 from deepthivenkat/issues/975/1
333d183 Issue #975 - Removed filter_new function and test_issues_new unit test
e98faea Issue #1088. Add pointer-events:none to non-button font icons.

→ git status
On branch master
Your branch is up-to-date with 'webcompat/master'.
nothing to commit, working directory clean

→  python run.py
secrets /Users/karl/code/webcompat.com
 * Running on http://localhost:5000/ (Press CTRL+C to quit)
 * Restarting with stat
secrets /Users/karl/code/webcompat.com

Then

→ http GET http://localhost:5000/api/issues/category/new 'Accept:application/json'
HTTP/1.0 500 INTERNAL SERVER ERROR
Connection: close
Content-Type: text/html; charset=utf-8
Date: Thu, 07 Jul 2016 01:42:29 GMT
Server: Werkzeug/0.10.4 Python/2.7.10
X-XSS-Protection: 0

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
  "http://www.w3.org/TR/html4/loose.dtd">

[…]

</html>

<!--

Traceback (most recent call last):
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1475, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/flask/app.py", line 1461, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/Users/karl/code/webcompat.com/webcompat/helpers.py", line 380, in wrapped_func
    return func(*args, **kwargs)
  File "/Users/karl/code/webcompat.com/webcompat/api/endpoints.py", line 132, in get_issue_category
    abort(301)
  File "/Users/karl/.virtualenvs/webcompatcom/lib/python2.7/site-packages/werkzeug/exceptions.py", line 604, in __call__
    raise LookupError('no exception for %r' % code)
LookupError: no exception for 301

-->
@miketaylr
Copy link
Member

Yeah, abort(301) doesn't make sense because 301 requires a Location header as well. What we want to do is something like Flask.redirect(new_endpoint_location, code=301).

We don't have any consumers of the http://localhost:5000/api/issues/category/new endpoint in the app now, but we should fix this.

deepthivenkat added a commit to deepthivenkat/webcompat.com that referenced this issue Jul 9, 2016
@deepthivenkat
Issue webcompat#1120 - Add status code 301 and redirect url for /issu…
9392650 
…es/category/new to /issues/category/needstriage, /issues/search/new to /issues/search/needtriage
deepthivenkat added a commit to deepthivenkat/webcompat.com that referenced this issue Jul 9, 2016
@deepthivenkat
Issue webcompat#1120 - Add status code 301 and redirect url for /issu…
6639edc 
…es/category/new to /issues/category/needstriage, /issues/search/new to /issues/search/needtriage
deepthivenkat added a commit to deepthivenkat/webcompat.com that referenced this issue Jul 13, 2016
@deepthivenkat
Issue webcompat#1120 - Add status code 301 and redirect url for /issu…
9e5fd0e 
…es/category/new to /issues/category/needstriage, /issues/search/new to /issues/search/needtriage
deepthivenkat added a commit to deepthivenkat/webcompat.com that referenced this issue Jul 13, 2016
@deepthivenkat
Issue webcompat#1120 - Add status code 301 and redirect url for /issu…
b6c9a63 
…es/category/new to /issues/category/needstriage, /issues/search/new to /issues/search/needtriage
deepthivenkat added a commit to deepthivenkat/webcompat.com that referenced this issue Jul 14, 2016
@deepthivenkat
Issue webcompat#1120 - Added return statement for \'new\' issue categ…
0c3b10f 
…ory and prepend api. to the method inside url_for
deepthivenkat added a commit to deepthivenkat/webcompat.com that referenced this issue Jul 14, 2016
@deepthivenkat
Issue webcompat#1120 - Added return statement for 'new' issue categor…
5dd4e21 
…y and prepend api. to the method inside url_for
@deepthivenkat
Copy link
Member

r? @karlcow @miketaylr

@miketaylr
Copy link
Member

r? @karlcow @miketaylr

@deepthivenkat: review requests should be done in pull requests, not the issue -- thanks!

@miketaylr miketaylr mentioned this issue Jul 14, 2016
Merged
miketaylr pushed a commit that referenced this issue Jul 14, 2016
Merge pull request #1130 from deepthivenkat/issues/1120/2
f69ae6e 
Fixes #1120 - Add status code 301 and redirect url for /issues/catego…
@miketaylr
Copy link
Member

This is fixed now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@miketaylr @karlcow @deepthivenkat