Merge "Support --header-icon-width and -height instead of just -size"
diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt
index 264ce73..7a042ba 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -1014,7 +1014,7 @@
inline comment ("Yeah, I see why, let me try again.").
[[security-fixes]]
--- Security Fixes
+== Security Fixes
If a security vulnerability is discovered you normally want to have an
embargo about it until fixed releases have been made available. This
diff --git a/java/com/google/gerrit/server/StarredChangesUtil.java b/java/com/google/gerrit/server/StarredChangesUtil.java
index fd08fa8..8f413f9 100644
--- a/java/com/google/gerrit/server/StarredChangesUtil.java
+++ b/java/com/google/gerrit/server/StarredChangesUtil.java
@@ -23,7 +23,6 @@
import com.google.common.base.CharMatcher;
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
-import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
@@ -39,21 +38,17 @@
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
-import com.google.gerrit.server.project.NoSuchChangeException;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
-import java.util.List;
import java.util.NavigableSet;
+import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import org.eclipse.jgit.lib.BatchRefUpdate;
@@ -166,20 +161,17 @@
private final GitReferenceUpdated gitRefUpdated;
private final AllUsersName allUsers;
private final Provider<PersonIdent> serverIdent;
- private final Provider<InternalChangeQuery> queryProvider;
@Inject
StarredChangesUtil(
GitRepositoryManager repoManager,
GitReferenceUpdated gitRefUpdated,
AllUsersName allUsers,
- @GerritPersonIdent Provider<PersonIdent> serverIdent,
- Provider<InternalChangeQuery> queryProvider) {
+ @GerritPersonIdent Provider<PersonIdent> serverIdent) {
this.repoManager = repoManager;
this.gitRefUpdated = gitRefUpdated;
this.allUsers = allUsers;
this.serverIdent = serverIdent;
- this.queryProvider = queryProvider;
}
public NavigableSet<String> getLabels(Account.Id accountId, Change.Id changeId) {
@@ -238,7 +230,7 @@
batchUpdate.setAllowNonFastForwards(true);
batchUpdate.setRefLogIdent(serverIdent.get());
batchUpdate.setRefLogMessage("Unstar change " + changeId.get(), true);
- for (Account.Id accountId : byChangeFromIndex(changeId).keySet()) {
+ for (Account.Id accountId : getStars(repo, changeId)) {
String refName = RefNames.refsStarredChanges(changeId, accountId);
Ref ref = repo.getRefDatabase().exactRef(refName);
if (ref != null) {
@@ -264,12 +256,7 @@
public ImmutableMap<Account.Id, StarRef> byChange(Change.Id changeId) {
try (Repository repo = repoManager.openRepository(allUsers)) {
ImmutableMap.Builder<Account.Id, StarRef> builder = ImmutableMap.builder();
- for (String refPart : getRefNames(repo, RefNames.refsStarredChangesPrefix(changeId))) {
- Integer id = Ints.tryParse(refPart);
- if (id == null) {
- continue;
- }
- Account.Id accountId = Account.id(id);
+ for (Account.Id accountId : getStars(repo, changeId)) {
builder.put(accountId, readLabels(repo, RefNames.refsStarredChanges(changeId, accountId)));
}
return builder.build();
@@ -308,22 +295,15 @@
}
}
- public ImmutableListMultimap<Account.Id, String> byChangeFromIndex(Change.Id changeId) {
- List<ChangeData> changeData =
- queryProvider
- .get()
- .setRequestedFields(ChangeField.CHANGE_ID_SPEC, ChangeField.STAR_SPEC)
- .byLegacyChangeId(changeId);
- if (changeData.size() != 1) {
- throw new NoSuchChangeException(changeId);
- }
- return changeData.get(0).stars();
- }
-
- private static Set<String> getRefNames(Repository repo, String prefix) throws IOException {
- RefDatabase refDb = repo.getRefDatabase();
+ private static Set<Account.Id> getStars(Repository allUsers, Change.Id changeId)
+ throws IOException {
+ String prefix = RefNames.refsStarredChangesPrefix(changeId);
+ RefDatabase refDb = allUsers.getRefDatabase();
return refDb.getRefsByPrefix(prefix).stream()
.map(r -> r.getName().substring(prefix.length()))
+ .map(refPart -> Ints.tryParse(refPart))
+ .filter(Objects::nonNull)
+ .map(id -> Account.id(id))
.collect(toSet());
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index ec1fcad..78615bf 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -334,7 +334,7 @@
* Map from {@link com.google.gerrit.entities.Account.Id} to the tip of the edit ref for this
* change and a given user.
*/
- private Table<Account.Id, PatchSet.Id, ObjectId> editsByUser;
+ private Table<Account.Id, PatchSet.Id, Ref> editRefsByUser;
private Set<Account.Id> reviewedBy;
/**
@@ -1097,8 +1097,8 @@
return editRefs().rowKeySet();
}
- public Table<Account.Id, PatchSet.Id, ObjectId> editRefs() {
- if (editsByUser == null) {
+ public Table<Account.Id, PatchSet.Id, Ref> editRefs() {
+ if (editRefsByUser == null) {
if (!lazyload()) {
return HashBasedTable.create();
}
@@ -1106,7 +1106,7 @@
if (c == null) {
return HashBasedTable.create();
}
- editsByUser = HashBasedTable.create();
+ editRefsByUser = HashBasedTable.create();
Change.Id id = requireNonNull(change.getId());
try (Repository repo = repoManager.openRepository(project())) {
for (Ref ref : repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_USERS)) {
@@ -1117,7 +1117,7 @@
if (id.equals(ps.changeId())) {
Account.Id accountId = Account.Id.fromRef(ref.getName());
if (accountId != null) {
- editsByUser.put(accountId, ps, ref.getObjectId());
+ editRefsByUser.put(accountId, ps, ref);
}
}
}
@@ -1125,7 +1125,7 @@
throw new StorageException(e);
}
}
- return editsByUser;
+ return editRefsByUser;
}
public Set<Account.Id> draftsByUser() {
@@ -1273,13 +1273,13 @@
}
ImmutableSetMultimap.Builder<NameKey, RefState> result = ImmutableSetMultimap.builder();
- for (Table.Cell<Account.Id, PatchSet.Id, ObjectId> edit : editRefs().cellSet()) {
+ for (Table.Cell<Account.Id, PatchSet.Id, Ref> edit : editRefs().cellSet()) {
result.put(
project,
RefState.create(
RefNames.refsEdit(
edit.getRowKey(), edit.getColumnKey().changeId(), edit.getColumnKey()),
- edit.getValue()));
+ edit.getValue().getObjectId()));
}
// TODO: instantiating the notes is too much. We don't want to parse NoteDb, we just want the
@@ -1309,21 +1309,6 @@
.forEach(r -> draftsByUser.put(Account.Id.fromRef(r.ref()), r.id()));
}
}
- if (editsByUser == null) {
- // Recover edit refs as well. Edits are represented as refs in the repository.
- // ChangeData exposes #editsByUser which just provides a Set of Account.Ids of users who
- // have edits on this change. Recovering this list from RefStates makes it available even
- // on ChangeData instances retrieved from the index.
- editsByUser = HashBasedTable.create();
- if (refStates.containsKey(project())) {
- refStates.get(project()).stream()
- .filter(r -> RefNames.isRefsEdit(r.ref()))
- .forEach(
- r ->
- editsByUser.put(
- Account.Id.fromRef(r.ref()), PatchSet.Id.fromEditRef(r.ref()), r.id()));
- }
- }
}
public ImmutableList<byte[]> getRefStatePatterns() {
diff --git a/java/com/google/gerrit/testing/TestChanges.java b/java/com/google/gerrit/testing/TestChanges.java
index 4a97bc5..c8f89cf 100644
--- a/java/com/google/gerrit/testing/TestChanges.java
+++ b/java/com/google/gerrit/testing/TestChanges.java
@@ -32,6 +32,7 @@
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Injector;
import java.time.ZoneId;
+import java.util.Optional;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.ObjectId;
@@ -77,15 +78,20 @@
}
public static ChangeUpdate newUpdate(
- Injector injector, Change c, CurrentUser user, boolean shouldExist) throws Exception {
+ Injector injector, Change c, Optional<CurrentUser> user, boolean shouldExist)
+ throws Exception {
injector =
injector.createChildInjector(
new FactoryModule() {
@Override
public void configure() {
- bind(CurrentUser.class).toInstance(user);
+ if (user.isPresent()) {
+ // user may be already bound in injector
+ bind(CurrentUser.class).toInstance(user.get());
+ }
}
});
+ CurrentUser currentUser = injector.getProvider(CurrentUser.class).get();
ChangeUpdate update =
injector
.getInstance(ChangeUpdate.Factory.class)
@@ -93,7 +99,7 @@
new ChangeNotes(
injector.getInstance(AbstractChangeNotes.Args.class), c, shouldExist, null)
.load(),
- user,
+ currentUser,
TimeUtil.now(),
Ordering.natural());
@@ -109,7 +115,9 @@
try (Repository repo = repoManager.openRepository(c.getProject());
TestRepository<Repository> tr = new TestRepository<>(repo)) {
PersonIdent ident =
- user.asIdentifiedUser().newCommitterIdent(update.getWhen(), ZoneId.systemDefault());
+ currentUser
+ .asIdentifiedUser()
+ .newCommitterIdent(update.getWhen(), ZoneId.systemDefault());
TestRepository<Repository>.CommitBuilder cb =
tr.commit()
.author(ident)
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index 37d8468..be8f1f9 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -76,6 +76,7 @@
import com.google.inject.TypeLiteral;
import java.time.Instant;
import java.time.ZoneId;
+import java.util.Optional;
import java.util.concurrent.ExecutorService;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
@@ -276,7 +277,7 @@
protected ChangeUpdate newUpdate(
Injector injector, Change c, CurrentUser user, boolean shouldExist) throws Exception {
- ChangeUpdate update = TestChanges.newUpdate(injector, c, user, shouldExist);
+ ChangeUpdate update = TestChanges.newUpdate(injector, c, Optional.of(user), shouldExist);
update.setPatchSetId(c.currentPatchSetId());
update.setAllowWriteToNewRef(true);
return update;
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index b1a9a49..8919e04 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -122,6 +122,7 @@
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.index.change.IndexedChangeQuery;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.Sequences;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
@@ -135,6 +136,7 @@
import com.google.gerrit.testing.GerritServerTests;
import com.google.gerrit.testing.InMemoryRepositoryManager;
import com.google.gerrit.testing.InMemoryRepositoryManager.Repo;
+import com.google.gerrit.testing.TestChanges;
import com.google.gerrit.testing.TestTimeUtil;
import com.google.inject.Inject;
import com.google.inject.Injector;
@@ -158,7 +160,6 @@
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.util.SystemReader;
@@ -3230,28 +3231,27 @@
@Test
public void reindexIfStale() throws Exception {
- Account.Id user = createAccount("user");
Project.NameKey project = Project.nameKey("repo");
TestRepository<Repo> repo = createProject(project.get());
Change change = insert(repo, newChange(repo));
String changeId = change.getKey().get();
- ChangeNotes notes = notesFactory.create(change.getProject(), change.getId());
- PatchSet ps = psUtil.get(notes, change.currentPatchSetId());
- requestContext.setContext(newRequestContext(user));
- gApi.changes().id(changeId).edit().create();
- assertQuery("has:edit", change);
+ Account.Id anotherUser = createAccount("another-user");
+ requestContext.setContext(newRequestContext(anotherUser));
+ gApi.changes().id(changeId).addReviewer(anotherUser.toString());
+
+ assertQuery("reviewer:self", change);
assertThat(indexer.reindexIfStale(project, change.getId()).get()).isFalse();
- // Delete edit ref behind index's back.
- RefUpdate ru = repo.getRepository().updateRef(RefNames.refsEdit(user, change.getId(), ps.id()));
- ru.setForceUpdate(true);
- assertThat(ru.delete()).isEqualTo(RefUpdate.Result.FORCED);
+ // Remove reviewer behind index's back.
+ ChangeUpdate update = newUpdate(change);
+ update.removeReviewer(anotherUser);
+ update.commit();
// Index is stale.
- assertQuery("has:edit", change);
+ assertQuery("reviewer:self", change);
assertThat(indexer.reindexIfStale(project, change.getId()).get()).isTrue();
- assertQuery("has:edit");
+ assertQuery("reviewer:self");
}
@Test
@@ -4413,4 +4413,12 @@
protected Schema<ChangeData> getSchema() {
return indexes.getSearchIndex().getSchema();
}
+
+ protected ChangeUpdate newUpdate(Change c) throws Exception {
+ ChangeUpdate update =
+ TestChanges.newUpdate(injector, c, Optional.empty(), /* shouldExist= */ true);
+ update.setPatchSetId(c.currentPatchSetId());
+ update.setAllowWriteToNewRef(true);
+ return update;
+ }
}
diff --git a/modules/jgit b/modules/jgit
index e74f385..801a56b 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit e74f3855ad9d54c986d60b0b2ea4c223d52b2cd1
+Subproject commit 801a56b48a7fe3c6e171073211cc62194184fe79
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
index 27fe620..d3562f2 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.ts
@@ -16,7 +16,11 @@
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, query, property, state} from 'lit/decorators.js';
import {assertIsDefined} from '../../../utils/common-util';
-import {AdminViewState} from '../../../models/views/admin';
+import {
+ AdminChildView,
+ AdminViewState,
+ createAdminUrl,
+} from '../../../models/views/admin';
import {createGroupUrl} from '../../../models/views/group';
import {whenVisible} from '../../../utils/dom-util';
import {modalStyles} from '../../../styles/gr-modal-styles';
@@ -29,8 +33,6 @@
@customElement('gr-admin-group-list')
export class GrAdminGroupList extends LitElement {
- readonly path = '/admin/groups';
-
@query('#createModal') private createModal?: HTMLDialogElement;
@query('#createNewModal') private createNewModal?: GrCreateGroupDialog;
@@ -87,7 +89,7 @@
.itemsPerPage=${this.groupsPerPage}
.loading=${this.loading}
.offset=${this.offset}
- .path=${this.path}
+ .path=${createAdminUrl({adminView: AdminChildView.GROUPS})}
@create-clicked=${() => this.handleCreateClicked()}
>
<table id="list" class="genericList">
@@ -167,18 +169,14 @@
*
* private but used in test
*/
- maybeOpenCreateModal(params?: AdminViewState) {
+ async maybeOpenCreateModal(params?: AdminViewState) {
if (params?.openCreateModal) {
- assertIsDefined(this.createModal, 'createModal');
- this.createModal.showModal();
+ await this.updateComplete;
+ if (!this.createModal?.open) this.createModal?.showModal();
}
}
- /**
- * Generates groups link (/admin/groups/<uuid>)
- *
- * private but used in test
- */
+ // private but used in test
computeGroupUrl(encodedId: string) {
const groupId = decodeURIComponent(encodedId) as GroupId;
return createGroupUrl({groupId});
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.ts b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.ts
index 1b128aa..e9b7ea0 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.ts
@@ -120,17 +120,17 @@
assert.equal(element.groups.slice(0, SHOWN_ITEMS_COUNT).length, 25);
});
- test('maybeOpenCreateModal', () => {
+ test('maybeOpenCreateModal', async () => {
const modalOpen = sinon.stub(
queryAndAssert<HTMLDialogElement>(element, '#createModal'),
'showModal'
);
- element.maybeOpenCreateModal();
+ await element.maybeOpenCreateModal();
assert.isFalse(modalOpen.called);
- element.maybeOpenCreateModal(undefined);
+ await element.maybeOpenCreateModal(undefined);
assert.isFalse(modalOpen.called);
value.openCreateModal = true;
- element.maybeOpenCreateModal(value);
+ await element.maybeOpenCreateModal(value);
assert.isTrue(modalOpen.called);
});
});
diff --git a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
index 4808d00..8d5689c 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog.ts
@@ -6,9 +6,8 @@
import '@polymer/iron-input/iron-input';
import '../../../styles/gr-form-styles';
import '../../../styles/shared-styles';
-import {encodeURL, getBaseUrl} from '../../../utils/url-util';
import {page} from '../../../utils/page-wrapper-utils';
-import {GroupName} from '../../../types/common';
+import {GroupId, GroupName} from '../../../types/common';
import {getAppContext} from '../../../services/app-context';
import {formStyles} from '../../../styles/gr-form-styles';
import {sharedStyles} from '../../../styles/shared-styles';
@@ -16,6 +15,7 @@
import {customElement, query, property} from 'lit/decorators.js';
import {BindValueChangeEvent} from '../../../types/events';
import {fireEvent} from '../../../utils/event-util';
+import {createGroupUrl} from '../../../models/views/group';
declare global {
interface HTMLElementTagNameMap {
@@ -75,10 +75,6 @@
fireEvent(this, 'has-new-group-name');
}
- private computeGroupUrl(groupId: string) {
- return getBaseUrl() + '/admin/groups/' + encodeURL(groupId, true);
- }
-
override focus() {
this.input.focus();
}
@@ -89,7 +85,9 @@
if (groupRegistered.status !== 201) return;
return this.restApiService.getGroupConfig(name).then(group => {
if (!group) return;
- page.show(this.computeGroupUrl(String(group.group_id!)));
+ const groupId = String(group.group_id!) as GroupId;
+ // TODO: Use navigation service instead of `page.show()` directly.
+ page.show(createGroupUrl({groupId}));
});
});
}
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
index a90abbd..bb59ccc 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
@@ -7,7 +7,6 @@
import '../../shared/gr-autocomplete/gr-autocomplete';
import '../../shared/gr-button/gr-button';
import '../../shared/gr-select/gr-select';
-import {encodeURL, getBaseUrl} from '../../../utils/url-util';
import {page} from '../../../utils/page-wrapper-utils';
import {
BranchName,
@@ -24,6 +23,7 @@
import {customElement, query, property, state} from 'lit/decorators.js';
import {fireEvent} from '../../../utils/event-util';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
+import {createRepoUrl} from '../../../models/views/repo';
declare global {
interface HTMLElementTagNameMap {
@@ -183,10 +183,6 @@
`;
}
- _computeRepoUrl(repoName: string) {
- return getBaseUrl() + '/admin/repos/' + encodeURL(repoName, true);
- }
-
override focus() {
this.input?.focus();
}
@@ -199,7 +195,8 @@
);
if (repoRegistered.status === 201) {
this.repoCreated = true;
- page.show(this._computeRepoUrl(this.repoConfig.name));
+ // TODO: Use navigation service instead of `page.show()` directly.
+ page.show(createRepoUrl({repo: this.repoConfig.name}));
}
return repoRegistered;
}
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index 389e7c4..52e0b3f 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../gr-access-section/gr-access-section';
-import {encodeURL, getBaseUrl, singleDecodeURL} from '../../../utils/url-util';
+import {singleDecodeURL} from '../../../utils/url-util';
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {toSortedPermissionsArray} from '../../../utils/access-util';
import {
@@ -44,6 +44,7 @@
import {resolve} from '../../../models/dependency';
import {createChangeUrl} from '../../../models/views/change';
import {throwingErrorCallback} from '../../shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper';
+import {createRepoUrl, RepoDetailView} from '../../../models/views/repo';
const NOTHING_TO_SAVE = 'No changes to save.';
@@ -726,10 +727,10 @@
computeParentHref() {
if (!this.inheritsFrom?.name) return '';
- return `${getBaseUrl()}/admin/repos/${encodeURL(
- this.inheritsFrom.name,
- true
- )},access`;
+ return createRepoUrl({
+ repo: this.inheritsFrom.name,
+ detail: RepoDetailView.ACCESS,
+ });
}
private handleEditInheritFromTextChanged(e: ValueChangedEvent) {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
index 3b193a6..632ec4c 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
@@ -11,7 +11,6 @@
import '../../shared/gr-list-view/gr-list-view';
import '../gr-create-pointer-dialog/gr-create-pointer-dialog';
import '../gr-confirm-delete-item-dialog/gr-confirm-delete-item-dialog';
-import {encodeURL} from '../../../utils/url-util';
import {GrCreatePointerDialog} from '../gr-create-pointer-dialog/gr-create-pointer-dialog';
import {
BranchInfo,
@@ -29,12 +28,16 @@
import {formStyles} from '../../../styles/gr-form-styles';
import {tableStyles} from '../../../styles/gr-table-styles';
import {sharedStyles} from '../../../styles/shared-styles';
-import {LitElement, PropertyValues, css, html} from 'lit';
+import {LitElement, PropertyValues, css, html, nothing} from 'lit';
import {customElement, query, property, state} from 'lit/decorators.js';
import {BindValueChangeEvent} from '../../../types/events';
import {assertIsDefined} from '../../../utils/common-util';
import {ifDefined} from 'lit/directives/if-defined.js';
-import {RepoDetailView, RepoViewState} from '../../../models/views/repo';
+import {
+ createRepoUrl,
+ RepoDetailView,
+ RepoViewState,
+} from '../../../models/views/repo';
import {modalStyles} from '../../../styles/gr-modal-styles';
const PGP_START = '-----BEGIN PGP SIGNATURE-----';
@@ -139,6 +142,7 @@
}
override render() {
+ if (!this.repo || !this.detailType) return nothing;
return html`
<gr-list-view
.createNew=${this.loggedIn}
@@ -147,7 +151,7 @@
.items=${this.items}
.loading=${this.loading}
.offset=${this.offset}
- .path=${this.getPath(this.repo, this.detailType)}
+ .path=${createRepoUrl({repo: this.repo, detail: this.detailType})}
@create-clicked=${() => {
this.handleCreateClicked();
}}
@@ -441,10 +445,6 @@
return Promise.reject(new Error('unknown detail type'));
}
- private getPath(repo?: RepoName, detailType?: RepoDetailView) {
- return `/admin/repos/${encodeURL(repo ?? '', false)},${detailType}`;
- }
-
private computeWeblink(repo: ProjectInfo | BranchInfo | TagInfo) {
if (!repo.web_links) return [];
const webLinks = repo.web_links;
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
index 28fc751..ec1bb82 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list_test.ts
@@ -2434,6 +2434,18 @@
});
suite('create new', () => {
+ setup(async () => {
+ stubRestApi('getRepoBranches').resolves(createBranchesList(3));
+
+ element.params = {
+ view: GerritView.REPO,
+ repo: 'test' as RepoName,
+ detail: RepoDetailView.BRANCHES,
+ };
+ await element.paramsChanged();
+ await element.updateComplete;
+ });
+
test('handleCreateClicked called when create-click fired', () => {
const handleCreateClickedStub = sinon.stub(
element,
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
index c6c6efd..2fd7e79 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list.ts
@@ -6,23 +6,23 @@
import '../../shared/gr-dialog/gr-dialog';
import '../../shared/gr-list-view/gr-list-view';
import '../gr-create-repo-dialog/gr-create-repo-dialog';
-import {
- RepoName,
- ProjectInfoWithName,
- WebLinkInfo,
-} from '../../../types/common';
+import {ProjectInfoWithName, WebLinkInfo} from '../../../types/common';
import {GrCreateRepoDialog} from '../gr-create-repo-dialog/gr-create-repo-dialog';
import {RepoState, SHOWN_ITEMS_COUNT} from '../../../constants/constants';
import {fireTitleChange} from '../../../utils/event-util';
import {getAppContext} from '../../../services/app-context';
-import {encodeURL, getBaseUrl} from '../../../utils/url-util';
import {tableStyles} from '../../../styles/gr-table-styles';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, property, query, state} from 'lit/decorators.js';
-import {AdminViewState} from '../../../models/views/admin';
+import {
+ AdminChildView,
+ AdminViewState,
+ createAdminUrl,
+} from '../../../models/views/admin';
import {createSearchUrl} from '../../../models/views/search';
import {modalStyles} from '../../../styles/gr-modal-styles';
+import {createRepoUrl} from '../../../models/views/repo';
declare global {
interface HTMLElementTagNameMap {
@@ -32,8 +32,6 @@
@customElement('gr-repo-list')
export class GrRepoList extends LitElement {
- readonly path = '/admin/repos';
-
@query('#createModal') private createModal?: HTMLDialogElement;
@query('#createNewModal') private createNewModal?: GrCreateRepoDialog;
@@ -103,7 +101,7 @@
.items=${this.repos}
.loading=${this.loading}
.offset=${this.offset}
- .path=${this.path}
+ .path=${createAdminUrl({adminView: AdminChildView.REPOS})}
@create-clicked=${() => this.handleCreateClicked()}
>
<table id="list" class="genericList">
@@ -157,11 +155,11 @@
return html`
<tr class="table">
<td class="name">
- <a href=${this.computeRepoUrl(item.name)}>${item.name}</a>
+ <a href=${createRepoUrl({repo: item.name})}>${item.name}</a>
</td>
<td class="repositoryBrowser">${this.renderWebLinks(item)}</td>
<td class="changesLink">
- <a href=${this.computeChangesLink(item.name)}>view all</a>
+ <a href=${createSearchUrl({repo: item.name})}>view all</a>
</td>
<td class="readOnly">
${item.state === RepoState.READ_ONLY ? 'Y' : ''}
@@ -210,14 +208,6 @@
}
}
- private computeRepoUrl(name: string) {
- return `${getBaseUrl()}${this.path}/${encodeURL(name, true)}`;
- }
-
- private computeChangesLink(name: string) {
- return createSearchUrl({repo: name as RepoName});
- }
-
private async getCreateRepoCapability() {
const account = await this.restApiService.getAccount();
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
index 7abfce9..faaee0b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
@@ -282,12 +282,14 @@
// private but used in test
handleNextPage() {
if (!this.nextArrow || !this.changesPerPage) return;
+ // TODO: Use navigation service instead of `page.show()` directly.
page.show(this.computeNavLink(1));
}
// private but used in test
handlePreviousPage() {
if (!this.prevArrow || !this.changesPerPage) return;
+ // TODO: Use navigation service instead of `page.show()` directly.
page.show(this.computeNavLink(-1));
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 6caa000..1b914bf 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -56,6 +56,7 @@
RepoViewState,
} from '../../../models/views/repo';
import {
+ createGroupUrl,
GroupDetailView,
GroupViewModel,
GroupViewState,
@@ -1067,7 +1068,8 @@
}
handleGroupInfoRoute(ctx: PageContext) {
- this.redirect('/admin/groups/' + encodeURIComponent(ctx.params[0]));
+ const groupId = ctx.params[0] as GroupId;
+ this.redirect(createGroupUrl({groupId}));
}
handleGroupSelfRedirectRoute(_: PageContext) {
@@ -1151,6 +1153,8 @@
}
}
+ // TODO: Change the route pattern to match `repo` and `detailView`
+ // separately, and then use `createRepoUrl()` here.
this.redirect(`/admin/repos/${params}`);
}
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 4860c87..e959e05 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -73,6 +73,7 @@
import {createDashboardUrl} from '../models/views/dashboard';
import {userModelToken} from '../models/user/user-model';
import {modalStyles} from '../styles/gr-modal-styles';
+import {AdminChildView, createAdminUrl} from '../models/views/admin';
interface ErrorInfo {
text: string;
@@ -209,6 +210,16 @@
createSearchUrl({query: 'is:watched is:open'})
)
);
+ this.shortcuts.addAbstract(Shortcut.GO_TO_REPOS, () =>
+ this.getNavigation().setUrl(
+ createAdminUrl({adminView: AdminChildView.REPOS})
+ )
+ );
+ this.shortcuts.addAbstract(Shortcut.GO_TO_GROUPS, () =>
+ this.getNavigation().setUrl(
+ createAdminUrl({adminView: AdminChildView.GROUPS})
+ )
+ );
subscribe(
this,
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 e5720b84..090dfef 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -130,6 +130,9 @@
@query('#confirmDeleteModal')
confirmDeleteModal?: HTMLDialogElement;
+ @query('#confirmDeleteCommentDialog')
+ confirmDeleteDialog?: GrConfirmDeleteCommentDialog;
+
@property({type: Object})
comment?: Comment;
@@ -200,9 +203,6 @@
unresolved = true;
@property({type: Boolean})
- showConfirmDeleteModal = false;
-
- @property({type: Boolean})
unableToSave = false;
@property({type: Boolean, attribute: 'show-patchset'})
@@ -940,11 +940,10 @@
}
private renderConfirmDialog() {
- if (!this.showConfirmDeleteModal) return;
return html`
<dialog id="confirmDeleteModal" tabindex="-1">
<gr-confirm-delete-comment-dialog
- id="confirmDeleteComment"
+ id="confirmDeleteCommentDialog"
@confirm=${this.handleConfirmDeleteComment}
@cancel=${this.closeDeleteCommentModal}
>
@@ -1264,15 +1263,14 @@
}
}
- private async openDeleteCommentModal() {
- this.showConfirmDeleteModal = true;
- await this.updateComplete;
- await this.confirmDeleteModal?.showModal();
+ private openDeleteCommentModal() {
+ this.confirmDeleteModal?.showModal();
+ whenVisible(this.confirmDeleteDialog!, () => {
+ this.confirmDeleteDialog!.resetFocus();
+ });
}
private closeDeleteCommentModal() {
- this.showConfirmDeleteModal = false;
- this.confirmDeleteModal?.remove();
this.confirmDeleteModal?.close();
}
@@ -1282,10 +1280,7 @@
*/
// private, but visible for testing
async handleConfirmDeleteComment() {
- const dialog = this.confirmDeleteModal?.querySelector(
- '#confirmDeleteComment'
- ) as GrConfirmDeleteCommentDialog | null;
- if (!dialog || !dialog.message) {
+ if (!this.confirmDeleteDialog || !this.confirmDeleteDialog.message) {
throw new Error('missing confirm delete dialog');
}
assertIsDefined(this.changeNum, 'changeNum');
@@ -1294,7 +1289,7 @@
await this.getCommentsModel().deleteComment(
this.changeNum,
this.comment,
- dialog.message
+ this.confirmDeleteDialog.message
);
this.closeDeleteCommentModal();
}
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 5b191c8..ec9c875 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -127,6 +127,10 @@
</div>
</div>
</gr-endpoint-decorator>
+ <dialog id="confirmDeleteModal" tabindex="-1">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
+ </gr-confirm-delete-comment-dialog>
+ </dialog>
`
);
});
@@ -166,6 +170,10 @@
</div>
</div>
</gr-endpoint-decorator>
+ <dialog id="confirmDeleteModal" tabindex="-1">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
+ </gr-confirm-delete-comment-dialog>
+ </dialog>
`
);
});
@@ -238,6 +246,10 @@
</div>
</div>
</gr-endpoint-decorator>
+ <dialog id="confirmDeleteModal" tabindex="-1">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
+ </gr-confirm-delete-comment-dialog>
+ </dialog>
`
);
});
@@ -336,6 +348,10 @@
</div>
</div>
</gr-endpoint-decorator>
+ <dialog id="confirmDeleteModal" tabindex="-1">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
+ </gr-confirm-delete-comment-dialog>
+ </dialog>
`
);
});
@@ -421,6 +437,10 @@
</div>
</div>
</gr-endpoint-decorator>
+ <dialog id="confirmDeleteModal" tabindex="-1">
+ <gr-confirm-delete-comment-dialog id="confirmDeleteCommentDialog">
+ </gr-confirm-delete-comment-dialog>
+ </dialog>
`
);
});
@@ -503,7 +523,7 @@
assertIsDefined(element.confirmDeleteModal, 'confirmDeleteModal');
const dialog = queryAndAssert<GrConfirmDeleteCommentDialog>(
element.confirmDeleteModal,
- '#confirmDeleteComment'
+ '#confirmDeleteCommentDialog'
);
dialog.message = 'removal reason';
await element.updateComplete;
diff --git a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.ts b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.ts
index b6512b3..285a41a 100644
--- a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.ts
+++ b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.ts
@@ -75,6 +75,7 @@
override render() {
return html` <gr-dialog
confirm-label="Delete"
+ ?disabled=${this.message === ''}
@confirm=${this.handleConfirmTap}
@cancel=${this.handleCancelTap}
>
diff --git a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_test.ts b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_test.ts
index bd84ac4..b7551c1 100644
--- a/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog_test.ts
@@ -7,6 +7,7 @@
import {fixture, html, assert} from '@open-wc/testing';
import {GrConfirmDeleteCommentDialog} from './gr-confirm-delete-comment-dialog';
import './gr-confirm-delete-comment-dialog';
+import {GrDialog} from '../gr-dialog/gr-dialog';
suite('gr-confirm-delete-comment-dialog tests', () => {
let element: GrConfirmDeleteCommentDialog;
@@ -17,7 +18,10 @@
);
});
- test('render', () => {
+ test('render', async () => {
+ element.message = 'Just cause';
+ await element.updateComplete;
+
// prettier and shadowDom string disagree about wrapping in <p> tag.
assert.shadowDom.equal(
element,
@@ -43,4 +47,13 @@
`
);
});
+
+ test('dialog is disabled when message is empty', async () => {
+ element.message = '';
+ await element.updateComplete;
+
+ assert.isTrue(
+ (element.shadowRoot!.querySelector('gr-dialog') as GrDialog).disabled
+ );
+ });
});
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
index fd43869..449c041 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.ts
@@ -177,9 +177,11 @@
() => {
if (!this.isConnected || !this.path) return;
if (filter) {
+ // TODO: Use navigation service instead of `page.show()` directly.
page.show(`${this.path}/q/filter:${encodeURL(filter, false)}`);
return;
}
+ // TODO: Use navigation service instead of `page.show()` directly.
page.show(this.path);
},
REQUEST_DEBOUNCE_INTERVAL_MS
diff --git a/polygerrit-ui/app/models/views/admin.ts b/polygerrit-ui/app/models/views/admin.ts
index 2ad95a2..3380637 100644
--- a/polygerrit-ui/app/models/views/admin.ts
+++ b/polygerrit-ui/app/models/views/admin.ts
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import {GerritView} from '../../services/router/router-model';
+import {getBaseUrl} from '../../utils/url-util';
import {define} from '../dependency';
import {Model} from '../model';
import {ViewState} from './base';
@@ -21,6 +22,17 @@
offset?: number | string;
}
+export function createAdminUrl(state: Omit<AdminViewState, 'view'>) {
+ switch (state.adminView) {
+ case AdminChildView.REPOS:
+ return `${getBaseUrl()}/admin/repos`;
+ case AdminChildView.GROUPS:
+ return `${getBaseUrl()}/admin/groups`;
+ case AdminChildView.PLUGINS:
+ return `${getBaseUrl()}/admin/plugins`;
+ }
+}
+
export const adminViewModelToken = define<AdminViewModel>('admin-view-model');
export class AdminViewModel extends Model<AdminViewState | undefined> {
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
index da61c41..9ca2213 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
@@ -35,6 +35,8 @@
GO_TO_MERGED_CHANGES = 'GO_TO_MERGED_CHANGES',
GO_TO_ABANDONED_CHANGES = 'GO_TO_ABANDONED_CHANGES',
GO_TO_WATCHED_CHANGES = 'GO_TO_WATCHED_CHANGES',
+ GO_TO_REPOS = 'GO_TO_REPOS',
+ GO_TO_GROUPS = 'GO_TO_GROUPS',
CURSOR_NEXT_CHANGE = 'CURSOR_NEXT_CHANGE',
CURSOR_PREV_CHANGE = 'CURSOR_PREV_CHANGE',
@@ -167,6 +169,16 @@
{key: 'w', combo: ComboKey.G}
);
describe(
+ Shortcut.GO_TO_REPOS,
+ ShortcutSection.EVERYWHERE,
+ 'Go to Repositories',
+ {key: 'r', combo: ComboKey.G}
+ );
+ describe(Shortcut.GO_TO_GROUPS, ShortcutSection.EVERYWHERE, 'Go to Groups', {
+ key: 'g',
+ combo: ComboKey.G,
+ });
+ describe(
Shortcut.TOGGLE_CHECKBOX,
ShortcutSection.ACTIONS,
'Toggle checkbox',
diff --git a/polygerrit-ui/app/styles/gr-menu-page-styles.ts b/polygerrit-ui/app/styles/gr-menu-page-styles.ts
index 7a44e79..17b7461 100644
--- a/polygerrit-ui/app/styles/gr-menu-page-styles.ts
+++ b/polygerrit-ui/app/styles/gr-menu-page-styles.ts
@@ -32,7 +32,7 @@
color: var(--deemphasized-text-color);
padding: var(--spacing-l);
}
- @media only screen and (max-width: 67em) {
+ @media only screen and (max-width: 70em) {
.main {
margin: var(--spacing-xxl) 0 var(--spacing-xxl) 15em;
}
diff --git a/polygerrit-ui/app/utils/admin-nav-util.ts b/polygerrit-ui/app/utils/admin-nav-util.ts
index 6f81d57..7916799 100644
--- a/polygerrit-ui/app/utils/admin-nav-util.ts
+++ b/polygerrit-ui/app/utils/admin-nav-util.ts
@@ -12,7 +12,7 @@
import {hasOwnProperty} from './common-util';
import {GerritView} from '../services/router/router-model';
import {MenuLink} from '../api/admin';
-import {AdminChildView} from '../models/views/admin';
+import {AdminChildView, createAdminUrl} from '../models/views/admin';
import {createGroupUrl, GroupDetailView} from '../models/views/group';
import {createRepoUrl, RepoDetailView} from '../models/views/repo';
@@ -20,7 +20,7 @@
{
name: 'Repositories',
noBaseUrl: true,
- url: '/admin/repos',
+ url: createAdminUrl({adminView: AdminChildView.REPOS}),
view: 'gr-repo-list' as GerritView,
viewableToAll: true,
},
@@ -28,7 +28,7 @@
name: 'Groups',
section: 'Groups',
noBaseUrl: true,
- url: '/admin/groups',
+ url: createAdminUrl({adminView: AdminChildView.GROUPS}),
view: 'gr-admin-group-list' as GerritView,
},
{
@@ -36,7 +36,7 @@
capability: 'viewPlugins',
section: 'Plugins',
noBaseUrl: true,
- url: '/admin/plugins',
+ url: createAdminUrl({adminView: AdminChildView.PLUGINS}),
view: 'gr-plugin-list' as GerritView,
},
];
