Merged
Conversation
added 2 commits
June 8, 2017 15:29
This involves updates to work with the new-model stripped-down 'request' Ruby objects, which don't have an embedded appId or challenges of their own. (And also updates to the example's own Gemfile, of course.)
This requires supplying appId and challenge values to JS separately from the request objects (which no longer have these items bundled), and deleting 'as_json' method invocations which appear to no longer be necessary.
Member
|
Thanks @rst for taking care of this. Looks good to me. Any comments from you side @mastahyeti ? |
btoews
reviewed
Jun 9, 2017
btoews
left a comment
btoews
left a comment
There was a problem hiding this comment.
One change, but otherwise this looks good to me.
README.md
Outdated
| var signRequests = <%= @sign_requests.as_json.to_json.html_safe %>; | ||
| var signRequests = <%= @sign_requests.to_json.html_safe %>; | ||
| var challenge = #{@challenge.to_json.html_safe}; | ||
| var appId = #{@app_id.to_json.html_safe}; |
There was a problem hiding this comment.
Should these two lines use ERB tags instead of #{}?
Contributor
Author
There was a problem hiding this comment.
Corrected. (I'd pasted those lines in from the HAML in the example, which, as before, uses #{...} syntax consistently.)
Corrects some usages of #{...} syntax taken from HAML in the
example code.
wallin
approved these changes
Jun 9, 2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR updates the example to work with the JS and Ruby APIs provided by version 1.0.0 of the gem. (This requires adjustments as the request objects no longer have separate appIds or challenges.) This resolves issues #31, #32, and provides examples which serve as better models for what the reporter was trying to do in #34.
It also updates the Ruby and JS examples in the README to match the current codebase, and adds a note that in actual usage, registration objects would be associated with a particular user on creation, and subsequent queries for them would ordinarily be scoped to those associated with the particular user trying to authenticate.