Merge "Minor improvements to non-personal dashboards"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index d9ab64d..5a2fc2e 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -384,6 +384,13 @@
* `CUSTOM_KEYED_VALUES`: include the custom key-value map
---
+[[star]]
+--
+* `STAR`: include the `starred` field in
+ link:#change-info[ChangeInfo], which indicates if the change is starred
+ by the current user or not.
+--
+
.Request
----
GET /changes/?q=97&o=CURRENT_REVISION&o=CURRENT_COMMIT&o=CURRENT_FILES&o=DOWNLOAD_COMMANDS HTTP/1.0
@@ -6972,6 +6979,7 @@
link:rest-api-accounts.html#account-info[ AccountInfo] entity.
|`starred` |not set if `false`|
Whether the calling user has starred this change with the default label.
+Only set if link:#star[requested].
|`stars` |optional|
A list of star labels that are applied by the calling user to this
change. The labels are lexicographically sorted.
diff --git a/java/com/google/gerrit/extensions/client/ListChangesOption.java b/java/com/google/gerrit/extensions/client/ListChangesOption.java
index de4326e..e2a7c1e 100644
--- a/java/com/google/gerrit/extensions/client/ListChangesOption.java
+++ b/java/com/google/gerrit/extensions/client/ListChangesOption.java
@@ -91,7 +91,10 @@
SUBMIT_REQUIREMENTS(24),
/** Include custom keyed values. */
- CUSTOM_KEYED_VALUES(25);
+ CUSTOM_KEYED_VALUES(25),
+
+ /** Include the 'starred' field, that is if the change is starred by the current user . */
+ STAR(26);
private final int value;
diff --git a/java/com/google/gerrit/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java
index 2d18054..cf04029 100644
--- a/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -49,10 +49,12 @@
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
+import java.util.List;
import java.util.NavigableSet;
import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
+import java.util.stream.Collectors;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -217,6 +219,29 @@
}
/**
+ * Returns a subset of change IDs among the input {@code changeIds} list that are starred by the
+ * {@code caller} user.
+ */
+ public Set<Change.Id> areStarred(
+ Repository allUsersRepo, List<Change.Id> changeIds, Account.Id caller) {
+ List<String> starRefs =
+ changeIds.stream()
+ .map(c -> RefNames.refsStarredChanges(c, caller))
+ .collect(Collectors.toList());
+ try {
+ return allUsersRepo.getRefDatabase().exactRef(starRefs.toArray(new String[0])).keySet()
+ .stream()
+ .map(r -> Change.Id.fromAllUsersRef(r))
+ .collect(Collectors.toSet());
+ } catch (IOException e) {
+ logger.atWarning().withCause(e).log(
+ "Failed getting starred changes for account %d within changes: %s",
+ caller.get(), Joiner.on(", ").join(changeIds));
+ return ImmutableSet.of();
+ }
+ }
+
+ /**
* Unstar the given change for all users.
*
* <p>Intended for use only when we're about to delete a change. For that reason, the change is
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 839629e..4875978 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -31,6 +31,7 @@
import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED;
import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWER_UPDATES;
import static com.google.gerrit.extensions.client.ListChangesOption.SKIP_DIFFSTAT;
+import static com.google.gerrit.extensions.client.ListChangesOption.STAR;
import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE;
import static com.google.gerrit.extensions.client.ListChangesOption.SUBMIT_REQUIREMENTS;
import static com.google.gerrit.extensions.client.ListChangesOption.TRACKING_IDS;
@@ -100,10 +101,12 @@
import com.google.gerrit.server.account.AccountInfoComparator;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.cancellation.RequestCancelledException;
+import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.experiments.ExperimentFeatures;
import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -133,6 +136,7 @@
import java.util.stream.Collectors;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
/**
* Produces {@link ChangeInfo} (which is serialized to JSON afterwards) from {@link ChangeData}.
@@ -222,12 +226,15 @@
}
}
+ private final GitRepositoryManager repoManager;
+ private final AllUsersName allUsers;
private final Provider<CurrentUser> userProvider;
private final PermissionBackend permissionBackend;
private final ChangeData.Factory changeDataFactory;
private final AccountLoader.Factory accountLoaderFactory;
private final ImmutableSet<ListChangesOption> options;
private final ChangeMessagesUtil cmUtil;
+ private final StarredChangesUtil starredChangesUtil;
private final Provider<ConsistencyChecker> checkerProvider;
private final ActionJson actionJson;
private final ChangeNotes.Factory notesFactory;
@@ -247,12 +254,15 @@
@Inject
ChangeJson(
+ GitRepositoryManager repoManager,
+ AllUsersName allUsers,
ExperimentFeatures experimentFeatures,
Provider<CurrentUser> user,
PermissionBackend permissionBackend,
ChangeData.Factory cdf,
AccountLoader.Factory ailf,
ChangeMessagesUtil cmUtil,
+ StarredChangesUtil starredChangesUtil,
Provider<ConsistencyChecker> checkerProvider,
ActionJson actionJson,
ChangeNotes.Factory notesFactory,
@@ -264,12 +274,15 @@
@GerritServerConfig Config cfg,
@Assisted Iterable<ListChangesOption> options,
@Assisted Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory) {
+ this.repoManager = repoManager;
+ this.allUsers = allUsers;
this.experimentFeatures = experimentFeatures;
this.userProvider = user;
this.changeDataFactory = cdf;
this.permissionBackend = permissionBackend;
this.accountLoaderFactory = ailf;
this.cmUtil = cmUtil;
+ this.starredChangesUtil = starredChangesUtil;
this.checkerProvider = checkerProvider;
this.actionJson = actionJson;
this.notesFactory = notesFactory;
@@ -539,6 +552,9 @@
"Omitting corrupt change %s from results", cd.getId());
}
}
+ if (has(STAR)) {
+ populateStarField(changeInfos);
+ }
return changeInfos;
}
}
@@ -956,6 +972,25 @@
return map.build();
}
+ /** Populate the 'starred' field. */
+ private void populateStarField(List<ChangeInfo> changeInfos) {
+ // We populate the 'starred' field for all change infos together so that we open the All-Users
+ // repository only once
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers)) {
+ List<Change.Id> changeIds =
+ changeInfos.stream().map(c -> Change.id(c._number)).collect(Collectors.toList());
+ Set<Change.Id> starredChanges =
+ starredChangesUtil.areStarred(
+ allUsersRepo, changeIds, userProvider.get().asIdentifiedUser().getAccountId());
+ if (starredChanges.isEmpty()) {
+ return;
+ }
+ changeInfos.stream().forEach(c -> c.starred = starredChanges.contains(Change.id(c._number)));
+ } catch (IOException e) {
+ logger.atWarning().withCause(e).log("Failed to open All-Users repo.");
+ }
+ }
+
private List<PluginDefinedInfo> getPluginInfos(ChangeData cd) {
return getPluginInfos(Collections.singleton(cd)).get(cd.getId());
}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index a188251..688e5e4 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -83,6 +83,7 @@
import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
+import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.ProjectWatchInfo;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
@@ -2886,7 +2887,8 @@
}
@Test
- public void byStar() throws Exception {
+ public void byStar_withStarOptionSet() throws Exception {
+ // When star option is set, the 'starred' field is set in the change infos in response.
repo = createAndOpenProject("repo");
Change change1 = insert("repo", newChangeWithStatus(repo, Change.Status.MERGED));
@@ -2899,6 +2901,32 @@
// check default star
assertQuery("has:star", change1);
assertQuery("is:starred", change1);
+
+ // The 'Star' bit in the change data is also set correctly
+ List<ChangeInfo> changeInfos =
+ gApi.changes().query("has:star").withOptions(ListChangesOption.STAR).get();
+ assertThat(changeInfos.get(0).starred).isTrue();
+ }
+
+ @Test
+ public void byStar_withStarOptionNotSet() throws Exception {
+ // When star option is not set, the 'starred' field is not set in the change infos in response.
+ repo = createAndOpenProject("repo");
+ Change change1 = insert("repo", newChangeWithStatus(repo, Change.Status.MERGED));
+
+ Account.Id user2 =
+ accountManager.authenticate(authRequestFactory.createForUser("anotheruser")).getAccountId();
+ requestContext.setContext(newRequestContext(user2));
+
+ gApi.accounts().self().starChange(change1.getId().toString());
+
+ // check default star
+ assertQuery("has:star", change1);
+ assertQuery("is:starred", change1);
+
+ // The 'Star' bit in the change data is not set if the backfilling option is not set
+ List<ChangeInfo> changeInfos = gApi.changes().query("has:star").get();
+ assertThat(changeInfos.get(0).starred).isNull();
}
@Test
diff --git a/plugins/delete-project b/plugins/delete-project
index b183ee5..79674d9 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit b183ee5230273670f3235cc5b3cf32562ccfb7ee
+Subproject commit 79674d9e00f8458a2f4f6d3a91bd032579c3f25c
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 6bdabec..2ba0cf8 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -317,7 +317,7 @@
>
${this.renderMarkers(
submittedTogetherMarkersPredicate(index)
- )}${this.renderSubmittedTogetherLine(change, true)}
+ )}${this.renderSubmittedTogetherLine(change)}
</div>`
)}
</gr-related-collapse>
@@ -327,17 +327,14 @@
</section>`;
}
- private renderSubmittedTogetherLine(
- change: ChangeInfo,
- showSubmittabilityCheck: boolean
- ) {
+ private renderSubmittedTogetherLine(change: ChangeInfo) {
const truncatedRepo = truncatePath(change.project, 2);
return html`
<gr-related-change
.label=${this.renderChangeTitle(change)}
.change=${change}
.href=${createChangeUrl({change, usp: 'submitted-together'})}
- ?show-submittable-check=${showSubmittabilityCheck}
+ show-submittable-check
>${change.subject}</gr-related-change
>
<span class="repo" .title=${change.project}>${truncatedRepo}</span
@@ -376,7 +373,7 @@
>
${this.renderMarkers(
sameTopicMarkersPredicate(index)
- )}${this.renderSubmittedTogetherLine(change, false)}
+ )}${this.renderSubmittedTogetherLine(change)}
</div>`
)}
</gr-related-collapse>
diff --git a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
index b8758e2..61534a4 100644
--- a/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
+++ b/polygerrit-ui/app/services/gr-reporting/gr-reporting_impl.ts
@@ -88,6 +88,9 @@
[Timing.WEB_COMPONENTS_READY]: 0,
};
+// List of timers that should NOT be reset before a location change.
+const LOCATION_CHANGE_OK_TIMERS: (string | Timing)[] = [Timing.SEND_REPLY];
+
const SLOW_RPC_THRESHOLD = 500;
export function initErrorReporter(reportingService: ReportingService) {
@@ -600,6 +603,7 @@
beforeLocationChanged() {
for (const prop of Object.keys(this._baselines)) {
+ if (LOCATION_CHANGE_OK_TIMERS.includes(prop)) continue;
delete this._baselines[prop];
}
this.time(Timing.CHANGE_DISPLAYED);
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index b1f9646..68ec76a 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -1805,7 +1805,8 @@
}
const options = listChangesOptionsToHex(
ListChangesOption.CURRENT_REVISION,
- ListChangesOption.CURRENT_COMMIT
+ ListChangesOption.CURRENT_COMMIT,
+ ListChangesOption.SUBMITTABLE
);
const params = {
O: options,
@@ -1855,7 +1856,8 @@
ListChangesOption.LABELS,
ListChangesOption.CURRENT_REVISION,
ListChangesOption.CURRENT_COMMIT,
- ListChangesOption.DETAILED_LABELS
+ ListChangesOption.DETAILED_LABELS,
+ ListChangesOption.SUBMITTABLE
);
const queryTerms = [`topic:${escapeAndWrapSearchOperatorValue(topic)}`];
if (options?.openChangesOnly) {
