opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.15' into stable-2.16

Browse Source 
* stable-2.15:
  ChangeApi: Add methods to get change comments/drafts as list
  ListChangeComments: Extend ListChangeDrafts
  detach -> detached
  Fix anchor tag for settings page

Change-Id: Id9f3cf0b5ec13ac38f6f011c611c5096f622c1aa
This commit is contained in:
David Pursehouse
2019-08-12 10:21:37 +09:00
parent 7d4f5297b2 f841099db5
commit 8450917187
6 changed files with 144 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
java/com/google/gerrit/extensions/api/changes/ChangeApi.java
View File

@@ -271,6 +271,15 @@ public interface ChangeApi {
*/
Map<String, List<CommentInfo>> comments() throws RestApiException;
/**
* Get all published comments on a change as a list.
*
* @return comments as a list; comments have the {@code revision} field set to indicate their
* patch set.
* @throws RestApiException
*/
List<CommentInfo> commentsAsList() throws RestApiException;
/**
* Get all robot comments on a change.
*
@@ -289,6 +298,15 @@ public interface ChangeApi {
*/
Map<String, List<CommentInfo>> drafts() throws RestApiException;
/**
* Get all draft comments for the current user on a change as a list.
*
* @return drafts as a list; comments have the {@code revision} field set to indicate their patch
* set.
* @throws RestApiException
*/
List<CommentInfo> draftsAsList() throws RestApiException;
ChangeInfo check() throws RestApiException;
ChangeInfo check(FixInput fix) throws RestApiException;
@@ -561,6 +579,11 @@ public interface ChangeApi {
throw new NotImplementedException();
}
@Override
public List<CommentInfo> commentsAsList() throws RestApiException {
throw new NotImplementedException();
}
@Override
public Map<String, List<RobotCommentInfo>> robotComments() throws RestApiException {
throw new NotImplementedException();
@@ -571,6 +594,11 @@ public interface ChangeApi {
throw new NotImplementedException();
}
@Override
public List<CommentInfo> draftsAsList() throws RestApiException {
throw new NotImplementedException();
}
@Override
public ChangeInfo check() throws RestApiException {
throw new NotImplementedException();

18
java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
View File

@@ -633,6 +633,15 @@ class ChangeApiImpl implements ChangeApi {
}
}
@Override
public List<CommentInfo> commentsAsList() throws RestApiException {
try {
return listComments.getComments(change);
} catch (Exception e) {
throw asRestApiException("Cannot get comments", e);
}
}
@Override
public Map<String, List<RobotCommentInfo>> robotComments() throws RestApiException {
try {
@@ -651,6 +660,15 @@ class ChangeApiImpl implements ChangeApi {
}
}
@Override
public List<CommentInfo> draftsAsList() throws RestApiException {
try {
return listDrafts.getComments(change);
} catch (Exception e) {
throw asRestApiException("Cannot get drafts", e);
}
}
@Override
public ChangeInfo check() throws RestApiException {
try {

38
java/com/google/gerrit/server/restapi/change/ListChangeComments.java
View File

@@ -14,27 +14,18 @@
package com.google.gerrit.server.restapi.change;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.List;
import java.util.Map;
@Singleton
public class ListChangeComments implements RestReadView<ChangeResource> {
private final Provider<ReviewDb> db;
private final ChangeData.Factory changeDataFactory;
private final Provider<CommentJson> commentJson;
private final CommentsUtil commentsUtil;
public class ListChangeComments extends ListChangeDrafts {
@Inject
ListChangeComments(
@@ -42,21 +33,22 @@ public class ListChangeComments implements RestReadView<ChangeResource> {
ChangeData.Factory changeDataFactory,
Provider<CommentJson> commentJson,
CommentsUtil commentsUtil) {
this.db = db;
this.changeDataFactory = changeDataFactory;
this.commentJson = commentJson;
this.commentsUtil = commentsUtil;
super(db, changeDataFactory, commentJson, commentsUtil);
}
@Override
public Map<String, List<CommentInfo>> apply(ChangeResource rsrc)
throws AuthException, OrmException, PermissionBackendException {
protected Iterable<Comment> listComments(ChangeResource rsrc) throws OrmException {
ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes());
return commentJson
.get()
.setFillAccounts(true)
.setFillPatchSet(true)
.newCommentFormatter()
.format(commentsUtil.publishedByChange(db.get(), cd.notes()));
return commentsUtil.publishedByChange(db.get(), cd.notes());
}
@Override
protected boolean includeAuthorInfo() {
return true;
}
@Override
public boolean requireAuthentication() {
return false;
}
}

44
java/com/google/gerrit/server/restapi/change/ListChangeDrafts.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.restapi.change.CommentJson.CommentFormatter;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -32,10 +33,10 @@ import java.util.Map;
@Singleton
public class ListChangeDrafts implements RestReadView<ChangeResource> {
private final Provider<ReviewDb> db;
private final ChangeData.Factory changeDataFactory;
private final Provider<CommentJson> commentJson;
private final CommentsUtil commentsUtil;
protected final Provider<ReviewDb> db;
protected final ChangeData.Factory changeDataFactory;
protected final Provider<CommentJson> commentJson;
protected final CommentsUtil commentsUtil;
@Inject
ListChangeDrafts(
@@ -49,20 +50,41 @@ public class ListChangeDrafts implements RestReadView<ChangeResource> {
this.commentsUtil = commentsUtil;
}
protected Iterable<Comment> listComments(ChangeResource rsrc) throws OrmException {
ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes());
return commentsUtil.draftByChangeAuthor(db.get(), cd.notes(), rsrc.getUser().getAccountId());
}
protected boolean includeAuthorInfo() {
return false;
}
public boolean requireAuthentication() {
return true;
}
@Override
public Map<String, List<CommentInfo>> apply(ChangeResource rsrc)
throws AuthException, OrmException, PermissionBackendException {
if (!rsrc.getUser().isIdentifiedUser()) {
if (requireAuthentication() && !rsrc.getUser().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes());
List<Comment> drafts =
commentsUtil.draftByChangeAuthor(db.get(), cd.notes(), rsrc.getUser().getAccountId());
return getCommentFormatter().format(listComments(rsrc));
}
public List<CommentInfo> getComments(ChangeResource rsrc)
throws AuthException, OrmException, PermissionBackendException {
if (requireAuthentication() && !rsrc.getUser().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
return getCommentFormatter().formatAsList(listComments(rsrc));
}
private CommentFormatter getCommentFormatter() {
return commentJson
.get()
.setFillAccounts(false)
.setFillAccounts(includeAuthorInfo())
.setFillPatchSet(true)
.newCommentFormatter()
.format(drafts);
.newCommentFormatter();
}
}

28
javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
View File

@@ -115,6 +115,11 @@ public class CommentsIT extends AbstractDaemonTest {
assertThat(result).hasSize(1);
CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
assertThat(comment).isEqualTo(infoToDraft(path).apply(actual));
List<CommentInfo> list = getDraftCommentsAsList(changeId);
assertThat(list).hasSize(1);
actual = list.get(0);
assertThat(comment).isEqualTo(infoToDraft(path).apply(actual));
}
}
@@ -137,6 +142,10 @@ public class CommentsIT extends AbstractDaemonTest {
assertThat(result).hasSize(1);
assertThat(Lists.transform(result.get(path), infoToDraft(path)))
.containsExactly(c1, c2, c3, c4);
List<CommentInfo> list = getDraftCommentsAsList(changeId);
assertThat(list).hasSize(4);
assertThat(Lists.transform(list, infoToDraft(path))).containsExactly(c1, c2, c3, c4);
}
}
@@ -239,6 +248,9 @@ public class CommentsIT extends AbstractDaemonTest {
assertThat(result).isNotEmpty();
assertThat(Lists.transform(result.get(file), infoToInput(file)))
.containsExactly(c1, c2, c3, c4);
List<CommentInfo> list = getPublishedCommentsAsList(changeId);
assertThat(Lists.transform(list, infoToInput(file))).containsExactly(c1, c2, c3, c4);
}
// for the commit message comments on the auto-merge are not possible
@@ -257,6 +269,9 @@ public class CommentsIT extends AbstractDaemonTest {
Map<String, List<CommentInfo>> result = getPublishedComments(changeId, revId);
assertThat(result).isNotEmpty();
assertThat(Lists.transform(result.get(file), infoToInput(file))).containsExactly(c1, c2, c3);
List<CommentInfo> list = getPublishedCommentsAsList(changeId);
assertThat(Lists.transform(list, infoToInput(file))).containsExactly(c1, c2, c3);
}
}
@@ -281,6 +296,7 @@ public class CommentsIT extends AbstractDaemonTest {
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
assertThat(getPublishedComments(changeId, revId)).isEmpty();
assertThat(getPublishedCommentsAsList(changeId)).isEmpty();
List<CommentInput> expectedComments = new ArrayList<>();
for (Integer line : lines) {
@@ -297,6 +313,10 @@ public class CommentsIT extends AbstractDaemonTest {
List<CommentInfo> actualComments = result.get(file);
assertThat(Lists.transform(actualComments, infoToInput(file)))
.containsExactlyElementsIn(expectedComments);
List<CommentInfo> list = getPublishedCommentsAsList(changeId);
assertThat(Lists.transform(list, infoToInput(file)))
.containsExactlyElementsIn(expectedComments);
}
@Test
@@ -1110,11 +1130,19 @@ public class CommentsIT extends AbstractDaemonTest {
return gApi.changes().id(changeId).revision(revId).comments();
}
private List<CommentInfo> getPublishedCommentsAsList(String changeId) throws Exception {
return gApi.changes().id(changeId).commentsAsList();
}
private Map<String, List<CommentInfo>> getDraftComments(String changeId, String revId)
throws Exception {
return gApi.changes().id(changeId).revision(revId).drafts();
}
private List<CommentInfo> getDraftCommentsAsList(String changeId) throws Exception {
return gApi.changes().id(changeId).draftsAsList();
}
private CommentInfo getDraftComment(String changeId, String revId, String uuid) throws Exception {
return gApi.changes().id(changeId).revision(revId).draft(uuid).get();
}

22
polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
View File

@@ -157,6 +157,9 @@
],
attached() {
// Polymer 2: anchor tag won't work on shadow DOM
// we need to manually calling scrollIntoView when hash changed
this.listen(window, 'location-change', '_handleLocationChange');
this.fire('title-change', {title: 'Settings'});
this._isDark = !!window.localStorage.getItem('dark-theme');
@@ -213,9 +216,28 @@
this._loadingPromise = Promise.all(promises).then(() => {
this._loading = false;
// Handle anchor tag for initial load
this._handleLocationChange();
});
},
detached() {
this.unlisten(window, 'location-change', '_handleLocationChange');
},
_handleLocationChange() {
// Handle anchor tag after dom attached
const urlHash = window.location.hash;
if (urlHash) {
// Use shadowRoot for Polymer 2
const elem = (this.shadowRoot || document).querySelector(urlHash);
if (elem) {
elem.scrollIntoView();
}
}
},
reloadAccountDetail() {
Promise.all([
this.$.accountInfo.loadData(),