Stop relying on server-side settings for option-hexes.
This was leading to duplicate code that needed to be kept in sync.
This also makes it easier to deal with option-lists for the case we
want option lists and not a hex (namely for sendreply).
Google-Bug-Id: b/296175151
Release-Notes: skip
Change-Id: I3f9bb4fb206e45a0db071f151fcc710e4c74d5e5
diff --git a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
index fb28d30..1d9a0af 100644
--- a/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexHtmlUtil.java
@@ -27,7 +27,6 @@
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.accounts.AccountApi;
import com.google.gerrit.extensions.api.config.Server;
-import com.google.gerrit.extensions.client.ListOption;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.json.OutputFormat;
@@ -95,8 +94,6 @@
case CHANGE:
case DIFF:
data.put(
- "defaultChangeDetailHex", ListOption.toHex(IndexPreloadingUtil.CHANGE_DETAIL_OPTIONS));
- data.put(
"changeRequestsPath",
IndexPreloadingUtil.computeChangeRequestsPath(requestedPath, page).get());
data.put("changeNum", IndexPreloadingUtil.computeChangeNum(requestedPath, page).get());
@@ -121,7 +118,6 @@
serializeObject(GSON, accountApi.getEditPreferences()));
data.put("userIsAuthenticated", true);
if (page == RequestedPage.DASHBOARD) {
- data.put("defaultDashboardHex", ListOption.toHex(IndexPreloadingUtil.DASHBOARD_OPTIONS));
data.put("dashboardQuery", IndexPreloadingUtil.computeDashboardQueryList());
}
} catch (AuthException e) {
diff --git a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
index 36fa61b..3ada18d 100644
--- a/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
+++ b/java/com/google/gerrit/httpd/raw/IndexPreloadingUtil.java
@@ -17,12 +17,10 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.UsedAt;
import com.google.gerrit.common.UsedAt.Project;
-import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.restapi.Url;
import java.net.URI;
import java.net.URISyntaxException;
@@ -83,25 +81,6 @@
NEW_USER)
.map(query -> query.replaceAll("\\$\\{user}", "self"))
.collect(toImmutableList());
- public static final ImmutableSet<ListChangesOption> DASHBOARD_OPTIONS =
- ImmutableSet.of(
- ListChangesOption.LABELS,
- ListChangesOption.DETAILED_ACCOUNTS,
- ListChangesOption.SUBMIT_REQUIREMENTS,
- ListChangesOption.STAR);
-
- public static final ImmutableSet<ListChangesOption> CHANGE_DETAIL_OPTIONS =
- ImmutableSet.of(
- ListChangesOption.ALL_COMMITS,
- ListChangesOption.ALL_REVISIONS,
- ListChangesOption.CHANGE_ACTIONS,
- ListChangesOption.DETAILED_LABELS,
- ListChangesOption.DOWNLOAD_COMMANDS,
- ListChangesOption.MESSAGES,
- ListChangesOption.SUBMITTABLE,
- ListChangesOption.WEB_LINKS,
- ListChangesOption.SKIP_DIFFSTAT,
- ListChangesOption.SUBMIT_REQUIREMENTS);
@Nullable
public static String getPath(@Nullable String requestedURL) throws URISyntaxException {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index eab6770..97ec978 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -160,7 +160,6 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.git.ObjectIds;
-import com.google.gerrit.httpd.raw.IndexPreloadingUtil;
import com.google.gerrit.index.IndexConfig;
import com.google.gerrit.index.query.PostFilterPredicate;
import com.google.gerrit.server.ChangeMessagesUtil;
@@ -239,6 +238,13 @@
@Inject private AccountControl.Factory accountControlFactory;
@Inject private ChangeOperations changeOperations;
+ public static final ImmutableSet<ListChangesOption> DASHBOARD_OPTIONS =
+ ImmutableSet.of(
+ ListChangesOption.LABELS,
+ ListChangesOption.DETAILED_ACCOUNTS,
+ ListChangesOption.SUBMIT_REQUIREMENTS,
+ ListChangesOption.STAR);
+
@Inject
@Named("diff_intraline")
private Cache<IntraLineDiffKey, IntraLineDiff> intraCache;
@@ -3132,7 +3138,7 @@
gApi.changes()
.query()
.withQuery("project:{" + project.get() + "} (status:open OR status:closed)")
- .withOptions(IndexPreloadingUtil.DASHBOARD_OPTIONS)
+ .withOptions(DASHBOARD_OPTIONS)
.get())
.hasSize(2);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index ab2f358..d1e6bcba 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -70,7 +70,6 @@
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.httpd.raw.IndexPreloadingUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.testing.TestLabels;
@@ -103,6 +102,13 @@
@Inject private ExtensionRegistry extensionRegistry;
@Inject private IndexOperations.Change changeIndexOperations;
+ private static final ImmutableSet<ListChangesOption> DASHBOARD_OPTIONS =
+ ImmutableSet.of(
+ ListChangesOption.LABELS,
+ ListChangesOption.DETAILED_ACCOUNTS,
+ ListChangesOption.SUBMIT_REQUIREMENTS,
+ ListChangesOption.STAR);
+
@Test
public void submitRecords() throws Exception {
PushOneCommit.Result r = createChange();
@@ -2952,7 +2958,7 @@
.withQuery("project:{" + project.get() + "} (status:open OR status:closed)")
.withOptions(
new ImmutableSet.Builder<ListChangesOption>()
- .addAll(IndexPreloadingUtil.DASHBOARD_OPTIONS)
+ .addAll(DASHBOARD_OPTIONS)
.add(ListChangesOption.SUBMIT_REQUIREMENTS)
.build())
.get();
diff --git a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
index cc1ee00..5cb7feb 100644
--- a/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
+++ b/javatests/com/google/gerrit/httpd/raw/IndexHtmlUtilTest.java
@@ -115,9 +115,7 @@
when(gerritApi.config()).thenReturn(configApi);
assertThat(dynamicTemplateData(gerritApi, "/c/project/+/123"))
- .containsAtLeast(
- "defaultChangeDetailHex", "1916314",
- "changeRequestsPath", "changes/project~123");
+ .containsAtLeast("changeRequestsPath", "changes/project~123");
}
private static SanitizedContent ordain(String s) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index a190b62..8fd619d 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -1442,17 +1442,13 @@
this.includeComments = true;
fireNoBubble(this, 'send', {});
fireIronAnnounce(this, 'Reply sent');
- return;
})
- .then(result => {
- this.disabled = false;
- return result;
- })
+ .then(result => result)
.catch(err => {
- this.disabled = false;
throw err;
})
.finally(() => {
+ this.disabled = false;
if (this.patchsetLevelGrComment) {
this.patchsetLevelGrComment.disableAutoSaving = false;
}
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 f6c3156..11b9ccb 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
@@ -1027,7 +1027,7 @@
/**
* Construct the uri to get list of changes.
*
- * If options is undefined then default options (see _getChangesOptionsHex) is
+ * If options is undefined then default options (see getListChangesOptionsHex) is
* used.
*/
getRequestForGetChanges(
@@ -1036,7 +1036,7 @@
offset?: 'n,z' | number,
options?: string
) {
- options = options || this._getChangesOptionsHex();
+ options = options || this.getListChangesOptionsHex();
if (offset === 'n,z') {
offset = 0;
}
@@ -1061,7 +1061,7 @@
/**
* For every query fetches the matching changes.
*
- * If options is undefined then default options (see _getChangesOptionsHex) is
+ * If options is undefined then default options (see getListChangesOptionsHex) is
* used.
*/
getChangesForMultipleQueries(
@@ -1108,7 +1108,7 @@
/**
* Fetches changes that match the query.
*
- * If options is undefined then default options (see _getChangesOptionsHex) is
+ * If options is undefined then default options (see getListChangesOptionsHex) is
* used.
*/
getChanges(
@@ -1183,34 +1183,31 @@
);
}
- getChangeDetail(
+ async getChangeDetail(
changeNum?: NumericChangeId,
errFn?: ErrorCallback,
cancelCondition?: CancelConditionCallback
): Promise<ParsedChangeInfo | undefined> {
- if (!changeNum) return Promise.resolve(undefined);
- return this.getConfig(false).then(config => {
- const optionsHex = this._getChangeOptionsHex(config);
- return this._getChangeDetail(
- changeNum,
- optionsHex,
- errFn,
- cancelCondition
- ).then(detail =>
- // detail has ChangeViewChangeInfo type because the optionsHex always
- // includes ALL_REVISIONS flag.
- GrReviewerUpdatesParser.parse(detail as ChangeViewChangeInfo)
- );
- });
+ if (!changeNum) return;
+ const optionsHex = await this.getChangeOptionsHex();
+
+ return this._getChangeDetail(
+ changeNum,
+ optionsHex,
+ errFn,
+ cancelCondition
+ ).then(detail =>
+ // detail has ChangeViewChangeInfo type because the optionsHex always
+ // includes ALL_REVISIONS flag.
+ GrReviewerUpdatesParser.parse(detail as ChangeViewChangeInfo)
+ );
}
- _getChangesOptionsHex() {
- if (
- window.DEFAULT_DETAIL_HEXES &&
- window.DEFAULT_DETAIL_HEXES.dashboardPage
- ) {
- return window.DEFAULT_DETAIL_HEXES.dashboardPage;
- }
+ /**
+ * Returns the options to use for querying multiple changes (e.g. dashboard or search).
+ * @return The options hex to use when fetching multiple changes.
+ */
+ private getListChangesOptionsHex() {
const options = [
ListChangesOption.LABELS,
ListChangesOption.DETAILED_ACCOUNTS,
@@ -1221,14 +1218,8 @@
return listChangesOptionsToHex(...options);
}
- _getChangeOptionsHex(config?: ServerInfo) {
- if (
- window.DEFAULT_DETAIL_HEXES &&
- window.DEFAULT_DETAIL_HEXES.changePage &&
- (!config || !(config.receive && config.receive.enable_signed_push))
- ) {
- return window.DEFAULT_DETAIL_HEXES.changePage;
- }
+ async getChangeOptionsHex(): Promise<string> {
+ const config = await this.getConfig(false);
// This list MUST be kept in sync with
// ChangeIT#changeDetailsDoesNotRequireIndex
diff --git a/polygerrit-ui/app/types/globals.ts b/polygerrit-ui/app/types/globals.ts
index 4a709b0..7448a27 100644
--- a/polygerrit-ui/app/types/globals.ts
+++ b/polygerrit-ui/app/types/globals.ts
@@ -23,12 +23,6 @@
// it's defined because of limitations from typescript, which don't import .mjs
page?: unknown;
hljs?: HighlightJS;
-
- DEFAULT_DETAIL_HEXES?: {
- diffPage?: string;
- changePage?: string;
- dashboardPage?: string;
- };
STATIC_RESOURCE_PATH?: string;
PRELOADED_QUERIES?: {
diff --git a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
index dbfef44..b5892794 100644
--- a/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
+++ b/resources/com/google/gerrit/httpd/raw/PolyGerritIndexHtml.soy
@@ -48,14 +48,6 @@
// Disable extra font load from paper-styles
window.polymerSkipLoadingFontRoboto = true;
window.CLOSURE_NO_DEPS = true;
- window.DEFAULT_DETAIL_HEXES = {lb}
- {if $defaultChangeDetailHex}
- changePage: '{$defaultChangeDetailHex}',
- {/if}
- {if $defaultDashboardHex}
- dashboardPage: '{$defaultDashboardHex}',
- {/if}
- {rb};
window.PRELOADED_QUERIES = {lb}
{if $userIsAuthenticated and $defaultDashboardHex and $dashboardQuery}
dashboardQuery: [{for $query in $dashboardQuery}{$query},{/for}],
