Fix import of subrelations on items with 1 emoji
What does this MR do and why?
- GitLab imports were sometimes randomly missing comments or notes on issues or merge requests. It was not obvious why but one theme seemed to be that they all had a single comment or system note. But some items with a single comment or note were still importing, so it didn't make sense why only some imports were failing.
- The specific note we saw missing was from a bot user. That same issue had no other notes but it did have 2 labels that were imported properly. It also had a single thumbsup on the issue.
- After much discovery using the import test project data: https://gitlab.com/gitlab-migration-large-import-test/migration-test-project I came to a discovery: the way to get an issue with one note to fail to import is to add a single emoji (thumbsup or anything else) to the issue.
- I then realized that this problem was not isolated to Issues + Notes. If
any item that is "awardable" (Issue, Merge Request, Epic, Snippet, Note)an Issue has a single award emoji on it, all other subrelations on that class, if there was only one item in the subrelation collection, would fail to import. - This means that an issue with one emoji and one note and one label would import with the emoji but without the note and label. Or an MR with one emoji and one one approval one assignee would import with the emoji but without the approval and assignee. Any subrelations with more than item would import as expected; the combo needed here is a single emoji plus any other subrelation with a count of 1.
- After much more discovery on this, I found that, by always importing subrelations separate from parent objects, this issue is avoided entirely. Previously, we were importing only subrelations with 2 or more records separately. This was to avoid validation errors with protected environments and deploy access levels: !79963 (comment 846600241)
- When a parent record had relations with just 1 record, the children were
previously saved when we called
save!
on the parent. By default, in ActiveRecord, new records are saved when their parent is saved, even if the:autosave
option is not specified: https://api.rubyonrails.org/classes/ActiveRecord/AutosaveAssociation.html - By changing the logic so that children are always saved separately,
the bug was fixed. But why? Because, when you pry deeper into the
code, you find that when
insert_record
is called on the child award emoji, all other relations on the parent are nulled out. This code is in ActiveRecord, not the Import code or even GitLab code. I confirmed that when creating objects outside of the context of import this also occurs. - Examples from a rails console:
issue = FactoryBot.build(:issue, project: Project.last, author: User.first, notes: [FactoryBot.build(:note, importing: true)], labels: [FactoryBot.build(:label)]) issue.save! => notes and labels saved issue - FactoryBot.build(:issue, project: Project.last, author: User.first, award_emoji: [FactoryBot.build(:award_emoji)], labels: [FactoryBot.build(:label)]) issue.save! => award emoji saved, label not saved
- So far, I am 90% sure this is a bug in ActiveRecord and relates to our
usage of
inverse_of
on the award emoji class and how that affects collection autosaving in ActiveRecord. I plan to submit a patch upstream. - In the meantime, the fix here should solve our import problem :)
- #417122 (closed)
Screenshots
How to set up and validate locally
- Create a project for import that contains an issue with a single emoji (thumbsup or any other emoji) and one of something else: a comment, an assignee, a label (or all of the above)
- Import the project
- Note that the issue was imported with all properties. Before this MR, the comment, assignee, and label would have been missing.
I prefer to do an import via Curl using GDK -> GDK:
curl --request POST --header "PRIVATE-TOKEN: GDK_PAT" "https://gdk.test:3443/api/v4/bulk_imports" \
--header "Content-Type: application/json" \
--data '{
"configuration": {
"url": "https://gdk.test:3443,
"access_token": "GDK_PAT"
},
"entities": [
{
"source_full_path": "export-group/export-project",
"source_type": "project_entity",
"destination_slug": "imported-project-path",
"destination_namespace": "imported-group-path"
}
]
}'
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Jessie Young