Skip to content

Bitbucket importer: Comments refactor

Madelein van Niekerk requested to merge 412614-refactor-mr-comments into master

What does this MR do and why?

Follow-up from Import notes from Bitbucket (!131149 - merged): Refactor on Bitbucket Importer to move MR note creation logic from the sequential importer to the parallel importer since the former will be deprecated.

The code is copied over from bitbucket_import/importer.rb to bitbucket_import/importers/pull_request_notes_importer.rb and a few changes were made:

  1. Don't create the note if it was deleted on bitbucket. I saw failures because deleted notes have a blank note content which fails validation. It's better to not create those notes at all.
  2. Try to create a DiffNote type note but if that is not valid, fall back to creating a standalone comment. This happens for example when the position is not valid because Bitbucket allows a comment on any position of a file and not only on the diff itself, whereas gitlab doesn't allow that. In this case we add a comment indicating where the comment was made (file and line). We log every time this is the case.

How to set up and validate locally

  1. Follow https://docs.gitlab.com/ee/integration/bitbucket.html to setup OAuth for BitBucket Cloud. You will need an account on https://bitbucket.org/. Make sure you add the bitbucket configuration in the development section of your config/gitlab.yml as shown at https://gitlab.com/gitlab-org/gitlab-development-kit/-/blob/main/doc/howto/gitlab-oauth2.md#set-up-gdk
  2. Create a project and repo on BitBucket. Create a few MRs with notes. Also create a note that is not on the actual diff
  3. Enable the feature flag: Feature.enable(:bitbucket_parallel_importer)
  4. On your gdk server, create a new project > click on Import project > Bitbucket Cloud > follow instructions to connect to https://bitbucket.org/.
  5. Import the project created in step 2.
  6. Verify that the notes were imported correctly.
  7. You can also view the importer logs to see when each step was executed or if there are errors: tail -f log/importer.log

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #412614 (closed)

Edited by Madelein van Niekerk

Merge request reports