Merge "CreateChange#CommitTreeSupplier: also pass the ObjectReader instance"
diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt
index 0da8aaa..a963edb 100644
--- a/Documentation/rest-api-config.txt
+++ b/Documentation/rest-api-config.txt
@@ -1770,6 +1770,33 @@
)]}'
----
+[[cleanup.changes]]
+=== Cleanup of stale changes
+
+This endpoint allows Gerrit administrators to abandon changes older than some given
+time. This allows to run change cleanup manually outside of the configured schedule or
+if change cleanup has been deactivated.
+
+The change cleanup will run asynchronously.
+
+.Request
+----
+ POST /config/server/cleanup.changes HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "after": "3 months",
+ "if_mergeable": true,
+ "message": "Abandoning stale changes."
+ }
+----
+
+.Response
+----
+ HTTP/1.1 202 Accepted
+ Content-Disposition: attachment
+----
+
[[experiment-endpoints]]
== Experiment Endpoints
@@ -2637,6 +2664,22 @@
name of the user is not set.
|====================================
+[[clean-changes-input]]
+=== CleanChanges.Input
+The `CleanChanges.Input` entity is being used to configure a run
+of change cleanup.
+
+[options="header",cols="1,^1,5"]
+|=============================
+|Field Name||Description
+|`after`||Abandon all changes that weren't updated in the
+timespan given here
+|`if_mergeable`|default: `false`|Whether to also abandon changes
+that are mergeable
+|`message`|optional|Message to post to changes abandoned by the
+cleanup
+|=============================
+
GERRIT
------
diff --git a/java/com/google/gerrit/server/change/AbandonUtil.java b/java/com/google/gerrit/server/change/AbandonUtil.java
index 07280ba..1a2c63d 100644
--- a/java/com/google/gerrit/server/change/AbandonUtil.java
+++ b/java/com/google/gerrit/server/change/AbandonUtil.java
@@ -23,7 +23,6 @@
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.InternalUser;
-import com.google.gerrit.server.config.ChangeCleanupConfig;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeQueryProcessor;
@@ -40,7 +39,6 @@
public class AbandonUtil {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final ChangeCleanupConfig cfg;
private final Provider<ChangeQueryProcessor> queryProvider;
private final Supplier<ChangeQueryBuilder> queryBuilderSupplier;
private final BatchAbandon batchAbandon;
@@ -48,27 +46,27 @@
@Inject
AbandonUtil(
- ChangeCleanupConfig cfg,
InternalUser.Factory internalUserFactory,
Provider<ChangeQueryProcessor> queryProvider,
Provider<ChangeQueryBuilder> queryBuilderProvider,
BatchAbandon batchAbandon) {
- this.cfg = cfg;
this.queryProvider = queryProvider;
this.queryBuilderSupplier = Suppliers.memoize(queryBuilderProvider::get);
this.batchAbandon = batchAbandon;
internalUser = internalUserFactory.create();
}
- public void abandonInactiveOpenChanges(BatchUpdate.Factory updateFactory) {
- if (cfg.getAbandonAfter() <= 0) {
+ public void abandonInactiveOpenChanges(
+ BatchUpdate.Factory updateFactory,
+ long abandonAfterMillis,
+ boolean abandonIfMergeable,
+ String message) {
+ if (abandonAfterMillis <= 0) {
return;
}
-
try {
- String query =
- "status:new age:" + TimeUnit.MILLISECONDS.toMinutes(cfg.getAbandonAfter()) + "m";
- if (!cfg.getAbandonIfMergeable()) {
+ String query = "status:new age:" + TimeUnit.MILLISECONDS.toMinutes(abandonAfterMillis) + "m";
+ if (!abandonIfMergeable) {
query += " -is:mergeable";
}
@@ -86,7 +84,6 @@
int count = 0;
ImmutableListMultimap<Project.NameKey, ChangeData> abandons = builder.build();
- String message = cfg.getAbandonMessage();
for (Project.NameKey project : abandons.keySet()) {
List<ChangeData> changes = getValidChanges(abandons.get(project), query);
try {
diff --git a/java/com/google/gerrit/server/change/ChangeCleanupRunner.java b/java/com/google/gerrit/server/change/ChangeCleanupRunner.java
index 2f2cff9..728830c 100644
--- a/java/com/google/gerrit/server/change/ChangeCleanupRunner.java
+++ b/java/com/google/gerrit/server/change/ChangeCleanupRunner.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.change;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.lifecycle.LifecycleModule;
@@ -25,6 +26,8 @@
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+import com.google.inject.assistedinject.AssistedInject;
/** Runnable to enable scheduling change cleanups to run periodically */
public class ChangeCleanupRunner implements Runnable {
@@ -34,18 +37,25 @@
@Override
protected void configure() {
listener().to(Lifecycle.class);
+ factory(Factory.class);
}
}
+ public interface Factory {
+ ChangeCleanupRunner create();
+
+ ChangeCleanupRunner create(long abandonAfterMillis, boolean abandonIfMergeable, String message);
+ }
+
static class Lifecycle implements LifecycleListener {
private final WorkQueue queue;
private final ChangeCleanupRunner runner;
private final ChangeCleanupConfig cfg;
@Inject
- Lifecycle(WorkQueue queue, ChangeCleanupRunner runner, ChangeCleanupConfig cfg) {
+ Lifecycle(WorkQueue queue, ChangeCleanupRunner.Factory runner, ChangeCleanupConfig cfg) {
this.queue = queue;
- this.runner = runner;
+ this.runner = runner.create();
this.cfg = cfg;
}
@@ -63,13 +73,38 @@
private final OneOffRequestContext oneOffRequestContext;
private final AbandonUtil abandonUtil;
private final RetryHelper retryHelper;
+ private final long abandonAfterMillis;
+ private final boolean abandonIfMergeable;
+ @Nullable private final String message;
- @Inject
+ @AssistedInject
ChangeCleanupRunner(
- OneOffRequestContext oneOffRequestContext, AbandonUtil abandonUtil, RetryHelper retryHelper) {
+ OneOffRequestContext oneOffRequestContext,
+ AbandonUtil abandonUtil,
+ RetryHelper retryHelper,
+ @Assisted long abandonAfterMillis,
+ @Assisted boolean abandonIfMergeable,
+ @Assisted @Nullable String message) {
this.oneOffRequestContext = oneOffRequestContext;
this.abandonUtil = abandonUtil;
this.retryHelper = retryHelper;
+ this.abandonAfterMillis = abandonAfterMillis;
+ this.abandonIfMergeable = abandonIfMergeable;
+ this.message = message;
+ }
+
+ @AssistedInject
+ ChangeCleanupRunner(
+ OneOffRequestContext oneOffRequestContext,
+ AbandonUtil abandonUtil,
+ RetryHelper retryHelper,
+ ChangeCleanupConfig cfg) {
+ this.oneOffRequestContext = oneOffRequestContext;
+ this.abandonUtil = abandonUtil;
+ this.retryHelper = retryHelper;
+ this.abandonAfterMillis = cfg.getAbandonAfter();
+ this.abandonIfMergeable = cfg.getAbandonIfMergeable();
+ this.message = cfg.getAbandonMessage();
}
@Override
@@ -85,7 +120,8 @@
.changeUpdate(
"abandonInactiveOpenChanges",
updateFactory -> {
- abandonUtil.abandonInactiveOpenChanges(updateFactory);
+ abandonUtil.abandonInactiveOpenChanges(
+ updateFactory, abandonAfterMillis, abandonIfMergeable, message);
return null;
})
.call();
diff --git a/java/com/google/gerrit/server/edit/tree/TreeCreator.java b/java/com/google/gerrit/server/edit/tree/TreeCreator.java
index b3ce564..ae80fe4 100644
--- a/java/com/google/gerrit/server/edit/tree/TreeCreator.java
+++ b/java/com/google/gerrit/server/edit/tree/TreeCreator.java
@@ -42,40 +42,49 @@
private final ObjectId baseTreeId;
private final ImmutableList<? extends ObjectId> baseParents;
private final Optional<ObjectInserter> objectInserter;
+ private final Optional<ObjectReader> objectReader;
private final List<TreeModification> treeModifications = new ArrayList<>();
public static TreeCreator basedOn(RevCommit baseCommit) {
requireNonNull(baseCommit, "baseCommit is required");
return new TreeCreator(
- baseCommit.getTree(), ImmutableList.copyOf(baseCommit.getParents()), Optional.empty());
+ baseCommit.getTree(),
+ ImmutableList.copyOf(baseCommit.getParents()),
+ Optional.empty(),
+ Optional.empty());
}
@UsedAt(UsedAt.Project.GOOGLE)
- public static TreeCreator basedOn(RevCommit baseCommit, ObjectInserter objectInserter) {
+ public static TreeCreator basedOn(
+ RevCommit baseCommit, ObjectInserter objectInserter, ObjectReader objectReader) {
requireNonNull(baseCommit, "baseCommit is required");
return new TreeCreator(
baseCommit.getTree(),
ImmutableList.copyOf(baseCommit.getParents()),
- Optional.of(objectInserter));
+ Optional.of(objectInserter),
+ Optional.of(objectReader));
}
public static TreeCreator basedOnTree(
ObjectId baseTreeId, ImmutableList<? extends ObjectId> baseParents) {
requireNonNull(baseTreeId, "baseTreeId is required");
- return new TreeCreator(baseTreeId, baseParents, Optional.empty());
+ return new TreeCreator(baseTreeId, baseParents, Optional.empty(), Optional.empty());
}
public static TreeCreator basedOnEmptyTree() {
- return new TreeCreator(ObjectId.zeroId(), ImmutableList.of(), Optional.empty());
+ return new TreeCreator(
+ ObjectId.zeroId(), ImmutableList.of(), Optional.empty(), Optional.empty());
}
private TreeCreator(
ObjectId baseTreeId,
ImmutableList<? extends ObjectId> baseParents,
- Optional<ObjectInserter> objectInserter) {
+ Optional<ObjectInserter> objectInserter,
+ Optional<ObjectReader> objectReader) {
this.baseTreeId = requireNonNull(baseTreeId, "baseTree is required");
this.baseParents = baseParents;
this.objectInserter = objectInserter;
+ this.objectReader = objectReader;
}
/**
@@ -138,14 +147,22 @@
}
private DirCache readBaseTree(Repository repository) throws IOException {
- try (ObjectReader objectReader = repository.newObjectReader()) {
- DirCache dirCache = DirCache.newInCore();
+ ObjectReader or = objectReader.orElseGet(() -> repository.newObjectReader());
+ try {
+ DirCache dirCache =
+ ObjectId.zeroId().equals(baseTreeId)
+ ? DirCache.newInCore()
+ : DirCache.read(or, baseTreeId);
DirCacheBuilder dirCacheBuilder = dirCache.builder();
if (!ObjectId.zeroId().equals(baseTreeId)) {
- dirCacheBuilder.addTree(new byte[0], DirCacheEntry.STAGE_0, objectReader, baseTreeId);
+ dirCacheBuilder.addTree(new byte[0], DirCacheEntry.STAGE_0, or, baseTreeId);
}
dirCacheBuilder.finish();
return dirCache;
+ } finally {
+ if (objectReader.isEmpty()) {
+ or.close();
+ }
}
}
diff --git a/java/com/google/gerrit/server/restapi/RestApiModule.java b/java/com/google/gerrit/server/restapi/RestApiModule.java
index 73991c9..359393e 100644
--- a/java/com/google/gerrit/server/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/RestApiModule.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.restapi;
+import com.google.gerrit.server.change.ChangeCleanupRunner.ChangeCleanupRunnerModule;
import com.google.gerrit.server.plugins.PluginRestApiModule;
import com.google.gerrit.server.restapi.access.AccessRestApiModule;
import com.google.gerrit.server.restapi.account.AccountRestApiModule;
@@ -42,5 +43,6 @@
install(new PluginRestApiModule());
install(new ProjectRestApiModule());
install(new ProjectRestApiModule.BatchModule());
+ install(new ChangeCleanupRunnerModule());
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
index 7d8c793..41faf22 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java
@@ -16,62 +16,43 @@
import static com.google.gerrit.server.notedb.ChangeNoteFooters.FOOTER_CHANGE_ID;
import static com.google.gerrit.server.project.ProjectCache.illegalState;
-import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.entities.BooleanProjectConfig;
import com.google.gerrit.entities.BranchNameKey;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.extensions.api.changes.ApplyPatchPatchSetInput;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.PreconditionFailedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.PatchSetInserter;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
import com.google.gerrit.server.git.CommitUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ContributorAgreementsChecker;
import com.google.gerrit.server.project.InvalidChangeOperationException;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
-import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException;
-import com.google.gerrit.server.update.context.RefUpdateContext;
-import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.time.Instant;
-import java.time.ZoneId;
import java.util.List;
-import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
-import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.patch.PatchApplier;
@@ -80,53 +61,42 @@
@Singleton
public class ApplyPatch implements RestModifyView<ChangeResource, ApplyPatchPatchSetInput> {
- private final ChangeJson.Factory jsonFactory;
+
private final ContributorAgreementsChecker contributorAgreements;
- private final Provider<IdentifiedUser> user;
private final GitRepositoryManager gitManager;
- private final BatchUpdate.Factory batchUpdateFactory;
- private final PatchSetInserter.Factory patchSetInserterFactory;
private final Provider<InternalChangeQuery> queryProvider;
- private final ZoneId serverZoneId;
private final ProjectCache projectCache;
private final ChangeUtil changeUtil;
+ private final PatchSetCreator patchSetCreator;
@Inject
ApplyPatch(
- ChangeJson.Factory jsonFactory,
ContributorAgreementsChecker contributorAgreements,
- Provider<IdentifiedUser> user,
GitRepositoryManager gitManager,
- BatchUpdate.Factory batchUpdateFactory,
- PatchSetInserter.Factory patchSetInserterFactory,
Provider<InternalChangeQuery> queryProvider,
- @GerritPersonIdent PersonIdent myIdent,
ProjectCache projectCache,
- ChangeUtil changeUtil) {
- this.jsonFactory = jsonFactory;
+ ChangeUtil changeUtil,
+ PatchSetCreator patchSetCreator) {
this.contributorAgreements = contributorAgreements;
- this.user = user;
this.gitManager = gitManager;
- this.batchUpdateFactory = batchUpdateFactory;
- this.patchSetInserterFactory = patchSetInserterFactory;
this.queryProvider = queryProvider;
- this.serverZoneId = myIdent.getZoneId();
this.projectCache = projectCache;
this.changeUtil = changeUtil;
+ this.patchSetCreator = patchSetCreator;
}
@Override
public Response<ChangeInfo> apply(ChangeResource rsrc, ApplyPatchPatchSetInput input)
throws IOException, UpdateException, RestApiException, PermissionBackendException,
ConfigInvalidException, NoSuchProjectException, InvalidChangeOperationException {
- NameKey project = rsrc.getProject();
- contributorAgreements.check(project, rsrc.getUser());
- BranchNameKey destBranch = rsrc.getChange().getDest();
-
if (input == null || input.patch == null || input.patch.patch == null) {
throw new BadRequestException("patch required");
}
+ NameKey project = rsrc.getProject();
+ contributorAgreements.check(project, rsrc.getUser());
+ BranchNameKey destBranch = rsrc.getChange().getDest();
+
try (Repository repo = gitManager.openRepository(project);
// This inserter and revwalk *must* be passed to any BatchUpdates
// created later on, to ensure the applied commit is flushed
@@ -140,22 +110,7 @@
String.format("Branch %s does not exist.", destBranch.branch()));
}
ChangeData destChange = rsrc.getChangeData();
- if (destChange == null) {
- throw new PreconditionFailedException(
- "patch:apply cannot be called without a destination change.");
- }
-
- if (destChange.change().isClosed()) {
- throw new PreconditionFailedException(
- String.format(
- "patch:apply with Change-Id %s could not update the existing change %d "
- + "in destination branch %s of project %s, because the change was closed (%s)",
- destChange.getId(),
- destChange.getId().get(),
- destBranch.branch(),
- destBranch.project(),
- destChange.change().getStatus().name()));
- }
+ patchSetCreator.validateChangeCanBeAppended(destChange, destBranch);
if (!Strings.isNullOrEmpty(input.base) && Boolean.TRUE.equals(input.amend)) {
throw new BadRequestException("amend only works with existing revisions. omit base.");
@@ -173,7 +128,8 @@
if (latestPatchset.getParentCount() != 1) {
throw new BadRequestException(
String.format(
- "Cannot parse base commit for a change with none or multiple parents. Change ID: %s.",
+ "Cannot parse base commit for a change with none or multiple parents. Change ID:"
+ + " %s.",
destChange.getId()));
}
if (Boolean.TRUE.equals(input.amend)) {
@@ -184,50 +140,37 @@
parents = ImmutableList.of(baseCommit);
}
}
+
+ List<ListChangesOption> opts = input.responseFormatOptions;
+ if (opts == null) {
+ opts = ImmutableList.of();
+ }
+
PatchApplier.Result applyResult =
ApplyPatchUtil.applyPatch(repo, oi, input.patch, baseCommit);
- ObjectId treeId = applyResult.getTreeId();
- Instant now = TimeUtil.now();
- PersonIdent committerIdent =
- Optional.ofNullable(latestPatchset.getCommitterIdent())
- .map(
- ident ->
- user.get()
- .newCommitterIdent(ident.getEmailAddress(), now, serverZoneId)
- .orElseGet(() -> user.get().newCommitterIdent(now, serverZoneId)))
- .orElseGet(() -> user.get().newCommitterIdent(now, serverZoneId));
- PersonIdent authorIdent =
- input.author == null
- ? committerIdent
- : new PersonIdent(input.author.name, input.author.email, now, serverZoneId);
String commitMessage =
buildFullCommitMessage(
project,
latestPatchset,
input,
- ApplyPatchUtil.getResultPatch(repo, reader, baseCommit, revWalk.lookupTree(treeId)),
+ ApplyPatchUtil.getResultPatch(
+ repo, reader, baseCommit, revWalk.lookupTree(applyResult.getTreeId())),
applyResult.getErrors());
- ObjectId appliedCommit =
- CommitUtil.createCommitWithTree(
- oi, authorIdent, committerIdent, parents, commitMessage, treeId);
- CodeReviewCommit commit = revWalk.parseCommit(appliedCommit);
- oi.flush();
-
- Change resultChange;
- try (BatchUpdate bu = batchUpdateFactory.create(project, user.get(), TimeUtil.now())) {
- bu.setRepository(repo, revWalk, oi);
- resultChange =
- insertPatchSet(bu, repo, patchSetInserterFactory, destChange.notes(), commit);
- } catch (NoSuchChangeException | RepositoryNotFoundException e) {
- throw new ResourceConflictException(e.getMessage());
- }
- List<ListChangesOption> opts = input.responseFormatOptions;
- if (opts == null) {
- opts = ImmutableList.of();
- }
- ChangeInfo changeInfo = jsonFactory.create(opts).format(resultChange);
+ ChangeInfo changeInfo =
+ patchSetCreator.createPatchSetWithSuppliedTree(
+ project,
+ destChange,
+ latestPatchset,
+ parents,
+ input.author,
+ opts,
+ repo,
+ oi,
+ revWalk,
+ applyResult.getTreeId(),
+ commitMessage);
return Response.ok(changeInfo);
}
}
@@ -275,28 +218,6 @@
return commitMessage;
}
- private static Change insertPatchSet(
- BatchUpdate bu,
- Repository git,
- PatchSetInserter.Factory patchSetInserterFactory,
- ChangeNotes destNotes,
- CodeReviewCommit commit)
- throws IOException, UpdateException, RestApiException {
- try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
- Change destChange = destNotes.getChange();
- PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId());
- PatchSetInserter inserter = patchSetInserterFactory.create(destNotes, psId, commit);
- inserter.setMessage(buildMessageForPatchSet(psId));
- bu.addOp(destChange.getId(), inserter);
- bu.execute();
- return inserter.getChange();
- }
- }
-
- private static String buildMessageForPatchSet(PatchSet.Id psId) {
- return new StringBuilder(String.format("Uploaded patch set %s.", psId.get())).toString();
- }
-
private String removeFooters(String originalMessage, List<FooterLine> footerLines) {
if (footerLines.isEmpty()) {
return originalMessage;
diff --git a/java/com/google/gerrit/server/restapi/change/PatchSetCreator.java b/java/com/google/gerrit/server/restapi/change/PatchSetCreator.java
new file mode 100644
index 0000000..60c8e89
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/PatchSetCreator.java
@@ -0,0 +1,175 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.change;
+
+import static com.google.gerrit.server.update.context.RefUpdateContext.RefUpdateType.CHANGE_MODIFICATION;
+import static java.util.Objects.requireNonNull;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.accounts.AccountInput;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.PreconditionFailedException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.change.ChangeJson;
+import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.git.CodeReviewCommit;
+import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.git.CommitUtil;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.UpdateException;
+import com.google.gerrit.server.update.context.RefUpdateContext;
+import com.google.gerrit.server.util.time.TimeUtil;
+import java.io.IOException;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.util.List;
+import java.util.Optional;
+import javax.inject.Inject;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+
+/** A utility class for creating a patch set on an existing change. */
+@Singleton
+public class PatchSetCreator {
+ private final Provider<IdentifiedUser> ident;
+ private final BatchUpdate.Factory batchUpdateFactory;
+ private final PatchSetInserter.Factory patchSetInserterFactory;
+ private final ChangeJson.Factory jsonFactory;
+ private final ZoneId serverZoneId;
+
+ @Inject
+ PatchSetCreator(
+ Provider<IdentifiedUser> ident,
+ BatchUpdate.Factory batchUpdateFactory,
+ PatchSetInserter.Factory patchSetInserterFactory,
+ @GerritPersonIdent PersonIdent myIdent,
+ ChangeJson.Factory jsonFactory) {
+ this.ident = ident;
+ this.batchUpdateFactory = batchUpdateFactory;
+ this.patchSetInserterFactory = patchSetInserterFactory;
+ this.serverZoneId = myIdent.getZoneId();
+ this.jsonFactory = jsonFactory;
+ }
+
+ public ChangeInfo createPatchSetWithSuppliedTree(
+ Project.NameKey project,
+ ChangeData destChange,
+ RevCommit latestPatchset,
+ List<RevCommit> parents,
+ @Nullable AccountInput author,
+ List<ListChangesOption> outputOptions,
+ Repository repo,
+ ObjectInserter oi,
+ CodeReviewRevWalk revWalk,
+ ObjectId commitTree,
+ String commitMessage)
+ throws IOException, RestApiException, UpdateException {
+ requireNonNull(destChange);
+ requireNonNull(latestPatchset);
+ requireNonNull(parents);
+ requireNonNull(outputOptions);
+
+ Instant now = TimeUtil.now();
+ PersonIdent committerIdent =
+ Optional.ofNullable(latestPatchset.getCommitterIdent())
+ .map(
+ id ->
+ ident
+ .get()
+ .newCommitterIdent(id.getEmailAddress(), now, serverZoneId)
+ .orElseGet(() -> ident.get().newCommitterIdent(now, serverZoneId)))
+ .orElseGet(() -> ident.get().newCommitterIdent(now, serverZoneId));
+ PersonIdent authorIdent =
+ author == null
+ ? committerIdent
+ : new PersonIdent(author.name, author.email, now, serverZoneId);
+
+ ObjectId appliedCommit =
+ CommitUtil.createCommitWithTree(
+ oi, authorIdent, committerIdent, parents, commitMessage, commitTree);
+ CodeReviewCommit commit = revWalk.parseCommit(appliedCommit);
+ oi.flush();
+
+ Change resultChange;
+ try (BatchUpdate bu = batchUpdateFactory.create(project, ident.get(), TimeUtil.now())) {
+ bu.setRepository(repo, revWalk, oi);
+ resultChange = insertPatchSet(bu, repo, patchSetInserterFactory, destChange.notes(), commit);
+ } catch (NoSuchChangeException | RepositoryNotFoundException e) {
+ throw new ResourceConflictException(e.getMessage());
+ }
+
+ return jsonFactory.create(outputOptions).format(resultChange);
+ }
+
+ public void validateChangeCanBeAppended(@Nullable ChangeData destChange, BranchNameKey destBranch)
+ throws PreconditionFailedException {
+ if (destChange == null) {
+ throw new PreconditionFailedException(
+ "cannot write a patch set without a destination change.");
+ }
+
+ if (destChange.change().isClosed()) {
+ throw new PreconditionFailedException(
+ String.format(
+ "patch:apply with Change-Id %s could not update the existing change %d "
+ + "in destination branch %s of project %s, because the change was closed (%s)",
+ destChange.getId(),
+ destChange.getId().get(),
+ destBranch.branch(),
+ destBranch.project(),
+ destChange.change().getStatus().name()));
+ }
+ }
+
+ private static Change insertPatchSet(
+ BatchUpdate bu,
+ Repository git,
+ PatchSetInserter.Factory patchSetInserterFactory,
+ ChangeNotes destNotes,
+ CodeReviewCommit commit)
+ throws IOException, UpdateException, RestApiException {
+ try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) {
+ Change destChange = destNotes.getChange();
+ PatchSet.Id psId = ChangeUtil.nextPatchSetId(git, destChange.currentPatchSetId());
+ PatchSetInserter inserter = patchSetInserterFactory.create(destNotes, psId, commit);
+ inserter.setMessage(buildMessageForPatchSet(psId));
+ bu.addOp(destChange.getId(), inserter);
+ bu.execute();
+ return inserter.getChange();
+ }
+ }
+
+ private static String buildMessageForPatchSet(PatchSet.Id psId) {
+ return String.format("Uploaded patch set %s.", psId.get());
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/config/CleanupChanges.java b/java/com/google/gerrit/server/restapi/config/CleanupChanges.java
new file mode 100644
index 0000000..9ae3637
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/config/CleanupChanges.java
@@ -0,0 +1,76 @@
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.config;
+
+import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.extensions.annotations.RequiresCapability;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.server.change.ChangeCleanupRunner;
+import com.google.gerrit.server.config.ConfigResource;
+import com.google.gerrit.server.config.ConfigUtil;
+import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.restapi.config.CleanupChanges.Input;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+
+@RequiresCapability(GlobalCapability.MAINTAIN_SERVER)
+@Singleton
+public class CleanupChanges implements RestModifyView<ConfigResource, Input> {
+ private final ChangeCleanupRunner.Factory runnerFactory;
+ private final WorkQueue workQueue;
+
+ public static class Input {
+ String after;
+ boolean ifMergeable;
+ String message;
+ }
+
+ @Inject
+ CleanupChanges(WorkQueue workQueue, ChangeCleanupRunner.Factory runnerFactory) {
+ this.runnerFactory = runnerFactory;
+ this.workQueue = workQueue;
+ }
+
+ @Override
+ public Response<?> apply(ConfigResource rsrc, Input input) throws BadRequestException {
+ if (taskAlreadyScheduled()) {
+ return Response.ok("Change cleaner already in queue.");
+ }
+ if (input.after == null) {
+ throw new BadRequestException("`after` must be specified.");
+ }
+ ChangeCleanupRunner runner =
+ runnerFactory.create(
+ ConfigUtil.getTimeUnit(input.after, 0, TimeUnit.MILLISECONDS),
+ input.ifMergeable,
+ input.message);
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError = workQueue.getDefaultQueue().submit(() -> runner.run());
+ return Response.accepted("Change cleaner task added to work queue.");
+ }
+
+ private boolean taskAlreadyScheduled() {
+ for (WorkQueue.Task<?> task : workQueue.getTasks()) {
+ if (task.toString().contains(ChangeCleanupRunner.class.getName())) {
+ return true;
+ }
+ }
+ return false;
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/config/ConfigRestApiModule.java b/java/com/google/gerrit/server/restapi/config/ConfigRestApiModule.java
index f42cb50..2f21da6 100644
--- a/java/com/google/gerrit/server/restapi/config/ConfigRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/config/ConfigRestApiModule.java
@@ -54,6 +54,7 @@
put(CONFIG_KIND, "preferences.edit").to(SetEditPreferences.class);
post(CONFIG_KIND, "reload").to(ReloadConfig.class);
post(CONFIG_KIND, "snapshot.indexes").to(SnapshotIndexes.class);
+ post(CONFIG_KIND, "cleanup.changes").to(CleanupChanges.class);
child(CONFIG_KIND, "tasks").to(TasksCollection.class);
delete(TASK_KIND).to(DeleteTask.class);
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index b0045e3..8e065b9 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -57,6 +57,7 @@
import com.google.gerrit.server.audit.AuditModule;
import com.google.gerrit.server.cache.h2.H2CacheModule;
import com.google.gerrit.server.cache.mem.DefaultMemoryCacheModule;
+import com.google.gerrit.server.change.ChangeCleanupRunner.ChangeCleanupRunnerModule;
import com.google.gerrit.server.change.FileInfoJsonModule;
import com.google.gerrit.server.config.AllProjectsConfigProvider;
import com.google.gerrit.server.config.AllProjectsName;
@@ -209,6 +210,7 @@
install(new RepoSequenceModule());
install(new NoteDbDraftCommentsModule());
install(new NoteDbStarredChangesModule());
+ install(new ChangeCleanupRunnerModule());
AuthConfig authConfig = cfgInjector.getInstance(AuthConfig.class);
install(new AuthModule(authConfig));
diff --git a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
index f0f262f..18969ad 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/AbandonIT.java
@@ -40,7 +40,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.change.AbandonUtil;
+import com.google.gerrit.server.change.ChangeCleanupRunner;
import com.google.gerrit.server.config.ChangeCleanupConfig;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.TestTimeUtil;
@@ -53,7 +53,7 @@
import org.junit.Test;
public class AbandonIT extends AbstractDaemonTest {
- @Inject private AbandonUtil abandonUtil;
+ @Inject private ChangeCleanupRunner.Factory cleanupRunner;
@Inject private ChangeCleanupConfig cleanupConfig;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@@ -140,7 +140,7 @@
assertThat(toChangeNumbers(query("is:open"))).containsExactly(id1, id2, id3);
assertThat(query("is:abandoned")).isEmpty();
- abandonUtil.abandonInactiveOpenChanges(batchUpdateFactory);
+ cleanupRunner.create().run();
assertThat(toChangeNumbers(query("is:open"))).containsExactly(id3);
assertThat(toChangeNumbers(query("is:abandoned"))).containsExactly(id1, id2);
}
@@ -182,7 +182,7 @@
assertThat(toChangeNumbers(query("is:merged"))).containsExactly(id3);
assertThat(toChangeNumbers(query("-is:mergeable"))).containsExactly(id4);
- abandonUtil.abandonInactiveOpenChanges(batchUpdateFactory);
+ cleanupRunner.create().run();
assertThat(toChangeNumbers(query("is:open"))).containsExactly(id5, id2, id1);
assertThat(toChangeNumbers(query("is:abandoned"))).containsExactly(id4);
}
@@ -229,7 +229,7 @@
assertThrows(BadRequestException.class, () -> query("-is:mergeable"));
assertThat(thrown).hasMessageThat().contains("operator is not supported");
- abandonUtil.abandonInactiveOpenChanges(batchUpdateFactory);
+ cleanupRunner.create().run();
assertThat(toChangeNumbers(query("is:open"))).containsExactly(id5);
assertThat(toChangeNumbers(query("is:abandoned"))).containsExactly(id4, id2, id1);
}
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index 62a095f..6cfb989 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -1371,8 +1371,21 @@
@Test
public void canCombineEdits() throws Exception {
- createEmptyEditFor(changeId);
+ String baseFileToDelete = "base_file_to_delete";
+ String baseFileToRename = "base_file_to_rename";
+ String baseFileNewName = "base_file_new_name";
+ String currPatchSetFileToRename = "current_patchset_file_to_rename";
+ String currPatchSetFileNewName = "current_patchset_file_new_name";
+ // Re-clone empty repo; TestRepository doesn't let us reset to unborn head.
+ testRepo = cloneProject(project);
+ String baseChangeId = newChangeWithFile(admin.newIdent(), baseFileToDelete, "content");
+ addNewPatchSetWithModifiedFile(baseChangeId, baseFileToRename, "content2");
+ gApi.changes().id(baseChangeId).current().review(ReviewInput.approve());
+ gApi.changes().id(baseChangeId).current().submit();
+ String changeId = newChange(admin.newIdent());
+ addNewPatchSetWithModifiedFile(changeId, currPatchSetFileToRename, "content3");
+ createEmptyEditFor(changeId);
// update author
gApi.changes()
.id(changeId)
@@ -1394,11 +1407,20 @@
.modifyIdentity(
"Test Committer", "test.committer@example.com", ChangeEditIdentityType.COMMITTER);
- // delete file
+ // delete current patch-set file
gApi.changes().id(changeId).edit().deleteFile(FILE_NAME);
- // rename file
- gApi.changes().id(changeId).edit().renameFile(FILE_NAME2, FILE_NAME3);
+ // delete base file
+ gApi.changes().id(changeId).edit().deleteFile(baseFileToDelete);
+
+ // rename current patch-set file
+ gApi.changes()
+ .id(changeId)
+ .edit()
+ .renameFile(currPatchSetFileToRename, currPatchSetFileNewName);
+
+ // rename base file
+ gApi.changes().id(changeId).edit().renameFile(baseFileToRename, baseFileNewName);
// publish edit
PublishChangeEditInput publishInput = new PublishChangeEditInput();
@@ -1419,7 +1441,12 @@
assertThat(currentCommit.author.name).isEqualTo("Test Author");
assertThat(currentCommit.author.email).isEqualTo("test.author@example.com");
assertThat(currentCommit.message).isEqualTo(msg);
- assertThat(currentRevision.files.keySet()).containsExactly(newFile, FILE_NAME3);
+ assertThat(currentRevision.files.keySet())
+ .containsExactly(newFile, baseFileToDelete, baseFileNewName, currPatchSetFileNewName);
+ assertThat(currentRevision.files.get(newFile).status).isEqualTo('A');
+ assertThat(currentRevision.files.get(baseFileToDelete).status).isEqualTo('D');
+ assertThat(currentRevision.files.get(baseFileNewName).status).isEqualTo('R');
+ assertThat(currentRevision.files.get(currPatchSetFileNewName).status).isEqualTo('A');
}
private void createArbitraryEditFor(String changeId) throws Exception {
@@ -1451,6 +1478,13 @@
return push.to("refs/for/master").getChangeId();
}
+ private String newChangeWithFile(PersonIdent ident, String filePath, String fileContent)
+ throws Exception {
+ PushOneCommit push =
+ pushFactory.create(ident, testRepo, PushOneCommit.SUBJECT, filePath, fileContent);
+ return push.to("refs/for/master").getChangeId();
+ }
+
private void addNewPatchSet(String changeId) throws Exception {
addNewPatchSetWithModifiedFile(changeId, FILE_NAME2, new String(CONTENT_NEW2, UTF_8));
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/ConfigRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/ConfigRestApiBindingsIT.java
index 9f2401a..bfc64a6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/ConfigRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/ConfigRestApiBindingsIT.java
@@ -66,7 +66,8 @@
RestCall.post("/config/server/deactivate.stale.accounts"),
RestCall.get("/config/server/tasks"),
RestCall.get("/config/server/top-menus"),
- RestCall.get("/config/server/version"));
+ RestCall.get("/config/server/version"),
+ RestCall.post("/config/server/cleanup.changes"));
/**
* Cache REST endpoints to be tested, the URLs contain a placeholder for the cache identifier.
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
index 2916f75..e63ac8f 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.ts
@@ -7,12 +7,14 @@
import '../../shared/gr-avatar/gr-avatar';
import {getUserName} from '../../../utils/display-name-util';
import {AccountInfo, DropdownLink, ServerInfo} from '../../../types/common';
-import {getAppContext} from '../../../services/app-context';
import {fire} from '../../../utils/event-util';
import {DropdownContent} from '../../shared/gr-dropdown/gr-dropdown';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, css, html} from 'lit';
-import {customElement, property} from 'lit/decorators.js';
+import {customElement, property, state} from 'lit/decorators.js';
+import {resolve} from '../../../models/dependency';
+import {configModelToken} from '../../../models/config/config-model';
+import {subscribe} from '../../lit/subscription-controller';
const INTERPOLATE_URL_PATTERN = /\${([\w]+)}/g;
@@ -30,34 +32,44 @@
@property({type: Object})
account?: AccountInfo;
- @property({type: Object})
+ // Private but used in test
+ @state()
config?: ServerInfo;
- @property({type: String})
- _path = '/';
+ @state()
+ private path = '/';
- @property({type: Boolean})
- _hasAvatars = false;
+ @state()
+ private hasAvatars = false;
- @property({type: String})
- _switchAccountUrl = '';
+ @state()
+ private switchAccountUrl = '';
- private readonly restApiService = getAppContext().restApiService;
+ // Private but used in test
+ readonly getConfigModel = resolve(this, configModelToken);
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ cfg => {
+ this.config = cfg;
+
+ if (cfg && cfg.auth && cfg.auth.switch_account_url) {
+ this.switchAccountUrl = cfg.auth.switch_account_url;
+ } else {
+ this.switchAccountUrl = '';
+ }
+ this.hasAvatars = !!(cfg && cfg.plugin && cfg.plugin.has_avatars);
+ }
+ );
+ }
override connectedCallback() {
super.connectedCallback();
this.handleLocationChange();
document.addEventListener('location-change', this.handleLocationChange);
- this.restApiService.getConfig().then(cfg => {
- this.config = cfg;
-
- if (cfg && cfg.auth && cfg.auth.switch_account_url) {
- this._switchAccountUrl = cfg.auth.switch_account_url;
- } else {
- this._switchAccountUrl = '';
- }
- this._hasAvatars = !!(cfg && cfg.plugin && cfg.plugin.has_avatars);
- });
}
override disconnectedCallback() {
@@ -88,15 +100,13 @@
link=""
.items=${this.links}
.topContent=${this.topContent}
- @tap-item-shortcuts=${this._handleShortcutsTap}
+ @tap-item-shortcuts=${this.handleShortcutsTap}
.horizontalAlign=${'right'}
>
- <span ?hidden=${this._hasAvatars}
- >${this._accountName(this.account)}</span
- >
+ <span ?hidden=${this.hasAvatars}>${this.accountName(this.account)}</span>
<gr-avatar
.account=${this.account}
- ?hidden=${!this._hasAvatars}
+ ?hidden=${!this.hasAvatars}
.imageSize=${56}
aria-label="Account avatar"
></gr-avatar>
@@ -104,14 +114,15 @@
}
get links(): DropdownLink[] | undefined {
- return this._getLinks(this._switchAccountUrl, this._path);
+ return this.getLinks(this.switchAccountUrl, this.path);
}
get topContent(): DropdownContent[] | undefined {
- return this._getTopContent(this.account);
+ return this.getTopContent(this.account);
}
- _getLinks(switchAccountUrl?: string, path?: string) {
+ // Private but used in test
+ getLinks(switchAccountUrl?: string, path?: string) {
if (switchAccountUrl === undefined || path === undefined) {
return undefined;
}
@@ -121,37 +132,39 @@
links.push({name: 'Keyboard Shortcuts', id: 'shortcuts'});
if (switchAccountUrl) {
const replacements = {path};
- const url = this._interpolateUrl(switchAccountUrl, replacements);
+ const url = this.interpolateUrl(switchAccountUrl, replacements);
links.push({name: 'Switch account', url, external: true});
}
links.push({name: 'Sign out', id: 'signout', url: '/logout'});
return links;
}
- _getTopContent(account?: AccountInfo) {
+ // Private but used in test
+ getTopContent(account?: AccountInfo) {
return [
- {text: this._accountName(account), bold: true},
+ {text: this.accountName(account), bold: true},
{text: account?.email ? account.email : ''},
] as DropdownContent[];
}
- _handleShortcutsTap() {
+ private handleShortcutsTap() {
fire(this, 'show-keyboard-shortcuts', {});
}
private readonly handleLocationChange = () => {
- this._path =
+ this.path =
window.location.pathname + window.location.search + window.location.hash;
};
- _interpolateUrl(url: string, replacements: {[key: string]: string}) {
+ // Private but used in test
+ interpolateUrl(url: string, replacements: {[key: string]: string}) {
return url.replace(
INTERPOLATE_URL_PATTERN,
(_, p1) => replacements[p1] || ''
);
}
- _accountName(account?: AccountInfo) {
+ private accountName(account?: AccountInfo) {
return getUserName(this.config, account);
}
}
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.ts b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.ts
index c224a6b..06dc3f8 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown_test.ts
@@ -78,14 +78,14 @@
test('switch account', () => {
// Missing params.
- assert.isUndefined(element._getLinks());
- assert.isUndefined(element._getLinks(undefined));
+ assert.isUndefined(element.getLinks());
+ assert.isUndefined(element.getLinks(undefined));
// No switch account link.
- assert.equal(element._getLinks('', '')!.length, 3);
+ assert.equal(element.getLinks('', '')!.length, 3);
// Unparameterized switch account link.
- let links = element._getLinks('/switch-account', '')!;
+ let links = element.getLinks('/switch-account', '')!;
assert.equal(links.length, 4);
assert.deepEqual(links[2], {
name: 'Switch account',
@@ -94,7 +94,7 @@
});
// Parameterized switch account link.
- links = element._getLinks('/switch-account${path}', '/c/123')!;
+ links = element.getLinks('/switch-account${path}', '/c/123')!;
assert.equal(links.length, 4);
assert.deepEqual(links[2], {
name: 'Switch account',
@@ -103,13 +103,13 @@
});
});
- test('_interpolateUrl', () => {
+ test('interpolateUrl', () => {
const replacements = {
foo: 'bar',
test: 'TEST',
};
const interpolate = (url: string) =>
- element._interpolateUrl(url, replacements);
+ element.interpolateUrl(url, replacements);
assert.equal(interpolate('test'), 'test');
assert.equal(interpolate('${test}'), 'TEST');
diff --git a/polygerrit-ui/app/elements/gr-app_test.ts b/polygerrit-ui/app/elements/gr-app_test.ts
index 15cfda6..cd19872 100644
--- a/polygerrit-ui/app/elements/gr-app_test.ts
+++ b/polygerrit-ui/app/elements/gr-app_test.ts
@@ -48,7 +48,7 @@
setup(async () => {
appStartedStub = sinon.stub(getAppContext().reportingService, 'appStarted');
- stubElement('gr-account-dropdown', '_getTopContent');
+ stubElement('gr-account-dropdown', 'getTopContent');
routerStartStub = sinon.stub(GrRouter.prototype, 'start');
stubRestApi('getAccount').returns(Promise.resolve(undefined));
stubRestApi('getAccountCapabilities').returns(Promise.resolve({}));
diff --git a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.ts b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.ts
index 985f9be..0b5f952 100644
--- a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.ts
+++ b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password.ts
@@ -9,8 +9,12 @@
import {grFormStyles} from '../../../styles/gr-form-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, css, html} from 'lit';
-import {customElement, property, query} from 'lit/decorators.js';
+import {customElement, query, state} from 'lit/decorators.js';
import {modalStyles} from '../../../styles/gr-modal-styles';
+import {resolve} from '../../../models/dependency';
+import {configModelToken} from '../../../models/config/config-model';
+import {userModelToken} from '../../../models/user/user-model';
+import {subscribe} from '../../lit/subscription-controller';
declare global {
interface HTMLElementTagNameMap {
@@ -23,47 +27,48 @@
@query('#generatedPasswordModal')
generatedPasswordModal?: HTMLDialogElement;
- @property({type: String})
+ @state()
username?: string;
- @property({type: String})
+ @state()
generatedPassword?: string;
- @property({type: String})
+ @state()
status?: string;
- @property({type: String})
+ @state()
passwordUrl: string | null = null;
private readonly restApiService = getAppContext().restApiService;
- override connectedCallback() {
- super.connectedCallback();
- this.loadData();
- }
+ // Private but used in test
+ readonly getConfigModel = resolve(this, configModelToken);
- loadData() {
- const promises = [];
+ // Private but used in test
+ readonly getUserModel = resolve(this, userModelToken);
- promises.push(
- this.restApiService.getAccount().then(account => {
- if (account) {
- this.username = account.username;
- }
- })
- );
-
- promises.push(
- this.restApiService.getConfig().then(info => {
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ info => {
if (info) {
this.passwordUrl = info.auth.http_password_url || null;
} else {
this.passwordUrl = null;
}
- })
+ }
);
-
- return Promise.all(promises);
+ subscribe(
+ this,
+ () => this.getUserModel().account$,
+ account => {
+ if (account) {
+ this.username = account.username;
+ }
+ }
+ );
}
static override get styles() {
diff --git a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_test.ts b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_test.ts
index 1abf8a7..d94638b 100644
--- a/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-http-password/gr-http-password_test.ts
@@ -14,7 +14,7 @@
import {AccountDetailInfo, ServerInfo} from '../../../types/common';
import {queryAndAssert} from '../../../test/test-utils';
import {GrButton} from '../../shared/gr-button/gr-button';
-import {fixture, html, assert} from '@open-wc/testing';
+import {fixture, html, assert, waitUntil} from '@open-wc/testing';
suite('gr-http-password tests', () => {
let element: GrHttpPassword;
@@ -29,7 +29,12 @@
stubRestApi('getConfig').returns(Promise.resolve(config));
element = await fixture(html`<gr-http-password></gr-http-password>`);
- await element.loadData();
+ await waitUntil(
+ () => element.getUserModel().getState().account === account
+ );
+ await waitUntil(
+ () => element.getConfigModel().getState().serverConfig === config
+ );
await waitEventLoop();
});
@@ -121,7 +126,8 @@
test('with http_password_url', async () => {
config.auth.http_password_url = 'http://example.com/';
- await element.loadData();
+ element.passwordUrl = config.auth.http_password_url;
+ await element.updateComplete;
assert.isNotNull(element.passwordUrl);
assert.equal(element.passwordUrl, config.auth.http_password_url);
});
diff --git a/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences.ts b/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences.ts
new file mode 100644
index 0000000..bb0f1e9
--- /dev/null
+++ b/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences.ts
@@ -0,0 +1,598 @@
+/**
+ * @license
+ * Copyright 2024 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '@polymer/iron-input/iron-input';
+import '../../shared/gr-button/gr-button';
+import '../../shared/gr-select/gr-select';
+import {AccountDetailInfo, PreferencesInput} from '../../../types/common';
+import {grFormStyles} from '../../../styles/gr-form-styles';
+import {menuPageStyles} from '../../../styles/gr-menu-page-styles';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {LitElement, html, css, nothing} from 'lit';
+import {customElement, query, state} from 'lit/decorators.js';
+import {convertToString} from '../../../utils/string-util';
+import {subscribe} from '../../lit/subscription-controller';
+import {resolve} from '../../../models/dependency';
+import {userModelToken} from '../../../models/user/user-model';
+import {
+ AppTheme,
+ DateFormat,
+ DiffViewMode,
+ EmailFormat,
+ EmailStrategy,
+ TimeFormat,
+} from '../../../constants/constants';
+import {getAppContext} from '../../../services/app-context';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {areNotificationsEnabled} from '../../../utils/worker-util';
+import {getDocUrl} from '../../../utils/url-util';
+import {configModelToken} from '../../../models/config/config-model';
+import {SuggestionsProvider} from '../../../api/suggestions';
+import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+
+/**
+ * This provides an interface to show settings for a user profile
+ * as defined in PreferencesInfo.
+ */
+@customElement('gr-preferences')
+export class GrPreferences extends LitElement {
+ @query('#themeSelect') themeSelect!: HTMLInputElement;
+
+ @query('#changesPerPageSelect') changesPerPageSelect!: HTMLInputElement;
+
+ @query('#dateTimeFormatSelect') dateTimeFormatSelect!: HTMLInputElement;
+
+ @query('#timeFormatSelect') timeFormatSelect!: HTMLInputElement;
+
+ @query('#emailNotificationsSelect')
+ emailNotificationsSelect!: HTMLInputElement;
+
+ @query('#emailFormatSelect') emailFormatSelect!: HTMLInputElement;
+
+ @query('#allowBrowserNotifications')
+ allowBrowserNotifications?: HTMLInputElement;
+
+ @query('#allowSuggestCodeWhileCommenting')
+ allowSuggestCodeWhileCommenting?: HTMLInputElement;
+
+ @query('#defaultBaseForMergesSelect')
+ defaultBaseForMergesSelect!: HTMLInputElement;
+
+ @query('#relativeDateInChangeTable')
+ relativeDateInChangeTable!: HTMLInputElement;
+
+ @query('#diffViewSelect') diffViewSelect!: HTMLInputElement;
+
+ @query('#showSizeBarsInFileList') showSizeBarsInFileList!: HTMLInputElement;
+
+ @query('#publishCommentsOnPush') publishCommentsOnPush!: HTMLInputElement;
+
+ @query('#workInProgressByDefault') workInProgressByDefault!: HTMLInputElement;
+
+ @query('#disableKeyboardShortcuts')
+ disableKeyboardShortcuts!: HTMLInputElement;
+
+ @query('#disableTokenHighlighting')
+ disableTokenHighlighting!: HTMLInputElement;
+
+ @query('#insertSignedOff') insertSignedOff!: HTMLInputElement;
+
+ @state() prefs?: PreferencesInput;
+
+ @state() private originalPrefs?: PreferencesInput;
+
+ @state() account?: AccountDetailInfo;
+
+ @state() private docsBaseUrl = '';
+
+ @state()
+ suggestionsProvider?: SuggestionsProvider;
+
+ readonly getUserModel = resolve(this, userModelToken);
+
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private readonly getPluginLoader = resolve(this, pluginLoaderToken);
+
+ // private but used in test
+ readonly flagsService = getAppContext().flagsService;
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getUserModel().preferences$,
+ prefs => {
+ this.originalPrefs = prefs;
+ this.prefs = {...prefs};
+ }
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().account$,
+ acc => {
+ this.account = acc;
+ }
+ );
+ subscribe(
+ this,
+ () => this.getConfigModel().docsBaseUrl$,
+ docsBaseUrl => (this.docsBaseUrl = docsBaseUrl)
+ );
+ }
+
+ override connectedCallback() {
+ super.connectedCallback();
+ this.getPluginLoader()
+ .awaitPluginsLoaded()
+ .then(() => {
+ const suggestionsPlugins =
+ this.getPluginLoader().pluginsModel.getState().suggestionsPlugins;
+ // We currently support results from only 1 provider.
+ this.suggestionsProvider = suggestionsPlugins?.[0]?.provider;
+ });
+ }
+
+ static override get styles() {
+ return [
+ sharedStyles,
+ menuPageStyles,
+ grFormStyles,
+ css`
+ :host {
+ border: none;
+ margin-bottom: var(--spacing-xxl);
+ }
+ h2 {
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h2);
+ font-weight: var(--font-weight-h2);
+ line-height: var(--line-height-h2);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`
+ <h2 id="Preferences" class=${this.hasUnsavedChanges() ? 'edited' : ''}>
+ Preferences
+ </h2>
+ <fieldset id="preferences">
+ <div id="preferences" class="gr-form-styles">
+ <section>
+ <label class="title" for="themeSelect">Theme</label>
+ <span class="value">
+ <gr-select
+ .bindValue=${this.prefs?.theme ?? AppTheme.AUTO}
+ @change=${() => {
+ this.prefs!.theme = this.themeSelect.value as AppTheme;
+ this.requestUpdate();
+ }}
+ >
+ <select id="themeSelect">
+ <option value="AUTO">Auto (based on OS prefs)</option>
+ <option value="LIGHT">Light</option>
+ <option value="DARK">Dark</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="changesPerPageSelect"
+ >Changes per page</label
+ >
+ <span class="value">
+ <gr-select
+ .bindValue=${convertToString(this.prefs?.changes_per_page)}
+ @change=${() => {
+ this.prefs!.changes_per_page = Number(
+ this.changesPerPageSelect.value
+ ) as 10 | 25 | 50 | 100;
+ this.requestUpdate();
+ }}
+ >
+ <select id="changesPerPageSelect">
+ <option value="10">10 rows per page</option>
+ <option value="25">25 rows per page</option>
+ <option value="50">50 rows per page</option>
+ <option value="100">100 rows per page</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="dateTimeFormatSelect"
+ >Date/time format</label
+ >
+ <span class="value">
+ <gr-select
+ .bindValue=${convertToString(this.prefs?.date_format)}
+ @change=${() => {
+ this.prefs!.date_format = this.dateTimeFormatSelect
+ .value as DateFormat;
+ this.requestUpdate();
+ }}
+ >
+ <select id="dateTimeFormatSelect">
+ <option value="STD">Jun 3 ; Jun 3, 2016</option>
+ <option value="US">06/03 ; 06/03/16</option>
+ <option value="ISO">06-03 ; 2016-06-03</option>
+ <option value="EURO">3. Jun ; 03.06.2016</option>
+ <option value="UK">03/06 ; 03/06/2016</option>
+ </select>
+ </gr-select>
+ <gr-select
+ .bindValue=${convertToString(this.prefs?.time_format)}
+ aria-label="Time Format"
+ @change=${() => {
+ this.prefs!.time_format = this.timeFormatSelect
+ .value as TimeFormat;
+ this.requestUpdate();
+ }}
+ >
+ <select id="timeFormatSelect">
+ <option value="HHMM_12">4:10 PM</option>
+ <option value="HHMM_24">16:10</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="emailNotificationsSelect"
+ >Email notifications</label
+ >
+ <span class="value">
+ <gr-select
+ .bindValue=${convertToString(this.prefs?.email_strategy)}
+ @change=${() => {
+ this.prefs!.email_strategy = this.emailNotificationsSelect
+ .value as EmailStrategy;
+ this.requestUpdate();
+ }}
+ >
+ <select id="emailNotificationsSelect">
+ <option value="CC_ON_OWN_COMMENTS">Every comment</option>
+ <option value="ENABLED">Only comments left by others</option>
+ <option value="ATTENTION_SET_ONLY">
+ Only when I am in the attention set
+ </option>
+ <option value="DISABLED">None</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="emailFormatSelect">Email format</label>
+ <span class="value">
+ <gr-select
+ .bindValue=${convertToString(this.prefs?.email_format)}
+ @change=${() => {
+ this.prefs!.email_format = this.emailFormatSelect
+ .value as EmailFormat;
+ this.requestUpdate();
+ }}
+ >
+ <select id="emailFormatSelect">
+ <option value="HTML_PLAINTEXT">HTML and plaintext</option>
+ <option value="PLAINTEXT">Plaintext only</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ ${this.renderBrowserNotifications()}
+ ${this.renderGenerateSuggestionWhenCommenting()}
+ ${this.renderDefaultBaseForMerges()}
+ <section>
+ <label class="title" for="relativeDateInChangeTable"
+ >Show Relative Dates In Changes Table</label
+ >
+ <span class="value">
+ <input
+ id="relativeDateInChangeTable"
+ type="checkbox"
+ ?checked=${this.prefs?.relative_date_in_change_table}
+ @change=${() => {
+ this.prefs!.relative_date_in_change_table =
+ this.relativeDateInChangeTable.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>
+ <section>
+ <span class="title">Diff view</span>
+ <span class="value">
+ <gr-select
+ .bindValue=${convertToString(this.prefs?.diff_view)}
+ @change=${() => {
+ this.prefs!.diff_view = this.diffViewSelect
+ .value as DiffViewMode;
+ this.requestUpdate();
+ }}
+ >
+ <select id="diffViewSelect">
+ <option value="SIDE_BY_SIDE">Side by side</option>
+ <option value="UNIFIED_DIFF">Unified diff</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label for="showSizeBarsInFileList" class="title"
+ >Show size bars in file list</label
+ >
+ <span class="value">
+ <input
+ id="showSizeBarsInFileList"
+ type="checkbox"
+ ?checked=${this.prefs?.size_bar_in_change_table}
+ @change=${() => {
+ this.prefs!.size_bar_in_change_table =
+ this.showSizeBarsInFileList.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>
+ <section>
+ <label for="publishCommentsOnPush" class="title"
+ >Publish comments on push</label
+ >
+ <span class="value">
+ <input
+ id="publishCommentsOnPush"
+ type="checkbox"
+ ?checked=${this.prefs?.publish_comments_on_push}
+ @change=${() => {
+ this.prefs!.publish_comments_on_push =
+ this.publishCommentsOnPush.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>
+ <section>
+ <label for="workInProgressByDefault" class="title"
+ >Set new changes to "work in progress" by default</label
+ >
+ <span class="value">
+ <input
+ id="workInProgressByDefault"
+ type="checkbox"
+ ?checked=${this.prefs?.work_in_progress_by_default}
+ @change=${() => {
+ this.prefs!.work_in_progress_by_default =
+ this.workInProgressByDefault.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>
+ <section>
+ <label for="disableKeyboardShortcuts" class="title"
+ >Disable all keyboard shortcuts</label
+ >
+ <span class="value">
+ <input
+ id="disableKeyboardShortcuts"
+ type="checkbox"
+ ?checked=${this.prefs?.disable_keyboard_shortcuts}
+ @change=${() => {
+ this.prefs!.disable_keyboard_shortcuts =
+ this.disableKeyboardShortcuts.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>
+ <section>
+ <label for="disableTokenHighlighting" class="title"
+ >Disable token highlighting on hover</label
+ >
+ <span class="value">
+ <input
+ id="disableTokenHighlighting"
+ type="checkbox"
+ ?checked=${this.prefs?.disable_token_highlighting}
+ @change=${() => {
+ this.prefs!.disable_token_highlighting =
+ this.disableTokenHighlighting.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>
+ <section>
+ <label for="insertSignedOff" class="title">
+ Insert Signed-off-by Footer For Inline Edit Changes
+ </label>
+ <span class="value">
+ <input
+ id="insertSignedOff"
+ type="checkbox"
+ ?checked=${this.prefs?.signed_off_by}
+ @change=${() => {
+ this.prefs!.signed_off_by = this.insertSignedOff.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>
+ </div>
+ <gr-button
+ id="savePrefs"
+ @click=${async () => {
+ await this.save();
+ }}
+ ?disabled=${!this.hasUnsavedChanges()}
+ >Save changes</gr-button
+ >
+ </fieldset>
+ `;
+ }
+
+ // When the experiment is over, move this back to render(),
+ // removing this function.
+ private renderBrowserNotifications() {
+ if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS))
+ return nothing;
+ if (
+ !this.flagsService.isEnabled(
+ KnownExperimentId.PUSH_NOTIFICATIONS_DEVELOPER
+ ) &&
+ !areNotificationsEnabled(this.account)
+ )
+ return nothing;
+ return html` <section id="allowBrowserNotificationsSection">
+ <div class="title">
+ <label for="allowBrowserNotifications"
+ >Allow browser notifications</label
+ >
+ <a
+ href=${getDocUrl(
+ this.docsBaseUrl,
+ 'user-attention-set.html#_browser_notifications'
+ )}
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ <gr-icon icon="help" title="read documentation"></gr-icon>
+ </a>
+ </div>
+ <span class="value">
+ <input
+ id="allowBrowserNotifications"
+ type="checkbox"
+ ?checked=${this.prefs?.allow_browser_notifications}
+ @change=${() => {
+ this.prefs!.allow_browser_notifications =
+ this.allowBrowserNotifications!.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>`;
+ }
+
+ // When the experiment is over, move this back to render(),
+ // removing this function.
+ private renderGenerateSuggestionWhenCommenting() {
+ if (
+ !this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT) ||
+ !this.suggestionsProvider
+ )
+ return nothing;
+ return html`
+ <section id="allowSuggestCodeWhileCommentingSection">
+ <div class="title">
+ <label for="allowSuggestCodeWhileCommenting"
+ >AI suggested fixes while commenting</label
+ >
+ <a
+ href=${this.suggestionsProvider.getDocumentationLink?.() ||
+ getDocUrl(
+ this.docsBaseUrl,
+ 'user-suggest-edits.html#_generate_suggestion'
+ )}
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ <gr-icon icon="help" title="read documentation"></gr-icon>
+ </a>
+ </div>
+ <span class="value">
+ <input
+ id="allowSuggestCodeWhileCommenting"
+ type="checkbox"
+ ?checked=${this.prefs?.allow_suggest_code_while_commenting}
+ @change=${() => {
+ this.prefs!.allow_suggest_code_while_commenting =
+ this.allowSuggestCodeWhileCommenting!.checked;
+ this.requestUpdate();
+ }}
+ />
+ </span>
+ </section>
+ `;
+ }
+
+ // When this is fixed and can be re-enabled, move this back to render()
+ // and remove function.
+ private renderDefaultBaseForMerges() {
+ if (!this.prefs?.default_base_for_merges) return nothing;
+ return nothing;
+ // TODO: Re-enable respecting the default_base_for_merges preference.
+ // See corresponding TODO in change-model.
+ // return html`
+ // <section>
+ // <span class="title">Default Base For Merges</span>
+ // <span class="value">
+ // <gr-select
+ // .bindValue=${convertToString(
+ // this.prefs?.default_base_for_merges
+ // )}
+ // @change=${() => {
+ // this.prefs!.default_base_for_merges = this
+ // .defaultBaseForMergesSelect.value as DefaultBase;
+ // this.requestUpdate();
+ // }}
+ // >
+ // <select id="defaultBaseForMergesSelect">
+ // <option value="AUTO_MERGE">Auto Merge</option>
+ // <option value="FIRST_PARENT">First Parent</option>
+ // </select>
+ // </gr-select>
+ // </span>
+ // </section>
+ // `;
+ }
+
+ // private but used in test
+ hasUnsavedChanges() {
+ // We have to wrap boolean values in Boolean() to ensure undefined values
+ // use false rather than undefined.
+ return (
+ this.originalPrefs?.theme !== this.prefs?.theme ||
+ this.originalPrefs?.changes_per_page !== this.prefs?.changes_per_page ||
+ this.originalPrefs?.date_format !== this.prefs?.date_format ||
+ this.originalPrefs?.time_format !== this.prefs?.time_format ||
+ this.originalPrefs?.email_strategy !== this.prefs?.email_strategy ||
+ this.originalPrefs?.email_format !== this.prefs?.email_format ||
+ Boolean(this.originalPrefs?.allow_browser_notifications) !==
+ Boolean(this.prefs?.allow_browser_notifications) ||
+ Boolean(this.originalPrefs?.allow_suggest_code_while_commenting) !==
+ Boolean(this.prefs?.allow_suggest_code_while_commenting) ||
+ this.originalPrefs?.default_base_for_merges !==
+ this.prefs?.default_base_for_merges ||
+ Boolean(this.originalPrefs?.relative_date_in_change_table) !==
+ Boolean(this.prefs?.relative_date_in_change_table) ||
+ this.originalPrefs?.diff_view !== this.prefs?.diff_view ||
+ Boolean(this.originalPrefs?.size_bar_in_change_table) !==
+ Boolean(this.prefs?.size_bar_in_change_table) ||
+ Boolean(this.originalPrefs?.publish_comments_on_push) !==
+ Boolean(this.prefs?.publish_comments_on_push) ||
+ Boolean(this.originalPrefs?.work_in_progress_by_default) !==
+ Boolean(this.prefs?.work_in_progress_by_default) ||
+ Boolean(this.originalPrefs?.disable_keyboard_shortcuts) !==
+ Boolean(this.prefs?.disable_keyboard_shortcuts) ||
+ Boolean(this.originalPrefs?.disable_token_highlighting) !==
+ Boolean(this.prefs?.disable_token_highlighting) ||
+ Boolean(this.originalPrefs?.signed_off_by) !==
+ Boolean(this.prefs?.signed_off_by)
+ );
+ }
+
+ async save() {
+ if (!this.prefs) return;
+ await this.getUserModel().updatePreferences(this.prefs);
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-preferences': GrPreferences;
+ }
+}
diff --git a/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences_test.ts b/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences_test.ts
new file mode 100644
index 0000000..818af06
--- /dev/null
+++ b/polygerrit-ui/app/elements/settings/gr-preferences/gr-preferences_test.ts
@@ -0,0 +1,454 @@
+/**
+ * @license
+ * Copyright 2024 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import './gr-preferences';
+import {
+ queryAll,
+ queryAndAssert,
+ stubFlags,
+ stubRestApi,
+ waitUntil,
+} from '../../../test/test-utils';
+import {GrPreferences} from './gr-preferences';
+import {PreferencesInfo, TopMenuItemInfo} from '../../../types/common';
+import {
+ AppTheme,
+ DateFormat,
+ DefaultBase,
+ DiffViewMode,
+ EmailFormat,
+ EmailStrategy,
+ TimeFormat,
+ createDefaultPreferences,
+} from '../../../constants/constants';
+import {fixture, html, assert} from '@open-wc/testing';
+import {GrSelect} from '../../shared/gr-select/gr-select';
+import {
+ createAccountDetailWithId,
+ createPreferences,
+} from '../../../test/test-data-generators';
+
+suite('gr-preferences tests', () => {
+ let element: GrPreferences;
+ let preferences: PreferencesInfo;
+
+ function valueOf(title: string, id: string): Element {
+ const sections = queryAll(element, `#${id} section`) ?? [];
+ let titleEl;
+ for (let i = 0; i < sections.length; i++) {
+ titleEl = sections[i].querySelector('.title');
+ if (titleEl?.textContent?.trim() === title) {
+ const el = sections[i].querySelector('.value');
+ if (el) return el;
+ }
+ }
+ assert.fail(`element with title ${title} not found`);
+ }
+
+ setup(async () => {
+ preferences = {
+ ...createPreferences(),
+ changes_per_page: 25,
+ theme: AppTheme.LIGHT,
+ date_format: DateFormat.UK,
+ time_format: TimeFormat.HHMM_12,
+ diff_view: DiffViewMode.UNIFIED,
+ email_strategy: EmailStrategy.ENABLED,
+ email_format: EmailFormat.HTML_PLAINTEXT,
+ default_base_for_merges: DefaultBase.FIRST_PARENT,
+ relative_date_in_change_table: false,
+ size_bar_in_change_table: true,
+ my: [
+ {url: '/first/url', name: 'first name', target: '_blank'},
+ {url: '/second/url', name: 'second name', target: '_blank'},
+ ] as TopMenuItemInfo[],
+ change_table: [],
+ };
+
+ stubRestApi('getPreferences').returns(Promise.resolve(preferences));
+
+ element = await fixture(html`<gr-preferences></gr-preferences>`);
+
+ await element.updateComplete;
+ });
+
+ test('renders', () => {
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <h2 id="Preferences">Preferences</h2>
+ <fieldset id="preferences">
+ <div class="gr-form-styles" id="preferences">
+ <section>
+ <label class="title" for="themeSelect"> Theme </label>
+ <span class="value">
+ <gr-select>
+ <select id="themeSelect">
+ <option value="AUTO">Auto (based on OS prefs)</option>
+ <option value="LIGHT">Light</option>
+ <option value="DARK">Dark</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="changesPerPageSelect">
+ Changes per page
+ </label>
+ <span class="value">
+ <gr-select>
+ <select id="changesPerPageSelect">
+ <option value="10">10 rows per page</option>
+ <option value="25">25 rows per page</option>
+ <option value="50">50 rows per page</option>
+ <option value="100">100 rows per page</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="dateTimeFormatSelect">
+ Date/time format
+ </label>
+ <span class="value">
+ <gr-select>
+ <select id="dateTimeFormatSelect">
+ <option value="STD">Jun 3 ; Jun 3, 2016</option>
+ <option value="US">06/03 ; 06/03/16</option>
+ <option value="ISO">06-03 ; 2016-06-03</option>
+ <option value="EURO">3. Jun ; 03.06.2016</option>
+ <option value="UK">03/06 ; 03/06/2016</option>
+ </select>
+ </gr-select>
+ <gr-select aria-label="Time Format">
+ <select id="timeFormatSelect">
+ <option value="HHMM_12">4:10 PM</option>
+ <option value="HHMM_24">16:10</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="emailNotificationsSelect">
+ Email notifications
+ </label>
+ <span class="value">
+ <gr-select>
+ <select id="emailNotificationsSelect">
+ <option value="CC_ON_OWN_COMMENTS">Every comment</option>
+ <option value="ENABLED">
+ Only comments left by others
+ </option>
+ <option value="ATTENTION_SET_ONLY">
+ Only when I am in the attention set
+ </option>
+ <option value="DISABLED">None</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="emailFormatSelect">
+ Email format
+ </label>
+ <span class="value">
+ <gr-select>
+ <select id="emailFormatSelect">
+ <option value="HTML_PLAINTEXT">HTML and plaintext</option>
+ <option value="PLAINTEXT">Plaintext only</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="relativeDateInChangeTable">
+ Show Relative Dates In Changes Table
+ </label>
+ <span class="value">
+ <input id="relativeDateInChangeTable" type="checkbox" />
+ </span>
+ </section>
+ <section>
+ <span class="title"> Diff view </span>
+ <span class="value">
+ <gr-select>
+ <select id="diffViewSelect">
+ <option value="SIDE_BY_SIDE">Side by side</option>
+ <option value="UNIFIED_DIFF">Unified diff</option>
+ </select>
+ </gr-select>
+ </span>
+ </section>
+ <section>
+ <label class="title" for="showSizeBarsInFileList">
+ Show size bars in file list
+ </label>
+ <span class="value">
+ <input checked="" id="showSizeBarsInFileList" type="checkbox" />
+ </span>
+ </section>
+ <section>
+ <label class="title" for="publishCommentsOnPush">
+ Publish comments on push
+ </label>
+ <span class="value">
+ <input id="publishCommentsOnPush" type="checkbox" />
+ </span>
+ </section>
+ <section>
+ <label class="title" for="workInProgressByDefault">
+ Set new changes to "work in progress" by default
+ </label>
+ <span class="value">
+ <input id="workInProgressByDefault" type="checkbox" />
+ </span>
+ </section>
+ <section>
+ <label class="title" for="disableKeyboardShortcuts">
+ Disable all keyboard shortcuts
+ </label>
+ <span class="value">
+ <input id="disableKeyboardShortcuts" type="checkbox" />
+ </span>
+ </section>
+ <section>
+ <label class="title" for="disableTokenHighlighting">
+ Disable token highlighting on hover
+ </label>
+ <span class="value">
+ <input id="disableTokenHighlighting" type="checkbox" />
+ </span>
+ </section>
+ <section>
+ <label class="title" for="insertSignedOff">
+ Insert Signed-off-by Footer For Inline Edit Changes
+ </label>
+ <span class="value">
+ <input id="insertSignedOff" type="checkbox" />
+ </span>
+ </section>
+ </div>
+ <gr-button
+ aria-disabled="true"
+ disabled=""
+ id="savePrefs"
+ role="button"
+ tabindex="-1"
+ >
+ Save changes
+ </gr-button>
+ </fieldset>
+ `
+ );
+ });
+
+ test('allow browser notifications', async () => {
+ stubFlags('isEnabled').returns(true);
+ element.account = createAccountDetailWithId();
+ await element.updateComplete;
+ assert.dom.equal(
+ queryAndAssert(element, '#allowBrowserNotificationsSection'),
+ /* HTML */ `<section id="allowBrowserNotificationsSection">
+ <div class="title">
+ <label for="allowBrowserNotifications">
+ Allow browser notifications
+ </label>
+ <a
+ href="/Documentation/user-attention-set.html#_browser_notifications"
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ <gr-icon icon="help" title="read documentation"> </gr-icon>
+ </a>
+ </div>
+ <span class="value">
+ <input checked="" id="allowBrowserNotifications" type="checkbox" />
+ </span>
+ </section>`
+ );
+ });
+
+ test('input values match preferences', () => {
+ // Rendered with the expected preferences selected.
+ assert.equal(
+ Number(
+ (
+ valueOf('Changes per page', 'preferences')!
+ .firstElementChild as GrSelect
+ ).bindValue
+ ),
+ preferences.changes_per_page
+ );
+ assert.equal(
+ (valueOf('Theme', 'preferences').firstElementChild as GrSelect).bindValue,
+ preferences.theme
+ );
+ assert.equal(
+ (
+ valueOf('Date/time format', 'preferences')!
+ .firstElementChild as GrSelect
+ ).bindValue,
+ preferences.date_format
+ );
+ assert.equal(
+ (valueOf('Date/time format', 'preferences')!.lastElementChild as GrSelect)
+ .bindValue,
+ preferences.time_format
+ );
+ assert.equal(
+ (
+ valueOf('Email notifications', 'preferences')!
+ .firstElementChild as GrSelect
+ ).bindValue,
+ preferences.email_strategy
+ );
+ assert.equal(
+ (valueOf('Email format', 'preferences')!.firstElementChild as GrSelect)
+ .bindValue,
+ preferences.email_format
+ );
+ assert.equal(
+ (
+ valueOf('Show Relative Dates In Changes Table', 'preferences')!
+ .firstElementChild as HTMLInputElement
+ ).checked,
+ false
+ );
+ assert.equal(
+ (valueOf('Diff view', 'preferences')!.firstElementChild as GrSelect)
+ .bindValue,
+ preferences.diff_view
+ );
+ assert.equal(
+ (
+ valueOf('Show size bars in file list', 'preferences')!
+ .firstElementChild as HTMLInputElement
+ ).checked,
+ true
+ );
+ assert.equal(
+ (
+ valueOf('Publish comments on push', 'preferences')!
+ .firstElementChild as HTMLInputElement
+ ).checked,
+ false
+ );
+ assert.equal(
+ (
+ valueOf(
+ 'Set new changes to "work in progress" by default',
+ 'preferences'
+ )!.firstElementChild as HTMLInputElement
+ ).checked,
+ false
+ );
+ assert.equal(
+ (
+ valueOf('Disable token highlighting on hover', 'preferences')!
+ .firstElementChild as HTMLInputElement
+ ).checked,
+ false
+ );
+ assert.equal(
+ (
+ valueOf(
+ 'Insert Signed-off-by Footer For Inline Edit Changes',
+ 'preferences'
+ )!.firstElementChild as HTMLInputElement
+ ).checked,
+ false
+ );
+
+ assert.isFalse(element.hasUnsavedChanges());
+ });
+
+ test('save changes', async () => {
+ assert.equal(element.prefs?.theme, AppTheme.LIGHT);
+
+ const themeSelect = valueOf('Theme', 'preferences')
+ .firstElementChild as GrSelect;
+ themeSelect.bindValue = AppTheme.DARK;
+
+ themeSelect.dispatchEvent(
+ new CustomEvent('change', {
+ composed: true,
+ bubbles: true,
+ })
+ );
+
+ const publishOnPush = valueOf('Publish comments on push', 'preferences')!
+ .firstElementChild! as HTMLSpanElement;
+
+ publishOnPush.click();
+
+ assert.isTrue(element.hasUnsavedChanges());
+
+ const savePrefStub = stubRestApi('savePreferences').resolves(
+ element.prefs as PreferencesInfo
+ );
+
+ await element.save();
+
+ // Wait for model state update, since this is not awaited by element.save()
+ await waitUntil(
+ () =>
+ element.getUserModel().getState().preferences?.theme === AppTheme.DARK
+ );
+ await waitUntil(
+ () => element.getUserModel().getState().preferences?.my === preferences.my
+ );
+ await waitUntil(
+ () =>
+ element.getUserModel().getState().preferences
+ ?.publish_comments_on_push === true
+ );
+
+ assert.isTrue(savePrefStub.called);
+ assert.isFalse(element.hasUnsavedChanges());
+ });
+
+ test('publish comments on push', async () => {
+ assert.isFalse(element.hasUnsavedChanges());
+
+ const publishCommentsOnPush = valueOf(
+ 'Publish comments on push',
+ 'preferences'
+ )!.firstElementChild! as HTMLSpanElement;
+ publishCommentsOnPush.click();
+
+ assert.isTrue(element.hasUnsavedChanges());
+
+ stubRestApi('savePreferences').callsFake(prefs => {
+ assert.equal(prefs.publish_comments_on_push, true);
+ return Promise.resolve(createDefaultPreferences());
+ });
+
+ // Save the change.
+ await element.save();
+ assert.isFalse(element.hasUnsavedChanges());
+ });
+
+ test('set new changes work-in-progress', async () => {
+ assert.isFalse(element.hasUnsavedChanges());
+
+ const newChangesWorkInProgress = valueOf(
+ 'Set new changes to "work in progress" by default',
+ 'preferences'
+ )!.firstElementChild! as HTMLSpanElement;
+ newChangesWorkInProgress.click();
+
+ assert.isTrue(element.hasUnsavedChanges());
+
+ stubRestApi('savePreferences').callsFake(prefs => {
+ assert.equal(prefs.work_in_progress_by_default, true);
+ return Promise.resolve(createDefaultPreferences());
+ });
+
+ // Save the change.
+ await element.save();
+ assert.isFalse(element.hasUnsavedChanges());
+ });
+});
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
index b002fc8..fd7d67b 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts
@@ -21,6 +21,7 @@
import '../gr-http-password/gr-http-password';
import '../gr-identities/gr-identities';
import '../gr-menu-editor/gr-menu-editor';
+import '../gr-preferences/gr-preferences';
import '../gr-ssh-editor/gr-ssh-editor';
import '../gr-watched-projects-editor/gr-watched-projects-editor';
import '../../shared/gr-dialog/gr-dialog';
@@ -39,17 +40,8 @@
import {GrEmailEditor} from '../gr-email-editor/gr-email-editor';
import {fireAlert, fireTitleChange} from '../../../utils/event-util';
import {getAppContext} from '../../../services/app-context';
-import {
- DateFormat,
- DefaultBase,
- DiffViewMode,
- EmailFormat,
- EmailStrategy,
- AppTheme,
- TimeFormat,
-} from '../../../constants/constants';
import {BindValueChangeEvent, ValueChangedEvent} from '../../../types/events';
-import {LitElement, css, html, nothing} from 'lit';
+import {LitElement, css, html} from 'lit';
import {customElement, query, queryAsync, state} from 'lit/decorators.js';
import {sharedStyles} from '../../../styles/shared-styles';
import {paperStyles} from '../../../styles/gr-paper-styles';
@@ -58,29 +50,24 @@
import {pageNavStyles} from '../../../styles/gr-page-nav-styles';
import {menuPageStyles} from '../../../styles/gr-menu-page-styles';
import {grFormStyles} from '../../../styles/gr-form-styles';
-import {KnownExperimentId} from '../../../services/flags/flags';
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {settingsViewModelToken} from '../../../models/views/settings';
-import {areNotificationsEnabled} from '../../../utils/worker-util';
import {
changeTablePrefs,
userModelToken,
} from '../../../models/user/user-model';
import {modalStyles} from '../../../styles/gr-modal-styles';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
-import {getDocUrl, rootUrl} from '../../../utils/url-util';
-import {configModelToken} from '../../../models/config/config-model';
-import {SuggestionsProvider} from '../../../api/suggestions';
-import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
+import {rootUrl} from '../../../utils/url-util';
const HTTP_AUTH = ['HTTP', 'HTTP_LDAP'];
-enum CopyPrefsDirection {
- PrefsToLocalPrefs,
- LocalPrefsToPrefs,
-}
-
+/**
+ * This provides an interface to show all settings for a user profile.
+ * In most cases a individual module is used per setting to make
+ * code more readable. In other cases, it is created within this module.
+ */
@customElement('gr-settings-view')
export class GrSettingsView extends LitElement {
/**
@@ -109,64 +96,17 @@
@query('#emailEditor', true) emailEditor!: GrEmailEditor;
- @query('#insertSignedOff') insertSignedOff!: HTMLInputElement;
-
- @query('#workInProgressByDefault') workInProgressByDefault!: HTMLInputElement;
-
- @query('#showSizeBarsInFileList') showSizeBarsInFileList!: HTMLInputElement;
-
- @query('#publishCommentsOnPush') publishCommentsOnPush!: HTMLInputElement;
-
- @query('#allowBrowserNotifications')
- allowBrowserNotifications?: HTMLInputElement;
-
- @query('#allowSuggestCodeWhileCommenting')
- allowSuggestCodeWhileCommenting?: HTMLInputElement;
-
- @query('#disableKeyboardShortcuts')
- disableKeyboardShortcuts!: HTMLInputElement;
-
- @query('#disableTokenHighlighting')
- disableTokenHighlighting!: HTMLInputElement;
-
- @query('#relativeDateInChangeTable')
- relativeDateInChangeTable!: HTMLInputElement;
-
- @query('#changesPerPageSelect') changesPerPageSelect!: HTMLInputElement;
-
- @query('#dateTimeFormatSelect') dateTimeFormatSelect!: HTMLInputElement;
-
- @query('#timeFormatSelect') timeFormatSelect!: HTMLInputElement;
-
- @query('#emailNotificationsSelect')
- emailNotificationsSelect!: HTMLInputElement;
-
- @query('#emailFormatSelect') emailFormatSelect!: HTMLInputElement;
-
- @query('#defaultBaseForMergesSelect')
- defaultBaseForMergesSelect!: HTMLInputElement;
-
- @query('#diffViewSelect') diffViewSelect!: HTMLInputElement;
-
- @query('#themeSelect') themeSelect!: HTMLInputElement;
-
@state() prefs: PreferencesInput = {};
@state() private accountInfoChanged = false;
// private but used in test
- @state() localPrefs: PreferencesInput = {};
-
- // private but used in test
@state() localChangeTableColumns: string[] = [];
@state() private loading = true;
@state() private changeTableChanged = false;
- // private but used in test
- @state() prefsChanged = false;
-
@state() private diffPrefsChanged = false;
@state() private watchedProjectsChanged = false;
@@ -199,11 +139,6 @@
@state() isDeletingAccount = false;
- @state() private docsBaseUrl = '';
-
- @state()
- suggestionsProvider?: SuggestionsProvider;
-
// private but used in test
public _testOnly_loadingPromise?: Promise<void>;
@@ -218,10 +153,6 @@
private readonly getNavigation = resolve(this, navigationToken);
- private readonly getConfigModel = resolve(this, configModelToken);
-
- private readonly getPluginLoader = resolve(this, pluginLoaderToken);
-
constructor() {
super();
subscribe(
@@ -248,16 +179,9 @@
}
this.prefs = prefs;
this.showNumber = !!prefs.legacycid_in_change_table;
- this.copyPrefs(CopyPrefsDirection.PrefsToLocalPrefs);
- this.prefsChanged = false;
this.localChangeTableColumns = changeTablePrefs(prefs);
}
);
- subscribe(
- this,
- () => this.getConfigModel().docsBaseUrl$,
- docsBaseUrl => (this.docsBaseUrl = docsBaseUrl)
- );
}
// private, but used in tests
@@ -275,14 +199,6 @@
// we need to manually calling scrollIntoView when hash changed
document.addEventListener('location-change', this.handleLocationChange);
fireTitleChange('Settings');
- this.getPluginLoader()
- .awaitPluginsLoaded()
- .then(() => {
- const suggestionsPlugins =
- this.getPluginLoader().pluginsModel.getState().suggestionsPlugins;
- // We currently support results from only 1 provider.
- this.suggestionsProvider = suggestionsPlugins?.[0]?.provider;
- });
}
override firstUpdated() {
@@ -459,32 +375,7 @@
</gr-dialog>
</dialog>
</fieldset>
- <h2
- id="Preferences"
- class=${this.computeHeaderClass(this.prefsChanged)}
- >
- Preferences
- </h2>
- <fieldset id="preferences">
- ${this.renderTheme()} ${this.renderChangesPerPages()}
- ${this.renderDateTimeFormat()} ${this.renderEmailNotification()}
- ${this.renderEmailFormat()} ${this.renderBrowserNotifications()}
- ${this.renderGenerateSuggestionWhenCommenting()}
- ${this.renderDefaultBaseForMerges()}
- ${this.renderRelativeDateInChangeTable()} ${this.renderDiffView()}
- ${this.renderShowSizeBarsInFileList()}
- ${this.renderPublishCommentsOnPush()}
- ${this.renderWorkInProgressByDefault()}
- ${this.renderDisableKeyboardShortcuts()}
- ${this.renderDisableTokenHighlighting()}
- ${this.renderInsertSignedOff()}
- <gr-button
- id="savePrefs"
- @click=${this.handleSavePreferences}
- ?disabled=${!this.prefsChanged}
- >Save changes</gr-button
- >
- </fieldset>
+ <gr-preferences id="preferences"></gr-preferences>
<h2
id="DiffPreferences"
class=${this.computeHeaderClass(this.diffPrefsChanged)}
@@ -701,436 +592,6 @@
super.disconnectedCallback();
}
- private renderTheme() {
- return html`
- <section>
- <label class="title" for="themeSelect">Theme</label>
- <span class="value">
- <gr-select
- .bindValue=${this.localPrefs.theme ?? AppTheme.AUTO}
- @change=${() => {
- this.localPrefs.theme = this.themeSelect.value as AppTheme;
- this.prefsChanged = true;
- }}
- >
- <select id="themeSelect">
- <option value="AUTO">Auto (based on OS prefs)</option>
- <option value="LIGHT">Light</option>
- <option value="DARK">Dark</option>
- </select>
- </gr-select>
- </span>
- </section>
- `;
- }
-
- private renderChangesPerPages() {
- return html`
- <section>
- <label class="title" for="changesPerPageSelect">Changes per page</label>
- <span class="value">
- <gr-select
- .bindValue=${this.convertToString(this.localPrefs.changes_per_page)}
- @change=${() => {
- this.localPrefs.changes_per_page = Number(
- this.changesPerPageSelect.value
- ) as 10 | 25 | 50 | 100;
- this.prefsChanged = true;
- }}
- >
- <select id="changesPerPageSelect">
- <option value="10">10 rows per page</option>
- <option value="25">25 rows per page</option>
- <option value="50">50 rows per page</option>
- <option value="100">100 rows per page</option>
- </select>
- </gr-select>
- </span>
- </section>
- `;
- }
-
- private renderDateTimeFormat() {
- return html`
- <section>
- <label class="title" for="dateTimeFormatSelect">Date/time format</label>
- <span class="value">
- <gr-select
- .bindValue=${this.convertToString(this.localPrefs.date_format)}
- @change=${() => {
- this.localPrefs.date_format = this.dateTimeFormatSelect
- .value as DateFormat;
- this.prefsChanged = true;
- }}
- >
- <select id="dateTimeFormatSelect">
- <option value="STD">Jun 3 ; Jun 3, 2016</option>
- <option value="US">06/03 ; 06/03/16</option>
- <option value="ISO">06-03 ; 2016-06-03</option>
- <option value="EURO">3. Jun ; 03.06.2016</option>
- <option value="UK">03/06 ; 03/06/2016</option>
- </select>
- </gr-select>
- <gr-select
- .bindValue=${this.convertToString(this.localPrefs.time_format)}
- aria-label="Time Format"
- @change=${() => {
- this.localPrefs.time_format = this.timeFormatSelect
- .value as TimeFormat;
- this.prefsChanged = true;
- }}
- >
- <select id="timeFormatSelect">
- <option value="HHMM_12">4:10 PM</option>
- <option value="HHMM_24">16:10</option>
- </select>
- </gr-select>
- </span>
- </section>
- `;
- }
-
- private renderEmailNotification() {
- return html`
- <section>
- <label class="title" for="emailNotificationsSelect"
- >Email notifications</label
- >
- <span class="value">
- <gr-select
- .bindValue=${this.convertToString(this.localPrefs.email_strategy)}
- @change=${() => {
- this.localPrefs.email_strategy = this.emailNotificationsSelect
- .value as EmailStrategy;
- this.prefsChanged = true;
- }}
- >
- <select id="emailNotificationsSelect">
- <option value="CC_ON_OWN_COMMENTS">Every comment</option>
- <option value="ENABLED">Only comments left by others</option>
- <option value="ATTENTION_SET_ONLY">
- Only when I am in the attention set
- </option>
- <option value="DISABLED">None</option>
- </select>
- </gr-select>
- </span>
- </section>
- `;
- }
-
- private renderEmailFormat() {
- if (!this.localPrefs.email_format) return nothing;
- return html`
- <section>
- <label class="title" for="emailFormatSelect">Email format</label>
- <span class="value">
- <gr-select
- .bindValue=${this.convertToString(this.localPrefs.email_format)}
- @change=${() => {
- this.localPrefs.email_format = this.emailFormatSelect
- .value as EmailFormat;
- this.prefsChanged = true;
- }}
- >
- <select id="emailFormatSelect">
- <option value="HTML_PLAINTEXT">HTML and plaintext</option>
- <option value="PLAINTEXT">Plaintext only</option>
- </select>
- </gr-select>
- </span>
- </section>
- `;
- }
-
- private renderBrowserNotifications() {
- if (!this.flagsService.isEnabled(KnownExperimentId.PUSH_NOTIFICATIONS))
- return nothing;
- if (
- !this.flagsService.isEnabled(
- KnownExperimentId.PUSH_NOTIFICATIONS_DEVELOPER
- ) &&
- !areNotificationsEnabled(this.account)
- )
- return nothing;
- return html`
- <section id="allowBrowserNotificationsSection">
- <div class="title">
- <label for="allowBrowserNotifications"
- >Allow browser notifications</label
- >
- <a
- href=${getDocUrl(
- this.docsBaseUrl,
- 'user-attention-set.html#_browser_notifications'
- )}
- target="_blank"
- rel="noopener noreferrer"
- >
- <gr-icon icon="help" title="read documentation"></gr-icon>
- </a>
- </div>
- <span class="value">
- <input
- id="allowBrowserNotifications"
- type="checkbox"
- ?checked=${this.localPrefs.allow_browser_notifications}
- @change=${() => {
- this.localPrefs.allow_browser_notifications =
- this.allowBrowserNotifications!.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
- private renderGenerateSuggestionWhenCommenting() {
- if (
- !this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT) ||
- !this.suggestionsProvider
- )
- return nothing;
- return html`
- <section id="allowSuggestCodeWhileCommentingSection">
- <div class="title">
- <label for="allowSuggestCodeWhileCommenting"
- >AI suggested fixes while commenting</label
- >
- <a
- href=${this.suggestionsProvider.getDocumentationLink?.() ||
- getDocUrl(
- this.docsBaseUrl,
- 'user-suggest-edits.html#_generate_suggestion'
- )}
- target="_blank"
- rel="noopener noreferrer"
- >
- <gr-icon icon="help" title="read documentation"></gr-icon>
- </a>
- </div>
- <span class="value">
- <input
- id="allowSuggestCodeWhileCommenting"
- type="checkbox"
- ?checked=${this.localPrefs.allow_suggest_code_while_commenting}
- @change=${() => {
- this.localPrefs.allow_suggest_code_while_commenting =
- this.allowSuggestCodeWhileCommenting!.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
- private renderDefaultBaseForMerges() {
- if (!this.localPrefs.default_base_for_merges) return nothing;
- return nothing;
- // TODO: Re-enable respecting the default_base_for_merges preference.
- // See corresponding TODO in change-model.
- // return html`
- // <section>
- // <span class="title">Default Base For Merges</span>
- // <span class="value">
- // <gr-select
- // .bindValue=${this.convertToString(
- // this.localPrefs.default_base_for_merges
- // )}
- // @change=${() => {
- // this.localPrefs.default_base_for_merges = this
- // .defaultBaseForMergesSelect.value as DefaultBase;
- // this.prefsChanged = true;
- // }}
- // >
- // <select id="defaultBaseForMergesSelect">
- // <option value="AUTO_MERGE">Auto Merge</option>
- // <option value="FIRST_PARENT">First Parent</option>
- // </select>
- // </gr-select>
- // </span>
- // </section>
- // `;
- }
-
- private renderRelativeDateInChangeTable() {
- return html`
- <section>
- <label class="title" for="relativeDateInChangeTable"
- >Show Relative Dates In Changes Table</label
- >
- <span class="value">
- <input
- id="relativeDateInChangeTable"
- type="checkbox"
- ?checked=${this.localPrefs.relative_date_in_change_table}
- @change=${() => {
- this.localPrefs.relative_date_in_change_table =
- this.relativeDateInChangeTable.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
- private renderDiffView() {
- return html`
- <section>
- <span class="title">Diff view</span>
- <span class="value">
- <gr-select
- .bindValue=${this.convertToString(this.localPrefs.diff_view)}
- @change=${() => {
- this.localPrefs.diff_view = this.diffViewSelect
- .value as DiffViewMode;
- this.prefsChanged = true;
- }}
- >
- <select id="diffViewSelect">
- <option value="SIDE_BY_SIDE">Side by side</option>
- <option value="UNIFIED_DIFF">Unified diff</option>
- </select>
- </gr-select>
- </span>
- </section>
- `;
- }
-
- private renderShowSizeBarsInFileList() {
- return html`
- <section>
- <label for="showSizeBarsInFileList" class="title"
- >Show size bars in file list</label
- >
- <span class="value">
- <input
- id="showSizeBarsInFileList"
- type="checkbox"
- ?checked=${this.localPrefs.size_bar_in_change_table}
- @change=${() => {
- this.localPrefs.size_bar_in_change_table =
- this.showSizeBarsInFileList.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
- private renderPublishCommentsOnPush() {
- return html`
- <section>
- <label for="publishCommentsOnPush" class="title"
- >Publish comments on push</label
- >
- <span class="value">
- <input
- id="publishCommentsOnPush"
- type="checkbox"
- ?checked=${this.localPrefs.publish_comments_on_push}
- @change=${() => {
- this.localPrefs.publish_comments_on_push =
- this.publishCommentsOnPush.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
- private renderWorkInProgressByDefault() {
- return html`
- <section>
- <label for="workInProgressByDefault" class="title"
- >Set new changes to "work in progress" by default</label
- >
- <span class="value">
- <input
- id="workInProgressByDefault"
- type="checkbox"
- ?checked=${this.localPrefs.work_in_progress_by_default}
- @change=${() => {
- this.localPrefs.work_in_progress_by_default =
- this.workInProgressByDefault.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
- private renderDisableKeyboardShortcuts() {
- return html`
- <section>
- <label for="disableKeyboardShortcuts" class="title"
- >Disable all keyboard shortcuts</label
- >
- <span class="value">
- <input
- id="disableKeyboardShortcuts"
- type="checkbox"
- ?checked=${this.localPrefs.disable_keyboard_shortcuts}
- @change=${() => {
- this.localPrefs.disable_keyboard_shortcuts =
- this.disableKeyboardShortcuts.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
- private renderDisableTokenHighlighting() {
- return html`
- <section>
- <label for="disableTokenHighlighting" class="title"
- >Disable token highlighting on hover</label
- >
- <span class="value">
- <input
- id="disableTokenHighlighting"
- type="checkbox"
- ?checked=${this.localPrefs.disable_token_highlighting}
- @change=${() => {
- this.localPrefs.disable_token_highlighting =
- this.disableTokenHighlighting.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
- private renderInsertSignedOff() {
- return html`
- <section>
- <label for="insertSignedOff" class="title">
- Insert Signed-off-by Footer For Inline Edit Changes
- </label>
- <span class="value">
- <input
- id="insertSignedOff"
- type="checkbox"
- ?checked=${this.localPrefs.signed_off_by}
- @change=${() => {
- this.localPrefs.signed_off_by = this.insertSignedOff.checked;
- this.prefsChanged = true;
- }}
- />
- </span>
- </section>
- `;
- }
-
private readonly handleLocationChange = () => {
// Handle anchor tag after dom attached
const urlHash = window.location.hash;
@@ -1147,30 +608,12 @@
Promise.all([this.accountInfo.loadData(), this.emailEditor.loadData()]);
}
- private copyPrefs(direction: CopyPrefsDirection) {
- if (direction === CopyPrefsDirection.LocalPrefsToPrefs) {
- this.prefs = {
- ...this.localPrefs,
- };
- } else {
- this.localPrefs = {
- ...this.prefs,
- };
- }
- }
-
// private but used in test
- handleSavePreferences() {
- return this.getUserModel().updatePreferences(this.localPrefs);
- }
-
- // private but used in test
- handleSaveChangeTable() {
+ async handleSaveChangeTable() {
this.prefs.change_table = this.localChangeTableColumns;
this.prefs.legacycid_in_change_table = this.showNumber;
- return this.restApiService.savePreferences(this.prefs).then(() => {
- this.changeTableChanged = false;
- });
+ await this.getUserModel().updatePreferences(this.prefs);
+ this.changeTableChanged = false;
}
private computeHeaderClass(changed?: boolean) {
@@ -1235,25 +678,6 @@
return false;
}
-
- /**
- * bind-value has type string so we have to convert anything inputed
- * to string.
- *
- * This is so typescript template checker doesn't fail.
- */
- private convertToString(
- key?:
- | DateFormat
- | DefaultBase
- | DiffViewMode
- | EmailFormat
- | EmailStrategy
- | TimeFormat
- | number
- ) {
- return key !== undefined ? String(key) : '';
- }
}
declare global {
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
index f9b1738..b6690b6 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts
@@ -6,13 +6,7 @@
import '../../../test/common-test-setup';
import './gr-settings-view';
import {GrSettingsView} from './gr-settings-view';
-import {
- queryAll,
- queryAndAssert,
- stubFlags,
- stubRestApi,
- waitEventLoop,
-} from '../../../test/test-utils';
+import {stubRestApi, waitEventLoop} from '../../../test/test-utils';
import {
AuthInfo,
AccountDetailInfo,
@@ -22,7 +16,6 @@
TopMenuItemInfo,
} from '../../../types/common';
import {
- createDefaultPreferences,
DateFormat,
DefaultBase,
DiffViewMode,
@@ -36,7 +29,6 @@
createPreferences,
createServerInfo,
} from '../../../test/test-data-generators';
-import {GrSelect} from '../../shared/gr-select/gr-select';
import {fixture, html, assert} from '@open-wc/testing';
suite('gr-settings-view tests', () => {
@@ -45,36 +37,6 @@
let preferences: PreferencesInfo;
let config: ServerInfo;
- function valueOf(title: string, id: string) {
- const sections = queryAll(element, `#${id} section`);
- let titleEl;
- for (let i = 0; i < sections.length; i++) {
- titleEl = sections[i].querySelector('.title');
- if (titleEl?.textContent?.trim() === title) {
- const el = sections[i].querySelector('.value');
- if (el) return el;
- }
- }
- assert.fail(`element with title ${title} not found`);
- }
-
- // Because deepEqual isn't behaving in Safari.
- function assertMenusEqual(
- actual?: TopMenuItemInfo[],
- expected?: TopMenuItemInfo[]
- ) {
- if (actual === undefined) {
- assert.fail("assertMenusEqual 'actual' param is undefined");
- } else if (expected === undefined) {
- assert.fail("assertMenusEqual 'expected' param is undefined");
- }
- assert.equal(actual.length, expected.length);
- for (let i = 0; i < actual.length; i++) {
- assert.equal(actual[i].name, expected[i].name);
- assert.equal(actual[i].url, expected[i].url);
- }
- }
-
function stubAddAccountEmail(statusCode: number) {
return stubRestApi('addAccountEmail').callsFake(() =>
Promise.resolve({status: statusCode} as Response)
@@ -193,172 +155,7 @@
</gr-dialog>
</dialog>
</fieldset>
- <h2 id="Preferences">Preferences</h2>
- <fieldset id="preferences">
- <section>
- <label class="title" for="themeSelect">
- Theme
- </label>
- <span class="value">
- <gr-select>
- <select id="themeSelect">
- <option value="AUTO">Auto (based on OS prefs)</option>
- <option value="LIGHT">Light</option>
- <option value="DARK">Dark</option>
- </select>
- </gr-select>
- </span>
- </section>
- <section>
- <label class="title" for="changesPerPageSelect">
- Changes per page
- </label>
- <span class="value">
- <gr-select>
- <select id="changesPerPageSelect">
- <option value="10">10 rows per page</option>
- <option value="25">25 rows per page</option>
- <option value="50">50 rows per page</option>
- <option value="100">100 rows per page</option>
- </select>
- </gr-select>
- </span>
- </section>
- <section>
- <label class="title" for="dateTimeFormatSelect">
- Date/time format
- </label>
- <span class="value">
- <gr-select>
- <select id="dateTimeFormatSelect">
- <option value="STD">Jun 3 ; Jun 3, 2016</option>
- <option value="US">06/03 ; 06/03/16</option>
- <option value="ISO">06-03 ; 2016-06-03</option>
- <option value="EURO">3. Jun ; 03.06.2016</option>
- <option value="UK">03/06 ; 03/06/2016</option>
- </select>
- </gr-select>
- <gr-select aria-label="Time Format">
- <select id="timeFormatSelect">
- <option value="HHMM_12">4:10 PM</option>
- <option value="HHMM_24">16:10</option>
- </select>
- </gr-select>
- </span>
- </section>
- <section>
- <label class="title" for="emailNotificationsSelect">
- Email notifications
- </label>
- <span class="value">
- <gr-select>
- <select id="emailNotificationsSelect">
- <option value="CC_ON_OWN_COMMENTS">Every comment</option>
- <option value="ENABLED">
- Only comments left by others
- </option>
- <option value="ATTENTION_SET_ONLY">
- Only when I am in the attention set
- </option>
- <option value="DISABLED">None</option>
- </select>
- </gr-select>
- </span>
- </section>
- <section>
- <label class="title" for="emailFormatSelect">
- Email format
- </label>
- <span class="value">
- <gr-select>
- <select id="emailFormatSelect">
- <option value="HTML_PLAINTEXT">HTML and plaintext</option>
- <option value="PLAINTEXT">Plaintext only</option>
- </select>
- </gr-select>
- </span>
- </section>
- <section>
- <label class="title" for="relativeDateInChangeTable">
- Show Relative Dates In Changes Table
- </label>
- <span class="value">
- <input id="relativeDateInChangeTable" type="checkbox" />
- </span>
- </section>
- <section>
- <span class="title"> Diff view </span>
- <span class="value">
- <gr-select>
- <select id="diffViewSelect">
- <option value="SIDE_BY_SIDE">Side by side</option>
- <option value="UNIFIED_DIFF">Unified diff</option>
- </select>
- </gr-select>
- </span>
- </section>
- <section>
- <label class="title" for="showSizeBarsInFileList">
- Show size bars in file list
- </label>
- <span class="value">
- <input
- checked=""
- id="showSizeBarsInFileList"
- type="checkbox"
- />
- </span>
- </section>
- <section>
- <label class="title" for="publishCommentsOnPush">
- Publish comments on push
- </label>
- <span class="value">
- <input id="publishCommentsOnPush" type="checkbox" />
- </span>
- </section>
- <section>
- <label class="title" for="workInProgressByDefault">
- Set new changes to "work in progress" by default
- </label>
- <span class="value">
- <input id="workInProgressByDefault" type="checkbox" />
- </span>
- </section>
- <section>
- <label class="title" for="disableKeyboardShortcuts">
- Disable all keyboard shortcuts
- </label>
- <span class="value">
- <input id="disableKeyboardShortcuts" type="checkbox" />
- </span>
- </section>
- <section>
- <label class="title" for="disableTokenHighlighting">
- Disable token highlighting on hover
- </label>
- <span class="value">
- <input id="disableTokenHighlighting" type="checkbox" />
- </span>
- </section>
- <section>
- <label class="title" for="insertSignedOff">
- Insert Signed-off-by Footer For Inline Edit Changes
- </label>
- <span class="value">
- <input id="insertSignedOff" type="checkbox" />
- </span>
- </section>
- <gr-button
- aria-disabled="true"
- disabled=""
- id="savePrefs"
- role="button"
- tabindex="-1"
- >
- Save changes
- </gr-button>
- </fieldset>
+ <gr-preferences id="preferences"> </gr-preferences>
<h2 id="DiffPreferences">Diff Preferences</h2>
<fieldset id="diffPreferences">
<gr-diff-preferences id="diffPrefs"> </gr-diff-preferences>
@@ -456,33 +253,6 @@
);
});
- test('allow browser notifications', async () => {
- stubFlags('isEnabled').returns(true);
- element = await fixture(html`<gr-settings-view></gr-settings-view>`);
- element.account = createAccountDetailWithId();
- await element.updateComplete;
- assert.dom.equal(
- queryAndAssert(element, '#allowBrowserNotificationsSection'),
- /* HTML */ `<section id="allowBrowserNotificationsSection">
- <div class="title">
- <label for="allowBrowserNotifications">
- Allow browser notifications
- </label>
- <a
- href="/Documentation/user-attention-set.html#_browser_notifications"
- target="_blank"
- rel="noopener noreferrer"
- >
- <gr-icon icon="help" title="read documentation"> </gr-icon>
- </a>
- </div>
- <span class="value">
- <input checked="" id="allowBrowserNotifications" type="checkbox" />
- </span>
- </section>`
- );
- });
-
test('calls the title-change event', async () => {
const titleChangedStub = sinon.stub();
const newElement = document.createElement('gr-settings-view');
@@ -497,167 +267,6 @@
assert.equal(titleChangedStub.getCall(0).args[0].detail.title, 'Settings');
});
- test('user preferences', async () => {
- // Rendered with the expected preferences selected.
- assert.equal(
- Number(
- (
- valueOf('Changes per page', 'preferences')!
- .firstElementChild as GrSelect
- ).bindValue
- ),
- preferences.changes_per_page
- );
- assert.equal(
- (valueOf('Theme', 'preferences').firstElementChild as GrSelect).bindValue,
- preferences.theme
- );
- assert.equal(
- (
- valueOf('Date/time format', 'preferences')!
- .firstElementChild as GrSelect
- ).bindValue,
- preferences.date_format
- );
- assert.equal(
- (valueOf('Date/time format', 'preferences')!.lastElementChild as GrSelect)
- .bindValue,
- preferences.time_format
- );
- assert.equal(
- (
- valueOf('Email notifications', 'preferences')!
- .firstElementChild as GrSelect
- ).bindValue,
- preferences.email_strategy
- );
- assert.equal(
- (valueOf('Email format', 'preferences')!.firstElementChild as GrSelect)
- .bindValue,
- preferences.email_format
- );
- assert.equal(
- (
- valueOf('Show Relative Dates In Changes Table', 'preferences')!
- .firstElementChild as HTMLInputElement
- ).checked,
- false
- );
- assert.equal(
- (valueOf('Diff view', 'preferences')!.firstElementChild as GrSelect)
- .bindValue,
- preferences.diff_view
- );
- assert.equal(
- (
- valueOf('Show size bars in file list', 'preferences')!
- .firstElementChild as HTMLInputElement
- ).checked,
- true
- );
- assert.equal(
- (
- valueOf('Publish comments on push', 'preferences')!
- .firstElementChild as HTMLInputElement
- ).checked,
- false
- );
- assert.equal(
- (
- valueOf(
- 'Set new changes to "work in progress" by default',
- 'preferences'
- )!.firstElementChild as HTMLInputElement
- ).checked,
- false
- );
- assert.equal(
- (
- valueOf('Disable token highlighting on hover', 'preferences')!
- .firstElementChild as HTMLInputElement
- ).checked,
- false
- );
- assert.equal(
- (
- valueOf(
- 'Insert Signed-off-by Footer For Inline Edit Changes',
- 'preferences'
- )!.firstElementChild as HTMLInputElement
- ).checked,
- false
- );
-
- assert.isFalse(element.prefsChanged);
-
- const themeSelect = valueOf('Theme', 'preferences')
- .firstElementChild as GrSelect;
- themeSelect.bindValue = 'DARK';
-
- themeSelect.dispatchEvent(
- new CustomEvent('change', {
- composed: true,
- bubbles: true,
- })
- );
-
- const publishOnPush = valueOf('Publish comments on push', 'preferences')!
- .firstElementChild! as HTMLSpanElement;
-
- publishOnPush.click();
-
- assert.isTrue(element.prefsChanged);
-
- stubRestApi('savePreferences').callsFake(prefs => {
- assertMenusEqual(prefs.my, preferences.my);
- assert.equal(prefs.publish_comments_on_push, true);
- assert.equal(prefs.theme, AppTheme.DARK);
- return Promise.resolve(createDefaultPreferences());
- });
-
- // Save the change.
- await element.handleSavePreferences();
- assert.isFalse(element.prefsChanged);
- });
-
- test('publish comments on push', async () => {
- const publishCommentsOnPush = valueOf(
- 'Publish comments on push',
- 'preferences'
- )!.firstElementChild! as HTMLSpanElement;
- publishCommentsOnPush.click();
-
- assert.isTrue(element.prefsChanged);
-
- stubRestApi('savePreferences').callsFake(prefs => {
- assert.equal(prefs.publish_comments_on_push, true);
- return Promise.resolve(createDefaultPreferences());
- });
-
- // Save the change.
- await element.handleSavePreferences();
- assert.isFalse(element.prefsChanged);
- });
-
- test('set new changes work-in-progress', async () => {
- const newChangesWorkInProgress = valueOf(
- 'Set new changes to "work in progress" by default',
- 'preferences'
- )!.firstElementChild! as HTMLSpanElement;
- newChangesWorkInProgress.click();
-
- assert.isTrue(element.prefsChanged);
-
- stubRestApi('savePreferences').callsFake(prefs => {
- assert.equal(prefs.work_in_progress_by_default, true);
- return Promise.resolve(createDefaultPreferences());
- });
-
- // Save the change.
- await element.handleSavePreferences();
- assert.isFalse(element.prefsChanged);
- });
-
test('add email validation', async () => {
assert.isFalse(element.isNewEmailValid('invalid email'));
assert.isTrue(element.isNewEmailValid('vaguely@valid.email'));
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
index 4c73fcf..d77bec9 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -23,15 +23,15 @@
import {createSearchUrl} from '../../../models/views/search';
import {accountsModelToken} from '../../../models/accounts-model/accounts-model';
import {resolve} from '../../../models/dependency';
+import {configModelToken} from '../../../models/config/config-model';
+import {userModelToken} from '../../../models/user/user-model';
+import {subscribe} from '../../lit/subscription-controller';
@customElement('gr-account-label')
export class GrAccountLabel extends LitElement {
@property({type: Object})
account?: AccountInfo;
- @property({type: Object})
- _selfAccount?: AccountInfo;
-
/**
* Optional ChangeInfo object, typically comes from the change page or
* from a row in a list of search results. This is needed for some change
@@ -67,9 +67,6 @@
@property({type: Boolean})
hideAvatar = false;
- @state()
- _config?: ServerInfo;
-
@property({type: Boolean, reflect: true})
selectionChipStyle = false;
@@ -94,12 +91,24 @@
@property({type: Boolean, reflect: true})
avatarShown = false;
+ // Private but used in tests.
+ @state()
+ selfAccount?: AccountInfo;
+
+ // Private but used in tests.
+ @state()
+ config?: ServerInfo;
+
readonly reporting = getAppContext().reportingService;
private readonly restApiService = getAppContext().restApiService;
private readonly getAccountsModel = resolve(this, accountsModelToken);
+ private readonly getConfigModel = resolve(this, configModelToken);
+
+ private readonly getUserModel = resolve(this, userModelToken);
+
static override get styles() {
return [
css`
@@ -201,13 +210,13 @@
}
override render() {
- const {account, change, highlightAttention, forceAttention, _config} = this;
+ const {account, change, highlightAttention, forceAttention, config} = this;
if (!account) return;
this.attentionIconShown =
forceAttention ||
this.hasUnforcedAttention(highlightAttention, account, change);
this.deselected = !this.selected;
- const hasAvatars = !!_config?.plugin?.has_avatars;
+ const hasAvatars = !!config?.plugin?.has_avatars;
this.avatarShown = !this.hideAvatar && hasAvatars;
return html`
@@ -227,7 +236,7 @@
account,
change,
false,
- this._selfAccount
+ this.selfAccount
)}
title=${this.computeAttentionIconTitle(
highlightAttention,
@@ -235,7 +244,7 @@
change,
forceAttention,
this.selected,
- this._selfAccount
+ this.selfAccount
)}
>
<gr-button
@@ -248,7 +257,7 @@
account,
change,
this.selected,
- this._selfAccount
+ this.selfAccount
)}
>
<div>
@@ -280,7 +289,7 @@
class="name"
part="gr-account-label-text"
>
- ${this.computeName(account, this.firstName, this._config)}
+ ${this.computeName(account, this.firstName, this.config)}
</span>
${this.renderAccountStatusPlugins()}
</span>
@@ -291,12 +300,16 @@
constructor() {
super();
- this.restApiService.getConfig().then(config => {
- this._config = config;
- });
- this.restApiService.getAccount().then(account => {
- this._selfAccount = account;
- });
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ x => (this.config = x)
+ );
+ subscribe(
+ this,
+ () => this.getUserModel().account$,
+ x => (this.selfAccount = x)
+ );
this.addEventListener('attention-set-updated', () => {
// For re-evaluation of everything that depends on 'change'.
if (this.change) this.change = {...this.change};
@@ -375,7 +388,7 @@
// We are deliberately updating the UI before making the API call. It is a
// risk that we are taking to achieve a better UX for 99.9% of the cases.
- const reason = getRemovedByIconClickReason(this._selfAccount, this._config);
+ const reason = getRemovedByIconClickReason(this.selfAccount, this.config);
if (this.change.attention_set)
delete this.change.attention_set[this.account._account_id];
// For re-evaluation of everything that depends on 'change'.
@@ -401,7 +414,7 @@
const targetId = this.account._account_id;
const ownerId =
(this.change && this.change.owner && this.change.owner._account_id) || -1;
- const selfId = this._selfAccount?._account_id || -1;
+ const selfId = this.selfAccount?._account_id || -1;
const reviewers =
this.change && this.change.reviewers && this.change.reviewers.REVIEWER
? [...this.change.reviewers.REVIEWER]
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.ts
index 71f8391..8b49cdd 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.ts
@@ -154,11 +154,11 @@
suite('attention set', () => {
setup(async () => {
element.highlightAttention = true;
- element._config = {
+ element.config = {
...createServerInfo(),
user: {anonymous_coward_name: 'Anonymous Coward'},
};
- element._selfAccount = kermit;
+ element.selfAccount = kermit;
element.account = {
...createAccountDetailWithIdNameAndEmail(42),
name: 'ernie',
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
index 863f4f8..3509146 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -1216,6 +1216,7 @@
this.reporting.reportInteraction(Interaction.GENERATE_SUGGESTION_RESPONSE, {
uuid: this.generatedSuggestionId,
type: 'suggest-code',
+ commentId: this.comment.id,
response: suggestionResponse.responseCode,
numSuggestions: suggestionResponse.suggestions.length,
hasNewRange: suggestionResponse.suggestions?.[0]?.newRange !== undefined,
@@ -1267,6 +1268,7 @@
this.reporting.reportInteraction(Interaction.GENERATE_SUGGESTION_RESPONSE, {
uuid: this.generatedSuggestionId,
type: 'suggest-fix',
+ commentId: this.comment.id,
response: suggestionResponse.responseCode,
numSuggestions: suggestionResponse.fix_suggestions.length,
});
diff --git a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
index d3a4013..c533a5e 100644
--- a/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
+++ b/polygerrit-ui/app/elements/shared/gr-suggestion-diff-preview/gr-suggestion-diff-preview.ts
@@ -256,6 +256,7 @@
});
this.reporting.timeEnd(Timing.PREVIEW_FIX_LOAD, {
uuid: this.uuid,
+ commentId: this.comment?.id ?? '',
});
if (currentPreviews.length > 0) {
this.preview = currentPreviews[0];
@@ -288,6 +289,7 @@
});
this.reporting.timeEnd(Timing.PREVIEW_FIX_LOAD, {
uuid: this.uuid,
+ commentId: this.comment?.id ?? '',
});
if (currentPreviews.length > 0) {
this.preview = currentPreviews[0];
@@ -347,6 +349,7 @@
fileExtension: getFileExtension(
fixSuggestion?.replacements?.[0].path ?? ''
),
+ commentId: this.comment?.id ?? '',
});
if (res?.ok) {
this.getNavigation().setUrl(
@@ -375,6 +378,7 @@
if (!this.suggestion) return;
this.reporting.reportInteraction(Interaction.GENERATE_SUGGESTION_ADDED, {
uuid: this.uuid,
+ commentId: this.comment?.id ?? '',
});
fire(this, 'add-generated-suggestion', {code: this.suggestion});
}
