Skip to content

Update redis session store and Rails to latest versions#1625

Merged
monfresh merged 2 commits intomasterfrom
mb-update-redis-session-store
Aug 18, 2017
Merged

Update redis session store and Rails to latest versions#1625
monfresh merged 2 commits intomasterfrom
mb-update-redis-session-store

Conversation

@monfresh
Copy link
Contributor

See the commit message for the Rails upgrade for more details.

**Why**: To pick up a change that allows the IdP app to be updated to
Rails 5.1
**Why**: To have the latest and greatest

**How**:
- Per the Rails docs, we have to create ApplicationRecord in models,
then make all of our models inherit from it.
- Per the Rails docs, we have to create ApplicationJob in jobs,
then make all of our jobs inherit from it.
- Rename `email_changed?` to `will_save_change_to_email?` and
`email_was` to `email_in_database` since the ActiveModel::Dirty API has
changed.
- Remove redundant CORS OPTIONS test

describe '#options' do
it 'is empty so it can be used as a pre-flight request' do
process :options, 'OPTIONS'
Copy link
Contributor

Choose a reason for hiding this comment

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

does 5.1 no longer support sending OPTIONS requests? I really like having this spec because it makes sure we handle the OPTIONS which is important for CORS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have that spec already here: https://github.com/18F/identity-idp/blob/master/spec/requests/openid_connect_cors_spec.rb#L39-L54
I added an expectation for the empty body to that test.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok thanks!

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM!

@monfresh monfresh merged commit c04a99b into master Aug 18, 2017
@monfresh monfresh deleted the mb-update-redis-session-store branch August 18, 2017 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants