Keep the draft note position when updating only its text

What does this MR do and why?

Updating a diff draft note through the API with only a new note body wiped its diff position. PUT /projects/:id/merge_requests/:merge_request_iid/draft_notes/:draft_note_id called:

draft_note.update!(note: params[:note], position: params[:position])

When position is omitted, params[:position] is nil, so the position was cleared. The draft turned into a general comment and was published off the diff line when the review was submitted.

This MR only updates the position when one is provided, so:

• a text-only update keeps the existing position;
• sending a position still moves the note, as before.

The API docs now say that omitting position keeps the existing one.

Database

Danger flagged the changed update! call. The statement is still a single-row update by primary key, as before. The only difference is that position is left out of the SET list when the request doesn't include it:

-- before, when position was omitted: position was overwritten with NULL
UPDATE "draft_notes" SET "note" = 'updated', "position" = NULL WHERE "draft_notes"."id" = 1;

-- after, when position is omitted
UPDATE "draft_notes" SET "note" = 'updated' WHERE "draft_notes"."id" = 1;

-- after, when position is provided (unchanged from before)
UPDATE "draft_notes" SET "note" = 'updated', "position" = '<serialized position>' WHERE "draft_notes"."id" = 1;

The row is found by primary key (draft_notes_pkey), and no new query shape is introduced. I don't have access to postgres.ai to attach a plan. Happy to add one if a reviewer can run it, or if you think a database review isn't needed for this.

References

• #631434 (closed)

Screenshots or screen recordings

Not applicable, API change.

How to set up and validate locally

1. Create a diff draft note on a merge request, for example by starting a review on a diff line.

2. Update only its body:

   curl --request PUT --header "PRIVATE-TOKEN: <token>" \
     --data "note=updated" \
     "http://gdk.test:3000/api/v4/projects/<id>/merge_requests/<iid>/draft_notes/<draft_note_id>"

3. The response, and a later GET of the draft note, still have the original position. Before this change, every position field was null.

Specs: bin/rspec spec/requests/api/draft_notes_spec.rb

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.