Fixes #2024 - Adds the ability to receive POST data with a JSON Payload #2582

karlcow · 2018-09-19T05:19:53Z

This PR fixes issue #2024

Proposed PR background

This should be able to receive POST data with a JSON body for pre-populating the form.

Works as before and accept URL parameters both from GET and POST
Adds a POST JSON data for the form
Submit the data as before with multiform/part

Possible Follow up issues to fill.

Be stricter in what is possible to do in the POST data (JSON validation)
Be stricter on the form validation.

The next step will be to push this to staging so we can test it deeply and thoroughly.

→ nosetests 
..................................................................................................
----------------------------------------------------------------------
Ran 98 tests in 1.133s

OK

r? @miketaylr

miketaylr · 2018-09-19T20:57:28Z

Possible Follow up issues to fill.

Also, all the clients we control that create issues via this form need to be updated.

miketaylr

This looks great, just a few questions to make sure that I understand how this works. Haven't tested locally yet, but that's the next step once I have a few more answers. Then we can deploy this thing to staging.

tests/unit/test_helpers.py

tests/unit/test_uploads.py

webcompat/form.py

webcompat/views.py

tests/unit/test_helpers.py

webcompat/views.py

miketaylr · 2018-09-19T22:20:55Z

This branch is out-of-date with the base branch

Could you also rebase against master, that will make deploying to staging a tiny bit simper. 😻

karlcow · 2018-09-19T23:28:42Z

Rebasing against master. Definitely!

This goes through the possible scenarios when requesting the form with a POST or a GET.

This will help us determine how we want to process the data when requesting.

This is handling a form_data object as a dictionary which is in charge of containing the data relative to the future Form()

…submission The tests were not defining the type and were then sending url-encoded stuff, which is not what we do in the HTML.

This is a minimal test which could become more complete if we wanted.

… for Form

A lot of the data can be collected at first request.

…ith a body The new form interaction are two folds 1. GET with parameters or POST with a body 2. POST as a multipart form to GitHub

Just polishing things here and there.

karlcow · 2018-09-19T23:59:25Z

Rebased and pushed. Let's see what travis said.
I haven't addressed the comment about form_data yet as our discussion is going on.

miketaylr

This is great.

Only "unresolved" issue is #2582 (comment), which isn't really an issue. We can leave it as-is and file a follow up, or make a decision it's the right way, or change it as I described in my comment. No strong feelings.

miketaylr · 2018-09-20T20:48:35Z

I've (temporarily? not seeing much pretriage activity...) deployed this branch to staging and when I try to file an issue:

502 Bad Gateway

2018/09/20 20:35:02 [error] 6998#0: *1839287 upstream prematurely closed connection while reading response header from upstream, client: xxx, server: staging.webcompat.com, request: "POST /issues/new HTTP/1.1", upstream: "uwsgi://unix:///tmp/uwsgi-staging.sock:", host: "staging.webcompat.com", referrer: "https://staging.webcompat.com/"

Trying to investigate more.

miketaylr · 2018-09-20T20:57:28Z

Once I log in, I'm able to file reports just fine. Logging out, and trying to report anonymously I get the following stack trace:

Traceback (most recent call last):
 [snip]
  File "./webcompat/views.py", line 245, in create_issue
    json_response = report_issue(form)
  File "./webcompat/issues.py", line 30, in report_issue
    json_response = github.post(path, build_formdata(form))
  File "/.../staging.webcompat.com/env/local/lib/python2.7/site-packages/flask_github.py", line 280, in post
    data=data, **kwargs)
  File "/.../staging.webcompat.com/env/local/lib/python2.7/site-packages/flask_github.py", line 247, in request
    raise GitHubError(response)
flask_github.GitHubError: 401: Bad credentials

Let me ensure I'm using the right credentials, I did mess with that to help out a contributor, and maybe deployed the wrong thing to staging.

miketaylr · 2018-09-20T21:27:14Z

OK, not a credentials thing.

It seems if I'm not logged in, and I click "report Anonymously", I get a 502.
If I log in and click "report Anonymously" it reports it as myself.

miketaylr · 2018-09-20T21:30:11Z

Ah!

https://github.com/webcompat/webcompat.com/pull/2582/files#diff-a50603a824fb762e9303ee20ff0f2945L237

proxy=True is missing for ~~auth~~ anon reports.

miketaylr

Let's add proxy=True to report_issue, then I'll re-deploy to staging.

karlcow · 2018-09-21T02:15:41Z

https://github.com/webcompat/webcompat.com/pull/2582/files#diff-a50603a824fb762e9303ee20ff0f2945L237

proxy=True is missing for auth anon reports.

Do you remember why we are using proxy=True or False instead of form.get('submit_type') inside report_issue?

1. Addresses the PR review 2. Adds tests for case for the submit type is wrong 3. Makes the reporting request a bit stricter.

We could probably pass again form_data again later when we need it for more complex things.

karlcow · 2018-09-21T07:04:26Z

@miketaylr this is working now for anonymous users. It can be deployed again on staging. :)

miketaylr · 2018-09-21T17:04:55Z

Do you remember why we are using proxy=True or False instead of form.get('submit_type') inside report_issue?

No, I don't remember if this was an intentional design decision (to allow it to be used outside of a form submission context), or just like, the way we wrote it. :S

