Skip to content
This repository was archived by the owner on Jul 24, 2020. It is now read-only.

[1471] Fix quick new user modal with CAS [master] #1473

Merged
merged 1 commit into from
Feb 12, 2016
Merged

Conversation

orenyk
Copy link
Contributor

@orenyk orenyk commented Feb 2, 2016

Resolves #1471, resolves #1469

  • controller takes CAS into account
  • cart reserver is now set as the new user after creation

@orenyk orenyk changed the title [1471] Fix quick new user modal with CAS [1471] Fix quick new user modal with CAS [master] Feb 2, 2016
@orenyk
Copy link
Contributor Author

orenyk commented Feb 2, 2016

Alright, should be good for review!

@esoterik
Copy link
Collaborator

esoterik commented Feb 9, 2016

should there be a feature test for this modal?

@orenyk
Copy link
Contributor Author

orenyk commented Feb 9, 2016

Not sure, this uses JS so it would be tricky; at the very least I should add a controller test. Good catch!

@orenyk
Copy link
Contributor Author

orenyk commented Feb 10, 2016

This turned out to be a lot trickier than I expected. I wanted to check both password and CAS authentication by using the ActiveModel validation on cas_login that is enabled when we use CAS. Unfortunately, RSpec loads the models only once, so if we use our EnvHelpers to wrap tests in a modified ENV, they don't affect code that runs at initialization, such as model definitions.

I was able to reload the User model using the following code:

Object.send(:remove_const, 'User')
load 'user.rb'

This kinda worked, except it then breaks Devise because Devise builds associations with models during initialization, and reloading said model breaks that association. This causes a bunch of tests to then fail when we try to authenticate. Going to have to go about things a bit more hack-ishly.

Resolves #1471, resolves #1469
- controller takes CAS into account
- cart reserver is now set as the new user after creation
- add controller specs for UsersController#quick_create
@orenyk orenyk force-pushed the 1471_quick_new_cas branch from caf7e46 to 83f73dc Compare February 10, 2016 06:57
@orenyk
Copy link
Contributor Author

orenyk commented Feb 10, 2016

@esoterik ready for re-review, thanks!

@esoterik
Copy link
Collaborator

this looks good

orenyk added a commit that referenced this pull request Feb 12, 2016
[1471] Fix quick new user modal with CAS
@orenyk orenyk merged commit c1697da into master Feb 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants