Fix persist before save in rails 5.2 #36
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -6,6 +6,9 @@ sudo: false | |||
| gemfile: | |||
| - Gemfile | |||
| - gemfiles/rails_4.2.gemfile | |||
| - gemfiles/rails_5.0.gemfile | |||
There was a problem hiding this comment.
Is it high complexity to just switch to a circleci config file instead?
There was a problem hiding this comment.
Probably. Seems that travis has some baked in matrix version combination stuff that we'd need to manage manually in circleci because it doesn't support it out of the box. I imagine this is why wwtd exists.
There was a problem hiding this comment.
Yes, we use TravisCI's config to run test with multiple versions of Ruby and ActiveRecord.
elenatanasoiu
approved these changes
Nov 12, 2018
We add the new rails gemfiles using the `appraisals` gem. This means that we can run `bundle exec appraisal rake` locally to run the specs against rails 4.2, 5, 5.1, and 5.2. We've also added these gemfiles to the `.travis.yml` so these rails versions are added to the ruby version test matrix that we run on CI. Note that although we use CircleCI to run our CI tests, we use the `wwtd` gem to read the `.travis.yml` config and do what it would do. This is confusing, but it works.
In rails 5.2 the behaviour of dirty tracking methods changed when they are run during callbacks. Methods like `<attribute>_changed?` and `changes` do the same thing in callbacks run before save, but will behave differently in callbacks run after save. We'd already written `__vault_encrypt_attribute` to understand this and use the `saved_change_to_attribute?` method instead of `<attribute>_changed?` method for rails 5.2. Unfortunately, this is only the correct approach when we're running `__vault_encrypt_attribute` in an `after_save` callback, which we're not if we have run `vault_persist_before_save!`. This means `__vault_encrypte_attribute` is run in a `before_save` callback where the `saved_change_to_attribute?` methods don't do what we want (because we haven't saved any changes yet). In this case we do want to use the old API that the raisl < 5.2 branch uses.
We use `Person` in the "when not used" context because that model class has not had `vault_persist_before_save!` called on it. `EagerPerson` has and that's why we use it in the "when used" context. It'd be clearer if we could explicitly call it in the spec, but there's no clean up option to unset it so we need separate models.
h-lame
force-pushed
the
fix-persist-before-save-in-rails-5.2
branch
from
77dcc1a
to
f99aebd
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
vault_persist_before_save!work properly in rails 5.2Check the commits individually for more info.