miketaylr

Looks great. Will do some quick testing on staging.

miketaylr · 2018-09-21T18:37:19Z

All good in my testing. Let's mergeeeeeee 💯 🍰 🌵

miketaylr · 2018-09-21T18:38:01Z

(ok, so the "Update branch" button does a merge commit, that's lame :P)

karlcow requested a review from miketaylr September 19, 2018 05:19

miketaylr reviewed Sep 19, 2018

View reviewed changes

karlcow added 13 commits September 20, 2018 08:56

Issue webcompat#2024 - Adds a better docstring to /issues/new route

f5e7025

Issue webcompat#2024 - Fixes tests syntax and style

9cb2201

Issue webcompat#2024 - Adds test for detecting the type of form request

598dda2

This goes through the possible scenarios when requesting the form with a POST or a GET.

Issue webcompat#2024 - Implements a form request detection

94b97b0

This will help us determine how we want to process the data when requesting.

Issue webcompat#2024 - Adds tests and methods for preparing form data

039562d

This is handling a form_data object as a dictionary which is in charge of containing the data relative to the future Form()

Issue webcompat#2024 - Fixes tests with proper content-type for form …

d62beb7

…submission The tests were not defining the type and were then sending url-encoded stuff, which is not what we do in the HTML.

Issue webcompat#2024 - Adds test for form rendering

5553cca

This is a minimal test which could become more complete if we wanted.

Issue webcompat#2024 - Fixes test to take into account the new format…

fd775ad

… for Form

Issue webcompat#2024 - Creates a more detailed form

bfdf016

A lot of the data can be collected at first request.

Issue webcompat#2024 - Modifies the views to take into account POST w…

0bc38e5

…ith a body The new form interaction are two folds 1. GET with parameters or POST with a body 2. POST as a multipart form to GitHub

Issue webcompat#2024 - Makes the syntax a bit stricter

97412da

Just polishing things here and there.

Issue webcompat#2024 - Makes the test for failed post more detailed.

2287364

Issue webcompat#2024 - Fixes typos after review

997aa64

karlcow force-pushed the 2024/1 branch from d73db92 to 997aa64 Compare September 19, 2018 23:58

miketaylr approved these changes Sep 20, 2018

View reviewed changes

miketaylr requested changes Sep 20, 2018

View reviewed changes

karlcow added 3 commits September 21, 2018 14:22

Issue webcompat#2024 - Fixes simple syntax issues

f99b270

Issue webcompat#2024 - Makes anonymous reporting more robust

435cf8c

1. Addresses the PR review 2. Adds tests for case for the submit type is wrong 3. Makes the reporting request a bit stricter.

Issue webcompat#2024 - Simplifies the session stashing for extra_labels

10dba44

We could probably pass again form_data again later when we need it for more complex things.

miketaylr approved these changes Sep 21, 2018

View reviewed changes

Merge branch 'master' into 2024/1

e8c8319

miketaylr merged commit 04e3e5d into webcompat:master Sep 21, 2018

miketaylr added status: add to changelog and removed status: add to changelog labels Sep 21, 2018

karlcow deleted the 2024/1 branch October 17, 2018 23:43

karlcow mentioned this pull request Oct 18, 2018

Fixes #2653. Encode details as JSON before filling input #2658

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #2024 - Adds the ability to receive POST data with a JSON Payload #2582

Fixes #2024 - Adds the ability to receive POST data with a JSON Payload #2582

karlcow commented Sep 19, 2018

miketaylr commented Sep 19, 2018

miketaylr left a comment

miketaylr commented Sep 19, 2018

karlcow commented Sep 19, 2018

karlcow commented Sep 19, 2018

miketaylr left a comment

miketaylr commented Sep 20, 2018

miketaylr commented Sep 20, 2018

miketaylr commented Sep 20, 2018

miketaylr commented Sep 20, 2018 •

edited

Loading

miketaylr left a comment

karlcow commented Sep 21, 2018

karlcow commented Sep 21, 2018

miketaylr commented Sep 21, 2018

miketaylr left a comment

miketaylr commented Sep 21, 2018

miketaylr commented Sep 21, 2018

Fixes #2024 - Adds the ability to receive POST data with a JSON Payload #2582

Fixes #2024 - Adds the ability to receive POST data with a JSON Payload #2582

Conversation

karlcow commented Sep 19, 2018

Proposed PR background

miketaylr commented Sep 19, 2018

miketaylr left a comment

Choose a reason for hiding this comment

miketaylr commented Sep 19, 2018

karlcow commented Sep 19, 2018

karlcow commented Sep 19, 2018

miketaylr left a comment

Choose a reason for hiding this comment

miketaylr commented Sep 20, 2018

miketaylr commented Sep 20, 2018

miketaylr commented Sep 20, 2018

miketaylr commented Sep 20, 2018 • edited Loading

miketaylr left a comment

Choose a reason for hiding this comment

karlcow commented Sep 21, 2018

karlcow commented Sep 21, 2018

miketaylr commented Sep 21, 2018

miketaylr left a comment

Choose a reason for hiding this comment

miketaylr commented Sep 21, 2018

miketaylr commented Sep 21, 2018

miketaylr commented Sep 20, 2018 •

edited

Loading