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.

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

Fix for ActiveRecord dirty attributes for rails 5.2 #76

Closed

Conversation

madding
Copy link

@madding madding commented May 4, 2019

Cause of bug:
rails/rails#33753 (comment)
You cannot call attribute_will_change! on something not registered with the attributes API -- There must be attribute :user_ids if you want that to be managed by Active Record.

For fixing this I added instantce variable where store changed attributes after save.

@hashicorp-cla
Copy link

hashicorp-cla commented May 4, 2019

CLA assistant check
All committers have signed the CLA.

@madding madding force-pushed the fix-dirty-changed-attributes branch from 75724bf to c5a50dd Compare May 4, 2019 18:49
@madding madding changed the title Fix for ActiveRecord dirty atributes for rails 5.2 Fix for ActiveRecord dirty attributes for rails 5.2 May 6, 2019
Copy link
Contributor

@justincampbell justincampbell left a comment

Choose a reason for hiding this comment

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

Thanks for submitting! This seems to have a lot of changes unrelated to the fix for Rails 5.2. Could you rebase the PR with only the tests and implementation for the fix? I'm happy to review the other changes too in separate PRs, but minimizing the scope of this PR would increase my confidence in merging and releasing it.

@madding
Copy link
Author

madding commented May 15, 2019

@justincampbell I left only the necessary changes

@madding madding force-pushed the fix-dirty-changed-attributes branch from 75f24dc to 81fb775 Compare May 29, 2019 18:29
@madding
Copy link
Author

madding commented May 29, 2019

I rebased branch

@madding madding force-pushed the fix-dirty-changed-attributes branch from 81fb775 to 99233c7 Compare June 24, 2019 19:12
@justincampbell
Copy link
Contributor

@madding Thanks for keeping this updated! I wasn't able to allocate time to get Rails 5.2 support into the 0.5.0 release, but I hope to find time to review this and get another release out soon. Thanks for your patience!

@dankozlowski
Copy link

Hey @justincampbell What's the ETA here? This merge is blocking use of the gem for us. Can I help?

@Valarissa
Copy link
Contributor

#67 was merged. Do you feel like that PR handles all the issues put forth in this PR, or do you feel like something in that PR misses something in your PR @madding ? If it does, then may I close this PR? If it doesn't, would you be able to elaborate what?

Thanks in advance, and sorry for leaving this to languish for so long.

@madding
Copy link
Author

madding commented May 5, 2020

@Valarissa I think you can close my PR. Yesterday was PRs birthday :)

@madding madding closed this May 5, 2020
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.

5 participants