THUND-107: Add support for a deprecitated ltf table#232
Conversation
dcaddell
left a comment
dcaddell
left a comment
There was a problem hiding this comment.
You mentioned this is still a draft, but here's a couple small things I noticed while skimming.
JQuinnie
left a comment
JQuinnie
left a comment
There was a problem hiding this comment.
Looking good! Just a minor comment...
CHANGELOG.md
Outdated
|
|
||
| Note: This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| ## [1.3.0] - 2025-06-04 |
There was a problem hiding this comment.
Hmm... looks like someone prior did not fill the changelog for version 1.2.0?
And question, will you be bumping the date just before merge?
There was a problem hiding this comment.
I can bump the date, and yes there is no CHANGELOG entry for 1.2 🤷
dcaddell
left a comment
dcaddell
left a comment
There was a problem hiding this comment.
Implementation looks good to me, just a couple of documentation-related requests!
test/unit/dummy/person_test.rb
Outdated
| # Ensure that the resume was saved in the new table | ||
| assert_predicate DummyLargeTextValue.where(owner: p, field_name: "resume"), :exists? | ||
|
|
||
| # Does not currently delete the old value | ||
| assert_predicate NamedTextValue.where(owner: p, field_name: "resume"), :exists? |
There was a problem hiding this comment.
⛏️ I think it would be helpful to assert on the values in this test. It's not clear if this is leaving the old value in place or if it's updating both tables with the new value.
There was a problem hiding this comment.
Currently it updates both, I will add the test
CHANGELOG.md
Outdated
|
|
||
| Note: This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| ## [1.3.0] - 2025-06-04 |
There was a problem hiding this comment.
🔧 Can we add a note to the changelog that we've changed when the association is being loaded (from when module is included -> when the large_text_field macro is called)? There probably isn't many lines of code executing between those two actions, but we don't know for sure that it won't break anyone's implementation. We should make it obvious in case anyone does have issues with this release.
3 participants
Add support for
large_text_field_deprecated_class_name.This is to allow for a model to transitioned over time to different table, so that it can eventually be placed into a separate database.