Merge "Remove unused methods"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index cc5a4b8..14330b5 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6919,10 +6919,14 @@
|==================================
|Field Name ||Description
|`id` ||
+The ID of the change. Subject to a 'GerritBackendFeature__return_new_change_info_id'
+ experiment, the format is either "'<project>\~<_number>'" (new format),
+or `triplet_id`. The experiment is needed to allow callers to migrate.
+'project' and '_number' are URL encoded. The callers must not rely on the format.
+|`triplet_id` ||
The ID of the change in the format "'<project>\~<branch>~<Change-Id>'",
where 'project', 'branch' and 'Change-Id' are URL encoded. For 'branch' the
-`refs/heads/` prefix is omitted. The callers must not rely on the format
- of the `id` field.
+`refs/heads/` prefix is omitted.
|`project` ||The name of the project.
|`branch` ||
The name of the target branch. +
diff --git a/java/com/google/gerrit/extensions/common/ChangeInfo.java b/java/com/google/gerrit/extensions/common/ChangeInfo.java
index dc9bc32..bfada78 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfo.java
@@ -37,6 +37,8 @@
// protected by any ListChangesOption.
public String id;
+ public String tripletId;
+
public String project;
public String branch;
public String topic;
diff --git a/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java b/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java
new file mode 100644
index 0000000..5333826
--- /dev/null
+++ b/java/com/google/gerrit/gpg/PublicKeyStoreUtil.java
@@ -0,0 +1,80 @@
+// Copyright (C) 2023 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.gpg;
+
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPGKEY;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.common.io.BaseEncoding;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIds;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.bouncycastle.openpgp.PGPException;
+import org.bouncycastle.openpgp.PGPPublicKey;
+import org.bouncycastle.openpgp.PGPPublicKeyRing;
+import org.eclipse.jgit.util.NB;
+
+public class PublicKeyStoreUtil {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final ExternalIds externalIds;
+ private final Provider<PublicKeyStore> storeProvider;
+
+ @Inject
+ public PublicKeyStoreUtil(ExternalIds externalIds, Provider<PublicKeyStore> storeProvider) {
+ this.externalIds = externalIds;
+ this.storeProvider = storeProvider;
+ }
+
+ public static byte[] parseFingerprint(ExternalId gpgKeyExtId) {
+ return BaseEncoding.base16().decode(gpgKeyExtId.key().id());
+ }
+
+ public static long keyIdFromFingerprint(byte[] fp) {
+ return NB.decodeInt64(fp, fp.length - 8);
+ }
+
+ public List<PGPPublicKey> listGpgKeysForUser(Account.Id id) throws PGPException, IOException {
+ List<PGPPublicKey> keys = new ArrayList<>();
+ try (PublicKeyStore store = storeProvider.get()) {
+ for (ExternalId extId : getGpgExtIds(id)) {
+ byte[] fp = parseFingerprint(extId);
+ boolean found = false;
+ for (PGPPublicKeyRing keyRing : store.get(keyIdFromFingerprint(fp))) {
+ if (Arrays.equals(keyRing.getPublicKey().getFingerprint(), fp)) {
+ found = true;
+ keys.add(keyRing.getPublicKey());
+ break;
+ }
+ }
+ if (!found) {
+ logger.atWarning().log(
+ "No public key stored for fingerprint %s", Fingerprint.toString(fp));
+ }
+ }
+ }
+ return keys;
+ }
+
+ public Iterable<ExternalId> getGpgExtIds(Account.Id id) throws IOException {
+ return externalIds.byAccount(id, SCHEME_GPGKEY);
+ }
+}
diff --git a/java/com/google/gerrit/gpg/server/GpgKeys.java b/java/com/google/gerrit/gpg/server/GpgKeys.java
index 00a0f57..9fb8286 100644
--- a/java/com/google/gerrit/gpg/server/GpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/GpgKeys.java
@@ -14,13 +14,10 @@
package com.google.gerrit.gpg.server;
-import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GPGKEY;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableList;
-import com.google.common.flogger.FluentLogger;
-import com.google.common.io.BaseEncoding;
import com.google.gerrit.extensions.common.GpgKeyInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -36,10 +33,10 @@
import com.google.gerrit.gpg.GerritPublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyStore;
+import com.google.gerrit.gpg.PublicKeyStoreUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.externalids.ExternalId;
-import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -48,36 +45,35 @@
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
+import java.util.List;
import java.util.Locale;
import java.util.Map;
import org.bouncycastle.bcpg.ArmoredOutputStream;
import org.bouncycastle.openpgp.PGPException;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
-import org.eclipse.jgit.util.NB;
@Singleton
public class GpgKeys implements ChildCollection<AccountResource, GpgKey> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final DynamicMap<RestView<GpgKey>> views;
private final Provider<CurrentUser> self;
+ private final PublicKeyStoreUtil publicKeyStoreUtil;
private final Provider<PublicKeyStore> storeProvider;
private final GerritPublicKeyChecker.Factory checkerFactory;
- private final ExternalIds externalIds;
@Inject
GpgKeys(
DynamicMap<RestView<GpgKey>> views,
Provider<CurrentUser> self,
+ PublicKeyStoreUtil publicKeyStoreUtil,
Provider<PublicKeyStore> storeProvider,
- GerritPublicKeyChecker.Factory checkerFactory,
- ExternalIds externalIds) {
+ GerritPublicKeyChecker.Factory checkerFactory) {
this.views = views;
this.self = self;
+ this.publicKeyStoreUtil = publicKeyStoreUtil;
this.storeProvider = storeProvider;
this.checkerFactory = checkerFactory;
- this.externalIds = externalIds;
}
@Override
@@ -90,10 +86,11 @@
throws ResourceNotFoundException, PGPException, IOException {
checkVisible(self, parent);
- ExternalId gpgKeyExtId = findGpgKey(id.get(), getGpgExtIds(parent));
- byte[] fp = parseFingerprint(gpgKeyExtId);
+ ExternalId gpgKeyExtId =
+ findGpgKey(id.get(), publicKeyStoreUtil.getGpgExtIds(parent.getUser().getAccountId()));
+ byte[] fp = PublicKeyStoreUtil.parseFingerprint(gpgKeyExtId);
try (PublicKeyStore store = storeProvider.get()) {
- long keyId = keyId(fp);
+ long keyId = PublicKeyStoreUtil.keyIdFromFingerprint(fp);
for (PGPPublicKeyRing keyRing : store.get(keyId)) {
PGPPublicKey key = keyRing.getPublicKey();
if (Arrays.equals(key.getFingerprint(), fp)) {
@@ -131,10 +128,6 @@
return gpgKeyExtId;
}
- static byte[] parseFingerprint(ExternalId gpgKeyExtId) {
- return BaseEncoding.base16().decode(gpgKeyExtId.key().id());
- }
-
@Override
public DynamicMap<RestView<GpgKey>> views() {
return views;
@@ -145,29 +138,17 @@
public Response<Map<String, GpgKeyInfo>> apply(AccountResource rsrc)
throws PGPException, IOException, ResourceNotFoundException {
checkVisible(self, rsrc);
- Map<String, GpgKeyInfo> keys = new HashMap<>();
+ List<PGPPublicKey> keys =
+ publicKeyStoreUtil.listGpgKeysForUser(rsrc.getUser().getAccountId());
+ Map<String, GpgKeyInfo> res = new HashMap<>();
try (PublicKeyStore store = storeProvider.get()) {
- for (ExternalId extId : getGpgExtIds(rsrc)) {
- byte[] fp = parseFingerprint(extId);
- boolean found = false;
- for (PGPPublicKeyRing keyRing : store.get(keyId(fp))) {
- if (Arrays.equals(keyRing.getPublicKey().getFingerprint(), fp)) {
- found = true;
- GpgKeyInfo info =
- toJson(
- keyRing.getPublicKey(), checkerFactory.create(rsrc.getUser(), store), store);
- keys.put(info.id, info);
- info.id = null;
- break;
- }
- }
- if (!found) {
- logger.atWarning().log(
- "No public key stored for fingerprint %s", Fingerprint.toString(fp));
- }
+ for (PGPPublicKey key : keys) {
+ GpgKeyInfo info = toJson(key, checkerFactory.create(rsrc.getUser(), store), store);
+ res.put(info.id, info);
+ info.id = null;
}
}
- return Response.ok(keys);
+ return Response.ok(res);
}
}
@@ -194,14 +175,6 @@
}
}
- private Iterable<ExternalId> getGpgExtIds(AccountResource rsrc) throws IOException {
- return externalIds.byAccount(rsrc.getUser().getAccountId(), SCHEME_GPGKEY);
- }
-
- private static long keyId(byte[] fp) {
- return NB.decodeInt64(fp, fp.length - 8);
- }
-
static void checkVisible(Provider<CurrentUser> self, AccountResource rsrc)
throws ResourceNotFoundException {
if (!BouncyCastleUtil.havePGP()) {
diff --git a/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/java/com/google/gerrit/gpg/server/PostGpgKeys.java
index d51ee6a..a45c400 100644
--- a/java/com/google/gerrit/gpg/server/PostGpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/PostGpgKeys.java
@@ -46,6 +46,7 @@
import com.google.gerrit.gpg.GerritPublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyChecker;
import com.google.gerrit.gpg.PublicKeyStore;
+import com.google.gerrit.gpg.PublicKeyStoreUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
@@ -175,7 +176,8 @@
for (String id : input.delete) {
try {
ExternalId gpgKeyExtId = GpgKeys.findGpgKey(id, existingExtIds);
- fingerprints.put(gpgKeyExtId, new Fingerprint(GpgKeys.parseFingerprint(gpgKeyExtId)));
+ fingerprints.put(
+ gpgKeyExtId, new Fingerprint(PublicKeyStoreUtil.parseFingerprint(gpgKeyExtId)));
} catch (ResourceNotFoundException e) {
// Skip removal.
}
diff --git a/java/com/google/gerrit/server/change/ActionJson.java b/java/com/google/gerrit/server/change/ActionJson.java
index e5a9534..bd36bfd 100644
--- a/java/com/google/gerrit/server/change/ActionJson.java
+++ b/java/com/google/gerrit/server/change/ActionJson.java
@@ -147,6 +147,7 @@
copy.unresolvedCommentCount = changeInfo.unresolvedCommentCount;
copy.workInProgress = changeInfo.workInProgress;
copy.id = changeInfo.id;
+ copy.tripletId = changeInfo.tripletId;
copy.cherryPickOfChange = changeInfo.cherryPickOfChange;
copy.cherryPickOfPatchSet = changeInfo.cherryPickOfPatchSet;
return copy;
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index f733a7b..ae4eff3 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -101,6 +101,8 @@
import com.google.gerrit.server.cancellation.RequestCancelledException;
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.index.change.ChangeField;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -240,9 +242,11 @@
private AccountLoader accountLoader;
private FixInput fix;
+ private ExperimentFeatures experimentFeatures;
@Inject
ChangeJson(
+ ExperimentFeatures experimentFeatures,
Provider<CurrentUser> user,
PermissionBackend permissionBackend,
ChangeData.Factory cdf,
@@ -259,6 +263,7 @@
@GerritServerConfig Config cfg,
@Assisted Iterable<ListChangesOption> options,
@Assisted Optional<PluginDefinedInfosFactory> pluginDefinedInfosFactory) {
+ this.experimentFeatures = experimentFeatures;
this.userProvider = user;
this.changeDataFactory = cdf;
this.permissionBackend = permissionBackend;
@@ -422,10 +427,17 @@
return info;
}
- private static void finish(ChangeInfo info) {
- info.id =
+ private static void finish(ChangeInfo info, ExperimentFeatures experimentFeatures) {
+ info.tripletId =
Joiner.on('~')
.join(Url.encode(info.project), Url.encode(info.branch), Url.encode(info.changeId));
+ if (experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants.GERRIT_BACKEND_FEATURE_RETURN_NEW_CHANGE_INFO_ID)) {
+ info.id =
+ Joiner.on('~').join(Url.encode(info.project), Url.encode(String.valueOf(info._number)));
+ } else {
+ info.id = info.tripletId;
+ }
}
private static boolean containsAnyOf(
@@ -563,7 +575,7 @@
info.isPrivate = c.isPrivate() ? true : null;
info.workInProgress = c.isWorkInProgress() ? true : null;
info.hasReviewStarted = c.hasReviewStarted();
- finish(info);
+ finish(info, experimentFeatures);
} else {
info._number = result.id().get();
info.problems = result.problems();
@@ -727,7 +739,7 @@
if (needMessages) {
out.messages = messages(cd);
}
- finish(out);
+ finish(out, experimentFeatures);
// This block must come after the ChangeInfo is mostly populated, since
// it will be passed to ActionVisitors as-is.
diff --git a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
index e294d55..fd7d504 100644
--- a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
+++ b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
@@ -29,4 +29,11 @@
/** On BatchUpdate, do not await index completion before returning to the user */
public static String GERRIT_BACKEND_FEATURE_DO_NOT_AWAIT_CHANGE_INDEXING =
"GerritBackendFeature__do_not_await_change_indexing";
+
+ /**
+ * Sets ChangeInfo.id to "'<project>~<_number>'", instead of "'<project>~<branch>~<Change-Id>'",
+ * spearing an index lookup if the id is used in the follow-up API calls.
+ */
+ public static String GERRIT_BACKEND_FEATURE_RETURN_NEW_CHANGE_INFO_ID =
+ "GerritBackendFeature__return_new_change_info_id";
}
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChain.java b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
index b8afcb7..343fb72 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChain.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChain.java
@@ -317,10 +317,18 @@
private List<PatchSetData> getChainForCurrentPatchSet(ChangeResource rsrc)
throws PermissionBackendException, IOException {
- return Lists.reverse(
- getRelatedChangesUtil.getAncestors(
- changeDataFactory.create(rsrc.getNotes()),
- patchSetUtil.current(rsrc.getNotes()),
- true));
+ List<PatchSetData> ancestors =
+ Lists.reverse(
+ getRelatedChangesUtil.getAncestors(
+ changeDataFactory.create(rsrc.getNotes()),
+ patchSetUtil.current(rsrc.getNotes()),
+ true));
+ int eldestOpenAncestor = 0;
+ for (PatchSetData ps : ancestors) {
+ if (ps.data().change().isMerged()) {
+ eldestOpenAncestor++;
+ }
+ }
+ return ancestors.subList(eldestOpenAncestor, ancestors.size());
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 118b31b..96c86d4 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -246,11 +246,14 @@
private Cache<DiffSummaryKey, DiffSummary> diffSummaryCache;
@Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value = "GerritBackendFeature__return_new_change_info_id")
public void get() throws Exception {
PushOneCommit.Result r = createChange();
- String triplet = project.get() + "~master~" + r.getChangeId();
- ChangeInfo c = info(triplet);
- assertThat(c.id).isEqualTo(triplet);
+ String id = project.get() + "~" + r.getChange().getId().get();
+ ChangeInfo c = info(id);
+ assertThat(c.id).isEqualTo(id);
assertThat(c.project).isEqualTo(project.get());
assertThat(c.branch).isEqualTo("master");
assertThat(c.status).isEqualTo(ChangeStatus.NEW);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
index fd37fb0..5ecb5a7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
@@ -986,7 +986,7 @@
public void rebaseChain() throws Exception {
// Create changes with the following hierarchy:
// * HEAD
- // * r1
+ // * r (merged)
// * r2
// * r3
// * r4
@@ -1043,7 +1043,7 @@
final String newContent = "new content";
// Create changes with the following revision hierarchy:
// * HEAD
- // * r1
+ // * r (merged)
// * r2
// * r3/1 r3/2
// * r4
@@ -1079,6 +1079,71 @@
}
@Test
+ public void rebaseChainWithMergedAncestor() throws Exception {
+ final String file = "modified_file.txt";
+ final String newContent = "new content";
+
+ // Create changes with the following hierarchy:
+ // * HEAD
+ // * r (merged)
+ // * r2.1 r2.2 (merged)
+ // * r3
+ // * r4
+ // *r5
+ PushOneCommit.Result r = createChange();
+ PushOneCommit.Result r2 = createChange();
+ PushOneCommit.Result r3 = createChange();
+ PushOneCommit.Result r4 = createChange();
+ PushOneCommit.Result r5 = createChange();
+
+ // Approve and submit the first change
+ RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+ testRepo.reset("HEAD~1");
+
+ // Create r2.2
+ gApi.changes()
+ .id(r2.getChangeId())
+ .edit()
+ .modifyFile(file, RawInputUtil.create(newContent.getBytes(UTF_8)));
+ gApi.changes().id(r2.getChangeId()).edit().publish();
+ // Approve and submit r2.2
+ revision = gApi.changes().id(r2.getChangeId()).current();
+ revision.review(ReviewInput.approve());
+ revision.submit();
+
+ // Add an approval whose score should be copied on trivial rebase
+ gApi.changes().id(r3.getChangeId()).current().review(ReviewInput.recommend());
+
+ // Rebase the chain through r4.
+ verifyRebaseChainResponse(gApi.changes().id(r4.getChangeId()).rebaseChain(), false, r3, r4);
+
+ // Only r3 and r4 are rebased.
+ verifyRebaseForChange(r3.getChange().getId(), r2.getChange().getId(), true);
+ verifyRebaseForChange(r4.getChange().getId(), r3.getChange().getId(), false);
+
+ verifyChangeIsUpToDate(r2);
+ verifyChangeIsUpToDate(r3);
+ verifyChangeIsUpToDate(r4);
+
+ // r5 wasn't rebased.
+ assertThat(
+ gApi.changes()
+ .id(r5.getChangeId())
+ .get(CURRENT_REVISION)
+ .getCurrentRevision()
+ ._number)
+ .isEqualTo(1);
+
+ // Rebasing r5
+ verifyRebaseChainResponse(
+ gApi.changes().id(r5.getChangeId()).rebaseChain(), false, r3, r4, r5);
+
+ verifyRebaseForChange(r5.getChange().getId(), r4.getChange().getId(), false);
+ }
+
+ @Test
public void rebaseChainWithConflicts_conflictsForbidden() throws Exception {
PushOneCommit.Result r1 = createChange();
gApi.changes()
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 4eee3cb..e0c28bc 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -42,7 +42,6 @@
BranchName,
ChangeActionDialog,
ChangeInfo,
- ChangeViewChangeInfo,
CherryPickInput,
CommitId,
InheritedBooleanInfo,
@@ -100,7 +99,7 @@
import {changeModelToken} from '../../../models/change/change-model';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, css, html, nothing} from 'lit';
-import {customElement, property, query, state} from 'lit/decorators.js';
+import {customElement, query, state} from 'lit/decorators.js';
import {ifDefined} from 'lit/directives/if-defined.js';
import {assertIsDefined, queryAll, uuid} from '../../../utils/common-util';
import {Interaction} from '../../../constants/reporting';
@@ -113,6 +112,9 @@
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {modalStyles} from '../../../styles/gr-modal-styles';
import {subscribe} from '../../lit/subscription-controller';
+import {userModelToken} from '../../../models/user/user-model';
+import {ParsedChangeInfo} from '../../../types/types';
+import {configModelToken} from '../../../models/config/config-model';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -377,76 +379,54 @@
RevisionActions = RevisionActions;
- @property({type: Object})
- change?: ChangeViewChangeInfo;
+ @state() change?: ParsedChangeInfo;
- @state()
- actions: ActionNameToActionInfoMap = {};
+ @state() actions: ActionNameToActionInfoMap = {};
- @property({type: Array})
- primaryActionKeys: PrimaryActionKey[] = [
+ @state() primaryActionKeys: PrimaryActionKey[] = [
ChangeActions.READY,
RevisionActions.SUBMIT,
];
- @property({type: Boolean})
- disableEdit = false;
-
- // private but used in test
@state() _hideQuickApproveAction = false;
- @property({type: Object})
- account?: AccountInfo;
+ @state() account?: AccountInfo;
- @property({type: String})
- changeNum?: NumericChangeId;
+ @state() changeNum?: NumericChangeId;
- @property({type: String})
- changeStatus?: ChangeStatus;
+ @state() changeStatus?: ChangeStatus;
- @property({type: String})
- commitNum?: CommitId;
+ @state() commitNum?: CommitId;
@state() latestPatchNum?: PatchSetNumber;
- @property({type: String})
- commitMessage = '';
+ @state() commitMessage = '';
- @property({type: Object})
- revisionActions: ActionNameToActionInfoMap = {};
+ @state() revisionActions: ActionNameToActionInfoMap = {};
- @state() private revisionSubmitAction?: ActionInfo | null;
+ @state() revisionSubmitAction?: ActionInfo | null;
- // used as a proprty type so cannot be private
@state() revisionRebaseAction?: ActionInfo | null;
- @property({type: String})
- privateByDefault?: InheritedBooleanInfo;
+ @state() privateByDefault?: InheritedBooleanInfo;
- // private but used in test
@state() loading = true;
- // private but used in test
@state() actionLoadingMessage = '';
- @state() private inProgressActionKeys = new Set<string>();
+ @state() inProgressActionKeys = new Set<string>();
- // _computeAllActions always returns an array
- // private but used in test
@state() allActionValues: UIActionInfo[] = [];
- // private but used in test
@state() topLevelActions?: UIActionInfo[];
- // private but used in test
@state() topLevelPrimaryActions?: UIActionInfo[];
- // private but used in test
@state() topLevelSecondaryActions?: UIActionInfo[];
- @state() private menuActions?: MenuAction[];
+ @state() menuActions?: MenuAction[];
- @state() private overflowActions: OverflowAction[] = [
+ @state() overflowActions: OverflowAction[] = [
{
type: ActionType.CHANGE,
key: ChangeActions.WIP,
@@ -493,29 +473,21 @@
},
];
- @state() private actionPriorityOverrides: ActionPriorityOverride[] = [];
+ @state() actionPriorityOverrides: ActionPriorityOverride[] = [];
- @state() private additionalActions: UIActionInfo[] = [];
+ @state() additionalActions: UIActionInfo[] = [];
- // private but used in test
@state() hiddenActions: string[] = [];
- // private but used in test
@state() disabledMenuActions: string[] = [];
- // private but used in test
- @state()
- editPatchsetLoaded = false;
+ @state() editPatchsetLoaded = false;
- @property({type: Boolean})
- editMode = false;
+ @state() editMode = false;
- // private but used in test
- @state()
- editBasedOnCurrentPatchSet = true;
+ @state() editBasedOnCurrentPatchSet = true;
- @property({type: Boolean})
- loggedIn = false;
+ @state() loggedIn = false;
private readonly restApiService = getAppContext().restApiService;
@@ -523,6 +495,10 @@
private readonly getPluginLoader = resolve(this, pluginLoaderToken);
+ private readonly getUserModel = resolve(this, userModelToken);
+
+ private readonly getConfigModel = resolve(this, configModelToken);
+
private readonly getChangeModel = resolve(this, changeModelToken);
private readonly getStorage = resolve(this, storageServiceToken);
@@ -546,6 +522,51 @@
() => this.getChangeModel().patchNum$,
x => (this.editPatchsetLoaded = x === 'edit')
);
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ x => (this.changeNum = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().change$,
+ x => (this.change = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().status$,
+ x => (this.changeStatus = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().editMode$,
+ x => (this.editMode = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().revision$,
+ rev => (this.commitNum = rev?.commit?.commit)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().latestRevision$,
+ rev => (this.commitMessage = rev?.commit?.message ?? '')
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().account$,
+ x => (this.account = x)
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().loggedIn$,
+ x => (this.loggedIn = x)
+ );
+ subscribe(
+ this,
+ () => this.getConfigModel().repoConfig$,
+ config => (this.privateByDefault = config?.private_by_default)
+ );
}
override connectedCallback() {
@@ -865,7 +886,7 @@
this.revisionActions = revisionActions;
this.sendShowRevisionActions({
- change,
+ change: change as ChangeInfo,
revisionActions,
});
this.handleLoadingComplete();
@@ -1031,18 +1052,7 @@
}
private editStatusChanged() {
- // Hide change edits if not logged in
- if (this.change === undefined || !this.loggedIn) {
- return;
- }
- if (this.disableEdit) {
- delete this.actions.rebaseEdit;
- delete this.actions.publishEdit;
- delete this.actions.deleteEdit;
- delete this.actions.stopEdit;
- delete this.actions.edit;
- return;
- }
+ if (!this.change || !this.loggedIn) return;
if (this.editPatchsetLoaded) {
// Only show actions that mutate an edit if an actual edit patch set
// is loaded.
@@ -1121,7 +1131,7 @@
if (!this.change || !this.change.labels || !this.change.permitted_labels) {
return null;
}
- if (this.change && this.change.status === ChangeStatus.MERGED) {
+ if (this.change?.status === ChangeStatus.MERGED) {
return null;
}
let result;
@@ -1323,18 +1333,18 @@
// private but used in test
canSubmitChange() {
- if (!this.change) {
- return false;
- }
+ if (!this.change) return false;
+ const change = this.change as ChangeInfo;
+ const revision = this.getRevision(change, this.latestPatchNum);
return this.getPluginLoader().jsApiService.canSubmitChange(
- this.change,
- this.getRevision(this.change, this.latestPatchNum)
+ change,
+ revision
);
}
// private but used in test
- getRevision(change: ChangeViewChangeInfo, patchNum?: PatchSetNumber) {
- for (const rev of Object.values(change.revisions)) {
+ getRevision(change: ChangeInfo, patchNum?: PatchSetNumber) {
+ for (const rev of Object.values(change.revisions ?? {})) {
if (rev._number === patchNum) {
return rev;
}
@@ -1805,7 +1815,7 @@
if (dialog.init) dialog.init();
dialog.hidden = false;
assertIsDefined(this.actionsModal, 'actionsModal');
- this.actionsModal.showModal();
+ if (this.actionsModal.isConnected) this.actionsModal.showModal();
whenVisible(dialog, () => {
if (dialog.resetFocus) {
dialog.resetFocus();
@@ -1818,7 +1828,7 @@
// private but used in test
setReviewOnRevert(newChangeId: NumericChangeId) {
const review = this.getPluginLoader().jsApiService.getReviewPostRevert(
- this.change
+ this.change as ChangeInfo
);
if (!review) {
return Promise.resolve(undefined);
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
index 946191b..b628cc1 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.ts
@@ -129,6 +129,7 @@
element = await fixture<GrChangeActions>(html`
<gr-change-actions></gr-change-actions>
`);
+ element.changeStatus = ChangeStatus.NEW;
element.change = {
...createChangeViewChange(),
actions: {
@@ -155,7 +156,7 @@
await element.reload();
});
- test('render', () => {
+ test('render', async () => {
assert.shadowDom.equal(
element,
/* HTML */ `
@@ -200,6 +201,24 @@
Rebase
</gr-button>
</gr-tooltip-content>
+ <gr-tooltip-content
+ has-tooltip=""
+ position-below=""
+ title="Edit this change"
+ >
+ <gr-button
+ aria-disabled="false"
+ class="edit"
+ data-action-key="edit"
+ data-label="Edit"
+ link=""
+ role="button"
+ tabindex="0"
+ >
+ <gr-icon filled="" icon="edit"> </gr-icon>
+ Edit
+ </gr-button>
+ </gr-tooltip-content>
</section>
<gr-button
aria-disabled="false"
@@ -705,29 +724,6 @@
});
suite('change edits', () => {
- test('disableEdit', async () => {
- element.editMode = false;
- element.editBasedOnCurrentPatchSet = false;
- element.change = {
- ...createChangeViewChange(),
- status: ChangeStatus.NEW,
- };
- element.disableEdit = true;
- await element.updateComplete;
-
- assert.isNotOk(
- query(element, 'gr-button[data-action-key="publishEdit"]')
- );
- assert.isNotOk(
- query(element, 'gr-button[data-action-key="rebaseEdit"]')
- );
- assert.isNotOk(
- query(element, 'gr-button[data-action-key="deleteEdit"]')
- );
- assert.isNotOk(query(element, 'gr-button[data-action-key="edit"]'));
- assert.isNotOk(query(element, 'gr-button[data-action-key="stopEdit"]'));
- });
-
test('shows confirm dialog for delete edit', async () => {
element.loggedIn = true;
element.editMode = true;
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index b7851a6..1cb25ea 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -82,6 +82,12 @@
import {createChangeUrl} from '../../../models/views/change';
import {getChangeWeblinks} from '../../../utils/weblink-util';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
+import {subscribe} from '../../lit/subscription-controller';
+import {userModelToken} from '../../../models/user/user-model';
+import {resolve} from '../../../models/dependency';
+import {configModelToken} from '../../../models/config/config-model';
+import {changeModelToken} from '../../../models/change/change-model';
+import {relatedChangesModelToken} from '../../../models/change/related-changes-model';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -118,54 +124,90 @@
export class GrChangeMetadata extends LitElement {
@query('#webLinks') webLinks?: HTMLElement;
- @property({type: Object}) change?: ParsedChangeInfo;
-
- @property({type: Object}) revertedChange?: ChangeInfo;
-
- @property({type: Object}) account?: AccountDetailInfo;
-
- @property({type: Object}) revision?: RevisionInfo | EditRevisionInfo;
-
- // TODO: Just use `revision.commit` instead.
- @property({type: Object}) commitInfo?: CommitInfoWithRequiredCommit;
-
- @property({type: Object}) serverConfig?: ServerInfo;
-
+ // TODO: Convert to @state. That requires the change model to keep track of
+ // current revision actions. Then we can also get rid of the
+ // `revision-actions-changed` event.
@property({type: Boolean}) parentIsCurrent?: boolean;
- @property({type: Object}) repoConfig?: ConfigInfo;
+ @state() change?: ParsedChangeInfo;
- // private but used in test
+ @state() revertedChange?: ChangeInfo;
+
+ @state() account?: AccountDetailInfo;
+
+ @state() revision?: RevisionInfo | EditRevisionInfo;
+
+ @state() serverConfig?: ServerInfo;
+
+ @state() repoConfig?: ConfigInfo;
+
@state() mutable = false;
- @state() private readonly notCurrentMessage = NOT_CURRENT_MESSAGE;
+ @state() readonly notCurrentMessage = NOT_CURRENT_MESSAGE;
- // private but used in test
@state() topicReadOnly = true;
- // private but used in test
@state() hashtagReadOnly = true;
- @state() private pushCertificateValidation?: PushCertificateValidationInfo;
+ @state() pushCertificateValidation?: PushCertificateValidationInfo;
- // private but used in test
@state() settingTopic = false;
- // private but used in test
@state() currentParents: ParentCommitInfo[] = [];
- @state() private showAllSections = false;
+ @state() showAllSections = false;
- @state() private queryTopic?: AutocompleteQuery;
+ @state() queryTopic?: AutocompleteQuery;
- @state() private queryHashtag?: AutocompleteQuery;
+ @state() queryHashtag?: AutocompleteQuery;
private restApiService = getAppContext().restApiService;
private readonly reporting = getAppContext().reportingService;
+ private readonly getUserModel = resolve(this, userModelToken);
+
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
+ private readonly getRelatedChangesModel = resolve(
+ this,
+ relatedChangesModelToken
+ );
+
constructor() {
super();
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ serverConfig => (this.serverConfig = serverConfig)
+ );
+ subscribe(
+ this,
+ () => this.getConfigModel().repoConfig$,
+ repoConfig => (this.repoConfig = repoConfig)
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().account$,
+ account => (this.account = account)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().change$,
+ change => (this.change = change)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().revision$,
+ revision => (this.revision = revision)
+ );
+ subscribe(
+ this,
+ () => this.getRelatedChangesModel().revertingChange$,
+ revertingChange => (this.revertedChange = revertingChange)
+ );
this.queryTopic = (input: string) => this.getTopicSuggestions(input);
this.queryHashtag = (input: string) => this.getHashtagSuggestions(input);
}
@@ -735,7 +777,10 @@
// private but used in test
computeWebLinks(): WebLinkInfo[] {
- return getChangeWeblinks(this.commitInfo?.web_links, this.serverConfig);
+ return getChangeWeblinks(
+ this.revision?.commit?.web_links,
+ this.serverConfig
+ );
}
private computeStrategy() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
index c46fe24..e0e1364 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
@@ -9,7 +9,6 @@
import {ChangeRole, GrChangeMetadata} from './gr-change-metadata';
import {
createServerInfo,
- createUserConfig,
createParsedChange,
createAccountWithId,
createCommitInfoWithRequiredCommit,
@@ -61,18 +60,9 @@
let element: GrChangeMetadata;
setup(async () => {
- stubRestApi('getLoggedIn').returns(Promise.resolve(false));
- stubRestApi('getConfig').returns(
- Promise.resolve({
- ...createServerInfo(),
- user: {
- ...createUserConfig(),
- anonymouscowardname: 'test coward name',
- },
- })
- );
element = await fixture(html`<gr-change-metadata></gr-change-metadata>`);
element.change = createParsedChange();
+ element.account = undefined;
await element.updateComplete;
});
@@ -251,16 +241,22 @@
});
test('weblinks hidden when no weblinks', async () => {
- element.commitInfo = createCommitInfoWithRequiredCommit();
+ element.revision = {
+ ...createRevision(),
+ commit: createCommitInfoWithRequiredCommit(),
+ };
element.serverConfig = createServerInfo();
await element.updateComplete;
assert.isNull(element.webLinks);
});
test('weblinks hidden when only gitiles weblink', async () => {
- element.commitInfo = {
- ...createCommitInfoWithRequiredCommit(),
- web_links: [{...createWebLinkInfo(), name: 'gitiles', url: '#'}],
+ element.revision = {
+ ...createRevision(),
+ commit: {
+ ...createCommitInfoWithRequiredCommit(),
+ web_links: [{...createWebLinkInfo(), name: 'gitiles', url: '#'}],
+ },
};
element.serverConfig = createServerInfo();
await element.updateComplete;
@@ -270,9 +266,12 @@
test('weblinks hidden when sole weblink is set as primary', async () => {
const browser = 'browser';
- element.commitInfo = {
- ...createCommitInfoWithRequiredCommit(),
- web_links: [{...createWebLinkInfo(), name: browser, url: '#'}],
+ element.revision = {
+ ...createRevision(),
+ commit: {
+ ...createCommitInfoWithRequiredCommit(),
+ web_links: [{...createWebLinkInfo(), name: browser, url: '#'}],
+ },
};
element.serverConfig = {
...createServerInfo(),
@@ -286,9 +285,12 @@
});
test('weblinks are visible when other weblinks', async () => {
- element.commitInfo = {
- ...createCommitInfoWithRequiredCommit(),
- web_links: [{...createWebLinkInfo(), name: 'test', url: '#'}],
+ element.revision = {
+ ...createRevision(),
+ commit: {
+ ...createCommitInfoWithRequiredCommit(),
+ web_links: [{...createWebLinkInfo(), name: 'test', url: '#'}],
+ },
};
await element.updateComplete;
const webLinks = element.webLinks!;
@@ -297,12 +299,15 @@
});
test('weblinks are visible when gitiles and other weblinks', async () => {
- element.commitInfo = {
- ...createCommitInfoWithRequiredCommit(),
- web_links: [
- {...createWebLinkInfo(), name: 'test', url: '#'},
- {...createWebLinkInfo(), name: 'gitiles', url: '#'},
- ],
+ element.revision = {
+ ...createRevision(),
+ commit: {
+ ...createCommitInfoWithRequiredCommit(),
+ web_links: [
+ {...createWebLinkInfo(), name: 'test', url: '#'},
+ {...createWebLinkInfo(), name: 'gitiles', url: '#'},
+ ],
+ },
};
await element.updateComplete;
const webLinks = element.webLinks!;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index e1ae35c..2e1a8b9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -335,10 +335,7 @@
@state()
private updateCheckTimerHandle?: number | null;
- // Private but used in tests.
- getEditMode(): boolean {
- return !!this.viewState?.edit || this.patchNum === EDIT;
- }
+ @state() editMode = false;
isSubmitEnabled(): boolean {
return !!(
@@ -664,6 +661,11 @@
);
subscribe(
this,
+ () => this.getChangeModel().editMode$,
+ editMode => (this.editMode = editMode)
+ );
+ subscribe(
+ this,
() => this.getChangeModel().patchNum$,
patchNum => (this.patchNum = patchNum)
);
@@ -1134,23 +1136,16 @@
${this.renderTabHeaders()} ${this.renderTabContent()}
${this.renderChangeLog()}
</div>
- <gr-apply-fix-dialog
- id="applyFixDialog"
- .change=${this.change}
- .changeNum=${this.changeNum}
- ></gr-apply-fix-dialog>
+ <gr-apply-fix-dialog id="applyFixDialog"></gr-apply-fix-dialog>
<dialog id="downloadModal" tabindex="-1">
<gr-download-dialog
id="downloadDialog"
- .change=${this.change}
- .config=${this.serverConfig?.download}
@close=${this.handleDownloadDialogClose}
></gr-download-dialog>
</dialog>
<dialog id="includedInModal" tabindex="-1">
<gr-included-in-dialog
id="includedInDialog"
- .changeNum=${this.changeNum}
@close=${this.handleIncludedInDialogClose}
></gr-included-in-dialog>
</dialog>
@@ -1287,27 +1282,18 @@
}
private renderCommitActions() {
- return html` <div class="commitActions">
- <!-- always show gr-change-actions regardless if logged in or not -->
- <gr-change-actions
- id="actions"
- .change=${this.change}
- .disableEdit=${false}
- .account=${this.account}
- .changeNum=${this.changeNum}
- .changeStatus=${this.change?.status}
- .commitNum=${this.revision?.commit?.commit}
- .commitMessage=${this.latestCommitMessage}
- .editMode=${this.getEditMode()}
- .privateByDefault=${this.projectConfig?.private_by_default}
- .loggedIn=${this.loggedIn}
- @edit-tap=${() => this.handleEditTap()}
- @stop-edit-tap=${() => this.handleStopEditTap()}
- @download-tap=${() => this.handleOpenDownloadDialog()}
- @included-tap=${() => this.handleOpenIncludedInDialog()}
- @revision-actions-changed=${this.handleRevisionActionsChanged}
- ></gr-change-actions>
- </div>`;
+ return html`
+ <div class="commitActions">
+ <gr-change-actions
+ id="actions"
+ @edit-tap=${() => this.handleEditTap()}
+ @stop-edit-tap=${() => this.handleStopEditTap()}
+ @download-tap=${() => this.handleOpenDownloadDialog()}
+ @included-tap=${() => this.handleOpenIncludedInDialog()}
+ @revision-actions-changed=${this.handleRevisionActionsChanged}
+ ></gr-change-actions>
+ </div>
+ `;
}
private renderChangeInfo() {
@@ -1315,20 +1301,13 @@
this.loggedIn,
this.editingCommitMessage,
this.change,
- this.getEditMode()
+ this.editMode
);
return html` <div class="changeInfo">
<div class="changeInfo-column changeMetadata">
<gr-change-metadata
id="metadata"
- .change=${this.change}
- .revertedChange=${this.revertingChange}
- .account=${this.account}
- .revision=${this.revision}
- .commitInfo=${this.revision?.commit}
- .serverConfig=${this.serverConfig}
.parentIsCurrent=${this.isParentCurrent()}
- .repoConfig=${this.projectConfig}
@show-reply-dialog=${this.handleShowReplyDialog}
>
</gr-change-metadata>
@@ -1461,7 +1440,7 @@
.changeNum=${this.changeNum}
.commitInfo=${this.revision?.commit}
.changeUrl=${this.computeChangeUrl()}
- .editMode=${this.getEditMode()}
+ .editMode=${this.editMode}
.loggedIn=${this.loggedIn}
.shownFileCount=${this.shownFileCount}
.filesExpanded=${this.fileList?.filesExpanded}
@@ -1475,7 +1454,7 @@
id="fileList"
.change=${this.change}
.changeNum=${this.changeNum}
- .editMode=${this.getEditMode()}
+ .editMode=${this.editMode}
@files-shown-changed=${(e: CustomEvent<{length: number}>) => {
this.shownFileCount = e.detail.length;
}}
@@ -1957,7 +1936,7 @@
change: this.change,
patchNum: this.patchNum,
basePatchNum: this.basePatchNum,
- edit: this.getEditMode(),
+ edit: this.editMode,
messageHash: hash,
});
history.replaceState(null, '', url);
@@ -2405,7 +2384,7 @@
// Private but used in tests.
computeHeaderClass() {
const classes = ['header'];
- if (this.getEditMode()) {
+ if (this.editMode) {
classes.push('editMode');
}
return classes.join(' ');
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 5331558..ae60449 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -73,7 +73,7 @@
import {Modifier} from '../../../utils/dom-util';
import {GrButton} from '../../shared/gr-button/gr-button';
import {GrCopyLinks} from '../gr-copy-links/gr-copy-links';
-import {ChangeChildView, ChangeViewState} from '../../../models/views/change';
+import {ChangeChildView} from '../../../models/views/change';
import {rootUrl} from '../../../utils/url-util';
import {testResolver} from '../../../test/common-test-setup';
import {UserModel, userModelToken} from '../../../models/user/user-model';
@@ -1318,10 +1318,9 @@
});
});
- test('header class computation', () => {
+ test('header class computation', async () => {
assert.equal(element.computeHeaderClass(), 'header');
- assertIsDefined(element.viewState);
- element.viewState.edit = true;
+ element.editMode = true;
assert.equal(element.computeHeaderClass(), 'header editMode');
});
@@ -1342,38 +1341,6 @@
assert.equal(scrollStub.lastCall.args[0], 'TEST');
});
- test('computeEditMode', async () => {
- const callCompute = async (viewState: ChangeViewState) => {
- element.viewState = viewState;
- element.patchNum = viewState.patchNum;
- element.basePatchNum = viewState.basePatchNum ?? PARENT;
- await element.updateComplete;
- return element.getEditMode();
- };
- assert.isTrue(
- await callCompute({
- ...createChangeViewState(),
- edit: true,
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- })
- );
- assert.isFalse(
- await callCompute({
- ...createChangeViewState(),
- basePatchNum: PARENT,
- patchNum: 1 as RevisionPatchSetNum,
- })
- );
- assert.isTrue(
- await callCompute({
- ...createChangeViewState(),
- basePatchNum: 1 as BasePatchSetNum,
- patchNum: EDIT,
- })
- );
- });
-
test('file-action-tap handling', async () => {
element.patchNum = 1 as RevisionPatchSetNum;
element.change = {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
index fedc377..e421c9c 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog.ts
@@ -15,6 +15,7 @@
import {resolve} from '../../../models/dependency';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {createSearchUrl} from '../../../models/views/search';
+import {ParsedChangeInfo} from '../../../types/types';
const ERR_COMMIT_NOT_FOUND = 'Unable to find the commit hash of this change.';
const INSERT_REASON_STRING = '<INSERT REASONING HERE>';
@@ -170,15 +171,23 @@
return this.revertType === RevertType.REVERT_SUBMISSION;
}
- modifyRevertMsg(change: ChangeInfo, commitMessage: string, message: string) {
+ modifyRevertMsg(
+ change: ParsedChangeInfo,
+ commitMessage: string,
+ message: string
+ ) {
return this.getPluginLoader().jsApiService.modifyRevertMsg(
- change,
+ change as ChangeInfo,
message,
commitMessage
);
}
- populate(change: ChangeInfo, commitMessage: string, changesCount: number) {
+ populate(
+ change: ParsedChangeInfo,
+ commitMessage: string,
+ changesCount: number
+ ) {
this.changesCount = changesCount;
// The option to revert a single change is always available
this.populateRevertSingleChangeMessage(
@@ -190,7 +199,7 @@
}
populateRevertSingleChangeMessage(
- change: ChangeInfo,
+ change: ParsedChangeInfo,
commitMessage: string,
commitHash?: CommitId
) {
@@ -215,18 +224,21 @@
}
private modifyRevertSubmissionMsg(
- change: ChangeInfo,
+ change: ParsedChangeInfo,
msg: string,
commitMessage: string
) {
return this.getPluginLoader().jsApiService.modifyRevertSubmissionMsg(
- change,
+ change as ChangeInfo,
msg,
commitMessage
);
}
- populateRevertSubmissionMessage(change: ChangeInfo, commitMessage: string) {
+ populateRevertSubmissionMessage(
+ change: ParsedChangeInfo,
+ commitMessage: string
+ ) {
// Follow the same convention of the revert
const commitHash = change.current_revision;
if (!commitHash) {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
index 904285f..8d71e15 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-revert-dialog/gr-confirm-revert-dialog_test.ts
@@ -5,7 +5,7 @@
*/
import {fixture, html, assert} from '@open-wc/testing';
import '../../../test/common-test-setup';
-import {createChange} from '../../../test/test-data-generators';
+import {createParsedChange} from '../../../test/test-data-generators';
import {ChangeSubmissionId, CommitId} from '../../../types/common';
import './gr-confirm-revert-dialog';
import {GrConfirmRevertDialog} from './gr-confirm-revert-dialog';
@@ -48,7 +48,7 @@
const alertStub = sinon.stub();
element.addEventListener('show-alert', alertStub);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'not a commitHash in sight',
undefined
);
@@ -58,7 +58,7 @@
test('single line', () => {
assert.isNotOk(element.message);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'one line commit\n\nChange-Id: abcdefg\n',
'abcd123' as CommitId
);
@@ -72,7 +72,7 @@
test('multi line', () => {
assert.isNotOk(element.message);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'many lines\ncommit\n\nmessage\n\nChange-Id: abcdefg\n',
'abcd123' as CommitId
);
@@ -86,7 +86,7 @@
test('issue above change id', () => {
assert.isNotOk(element.message);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'much lines\nvery\n\ncommit\n\nBug: Issue 42\nChange-Id: abcdefg\n',
'abcd123' as CommitId
);
@@ -100,7 +100,7 @@
test('revert a revert', () => {
assert.isNotOk(element.message);
element.populateRevertSingleChangeMessage(
- createChange(),
+ createParsedChange(),
'Revert "one line commit"\n\nChange-Id: abcdefg\n',
'abcd123' as CommitId
);
@@ -115,7 +115,7 @@
element.changesCount = 3;
element.populateRevertSubmissionMessage(
{
- ...createChange(),
+ ...createParsedChange(),
submission_id: '5545' as ChangeSubmissionId,
current_revision: 'abcd123' as CommitId,
},
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
index 11dc890..e1808bf 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
@@ -5,7 +5,7 @@
*/
import '../../shared/gr-download-commands/gr-download-commands';
import {changeBaseURL, getRevisionKey} from '../../../utils/change-util';
-import {ChangeInfo, DownloadInfo, PatchSetNum} from '../../../types/common';
+import {DownloadInfo, PatchSetNum} from '../../../types/common';
import {GrDownloadCommands} from '../../shared/gr-download-commands/gr-download-commands';
import {GrButton} from '../../shared/gr-button/gr-button';
import {copyToClipbard, hasOwnProperty} from '../../../utils/common-util';
@@ -13,13 +13,15 @@
import {fontStyles} from '../../../styles/gr-font-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css} from 'lit';
-import {customElement, property, state, query} from 'lit/decorators.js';
+import {customElement, state, query} from 'lit/decorators.js';
import {assertIsDefined} from '../../../utils/common-util';
import {BindValueChangeEvent} from '../../../types/events';
import {ShortcutController} from '../../lit/shortcut-controller';
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
+import {ParsedChangeInfo} from '../../../types/types';
+import {configModelToken} from '../../../models/config/config-model';
@customElement('gr-download-dialog')
export class GrDownloadDialog extends LitElement {
@@ -35,27 +37,37 @@
@query('#closeButton') protected closeButton?: GrButton;
- @property({type: Object})
- change: ChangeInfo | undefined;
+ @state() change?: ParsedChangeInfo;
- @property({type: Object})
- config?: DownloadInfo;
+ @state() config?: DownloadInfo;
@state() patchNum?: PatchSetNum;
- @state() private selectedScheme?: string;
+ @state() selectedScheme?: string;
private readonly shortcuts = new ShortcutController(this);
private readonly getChangeModel = resolve(this, changeModelToken);
+ private readonly getConfigModel = resolve(this, configModelToken);
+
constructor() {
super();
subscribe(
this,
+ () => this.getChangeModel().change$,
+ x => (this.change = x)
+ );
+ subscribe(
+ this,
() => this.getChangeModel().patchNum$,
x => (this.patchNum = x)
);
+ subscribe(
+ this,
+ () => this.getConfigModel().download$,
+ x => (this.config = x)
+ );
for (const key of ['1', '2', '3', '4', '5']) {
this.shortcuts.addLocal({key}, e => this.handleNumberKey(e));
}
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
index e5f40a6..73d7618 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog_test.ts
@@ -8,6 +8,7 @@
createChange,
createCommit,
createDownloadInfo,
+ createParsedChange,
createRevision,
} from '../../../test/test-data-generators';
import {
@@ -160,7 +161,7 @@
suite('gr-download-dialog tests with no fetch options', () => {
setup(async () => {
element.change = {
- ...createChange(),
+ ...createParsedChange(),
revisions: {
r1: {
...createRevision(),
@@ -204,7 +205,7 @@
test('computed fields', () => {
element.change = {
- ...createChange(),
+ ...createParsedChange(),
project: 'test/project' as RepoName,
_number: 123 as NumericChangeId,
};
@@ -233,7 +234,7 @@
element.patchNum = 1 as PatchSetNum;
element.change = {
- ...createChange(),
+ ...createParsedChange(),
revisions: {
r1: {...createRevision(), commit: createCommit()},
},
@@ -241,7 +242,7 @@
assert.isTrue(element.computeHidePatchFile());
element.change = {
- ...createChange(),
+ ...createParsedChange(),
revisions: {
r1: {
...createRevision(),
@@ -255,7 +256,7 @@
assert.isFalse(element.computeHidePatchFile());
element.change = {
- ...createChange(),
+ ...createParsedChange(),
revisions: {
r1: {
...createRevision(),
diff --git a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
index 33dfe82..32da400 100644
--- a/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-included-in-dialog/gr-included-in-dialog.ts
@@ -10,9 +10,12 @@
import {fontStyles} from '../../../styles/gr-font-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css} from 'lit';
-import {customElement, property, state} from 'lit/decorators.js';
+import {customElement, state} from 'lit/decorators.js';
import {BindValueChangeEvent} from '../../../types/events';
import {fireNoBubble} from '../../../utils/event-util';
+import {resolve} from '../../../models/dependency';
+import {changeModelToken} from '../../../models/change/change-model';
+import {subscribe} from '../../lit/subscription-controller';
interface DisplayGroup {
title: string;
@@ -27,19 +30,18 @@
* @event close
*/
- @property({type: Object})
- changeNum?: NumericChangeId;
+ @state() changeNum?: NumericChangeId;
- // private but used in test
@state() includedIn?: IncludedInInfo;
@state() private loaded = false;
- // private but used in test
@state() filterText = '';
private readonly restApiService = getAppContext().restApiService;
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
static override get styles() {
return [
fontStyles,
@@ -93,6 +95,15 @@
];
}
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ changeNum => (this.changeNum = changeNum)
+ );
+ }
+
override render() {
return html`
<header>
diff --git a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
index 8bcbb2b..4f1ce7a 100644
--- a/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
+++ b/polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.ts
@@ -24,7 +24,7 @@
import {GrButton} from '../../shared/gr-button/gr-button';
import {TokenHighlightLayer} from '../../../embed/diff/gr-diff-builder/token-highlight-layer';
import {css, html, LitElement, nothing} from 'lit';
-import {customElement, property, query, state} from 'lit/decorators.js';
+import {customElement, query, state} from 'lit/decorators.js';
import {sharedStyles} from '../../../styles/shared-styles';
import {subscribe} from '../../lit/subscription-controller';
import {assert} from '../../../utils/common-util';
@@ -39,6 +39,7 @@
import {fireReload} from '../../../utils/event-util';
import {when} from 'lit/directives/when.js';
import {Timing} from '../../../constants/reporting';
+import {changeModelToken} from '../../../models/change/change-model';
interface FilePreview {
filepath: string;
@@ -62,10 +63,10 @@
@query('#nextFix')
nextFix?: GrButton;
- @property({type: Object})
+ @state()
change?: ParsedChangeInfo;
- @property({type: Number})
+ @state()
changeNum?: NumericChangeId;
@state()
@@ -102,6 +103,8 @@
private readonly getUserModel = resolve(this, userModelToken);
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
private readonly getNavigation = resolve(this, navigationToken);
private readonly reporting = getAppContext().reportingService;
@@ -133,6 +136,16 @@
this.syntaxLayer.setEnabled(!!this.diffPrefs.syntax_highlighting);
}
);
+ subscribe(
+ this,
+ () => this.getChangeModel().change$,
+ change => (this.change = change)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().changeNum$,
+ changeNum => (this.changeNum = changeNum)
+ );
}
static override styles = [
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index bdb634b..0f54ad1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -46,7 +46,6 @@
PreferencesInfo,
RepoName,
RevisionPatchSetNum,
- ServerInfo,
CommentMap,
} from '../../../types/common';
import {DiffInfo, DiffPreferencesInfo, WebLinkInfo} from '../../../types/diff';
@@ -80,7 +79,6 @@
import {ShortcutController} from '../../lit/shortcut-controller';
import {subscribe} from '../../lit/subscription-controller';
import {customElement, property, query, state} from 'lit/decorators.js';
-import {configModelToken} from '../../../models/config/config-model';
import {a11yStyles} from '../../../styles/gr-a11y-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {ifDefined} from 'lit/directives/if-defined.js';
@@ -194,9 +192,6 @@
@property({type: Object})
prefs?: DiffPreferencesInfo;
- @state()
- private serverConfig?: ServerInfo;
-
// Private but used in tests.
@state()
userPrefs?: PreferencesInfo;
@@ -241,8 +236,6 @@
private readonly getShortcutsService = resolve(this, shortcutsServiceToken);
- private readonly getConfigModel = resolve(this, configModelToken);
-
private readonly getViewModel = resolve(this, changeViewModelToken);
private throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
@@ -340,13 +333,6 @@
);
subscribe(
this,
- () => this.getConfigModel().serverConfig$,
- config => {
- this.serverConfig = config;
- }
- );
- subscribe(
- this,
() => this.getCommentsModel().changeComments$,
changeComments => {
this.changeComments = changeComments;
@@ -966,12 +952,8 @@
}
private renderDialogs() {
- return html` <gr-apply-fix-dialog
- id="applyFixDialog"
- .change=${this.change}
- .changeNum=${this.changeNum}
- >
- </gr-apply-fix-dialog>
+ return html`
+ <gr-apply-fix-dialog id="applyFixDialog"></gr-apply-fix-dialog>
<gr-diff-preferences-dialog
id="diffPreferencesDialog"
@reload-diff-preference=${this.handleReloadingDiffPreference}
@@ -980,12 +962,10 @@
<dialog id="downloadModal" tabindex="-1">
<gr-download-dialog
id="downloadDialog"
- .change=${this.change}
- .patchNum=${this.patchNum}
- .config=${this.serverConfig?.download}
@close=${this.handleDownloadDialogClose}
></gr-download-dialog>
- </dialog>`;
+ </dialog>
+ `;
}
/**
diff --git a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
index e230faf..6e0a11f 100644
--- a/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
+++ b/polygerrit-ui/app/elements/shared/gr-formatted-text/gr-formatted-text.ts
@@ -21,7 +21,10 @@
import '../gr-user-suggestion-fix/gr-user-suggestion-fix';
import {KnownExperimentId} from '../../../services/flags/flags';
import {getAppContext} from '../../../services/app-context';
-import {USER_SUGGESTION_INFO_STRING} from '../../../utils/comment-util';
+import {
+ getUserSuggestionFromString,
+ USER_SUGGESTION_INFO_STRING,
+} from '../../../utils/comment-util';
/**
* This element optionally renders markdown and also applies some regex
@@ -298,9 +301,16 @@
}
private convertCodeToSuggestions() {
- for (const userSuggestionMark of this.renderRoot.querySelectorAll('mark')) {
+ const marks = this.renderRoot.querySelectorAll('mark');
+ if (marks.length > 0) {
+ const userSuggestionMark = marks[0];
const userSuggestion = document.createElement('gr-user-suggestion-fix');
- userSuggestion.textContent = userSuggestionMark.textContent ?? '';
+ // Temporary workaround for bug - tabs replacement
+ if (this.content.includes('\t')) {
+ userSuggestion.textContent = getUserSuggestionFromString(this.content);
+ } else {
+ userSuggestion.textContent = userSuggestionMark.textContent ?? '';
+ }
userSuggestionMark.parentNode?.replaceChild(
userSuggestion,
userSuggestionMark
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index 6d111aa..f5d7fef 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -281,6 +281,12 @@
viewModelState?.patchNum || latestPatchN
);
+ /** The user can enter edit mode without an `EDIT` patchset existing yet. */
+ public readonly editMode$ = select(
+ combineLatest([this.viewModel.edit$, this.patchNum$]),
+ ([edit, patchNum]) => !!edit || patchNum === EDIT
+ );
+
/**
* Emits the base patchset number. This is identical to the
* `viewModel.basePatchNum$`, but has some special logic for merges.
diff --git a/polygerrit-ui/app/models/config/config-model.ts b/polygerrit-ui/app/models/config/config-model.ts
index 168e0f4..4c0df60 100644
--- a/polygerrit-ui/app/models/config/config-model.ts
+++ b/polygerrit-ui/app/models/config/config-model.ts
@@ -34,6 +34,11 @@
configState => configState.serverConfig
);
+ public download$ = select(
+ this.serverConfig$,
+ serverConfig => serverConfig?.download
+ );
+
public mergeabilityComputationBehavior$ = select(
this.serverConfig$,
serverConfig => serverConfig?.change?.mergeability_computation_behavior
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 4646bf6..ee1a44c 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -476,13 +476,17 @@
return comment.message?.includes(USER_SUGGESTION_START_PATTERN) ?? false;
}
+export function getUserSuggestionFromString(content: string) {
+ const start =
+ content.indexOf(USER_SUGGESTION_START_PATTERN) +
+ USER_SUGGESTION_START_PATTERN.length;
+ const end = content.indexOf('\n```', start);
+ return content.substring(start, end);
+}
+
export function getUserSuggestion(comment: Comment) {
if (!comment.message) return;
- const start =
- comment.message.indexOf(USER_SUGGESTION_START_PATTERN) +
- USER_SUGGESTION_START_PATTERN.length;
- const end = comment.message.indexOf('\n```', start);
- return comment.message.substring(start, end);
+ return getUserSuggestionFromString(comment.message);
}
export function getContentInCommentRange(
