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

Added options hash to create_permission_url and makes redirect_uri required #670

Merged
merged 1 commit into from
Jan 20, 2020
Merged

Added options hash to create_permission_url and makes redirect_uri required #670

merged 1 commit into from
Jan 20, 2020

Conversation

jdjkelly
Copy link
Contributor

@jdjkelly jdjkelly commented Jan 18, 2020 •
edited
Loading

Fixes #466

A re-attempt at the solution first investigated in #467

  • Makes a breaking change in requiring redirect_uri to create_permissions_url, since this is actually required today
  • Adds an options hash so that we don't break the interface again, which for now supports passing state
  • Updates readme

cc @jamiemtdwyer @richter-alex let me know your thoughts

@jdjkelly jdjkelly requested a review from a team as a code owner January 18, 2020 17:45
tanema
tanema approved these changes Jan 20, 2020
View reviewed changes
Copy link
Contributor

@tanema tanema left a comment

Choose a reason for hiding this comment

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

This is great! Thanks for updating the documentation as well.

@tanema
Copy link
Contributor

tanema commented Jan 20, 2020

If you could rebase with master, I can merge this in.

@jdjkelly jdjkelly force-pushed the feature/add-options-hash branch from a0078dd to 0f67346 Compare January 20, 2020 16:10
@jdjkelly
Copy link
Contributor Author

If you could rebase with master, I can merge this in.

Done

@tanema tanema merged commit 7a81d00 into Shopify:master Jan 20, 2020
@jdjkelly jdjkelly deleted the feature/add-options-hash branch January 20, 2020 20:53
@linzhao125 linzhao125 temporarily deployed to rubygems January 27, 2020 20:49 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tanema tanema tanema approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

create_permission_url does not accept state parameter
3 participants
@jdjkelly @tanema @linzhao125