Repository navigation
Conversation
`record_scope` is how Mechanisms reads and moves the record's own row, but it applied the model's default scope and used the in-memory primary key: - A default scope `where` that hides the row (e.g. the archived/soft deleted pattern from #59, with the timestamp in the positioning scope) meant restoring a record raised NoMethodError on nil. - A default scope `select` that omits the scope columns raised MissingAttributeError on any scope change. - Changing part of a composite primary key in the same save as the position meant the row couldn't be found and update_position raised. Use `unscoped` and `id_in_database`; this only affects queries for the record's own row, not the list scope. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`position_was` is memoized with `||=`, which doesn't memoize nil. So for a row whose position is NULL, `move_out_of_the_way` set it to 0 and the next call read back 0, and the record was then treated as if it had been at position 0. The result was a 0 left in the list and every other record shifted down by one, on update, destroy and scope change. With a relative position it raised NoMethodError instead. That affects tables that gained a position column but haven't been healed yet, and destroying a record whose row is already gone, e.g. a repeated request (#39, #30), which raised `undefined method '+' for nil`. Memoize nil properly and treat a NULL position as not being in the list yet: there's no gap to close when it leaves, and it's inserted (rather than moved) when it's given a position. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Positions were shuffled in a before_destroy callback. If a callback declared after `positioned` then halted the destroy (for example `has_many ..., dependent: :restrict_with_error`), the destroy's rollback is swallowed when it runs inside an outer transaction, so the record was left at position 0 with the rest of the list already shifted over it. Take the lock and read the original position before the destroy as before, but contract the list in an around_destroy after the row has been deleted. Moving the record out of the way first is no longer needed since the row is gone by then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Positions were shuffled in a
before_destroycallback. If a callback declared afterpositionedthen halts the destroy, for example the very commonthe destroy is rolled back. At the top level that's fine, but inside an outer transaction (a
transactionblock, nested attributes, a parent's save) Rails swallows the nested rollback, so the shuffle stays. The record is left at position 0 and the next record has already moved into its slot. Onmain, on all three databases:[1, 0, 2].Fix
Take the lock and read the original position before the destroy, as before, but contract the list in an
around_destroyafter the row has actually been deleted, and only if it was. Moving the record out of the way is no longer needed since its row is gone.The
destroy_allstale-position case from 0.2.2 is still handled (the position is read from the database before the delete), and the existing threaded destroy test still passes.Creates and updates have the same exposure to halting
before_create/before_updatecallbacks declared afterpositioned. Those are harder to restructure, so the docs PR suggests declaringpositionedafter such callbacks instead.Changelog: Fix a halted destroy (e.g.
dependent: :restrict_with_error) inside a transaction leaving the list corrupted.🤖 Generated with Claude Code