Create cop about rescuing ActiveRecord::RecordNotUnique
Description of the proposal
Based on the discussion in this MR on dev (https://dev.gitlab.org/gitlab/gitlab-ee/merge_requests/866#note_158630) with @abrandl and @godfat, it's good practice to create a new nested transaction so any ongoing parent transaction can still continue (run more queries) or retry the failed transaction. In the case when there's a unique index to a table and ActiveRecord::RecordNotUnique gets thrown, if there's no nested transaction, the transaction will be in a state where it cannot run subsequent queries. Or if we need to retry, we won't be able to retry without rolling back the entire transaction.
As a result of the discussion in that MR, ApplicationRecord::safe_ensure_unique was added and re-used in ApplicationRecord::safe_find_or_create_by.
To ensure the usage of #safe_ensure_unique in all places as much as possible, we're proposing to add a cop (most likely a custom one) to throw a warning saying to use #safe_ensure_unique whenever ActiveRecord::RecordNotUnique is rescued.
-
Mention the proposal in the next backend weekly call and the #backend channel to encourage contribution -
Proceed with the proposal once 50% of the maintainers have weighed in, and 80% of the votes are 👍 -
Once approved, mention it again in the next backend weekly call and the #backend channel