Skip to content

[Frontend only] Batch comments on merge requests

André Luís requested to merge 1984-frontend-for-batch-comments into master

CE backport: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22158

What does this MR do?

This MR implements the rest of the frontend work left for #1984 (closed), standing on top of !4213 (merged)

Status of Features

  • Start a review by posting the first draft
  • Add drafts to existing review
  • Show Review bar at the bottom of the screen
    • Make Review Bar display regardless of which tab is selected
    • Show Reviews count
    • Submit Review
    • Dismiss Review
  • Show Drafts in the diff line
    • Inline view
    • Parallel view
    • Show the correct content for the Draft
    • Show discussion resolution status on draft
    • Add actions to draft:
      • edit
      • delete (still might need refactoring)
    • Publish draft individually
  • Send resolve/unresolve status
  • Check that quick actions work at least in Discussion tab
    • They don't. Need to add support to Draft Note vue component to list actions to be taken when published (per mockup)

Status of low-level tasks

  • Render drafts using same mechanism as discussions (incremental rendering, after diffs)
  • Use old icons for actions (edit / delete) to sync-up visually with the Notes' ones
    • create follow-up issue
  • Rewrite mixin (experiment) in app/assets/javascripts/diffs/components/diff_line_note_form.vue to an action in the batchComments store module we kept the mixin. ...

Bugs to fix

  • Don't fetch/render drafts when signed out
  • Clear form properly after draft submission
  • Prevent Batch Comments logic in Issues (start a discussion in an issue is broken)
  • Don't display batch comments buttons while editing a current existing note/draft.
  • Drafts are shown on same line of all files changed (need to add an extra check)

Nice-to-haves for first release

  • Improve "pending" area of Draft (see discussion).
  • Improve badge style when “Submit review” button is disabled/loading (screenshot)
  • Set drafts on lines upon fetching, as we do with discussions (follow up)

Tests tasks

  • Add a complete feature test to ensure with breaks if a conflict is badly resolved between CE-EE

What are the relevant issue numbers?

#1984 (closed)

Does this MR meet the acceptance criteria?

Edited by 🤖 GitLab Bot 🤖

Merge request reports