Skip to content

Pivot "VR in MR" feature from a suggestion to a MR-based approach

What does this MR do and why?

Context

We have an epic to build a workflow for the Duo Vulnerability Resolution feature into the Security MR widget.

The design for this workflow settled on using in-line suggestions on an MR to supply the fixes.1

During a limited feature flag roll-out, a discussion started around the limitations of a suggestion-based approach for vulnerabilities that span multiple files.

The result of that discussion was that we should pivot to an MR-based approach.2

This change

This is a first iteration to pivot to a workflow of creating a separate MR rather than using suggestions.

It re-uses most of the existing workflow's code and:

  • renamed and updated graphql attributes
  • Extracted note creation from CreateFromVulnerabilityDataService
  • consolidated create_merge_request_suggestion and create_merge_request into one method
  • Updated the specs
  • Added verbose comments for follow-up3

This feature is still behind a feature flag and we we continue iterating based on user acceptance testing

Screenshots or screen recordings

Before After
before after

How to set up and validate locally

Prerequisites

  1. follow this setup guide

Steps

  1. import the CWE Samples project into your local gdk
  2. run a pipeline on the main branch
  3. The project you cloned comes pre-populated with some branches you can create an MR from
  4. create an MR and follow the workflow shown in the screenrecordings

epic: &14862 (closed)
MR: !172185 (merged)
resolves: #503403 (closed)
Changelog: changed
EE: true

  1. https://gitlab.com/gitlab-org/gitlab/-/issues/407275/designs/design_1722547971389.png

  2. That decision is discussed here. There is also a requirement of keeping the original release timeline, so I have marked off some TODOs that we will be able to follow up on after this MR merges

  3. I added some overly verbose comments to help facilitate with follow-up work. As I was replacing the suggestion workflow, I noticed some areas that could possibly be moved to async workers (like the note creation). As I inherited this MR, I left the comments so I can discuss with @subashis and @darbyfrey when they are back from OOO

Edited by Michael Becker

Merge request reports

Loading