Skip to content

Fix under rails 4.0.4+#138

Merged
arthurnn merged 4 commits into
masterfrom
fix_rails_404
Mar 26, 2014
Merged

Fix under rails 4.0.4+#138
arthurnn merged 4 commits into
masterfrom
fix_rails_404

Conversation

@arthurnn

Copy link
Copy Markdown
Contributor

This fixes tests on rails 4.0.4.

Problem

PR rails/rails#12031 was merged into rails 4.0.4, which means that the touch method will now call after_commit callbacks.

Solution.

Dont use the after_touch callback anymore.

review @dylanahsmith @camilo
cc @jnormore @airhorns

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we need to add timestamps to some models, otherwise .touch will fail

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are these additional timestamps why test/fixtures/serialized_record changed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

because now those fields need to be serialized on the fixture.

@arthurnn

Copy link
Copy Markdown
Contributor Author

Can I merge this and release a new 0.1.0 version? and explain the changes?

@camilo

camilo commented Mar 26, 2014

Copy link
Copy Markdown
Contributor

👍

On Tuesday, March 25, 2014, Arthur Nogueira Neves notifications@github.com
wrote:

Can I merge this and release a new 0.1.0 version? and explain the changes?

Reply to this email directly or view it on GitHubhttps://github.com//pull/138#issuecomment-38616641
.

arthurnn added a commit that referenced this pull request Mar 26, 2014
@arthurnn
arthurnn merged commit 202f72b into master Mar 26, 2014
@arthurnn
arthurnn deleted the fix_rails_404 branch March 26, 2014 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants