Y24-225 fix location count #819
Conversation
…sync children_count column to the correct value
| Location.find_each do |location| | ||
| correct_count = location.children.count | ||
| if location.children_count != correct_count | ||
| location.update_column(:children_count, correct_count) # rubocop:disable Rails/SkipsModelValidations |
There was a problem hiding this comment.
We probably don't want to skip model validations as rubocop is warning us here. It might be that update_column is trying to bypass them. An alternative way of writing this would be
location.children_count = correct_count
location.save
There was a problem hiding this comment.
Thanks Ben. Sure, will change it.
There was a problem hiding this comment.
On second thoughts im not sure skipping validations is a bad thing here as we are editing a column that we don't validate or control. Is there documentation in the ancestry gem on how to update this? Do the use a similar rake task with update_column?
There was a problem hiding this comment.
Yes, I thought the same. As we were not quite sure how the ancestry gem internally works and what exactly causes the bug, I was not sure calling a save method would make ancestry to update the column as we expected. So, I thought the best way is to override this column that is externally managed by ancestry gem
| @grandchild = create(:location, parent: @child1) | ||
|
|
||
| # Manually set children_count to 0 for all locations | ||
| # rubocop:disable Rails/SkipsModelValidations |
There was a problem hiding this comment.
Can you set the children_count on creation instead?
@parent = create(:location, children_count: 0)
@child1 = create(:location, parent: @parent, children_count: 0)
@child2 = create(:location, parent: @parent, children_count: 0)
@grandchild = create(:location, parent: @child1, children_count: 0)
This might not work as ancestry might override it each time.
There was a problem hiding this comment.
Might also be worth having a children_count that is set to 100 (or some other non zero value) and confirm that also gets corrected.
|
|
||
| it 'fixes the children_count for all locations and outputs the correct message' do | ||
| expect do | ||
| Rake::Task['location:fix_children_count'].invoke |
There was a problem hiding this comment.
I would add expectations to confirm that children_count is 0 before running the rake tests as otherwise we can't know for sure that children_count is actually updated.
Closes #813
Changes proposed in this pull request
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code
[Production Merges to
main]- Check story numbers included
- Check for debug code
- Check version