TAN-5720 Inconsistent custom fields ordering (Part 2) #12401
Conversation
|
| add_index :custom_fields, :ordering, | ||
| unique: true, | ||
| where: 'resource_id IS NULL', | ||
| algorithm: :concurrently |
There was a problem hiding this comment.
Learn something every day. Did not know that you could have a conditional on an index
There was a problem hiding this comment.
Is it needed though? Does the first index not create an index on [null, 1] for example?
There was a problem hiding this comment.
@jamesspeake Apparently uniqueness does not apply to null values. Duplicates like [null, 1] and [null, 1] will be allowed if we would make one index. That's why a separate index is needed to deal with the registration forms.
| update_statements! field, statements_params, errors, index if statements_params | ||
| relate_map_config_to_field(field, field_params, errors, index) | ||
| field.set_list_position(index) | ||
| field.move_to_bottom |
There was a problem hiding this comment.
Does this work fine where new fields are added in the middle of the form? I have it in my mind that we changed update_all a while back so that it was not making updates to every field, every time. ie the next statements on line 152, 155
There was a problem hiding this comment.
@jamesspeake I wasn't aware of this change. Will do more testing around these cases (new field in the middle and the other field) and will likely use a different method to insert at that position. But it would have to be a different method because set_list_position will temporarily have two fields with the same ordering.
One fix to rule them all.
There was a lot of code spread around the code base that attempted to fix this bug, but the issue remained. Adding a unique constraint on the database level should fix this once and for all.
Ideally we should do this for all ordering columns we have today.
This task will be ran before releasing:
This flow was tested thoroughly on a snapshot of the Benelux cluster.
Changelog
Fixed