Remove DraftHandling.DELETE

Using this as the default is more dangerous than useful: if a caller
forgets to populate the `drafts` field in ReviewInput, then drafts are
unceremoniously deleted. A much safer default is KEEP; this also comes
closest to preserving the old behavior, which also does not publish any
drafts. We don't know of anybody relying on DELETE as the default, but
just in case, this is probably the safest new default.

An alternative proposal[1] would phase out the old default more slowly.
However, given that the community couldn't come up with any use cases
for the old default[2], that much caution is unnecessary, especially
when compared with the real risk of data loss in the current state.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/140055
[2] https://groups.google.com/d/topic/repo-discuss/caWj8VqUVDY/discussion

Change-Id: Idd8cdd6e93c17f296dc62c400980323f0167ae9f

Documentation/rest-api-changes.txt

@@ -6709,12 +6709,11 @@ to a list of link:#robot-comment-input[RobotCommentInput] entities.
 Draft handling that defines how draft comments are handled that are
 already in the database but that were not also described in this
 input. +
-Allowed values are `DELETE`, `PUBLISH`, `PUBLISH_ALL_REVISIONS` and
-`KEEP`. All values except `PUBLISH_ALL_REVISIONS` operate only on drafts
-for a single revision. +
+Allowed values are `PUBLISH`, `PUBLISH_ALL_REVISIONS` and `KEEP`. All values
+except `PUBLISH_ALL_REVISIONS` operate only on drafts for a single revision. +
 Only `KEEP` is allowed when used in conjunction with `on_behalf_of`. +
-If not set, the default is `DELETE`, unless `on_behalf_of` is set, in
-which case the default is `KEEP` and any other value is disallowed.
+If not set, the default is `KEEP`. If `on_behalf_of` is set, then no other value
+besides `KEEP` is allowed.
 |`notify`                 |optional|
 Notify handling that defines to whom email notifications should be sent
 after the review is stored. +

java/com/google/gerrit/extensions/api/changes/ReviewInput.java

@@ -48,8 +48,8 @@ public class ReviewInput {
    * How to process draft comments already in the database that were not also described in this
    * input request.
    *
-   * <p>Defaults to DELETE, unless {@link #onBehalfOf} is set, in which case it defaults to KEEP and
-   * any other value is disallowed.
+   * <p>If not set, the default is {@link DraftHandling#KEEP}. If {@link #onBehalfOf} is set, then
+   * no other value besides {@code KEEP} is allowed.
    */
   public DraftHandling drafts;
 
@@ -87,15 +87,12 @@ public class ReviewInput {
   public boolean ready;
 
   public enum DraftHandling {
-    /** Delete pending drafts on this revision only. */
-    DELETE,
+    /** Leave pending drafts alone. */
+    KEEP,
 
     /** Publish pending drafts on this revision only. */
     PUBLISH,
 
-    /** Leave pending drafts alone. */
-    KEEP,
-
     /** Publish pending drafts on all revisions. */
     PUBLISH_ALL_REVISIONS
   }

java/com/google/gerrit/server/restapi/change/PostReview.java

@@ -232,10 +232,9 @@ public class PostReview
     }
     ProjectState projectState = projectCache.checkedGet(revision.getProject());
     LabelTypes labelTypes = projectState.getLabelTypes(revision.getNotes(), revision.getUser());
+    input.drafts = firstNonNull(input.drafts, DraftHandling.KEEP);
     if (input.onBehalfOf != null) {
       revision = onBehalfOf(revision, labelTypes, input);
-    } else if (input.drafts == null) {
-      input.drafts = DraftHandling.DELETE;
     }
     if (input.labels != null) {
       checkLabels(revision, labelTypes, input.labels);
@@ -435,9 +434,6 @@ public class PostReview
       throw new AuthException(
           String.format("label required to post review on behalf of \"%s\"", in.onBehalfOf));
     }
-    if (in.drafts == null) {
-      in.drafts = DraftHandling.KEEP;
-    }
     if (in.drafts != DraftHandling.KEEP) {
       throw new AuthException("not allowed to modify other user's drafts");
     }
@@ -900,7 +896,6 @@ public class PostReview
         }
       }
 
-      List<Comment> toDel = new ArrayList<>();
       List<Comment> toPublish = new ArrayList<>();
 
       Set<CommentSetEntry> existingIds =
@@ -934,9 +929,6 @@ public class PostReview
         case KEEP:
         default:
           break;
-        case DELETE:
-          toDel.addAll(drafts.values());
-          break;
         case PUBLISH:
         case PUBLISH_ALL_REVISIONS:
           commentsUtil.publish(ctx, psId, drafts.values(), in.tag);
@@ -944,10 +936,9 @@ public class PostReview
           break;
       }
       ChangeUpdate u = ctx.getUpdate(psId);
-      commentsUtil.deleteComments(ctx.getDb(), u, toDel);
       commentsUtil.putComments(ctx.getDb(), u, Status.PUBLISHED, toPublish);
       comments.addAll(toPublish);
-      return !toDel.isEmpty() || !toPublish.isEmpty();
+      return !toPublish.isEmpty();
     }
 
     private boolean insertRobotComments(ChangeContext ctx) throws OrmException {