Skip to content

Support the use of primary keys other than "id" in ActiveRecord.#237

Merged
mcmire merged 1 commit into
splitwise:mainfrom
benk-gc:benk--support-custom-primary-key-fields-activerecord
Apr 25, 2024
Merged

Support the use of primary keys other than "id" in ActiveRecord.#237
mcmire merged 1 commit into
splitwise:mainfrom
benk-gc:benk--support-custom-primary-key-fields-activerecord

Conversation

@benk-gc

@benk-gc benk-gc commented Apr 1, 2024

Copy link
Copy Markdown
Contributor

Previously the diffing code made an assumption that "id" would always be the primary field. This is not always the case, and the use of read_attribute(:id) is deprecated in rails/rails@39997a0.

This changes adds support for use of custom primary key fields, and updates the Person test model to use the "person_id" primary key.

Note that this will almost certainly still run into issues if the model uses the newly introduced composite primary key type, so this would require additional changes outside the scope of this fix.

@benk-gc benk-gc force-pushed the benk--support-custom-primary-key-fields-activerecord branch from cb9b75c to 4695d54 Compare April 1, 2024 14:14
@benk-gc benk-gc marked this pull request as ready for review April 1, 2024 14:14
Previously the diffing code made an assumption that "id" would always be
the primary field. This is not always the case. Additionally, the use of
read_attribute(:id) is deprecated in rails/rails@39997a0.

This changes adds support for use of custom primary key fields, and
updates the Person test model to use the "person_id" primary key.

Note that this will almost certainly still run into issues if the model
uses the newly introduced composite primary key type, so this would
require additional changes outside the scope of this fix.
@benk-gc benk-gc force-pushed the benk--support-custom-primary-key-fields-activerecord branch from 4695d54 to 7ffc318 Compare April 1, 2024 15:26
@benk-gc

benk-gc commented Apr 4, 2024

Copy link
Copy Markdown
Contributor Author

@mcmire Any chance you could approve the test workflow and give this one a look? This would unblock us using the gem in Rails 7.1. Thanks!

@benk-gc

benk-gc commented Apr 10, 2024

Copy link
Copy Markdown
Contributor Author

@mcmire Specs looks happy - what do you think about a merge + release for this one? Happy to make any changes.

@benk-gc

benk-gc commented Apr 24, 2024

Copy link
Copy Markdown
Contributor Author

@mcmire Ping on the above when you have a sec - thanks!

@mcmire mcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sorry about the delay in reviewing. Looks good. Thank you!

@mcmire mcmire merged commit 8b971a8 into splitwise:main Apr 25, 2024
@mcmire

mcmire commented Apr 25, 2024

Copy link
Copy Markdown
Collaborator

I've released this under 0.12.0: https://github.com/mcmire/super_diff/releases/tag/v0.12.0. Thanks again!

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.

2 participants