-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
Pass request to after-session-save
#49
Conversation
Sure. Did you break the tests? 😊 |
Yes I did :p Let me fix them ! |
Not really clear to me how to run them locally. I did :
and I get : |
Okay, a |
I fixed the specs ;-) But I realise I introduced a breaking change in the API : |
@aurels you could make it optional by assigning |
@kinduff Yes I can do
I thought about that initially but could not find a good way because it's called in |
Perhaps we could use |
I'll try that ! |
@@ -30,4 +31,5 @@ | |||
t.datetime "created_at", null: false | |||
t.datetime "updated_at", null: false | |||
end | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.
@@ -11,6 +11,7 @@ | |||
# It's strongly recommended that you check this file into your version control system. | |||
|
|||
ActiveRecord::Schema.define(version: 2017_11_04_225303) do | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body beginning.
|
||
post "/users/sign_in", | ||
params: {passwordless: {email: "A@a"}}, | ||
headers: {'User-Agent': "an actual monkey"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.
User.create email: "a@a" | ||
|
||
post "/users/sign_in", | ||
params: {passwordless: {email: "A@a"}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/AlignParameters: Align the parameters of a method call if they span more than one line.
Layout/SpaceInsideHashLiteralBraces: Space inside { missing.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Layout/SpaceInsideHashLiteralBraces: Space inside } missing.
|
||
User.create email: "a@a" | ||
|
||
post "/users/sign_in", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
called = false | ||
Passwordless.after_session_save = ->(_, _) { called = true } | ||
|
||
User.create email: "a@a" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
@@ -43,6 +43,23 @@ def create_session_for(user) | |||
Passwordless.after_session_save = old_proc | |||
end | |||
|
|||
test "magic link will send by custom method (with request param)" do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
@@ -13,5 +13,5 @@ module Passwordless | |||
mattr_accessor(:expires_at) { lambda { 1.year.from_now } } | |||
mattr_accessor(:timeout_at) { lambda { 1.hour.from_now } } | |||
|
|||
mattr_accessor(:after_session_save) { lambda { |session| Mailer.magic_link(session).deliver_now } } | |||
mattr_accessor(:after_session_save) { lambda { |session, _request| Mailer.magic_link(session).deliver_now } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/Lambda: Use the -> { ... } lambda literal syntax for single line lambdas.
Metrics/LineLength: Line is too long. [111/80]
What do you guys think of d4d0428 ? |
LGTM 👍 |
Mmm most of them come from code I didn't write, right ? |
I’m not even sure at this point. I’ll find out and merge when I find a minute. |
Merged this in #55 – thanks a bunch! |
I would find it useful, what do you think ?
For example : my app is multi-domain and I'd like to know on which domain the user has initiated the login.