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

Should the amp-user-notification component do a GET or POST request to the data-show-if-href url #1228

Closed
erwinmombay opened this issue Dec 22, 2015 · 2 comments · Fixed by #1321

Comments

@erwinmombay
Copy link
Member

Got feedback from @dvoytenko on this (quote: a POST without user action is generally suspicious).

Currently I switched the request to a POST so that we're able to transmit the json data { ampUserId, elementId } to the server and it can determine wether to show the notification to the current visitor or not.

The alternative is to switch back to a GET request and transmit the data as query params.

/cc @dvoytenko @cramforce for input

@dvoytenko
Copy link
Contributor

Additional thing that GET buys us is that we can inject query parameters via URL substitutions: https://github.com/ampproject/amphtml/blob/master/spec/amp-var-substitutions.md

I don't know if this is useful.

@erwinmombay erwinmombay self-assigned this Jan 4, 2016
@erwinmombay
Copy link
Member Author

@cramforce unless you have any objections, i'll switch this to a GET with the query params elementId and ampUserId keys.

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

Successfully merging a pull request may close this issue.

2 participants