Skip to content

Fix import of subrelations on items with 1 emoji

Jessie Young requested to merge jy-fix-awardable-import-issue into master

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

Issue to export (original) Issue imported before these MR changes Issue imported with these MR changes
1 emoji, 1 assignee, 1 comment (assignment info in timeline counts as a note so 1 comment + 1 assignment = 2 notes so these will import fine in both before and after scenarios) Label and Assignee missing even though assignee note is present because there were 2 notes so import of those succeeded 1 emoji, 1 assignee, 1 comment (and assignee note on timeline) 💯
Screenshot_2023-09-16_at_4.54.14_PM Screenshot_2023-09-16_at_5.06.06_PM imported_correcly

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.

Edited by Jessie Young

Merge request reports