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

Merge STI model fix #27

Merged
merged 2 commits into from
Jun 22, 2018
Merged

Merge STI model fix #27

merged 2 commits into from
Jun 22, 2018

Conversation

mikker
Copy link
Owner

@mikker mikker commented Jun 22, 2018

Closes #26.
💙💚💛💜❤️

iancanderson and others added 2 commits June 22, 2018 11:19
@iancanderson @mikker
Enable logging in with STI subclass
c937b25 
Say you have a `User` model using STI, with an `Admin` subclass.
Previously, if an `Admin` attempted to log in, their id would be saved in the `admin_id` key.
Then, when calling `authenticate_by_cookie(User)`, no user would be loaded, since it would look for the `user_id` key.

This changes the session keys to use the `base_class`, which is the root class of the STI hierarchy. That way, for the above scenario, the ID is both saved and loaded using the `user_id` key, so logging in works.

This shouldn't affect non-STI models, since `base_class` would just return the class itself
@mikker
Add test case for STI models
1917ce1
@mikker
Copy link
Owner Author

mikker commented Jun 22, 2018

Thanks a bunch @iancanderson !

@mikker mikker merged commit 589e62c into master Jun 22, 2018
iancanderson
iancanderson reviewed Jun 22, 2018
View reviewed changes
test/controllers/passwordless/sessions_controller_test.rb

assert_equal 200, status
assert_equal '/', path
assert_not_nil cookies[:user_id]
Copy link

Choose a reason for hiding this comment

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

Thanks for writing the test! I meant to add it, but didn't get around to it in time 😄

Copy link
Owner Author

@mikker mikker Jun 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

¯\_(ツ)_/¯ np

@mikker mikker deleted the iancanderson-patch-1 branch July 31, 2018 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iancanderson iancanderson iancanderson left review comments

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@mikker @iancanderson