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

✨Request Bank: Fixed AMP CORs and Multipart form data #20879

Merged
merged 2 commits into from Feb 21, 2019

Conversation

torch2424
Copy link
Contributor

@torch2424 torch2424 commented Feb 15, 2019

closes #20868
closes #20869
relates to #20541

This adds multipart form data parsing into the request body, which AMP Form uses, to the RequestBank Express Middleware, as well as, enables AMP Cors on request bank (which included some refactoring in other places).

Example Requests

These examples are made from just switching around the endpoint of the POST request on the recaptcha example

Submitting to a standard endpoint on the dev server

screen shot 2019-02-14 at 5 46 00 pm

screen shot 2019-02-14 at 5 46 19 pm

Depositing to request bank

screen shot 2019-02-14 at 6 11 26 pm

screen shot 2019-02-14 at 6 11 46 pm

Withdrawing from request bank

screen shot 2019-02-14 at 6 11 16 pm

Copy link
Contributor

@lannka lannka left a comment

Choose a reason for hiding this comment

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

thanks for improving our bank!

}
}

module.exports = {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can do:

module.exports = enableCors;

and to import

const enableCors = require('./amp-cors');

});
app.use(
'/request-bank/:bid/deposit/:id/',
upload.array(),
Copy link
Contributor

Choose a reason for hiding this comment

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

i suppose this is a no-op for non-form POST?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup 😄

screen shot 2019-02-20 at 1 00 51 pm

@torch2424 torch2424 merged commit 96b2b53 into ampproject:master Feb 21, 2019
@torch2424 torch2424 deleted the request-bank-fixes branch February 21, 2019 01:01
noranazmy pushed a commit to noranazmy/amphtml that referenced this pull request Mar 22, 2019
* Fixed AMP CORs and Multipart form data for Request Bank

* Made requested changes
bramanudom pushed a commit to bramanudom/amphtml that referenced this pull request Mar 22, 2019
* Fixed AMP CORs and Multipart form data for Request Bank

* Made requested changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request Bank: Send back POST Form Data in the body Request Bank: Send back AMP CORS Headers
3 participants