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

Pass request to `after-session-save` #49

Closed

aurels wants to merge 3 commits into mikker:master from aurels:pass-request-to-after-session-save

aurels commented Apr 2, 2019

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.


Pass request to

338c4d9

mikker approved these changes

View reviewed changes

Owner

mikker commented Apr 2, 2019

Sure. Did you break the tests? 😊

Author

aurels commented Apr 2, 2019

Yes I did :p Let me fix them !

Author

aurels commented Apr 2, 2019

Not really clear to me how to run them locally.

I did :

rbenv shell 2.4.5
bundle
cd test/dummy
bin/rails db:migrate RAILS_ENV=test

and I get : ActiveRecord::StatementInvalid: SQLite3::SQLException: table "passwordless_sessions" already exists: CREATE TABLE "passwordless_sessions"

Author

aurels commented Apr 2, 2019

Okay, a bin/rails db:test:prepare did it ;-)


fix tests

8ae00ca

Author

aurels commented Apr 2, 2019

I fixed the specs ;-)

But I realise I introduced a breaking change in the API : Passwordless.after_session_save MUST now always have two parameters.

kinduff commented Apr 17, 2019

@aurels you could make it optional by assigning _request = nil at the lambda. I like this change but I see it's not flexible. How about to optionally be able to pass anything to the after_session_save method?

Author

aurels commented Apr 18, 2019

@kinduff Yes I can do _request = nil.

How about to optionally be able to pass anything to the after_session_save method?

I thought about that initially but could not find a good way because it's called in SessionsController. Any idea ?

Owner

mikker commented Apr 26, 2019

Perhaps we could use Proc#arity to determine how many arguments to pass? https://ruby-doc.org/core-2.2.0/Proc.html#method-i-arity

Author

aurels commented Apr 26, 2019

I'll try that !


use Proc#arity to allow Passwordless.after_session_save to have one o…

d4d0428

…r two args

houndci-bot reviewed

View reviewed changes

test/dummy/db/schema.rb

@@ @@ -30,4 +31,5 @@ @@
  t.datetime "created_at", null: false
  t.datetime "updated_at", null: false
  end

houndci-bot Apr 26, 2019

Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end.

test/dummy/db/schema.rb

		@@ -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

houndci-bot Apr 26, 2019

Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body beginning.

test/controllers/passwordless/sessions_controller_test.rb

+ post "/users/sign_in",
+ params: {passwordless: {email: "A@a"}},
+ headers: {'User-Agent': "an actual monkey"}

houndci-bot Apr 26, 2019

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.

test/controllers/passwordless/sessions_controller_test.rb

+ User.create email: "a@a"
+ post "/users/sign_in",
+ params: {passwordless: {email: "A@a"}},

houndci-bot Apr 26, 2019

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.

test/controllers/passwordless/sessions_controller_test.rb


		User.create email: "a@a"

		post "/users/sign_in",

houndci-bot Apr 26, 2019

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

test/controllers/passwordless/sessions_controller_test.rb

+ called = false
+ Passwordless.after_session_save = ->(_, _) { called = true }
+ User.create email: "a@a"

houndci-bot Apr 26, 2019

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

test/controllers/passwordless/sessions_controller_test.rb

@@ @@ -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

houndci-bot Apr 26, 2019

Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.

lib/passwordless.rb

@@ @@ -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 } }

houndci-bot Apr 26, 2019

Style/Lambda: Use the -> { ... } lambda literal syntax for single line lambdas.
Metrics/LineLength: Line is too long. [111/80]

Author

aurels commented Apr 26, 2019

What do you guys think of d4d0428 ?

Owner

mikker commented Apr 26, 2019

LGTM 👍
Fix the rubocop warnings and I'll be happy to include this.

Author

aurels commented Apr 26, 2019

Mmm most of them come from code I didn't write, right ?

Owner

mikker commented Apr 26, 2019

I’m not even sure at this point. I’ll find out and merge when I find a minute.

mikker mentioned this pull request

Optionally pass request to after_session_save #55

Merged

Owner

mikker commented May 21, 2019

Merged this in #55 – thanks a bunch!
💙💚💛💜❤️

mikker closed this

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