Disallow usage of columns with `_id` to anything else than relations
This builds on top of suggestion from: #211972.
The column ending with _id
has a very special meaning in Active Record: it indicates a relation.
This can result in us unintentionally breaking that assumption, like recent problem with ~import #212182 (comment 309851204).
I believe here, the recommendation is actually two-fold:
- each column that ends with
_id
needs to have associatedbelongs_to
- you cannot use ambiguous column like:
name_id
andname
in a single table
Proposal
Similar to our convention of using _iid
for internal IDs, we propose to use the _xid
suffix for external IDs.
We already have a few examples where we use this:
jira_imports.jira_project_xid
metrics_dashboard_annotations.panel_xid
terraform_states.lock_xid
Implementation steps
-
Add documentation on this convention -
Create a spec to ensure that we do not allow adding new columns that end with _id
and are:- not having FK configured: I think we want all
_id
to have FK - do not have
has_many/has_one/belongs_to
.
- not having FK configured: I think we want all
-
See how many offenses to this rule we have and create relevant issues to fix them.
Check-list
-
Make sure this MR enables a static analysis check rule for new usage but ignores current offenses -
Mention this proposal in the relevant Slack channels (e.g. #development
,#backend
,#frontend
) -
The MR doesn't have significant objections, and is getting a majority of 👍 vs👎 (remember that we don't need to reach a consensus) -
Create a follow-up issue to fix the current offenses as a separate iteration: ISSUE_LINK -
Follow the review process as usual -
Once approved and merged by a maintainer, mention it again: -
In the relevant Slack channels (e.g. #development
,#backend
,#frontend
) -
(Optional depending on the impact of the change) In the Engineering Week in Review
-
Edited by Toon Claes