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

Handle assignments in an ActiveModel::Dirty-friendly way #2658

Merged
merged 1 commit into from
Mar 12, 2023

Conversation

mshibuya
Copy link
Member

This is a proposal for solving ActiveModel::Dirty related issues.
Previously saving an attachment takes following process:

  • On attachment assignment: does not assign the attribute value but call #{column}_will_change!
  • Before save: assign identifier as the attribute value

After this PR, it will be like:

  • On attachment assignment: assign temporary identifier as the attribute value (this marks the column as changed, so we don't need to call #{column}_will_change!)
  • Before save: assign the true identifier as the attribute value

The reason for introducing the idea of temporary identifier is that there are the cases we can't determine the identifier on the attachment assignment - an identifier is made from the filename, but filename can be overridden to reference another attribute in the model and in this case we can't determine the final filename until the record is saved (i.e. that column can be assigned after the attachment assignment).

Doing this allows us to reflect #changes correctly, resulting in fixing #2404. Also not needing to call #{column}_will_change! fixes #2409 and #2468, as empty changes will no longer be picked up.

Any feedback would be appreciated, thanks.

@mshibuya mshibuya merged commit 4597f39 into master Mar 12, 2023
@mshibuya mshibuya deleted the activemodel-dirty-compatibility branch March 12, 2023 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant