Add API for deleting draft comments of a user
Now that changes with unresolved comments appear in dashboards, some
users are discovering that they have a lot of them and would like to
clean them up. Add an account API endpoint to do so, where callers may
restrict the set of changes to delete draft comments on by passing a
query in the input.
This endpoint is not yet wired up into the UI, but the intent is that
the input structure is flexible enough to accommodate a specialized flow
in the UI to delete draft comments on some or all changes in the
dashboard section, as well as custom queries from the command line.
An interface to select and delete subsets of comments from matching
changes is not in scope, although in theory the DeleteDraftCommentsInput
class could be extended to support this.
Change-Id: Id05adcc1a63b213d67b163d5fcaa47ab8c2679c3
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index e28a9c4..1050d56 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1805,6 +1805,69 @@
]
----
+[delete-draft-comments]
+=== Delete Draft Comments
+--
+'POST /accounts/link:#account-id[\{account-id\}]/drafts:delete'
+--
+
+Deletes some or all of a user's draft comments. The set of comments to delete is
+specified as a link:#delete-draft-comments-input[DeleteDraftCommentsInput]
+entity. An empty input entity deletes all comments.
+
+Only drafts belonging to the caller may be deleted.
+
+.Request
+----
+ POST /accounts/self/drafts.delete HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "query": "is:abandoned"
+ }
+----
+
+As a response, a list of
+link:#deleted-draft-comment-info[DeletedDraftCommentInfo] entities is returned.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ [
+ {
+ "change": {
+ "id": "myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940",
+ "project": "myProject",
+ "branch": "master",
+ "change_id": "I8473b95934b5732ac55d26311a706c9c2bde9940",
+ "subject": "Implementing Feature X",
+ "status": "ABANDONED",
+ "created": "2013-02-01 09:59:32.126000000",
+ "updated": "2013-02-21 11:16:36.775000000",
+ "insertions": 34,
+ "deletions": 101,
+ "_number": 3965,
+ "owner": {
+ "name": "John Doe"
+ }
+ },
+ "deleted": [
+ {
+ "id": "TvcXrmjM",
+ "path": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java",
+ "line": 23,
+ "message": "[nit] trailing whitespace",
+ "updated": "2013-02-26 15:40:43.986000000"
+ }
+ ]
+ }
+ ]
+----
+
[[sign-contributor-agreement]]
=== Sign Contributor Agreement
--
@@ -2316,6 +2379,37 @@
|`name` |The name of the agreement.
|=================================
+[[delete-draft-comments-input]]
+=== DeleteDraftCommentsInput
+The `DeleteDraftCommentsInput` entity contains information specifying a set of
+draft comments that should be deleted.
+
+[options="header",cols="1,^1,5"]
+|=================================
+|Field Name ||Description
+|`query` |optional|
+A link:user-search.html[change query] limiting results to changes matching this
+query; `has:draft` is implied and not necessary to list explicitly. If not set,
+matches all changes with drafts.
+|=================================
+
+[[deleted-draft-comment-info]]
+=== DeletedDraftCommentInfo
+The `DeletedDraftCommentInfo` entity contains information about draft comments
+that were deleted.
+
+[options="header",cols="1,6"]
+|=================================
+|Field Name |Description
+|`change` |
+link:rest-api-changes.html#change-info[ChangeInfo] entity describing the change
+on which one or more comments was deleted. Populated with only the
+link:rest-api-changes.html#skip_mergeable[SKIP_MERGEABLE] option.
+|`deleted` |
+List of link:rest-api-changes.html#comment-info[CommentInfo] entities for each
+comment that was deleted.
+|=================================
+
[[diff-preferences-info]]
=== DiffPreferencesInfo
The `DiffPreferencesInfo` entity contains information about the diff
diff --git a/java/com/google/gerrit/acceptance/AccountCreator.java b/java/com/google/gerrit/acceptance/AccountCreator.java
index 1416797..69492a9 100644
--- a/java/com/google/gerrit/acceptance/AccountCreator.java
+++ b/java/com/google/gerrit/acceptance/AccountCreator.java
@@ -16,6 +16,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
@@ -150,6 +151,10 @@
accounts.values().removeIf(a -> ids.contains(a.id));
}
+ public ImmutableList<TestAccount> getAll() {
+ return ImmutableList.copyOf(accounts.values());
+ }
+
private void addGroupMember(AccountGroup.UUID groupUuid, Account.Id accountId)
throws OrmException, IOException, NoSuchGroupException, ConfigInvalidException {
InternalGroupUpdate groupUpdate =
diff --git a/java/com/google/gerrit/extensions/api/accounts/AccountApi.java b/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
index 6adc978..b7fce5f 100644
--- a/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
+++ b/java/com/google/gerrit/extensions/api/accounts/AccountApi.java
@@ -109,6 +109,9 @@
void deleteExternalIds(List<String> externalIds) throws RestApiException;
+ List<DeletedDraftCommentInfo> deleteDraftComments(DeleteDraftCommentsInput input)
+ throws RestApiException;
+
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -301,5 +304,11 @@
public void deleteExternalIds(List<String> externalIds) throws RestApiException {
throw new NotImplementedException();
}
+
+ @Override
+ public List<DeletedDraftCommentInfo> deleteDraftComments(DeleteDraftCommentsInput input)
+ throws RestApiException {
+ throw new NotImplementedException();
+ }
}
}
diff --git a/java/com/google/gerrit/extensions/api/accounts/DeleteDraftCommentsInput.java b/java/com/google/gerrit/extensions/api/accounts/DeleteDraftCommentsInput.java
new file mode 100644
index 0000000..113f3d4
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/accounts/DeleteDraftCommentsInput.java
@@ -0,0 +1,35 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.accounts;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.restapi.DefaultInput;
+
+public class DeleteDraftCommentsInput {
+ /**
+ * Delete comments only on changes that match this query.
+ *
+ * <p>If null or empty, delete comments on all changes.
+ */
+ @DefaultInput public String query;
+
+ public DeleteDraftCommentsInput() {
+ this(null);
+ }
+
+ public DeleteDraftCommentsInput(@Nullable String query) {
+ this.query = query;
+ }
+}
diff --git a/java/com/google/gerrit/extensions/api/accounts/DeletedDraftCommentInfo.java b/java/com/google/gerrit/extensions/api/accounts/DeletedDraftCommentInfo.java
new file mode 100644
index 0000000..c15d5bc
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/accounts/DeletedDraftCommentInfo.java
@@ -0,0 +1,24 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.accounts;
+
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.CommentInfo;
+import java.util.List;
+
+public class DeletedDraftCommentInfo {
+ public ChangeInfo change;
+ public List<CommentInfo> deleted;
+}
diff --git a/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java b/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
index 9ef5753..15e21fe 100644
--- a/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
+++ b/java/com/google/gerrit/server/api/accounts/AccountApiImpl.java
@@ -20,6 +20,8 @@
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.extensions.api.accounts.AccountApi;
import com.google.gerrit.extensions.api.accounts.AgreementInput;
+import com.google.gerrit.extensions.api.accounts.DeleteDraftCommentsInput;
+import com.google.gerrit.extensions.api.accounts.DeletedDraftCommentInfo;
import com.google.gerrit.extensions.api.accounts.EmailApi;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.accounts.GpgKeyApi;
@@ -51,6 +53,7 @@
import com.google.gerrit.server.restapi.account.AddSshKey;
import com.google.gerrit.server.restapi.account.CreateEmail;
import com.google.gerrit.server.restapi.account.DeleteActive;
+import com.google.gerrit.server.restapi.account.DeleteDraftComments;
import com.google.gerrit.server.restapi.account.DeleteEmail;
import com.google.gerrit.server.restapi.account.DeleteExternalIds;
import com.google.gerrit.server.restapi.account.DeleteSshKey;
@@ -125,6 +128,7 @@
private final Index index;
private final GetExternalIds getExternalIds;
private final DeleteExternalIds deleteExternalIds;
+ private final DeleteDraftComments deleteDraftComments;
private final PutStatus putStatus;
private final GetGroups getGroups;
private final EmailApiImpl.Factory emailApi;
@@ -165,6 +169,7 @@
Index index,
GetExternalIds getExternalIds,
DeleteExternalIds deleteExternalIds,
+ DeleteDraftComments deleteDraftComments,
PutStatus putStatus,
GetGroups getGroups,
EmailApiImpl.Factory emailApi,
@@ -204,6 +209,7 @@
this.index = index;
this.getExternalIds = getExternalIds;
this.deleteExternalIds = deleteExternalIds;
+ this.deleteDraftComments = deleteDraftComments;
this.putStatus = putStatus;
this.getGroups = getGroups;
this.emailApi = emailApi;
@@ -561,4 +567,14 @@
throw asRestApiException("Cannot delete external IDs", e);
}
}
+
+ @Override
+ public List<DeletedDraftCommentInfo> deleteDraftComments(DeleteDraftCommentsInput input)
+ throws RestApiException {
+ try {
+ return deleteDraftComments.apply(account, input);
+ } catch (Exception e) {
+ throw asRestApiException("Cannot delete draft comments", e);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
new file mode 100644
index 0000000..26d6cf4
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
@@ -0,0 +1,211 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.account;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.gerrit.server.CommentsUtil.setCommentRevId;
+
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.extensions.api.accounts.DeleteDraftCommentsInput;
+import com.google.gerrit.extensions.api.accounts.DeletedDraftCommentInfo;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Account.Id;
+import com.google.gerrit.reviewdb.client.Comment;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CommentsUtil;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.account.AccountResource;
+import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.HasDraftByPredicate;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.restapi.change.CommentJson;
+import com.google.gerrit.server.restapi.change.CommentJson.CommentFormatter;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.BatchUpdate.Factory;
+import com.google.gerrit.server.update.BatchUpdateListener;
+import com.google.gerrit.server.update.BatchUpdateOp;
+import com.google.gerrit.server.update.ChangeContext;
+import com.google.gerrit.server.update.UpdateException;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+@Singleton
+public class DeleteDraftComments
+ implements RestModifyView<AccountResource, DeleteDraftCommentsInput> {
+
+ private final Provider<ReviewDb> db;
+ private final Provider<CurrentUser> userProvider;
+ private final BatchUpdate.Factory batchUpdateFactory;
+ private final Provider<ChangeQueryBuilder> queryBuilderProvider;
+ private final Provider<InternalChangeQuery> queryProvider;
+ private final ChangeData.Factory changeDataFactory;
+ private final ChangeJson.Factory changeJsonFactory;
+ private final Provider<CommentJson> commentJsonProvider;
+ private final CommentsUtil commentsUtil;
+ private final PatchSetUtil psUtil;
+ private final PatchListCache patchListCache;
+
+ @Inject
+ DeleteDraftComments(
+ Provider<ReviewDb> db,
+ Provider<CurrentUser> userProvider,
+ Factory batchUpdateFactory,
+ Provider<ChangeQueryBuilder> queryBuilderProvider,
+ Provider<InternalChangeQuery> queryProvider,
+ ChangeData.Factory changeDataFactory,
+ ChangeJson.Factory changeJsonFactory,
+ Provider<CommentJson> commentJsonProvider,
+ CommentsUtil commentsUtil,
+ PatchSetUtil psUtil,
+ PatchListCache patchListCache) {
+ this.db = db;
+ this.userProvider = userProvider;
+ this.batchUpdateFactory = batchUpdateFactory;
+ this.queryBuilderProvider = queryBuilderProvider;
+ this.queryProvider = queryProvider;
+ this.changeDataFactory = changeDataFactory;
+ this.changeJsonFactory = changeJsonFactory;
+ this.commentJsonProvider = commentJsonProvider;
+ this.commentsUtil = commentsUtil;
+ this.psUtil = psUtil;
+ this.patchListCache = patchListCache;
+ }
+
+ @Override
+ public ImmutableList<DeletedDraftCommentInfo> apply(
+ AccountResource rsrc, DeleteDraftCommentsInput input)
+ throws RestApiException, OrmException, UpdateException {
+ CurrentUser user = userProvider.get();
+ if (!user.isIdentifiedUser()) {
+ throw new AuthException("Authentication required");
+ }
+ if (!rsrc.getUser().hasSameAccountId(user)) {
+ // Disallow even for admins or users with Modify Account. Drafts are not like preferences or
+ // other account info; there is no way even for admins to read or delete another user's drafts
+ // using the normal draft endpoints under the change resource, so disallow it here as well.
+ // (Admins may still call this endpoint with impersonation, but in that case it would pass the
+ // hasSameAccountId check.)
+ throw new AuthException("Cannot delete drafts of other user");
+ }
+
+ CommentFormatter commentFormatter = commentJsonProvider.get().newCommentFormatter();
+ Account.Id accountId = rsrc.getUser().getAccountId();
+ Timestamp now = TimeUtil.nowTs();
+ Map<Project.NameKey, BatchUpdate> updates = new LinkedHashMap<>();
+ List<Op> ops = new ArrayList<>();
+ for (ChangeData cd :
+ queryProvider
+ .get()
+ // Don't attempt to mutate any changes the user can't currently see.
+ .enforceVisibility(true)
+ .query(predicate(accountId, input))) {
+ BatchUpdate update =
+ updates.computeIfAbsent(
+ cd.project(), p -> batchUpdateFactory.create(db.get(), p, rsrc.getUser(), now));
+ Op op = new Op(commentFormatter, accountId);
+ update.addOp(cd.getId(), op);
+ ops.add(op);
+ }
+
+ // Currently there's no way to let some updates succeed even if others fail. Even if there were,
+ // all updates from this operation only happen in All-Users and thus are fully atomic, so
+ // allowing partial failure would have little value.
+ batchUpdateFactory.execute(updates.values(), BatchUpdateListener.NONE, false);
+
+ return ops.stream().map(Op::getResult).filter(Objects::nonNull).collect(toImmutableList());
+ }
+
+ private Predicate<ChangeData> predicate(Account.Id accountId, DeleteDraftCommentsInput input)
+ throws BadRequestException {
+ Predicate<ChangeData> hasDraft = new HasDraftByPredicate(accountId);
+ if (CharMatcher.whitespace().trimFrom(Strings.nullToEmpty(input.query)).isEmpty()) {
+ return hasDraft;
+ }
+ try {
+ return Predicate.and(hasDraft, queryBuilderProvider.get().parse(input.query));
+ } catch (QueryParseException e) {
+ throw new BadRequestException("Invalid query: " + e.getMessage(), e);
+ }
+ }
+
+ private class Op implements BatchUpdateOp {
+ private final CommentFormatter commentFormatter;
+ private final Account.Id accountId;
+ private DeletedDraftCommentInfo result;
+
+ Op(CommentFormatter commentFormatter, Id accountId) {
+ this.commentFormatter = commentFormatter;
+ this.accountId = accountId;
+ }
+
+ @Override
+ public boolean updateChange(ChangeContext ctx)
+ throws OrmException, PatchListNotAvailableException, PermissionBackendException {
+ ImmutableList.Builder<CommentInfo> comments = ImmutableList.builder();
+ boolean dirty = false;
+ for (Comment c : commentsUtil.draftByChangeAuthor(ctx.getDb(), ctx.getNotes(), accountId)) {
+ dirty = true;
+ PatchSet.Id psId = new PatchSet.Id(ctx.getChange().getId(), c.key.patchSetId);
+ setCommentRevId(
+ c, patchListCache, ctx.getChange(), psUtil.get(ctx.getDb(), ctx.getNotes(), psId));
+ commentsUtil.deleteComments(ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(c));
+ comments.add(commentFormatter.format(c));
+ }
+ if (dirty) {
+ result = new DeletedDraftCommentInfo();
+ result.change =
+ changeJsonFactory
+ .create(ListChangesOption.SKIP_MERGEABLE)
+ .format(changeDataFactory.create(ctx.getDb(), ctx.getNotes()));
+ result.deleted = comments.build();
+ }
+ return dirty;
+ }
+
+ @Nullable
+ DeletedDraftCommentInfo getResult() {
+ return result;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/account/Module.java b/java/com/google/gerrit/server/restapi/account/Module.java
index be0924a..9b012f7 100644
--- a/java/com/google/gerrit/server/restapi/account/Module.java
+++ b/java/com/google/gerrit/server/restapi/account/Module.java
@@ -107,6 +107,8 @@
get(ACCOUNT_KIND, "external.ids").to(GetExternalIds.class);
post(ACCOUNT_KIND, "external.ids:delete").to(DeleteExternalIds.class);
+ post(ACCOUNT_KIND, "drafts:delete").to(DeleteDraftComments.class);
+
// The gpgkeys REST endpoints are bound via GpgApiModule.
factory(AccountsUpdate.Factory.class);
diff --git a/java/com/google/gerrit/server/restapi/change/CommentJson.java b/java/com/google/gerrit/server/restapi/change/CommentJson.java
index a562592..7112bbf 100644
--- a/java/com/google/gerrit/server/restapi/change/CommentJson.java
+++ b/java/com/google/gerrit/server/restapi/change/CommentJson.java
@@ -41,7 +41,7 @@
import java.util.Map;
import java.util.TreeMap;
-class CommentJson {
+public class CommentJson {
private final AccountLoader.Factory accountLoaderFactory;
@@ -161,7 +161,7 @@
}
}
- class CommentFormatter extends BaseCommentFormatter<Comment, CommentInfo> {
+ public class CommentFormatter extends BaseCommentFormatter<Comment, CommentInfo> {
@Override
protected CommentInfo toInfo(Comment c, AccountLoader loader) {
CommentInfo ci = new CommentInfo();
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index de66b87..1eaadca 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -15,8 +15,10 @@
package com.google.gerrit.acceptance.api.accounts;
import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.common.truth.Truth.assert_;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.GitUtil.deleteRef;
import static com.google.gerrit.acceptance.GitUtil.fetch;
@@ -62,8 +64,11 @@
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule.Action;
import com.google.gerrit.extensions.api.accounts.AccountInput;
+import com.google.gerrit.extensions.api.accounts.DeleteDraftCommentsInput;
+import com.google.gerrit.extensions.api.accounts.DeletedDraftCommentInfo;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.StarsInput;
import com.google.gerrit.extensions.api.config.ConsistencyCheckInfo;
@@ -73,6 +78,7 @@
import com.google.gerrit.extensions.common.AccountDetailInfo;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.EmailInfo;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.common.GroupInfo;
@@ -92,6 +98,7 @@
import com.google.gerrit.gpg.testing.TestKey;
import com.google.gerrit.mail.Address;
import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
@@ -2583,6 +2590,160 @@
assertThat(stalenessChecker.isStale(accountId)).isFalse();
}
+ @Test
+ public void deleteAllDraftComments() throws Exception {
+ try {
+ TestTimeUtil.resetWithClockStep(1, SECONDS);
+ Project.NameKey project2 = createProject("project2");
+ PushOneCommit.Result r1 = createChange();
+
+ TestRepository<?> tr2 = cloneProject(project2);
+ PushOneCommit.Result r2 =
+ createChange(
+ tr2,
+ "refs/heads/master",
+ "Change in project2",
+ PushOneCommit.FILE_NAME,
+ "content2",
+ null);
+
+ // Create 2 drafts each on both changes for user.
+ setApiUser(user);
+ createDraft(r1, PushOneCommit.FILE_NAME, "draft 1a");
+ createDraft(r1, PushOneCommit.FILE_NAME, "draft 1b");
+ createDraft(r2, PushOneCommit.FILE_NAME, "draft 2a");
+ createDraft(r2, PushOneCommit.FILE_NAME, "draft 2b");
+ assertThat(gApi.changes().id(r1.getChangeId()).current().draftsAsList()).hasSize(2);
+ assertThat(gApi.changes().id(r2.getChangeId()).current().draftsAsList()).hasSize(2);
+
+ // Create 1 draft on first change for admin.
+ setApiUser(admin);
+ createDraft(r1, PushOneCommit.FILE_NAME, "admin draft");
+ assertThat(gApi.changes().id(r1.getChangeId()).current().draftsAsList()).hasSize(1);
+
+ // Delete user's draft comments; leave admin's alone.
+ setApiUser(user);
+ List<DeletedDraftCommentInfo> result =
+ gApi.accounts().self().deleteDraftComments(new DeleteDraftCommentsInput());
+
+ // Results are ordered according to the change search, most recently updated first.
+ assertThat(result).hasSize(2);
+ DeletedDraftCommentInfo del2 = result.get(0);
+ assertThat(del2.change.changeId).isEqualTo(r2.getChangeId());
+ assertThat(del2.deleted.stream().map(c -> c.message)).containsExactly("draft 2a", "draft 2b");
+ DeletedDraftCommentInfo del1 = result.get(1);
+ assertThat(del1.change.changeId).isEqualTo(r1.getChangeId());
+ assertThat(del1.deleted.stream().map(c -> c.message)).containsExactly("draft 1a", "draft 1b");
+
+ assertThat(gApi.changes().id(r1.getChangeId()).current().draftsAsList()).isEmpty();
+ assertThat(gApi.changes().id(r2.getChangeId()).current().draftsAsList()).isEmpty();
+
+ setApiUser(admin);
+ assertThat(gApi.changes().id(r1.getChangeId()).current().draftsAsList()).hasSize(1);
+ } finally {
+ cleanUpDrafts();
+ }
+ }
+
+ @Test
+ public void deleteDraftCommentsByQuery() throws Exception {
+ try {
+ PushOneCommit.Result r1 = createChange();
+ PushOneCommit.Result r2 = createChange();
+
+ createDraft(r1, PushOneCommit.FILE_NAME, "draft a");
+ createDraft(r2, PushOneCommit.FILE_NAME, "draft b");
+ assertThat(gApi.changes().id(r1.getChangeId()).current().draftsAsList()).hasSize(1);
+ assertThat(gApi.changes().id(r2.getChangeId()).current().draftsAsList()).hasSize(1);
+
+ List<DeletedDraftCommentInfo> result =
+ gApi.accounts()
+ .self()
+ .deleteDraftComments(new DeleteDraftCommentsInput("change:" + r1.getChangeId()));
+ assertThat(result).hasSize(1);
+ assertThat(result.get(0).change.changeId).isEqualTo(r1.getChangeId());
+ assertThat(result.get(0).deleted.stream().map(c -> c.message)).containsExactly("draft a");
+
+ assertThat(gApi.changes().id(r1.getChangeId()).current().draftsAsList()).isEmpty();
+ assertThat(gApi.changes().id(r2.getChangeId()).current().draftsAsList()).hasSize(1);
+ } finally {
+ cleanUpDrafts();
+ }
+ }
+
+ @Test
+ public void deleteOtherUsersDraftCommentsDisallowed() throws Exception {
+ try {
+ PushOneCommit.Result r = createChange();
+ setApiUser(user);
+ createDraft(r, PushOneCommit.FILE_NAME, "draft");
+ setApiUser(admin);
+ try {
+ gApi.accounts().id(user.id.get()).deleteDraftComments(new DeleteDraftCommentsInput());
+ assert_().fail("expected AuthException");
+ } catch (AuthException e) {
+ assertThat(e).hasMessageThat().isEqualTo("Cannot delete drafts of other user");
+ }
+ } finally {
+ cleanUpDrafts();
+ }
+ }
+
+ @Test
+ public void deleteDraftCommentsSkipsInvisibleChanges() throws Exception {
+ try {
+ createBranch(new Branch.NameKey(project, "secret"));
+ PushOneCommit.Result r1 = createChange();
+ PushOneCommit.Result r2 = createChange("refs/for/secret");
+
+ setApiUser(user);
+ createDraft(r1, PushOneCommit.FILE_NAME, "draft a");
+ createDraft(r2, PushOneCommit.FILE_NAME, "draft b");
+ assertThat(gApi.changes().id(r1.getChangeId()).current().draftsAsList()).hasSize(1);
+ assertThat(gApi.changes().id(r2.getChangeId()).current().draftsAsList()).hasSize(1);
+
+ block(project, "refs/heads/secret", Permission.READ, REGISTERED_USERS);
+ List<DeletedDraftCommentInfo> result =
+ gApi.accounts().self().deleteDraftComments(new DeleteDraftCommentsInput());
+ assertThat(result).hasSize(1);
+ assertThat(result.get(0).change.changeId).isEqualTo(r1.getChangeId());
+ assertThat(result.get(0).deleted.stream().map(c -> c.message)).containsExactly("draft a");
+
+ removePermission(project, "refs/heads/secret", Permission.READ);
+ assertThat(gApi.changes().id(r1.getChangeId()).current().draftsAsList()).isEmpty();
+ // Draft still exists since change wasn't visible when drafts where deleted.
+ assertThat(gApi.changes().id(r2.getChangeId()).current().draftsAsList()).hasSize(1);
+ } finally {
+ cleanUpDrafts();
+ }
+ }
+
+ private void createDraft(PushOneCommit.Result r, String path, String message) throws Exception {
+ DraftInput in = new DraftInput();
+ in.path = path;
+ in.line = 1;
+ in.message = message;
+ gApi.changes().id(r.getChangeId()).current().createDraft(in);
+ }
+
+ private void cleanUpDrafts() throws Exception {
+ for (TestAccount testAccount : accountCreator.getAll()) {
+ setApiUser(testAccount);
+ for (ChangeInfo changeInfo : gApi.changes().query("has:draft").get()) {
+ for (CommentInfo c :
+ gApi.changes()
+ .id(changeInfo.id)
+ .drafts()
+ .values()
+ .stream()
+ .flatMap(List::stream)
+ .collect(toImmutableList())) {
+ gApi.changes().id(changeInfo.id).revision(c.patchSet).draft(c.id).delete();
+ }
+ }
+ }
+ }
+
private static Correspondence<GroupInfo, String> getGroupToNameCorrespondence() {
return new Correspondence<GroupInfo, String>() {
@Override
diff --git a/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
index 1a401b0..b0adba7 100644
--- a/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
@@ -88,6 +88,7 @@
RestCall.put("/accounts/%s/agreements"),
RestCall.get("/accounts/%s/external.ids"),
RestCall.post("/accounts/%s/external.ids:delete"),
+ RestCall.post("/accounts/%s/drafts:delete"),
RestCall.get("/accounts/%s/oauthtoken"),
RestCall.get("/accounts/%s/capabilities"),
RestCall.get("/accounts/%s/capabilities/viewPlugins"),
