Merge "Allow to bind a different implementation for creating reflog identities"
diff --git a/Documentation/user-review-ui.txt b/Documentation/user-review-ui.txt
index 73668d7..780d3ec 100644
--- a/Documentation/user-review-ui.txt
+++ b/Documentation/user-review-ui.txt
@@ -88,7 +88,7 @@
image::images/user-review-ui-change-metadata.png[width=600, link="images/user-review-ui-change-metadata.png"]
-- [[owner]]Owner/Uploader/Author/Committer
+- [[owner]]Owner/Uploader/Author/Committer:
+
Owner is the person who created the change
+
@@ -170,11 +170,11 @@
The SHA of the commit corresponding to the merged change on the destination
branch.
-- [[revert-created-as]]Revert (Created|Submitted) As
+- [[revert-created-as]]Revert (Created|Submitted) As:
+
Points to the revert change, if one was created.
-- [[cherry-pick-of]]Cherry-pick of
+- [[cherry-pick-of]]Cherry-pick of:
+
If the change was created as cherry-pick of some other change to a different
branch, points to the original change.
@@ -207,7 +207,7 @@
link:config-submit-requirements.html[Submit Requirement Configuration] page.
[[actions]]
-=== Actions:
+=== Actions
Actions buttons are at the top right and in the overflow menu.
Depending on the change state and the permissions of the user, different
actions are available on the change:
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
index c1029be..0966bbe 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
@@ -441,8 +441,10 @@
PatchSetInserter patchSetInserter =
getPatchSetInserter(changeNotes, newPatchsetCommit, patchsetId);
- IdentifiedUser changeOwner = userFactory.create(changeNotes.getChange().getOwner());
- try (BatchUpdate batchUpdate = batchUpdateFactory.create(project, changeOwner, now)) {
+ Account.Id uploaderId =
+ patchsetCreation.uploader().orElse(changeNotes.getChange().getOwner());
+ IdentifiedUser uploader = userFactory.create(uploaderId);
+ try (BatchUpdate batchUpdate = batchUpdateFactory.create(project, uploader, now)) {
batchUpdate.setRepository(repository, revWalk, objectInserter);
batchUpdate.addOp(changeId, patchSetInserter);
batchUpdate.execute();
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
index 22a4da6..32731c1 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestPatchsetCreation.java
@@ -17,6 +17,7 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.testsuite.ThrowingFunction;
+import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.server.edit.tree.TreeModification;
import java.util.Optional;
@@ -25,6 +26,8 @@
@AutoValue
public abstract class TestPatchsetCreation {
+ public abstract Optional<Account.Id> uploader();
+
public abstract Optional<String> commitMessage();
public abstract ImmutableList<TreeModification> treeModifications();
@@ -40,6 +43,11 @@
@AutoValue.Builder
public abstract static class Builder {
+ /**
+ * The uploader for the new patch set. If not set the new patch set is uploaded by the change
+ * owner.
+ */
+ public abstract Builder uploader(Account.Id uploader);
public abstract Builder commitMessage(String commitMessage);
diff --git a/java/com/google/gerrit/server/account/AccountResolver.java b/java/com/google/gerrit/server/account/AccountResolver.java
index fcfc805..d2417d7 100644
--- a/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/java/com/google/gerrit/server/account/AccountResolver.java
@@ -790,7 +790,7 @@
@Override
public Predicate<AccountState> get() {
- return accountControlFactory.get(asUser.asIdentifiedUser())::canSee;
+ return accountControlFactory.get(asUser)::canSee;
}
}
diff --git a/java/com/google/gerrit/server/comment/CommentContextLoader.java b/java/com/google/gerrit/server/comment/CommentContextLoader.java
index 8fbb259..0df7729 100644
--- a/java/com/google/gerrit/server/comment/CommentContextLoader.java
+++ b/java/com/google/gerrit/server/comment/CommentContextLoader.java
@@ -225,6 +225,11 @@
private static Optional<Range> getStartAndEndLines(ContextInput comment) {
if (comment.range() != null) {
+ if (comment.range().endLine < comment.range().startLine) {
+ // Seems like comments, created in reply to robot comments sometimes have invalid ranges
+ // Fix here, otherwise the range is invalid and we throw an error later on.
+ return Optional.of(Range.create(comment.range().startLine, comment.range().startLine + 1));
+ }
return Optional.of(Range.create(comment.range().startLine, comment.range().endLine + 1));
} else if (comment.lineNumber() > 0) {
return Optional.of(Range.create(comment.lineNumber(), comment.lineNumber() + 1));
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 6f7d761..e36ce7b 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -216,7 +216,10 @@
public void check(ChangePermissionOrLabel perm)
throws AuthException, PermissionBackendException {
if (!can(perm)) {
- throw new AuthException(perm.describeForException() + " not permitted");
+ throw new AuthException(
+ perm.describeForException()
+ + " not permitted"
+ + perm.hintForException().map(hint -> " (" + hint + ")").orElse(""));
}
}
diff --git a/java/com/google/gerrit/server/permissions/ChangePermission.java b/java/com/google/gerrit/server/permissions/ChangePermission.java
index 63b0378..6ceed3e 100644
--- a/java/com/google/gerrit/server/permissions/ChangePermission.java
+++ b/java/com/google/gerrit/server/permissions/ChangePermission.java
@@ -16,7 +16,9 @@
import static java.util.Objects.requireNonNull;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.access.GerritPermission;
+import java.util.Optional;
public enum ChangePermission implements ChangePermissionOrLabel {
READ,
@@ -53,24 +55,40 @@
* <p>Before checking this permission, the caller should first verify the current patch set of the
* change is not locked by calling {@code PatchSetUtil.isPatchSetLocked}.
*/
- REBASE,
+ REBASE(
+ /* description= */ null,
+ /* hint= */ "change owners and users with the 'Submit' or 'Rebase' permission can rebase"
+ + " if they have the 'Push' permission"),
REVERT,
SUBMIT,
SUBMIT_AS("submit on behalf of other users"),
TOGGLE_WORK_IN_PROGRESS_STATE;
private final String description;
+ private final String hint;
ChangePermission() {
this.description = null;
+ this.hint = null;
}
ChangePermission(String description) {
this.description = requireNonNull(description);
+ this.hint = null;
+ }
+
+ ChangePermission(@Nullable String description, String hint) {
+ this.description = description;
+ this.hint = requireNonNull(hint);
}
@Override
public String describeForException() {
return description != null ? description : GerritPermission.describeEnumValue(this);
}
+
+ @Override
+ public Optional<String> hintForException() {
+ return Optional.ofNullable(hint);
+ }
}
diff --git a/java/com/google/gerrit/server/permissions/ChangePermissionOrLabel.java b/java/com/google/gerrit/server/permissions/ChangePermissionOrLabel.java
index f59ba02..9254158 100644
--- a/java/com/google/gerrit/server/permissions/ChangePermissionOrLabel.java
+++ b/java/com/google/gerrit/server/permissions/ChangePermissionOrLabel.java
@@ -15,6 +15,17 @@
package com.google.gerrit.server.permissions;
import com.google.gerrit.extensions.api.access.GerritPermission;
+import java.util.Optional;
/** A {@link ChangePermission} or a {@link AbstractLabelPermission}. */
-public interface ChangePermissionOrLabel extends GerritPermission {}
+public interface ChangePermissionOrLabel extends GerritPermission {
+ /**
+ * A hint that explains under which conditions this permission is permitted.
+ *
+ * <p>This is useful for permissions that are not directly assigned but are indirectly permitted
+ * by the user having other permissions or being the change owner.
+ */
+ default Optional<String> hintForException() {
+ return Optional.empty();
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
index 74bd94e..522013e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java
@@ -399,7 +399,11 @@
String changeId = r2.getChangeId();
requestScopeOperations.setApiUser(user.id());
AuthException thrown = assertThrows(AuthException.class, () -> rebaseCall.call(changeId));
- assertThat(thrown).hasMessageThat().contains("rebase not permitted");
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "rebase not permitted (change owners and users with the 'Submit' or 'Rebase'"
+ + " permission can rebase if they have the 'Push' permission)");
}
@Test
@@ -449,7 +453,11 @@
String changeId = r2.getChangeId();
requestScopeOperations.setApiUser(user.id());
AuthException thrown = assertThrows(AuthException.class, () -> rebaseCall.call(changeId));
- assertThat(thrown).hasMessageThat().contains("rebase not permitted");
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "rebase not permitted (change owners and users with the 'Submit' or 'Rebase'"
+ + " permission can rebase if they have the 'Push' permission)");
}
@Test
@@ -473,7 +481,11 @@
// Rebase the second
String changeId = r2.getChangeId();
AuthException thrown = assertThrows(AuthException.class, () -> rebaseCall.call(changeId));
- assertThat(thrown).hasMessageThat().contains("rebase not permitted");
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "rebase not permitted (change owners and users with the 'Submit' or 'Rebase'"
+ + " permission can rebase if they have the 'Push' permission)");
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index 242c278..ab2f358 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -33,6 +33,7 @@
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.acceptance.UseTimezone;
import com.google.gerrit.acceptance.VerifyNoPiiInChangeNotes;
import com.google.gerrit.acceptance.testsuite.change.IndexOperations;
@@ -424,6 +425,26 @@
}
@Test
+ public void checkSubmitRequirement_verifiesUploader() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ voteLabel(changeId, "Code-Review", 2);
+ TestAccount anotherUser = accountCreator.createValid("anotherUser");
+
+ SubmitRequirementInput in =
+ createSubmitRequirementInput(
+ "Foo", /* submittabilityExpression= */ "uploader:" + anotherUser.id());
+ SubmitRequirementResultInfo result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.UNSATISFIED);
+
+ in =
+ createSubmitRequirementInput(
+ "Foo", /* submittabilityExpression= */ "uploader:" + r.getChange().change().getOwner());
+ result = gApi.changes().id(changeId).checkSubmitRequirement(in);
+ assertThat(result.status).isEqualTo(SubmitRequirementResultInfo.Status.SATISFIED);
+ }
+
+ @Test
public void submitRequirement_withLabelEqualsMax() throws Exception {
configSubmitRequirement(
project,
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index 6c629c9..fd5f6fc 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -795,6 +795,23 @@
}
@Test
+ public void newPatchsetCanHaveDifferentUploader() throws Exception {
+ Account.Id changeOwner = accountOperations.newAccount().create();
+ Change.Id changeId = changeOperations.newChange().owner(changeOwner).create();
+
+ ChangeInfo change = getChangeFromServer(changeId);
+ RevisionInfo currentPatchsetRevision = change.revisions.get(change.currentRevision);
+ assertThat(currentPatchsetRevision.uploader._accountId).isEqualTo(changeOwner.get());
+
+ Account.Id newUploader = accountOperations.newAccount().create();
+ changeOperations.change(changeId).newPatchset().uploader(newUploader).create();
+
+ change = getChangeFromServer(changeId);
+ currentPatchsetRevision = change.revisions.get(change.currentRevision);
+ assertThat(currentPatchsetRevision.uploader._accountId).isEqualTo(newUploader.get());
+ }
+
+ @Test
public void newPatchsetCanHaveUpdatedCommitMessage() throws Exception {
Change.Id changeId = changeOperations.newChange().commitMessage("Old message").create();
diff --git a/modules/jgit b/modules/jgit
index 801a56b..a190130 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 801a56b48a7fe3c6e171073211cc62194184fe79
+Subproject commit a1901305b26ed5e0116f138bc02837713d2cf5c3
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 8d5689c..96688e9 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,7 +6,6 @@
import '@polymer/iron-input/iron-input';
import '../../../styles/gr-form-styles';
import '../../../styles/shared-styles';
-import {page} from '../../../utils/page-wrapper-utils';
import {GroupId, GroupName} from '../../../types/common';
import {getAppContext} from '../../../services/app-context';
import {formStyles} from '../../../styles/gr-form-styles';
@@ -16,6 +15,8 @@
import {BindValueChangeEvent} from '../../../types/events';
import {fireEvent} from '../../../utils/event-util';
import {createGroupUrl} from '../../../models/views/group';
+import {resolve} from '../../../models/dependency';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
declare global {
interface HTMLElementTagNameMap {
@@ -32,6 +33,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
static override get styles() {
return [
formStyles,
@@ -86,8 +89,7 @@
return this.restApiService.getGroupConfig(name).then(group => {
if (!group) return;
const groupId = String(group.group_id!) as GroupId;
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(createGroupUrl({groupId}));
+ this.getNavigation().setUrl(createGroupUrl({groupId}));
});
});
}
diff --git a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts
index 2a0b539..6e36d8c 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-group-dialog/gr-create-group-dialog_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-create-group-dialog';
import {GrCreateGroupDialog} from './gr-create-group-dialog';
-import {page} from '../../../utils/page-wrapper-utils';
import {
mockPromise,
queryAndAssert,
@@ -15,6 +14,8 @@
import {IronInputElement} from '@polymer/iron-input';
import {GroupId} from '../../../types/common';
import {fixture, html, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
suite('gr-create-group-dialog tests', () => {
let element: GrCreateGroupDialog;
@@ -68,9 +69,9 @@
Promise.resolve({id: 'testId551' as GroupId, group_id: 551})
);
- const showStub = sinon.stub(page, 'show');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
await element.handleCreateGroup();
- assert.isTrue(showStub.calledWith('/admin/groups/551'));
+ assert.isTrue(setUrlStub.calledWith('/admin/groups/551'));
});
test('test for unsuccessful group creation', async () => {
@@ -81,8 +82,8 @@
Promise.resolve({id: 'testId551' as GroupId, group_id: 551})
);
- const showStub = sinon.stub(page, 'show');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
await element.handleCreateGroup();
- assert.isFalse(showStub.called);
+ assert.isFalse(setUrlStub.called);
});
});
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 bb59ccc..ed57830 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 {page} from '../../../utils/page-wrapper-utils';
import {
BranchName,
GroupId,
@@ -24,6 +23,8 @@
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';
+import {resolve} from '../../../models/dependency';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
declare global {
interface HTMLElementTagNameMap {
@@ -71,6 +72,8 @@
private readonly restApiService = getAppContext().restApiService;
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.query = (input: string) => this.getRepoSuggestions(input);
@@ -195,8 +198,7 @@
);
if (repoRegistered.status === 201) {
this.repoCreated = true;
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(createRepoUrl({repo: this.repoConfig.name}));
+ this.getNavigation().setUrl(createRepoUrl({repo: this.repoConfig.name}));
}
return repoRegistered;
}
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 9b9cf29..9a67fda 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
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-repo-detail-list';
import {GrRepoDetailList} from './gr-repo-detail-list';
-import {page} from '../../../utils/page-wrapper-utils';
import {
addListenerForTest,
mockPromise,
@@ -34,6 +33,8 @@
import {GrListView} from '../../shared/gr-list-view/gr-list-view';
import {fixture, html, assert} from '@open-wc/testing';
import {RepoDetailView} from '../../../models/views/repo';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
function branchGenerator(counter: number) {
return {
@@ -95,7 +96,7 @@
html`<gr-repo-detail-list></gr-repo-detail-list>`
);
element.detailType = RepoDetailView.BRANCHES;
- sinon.stub(page, 'show');
+ sinon.stub(testResolver(navigationToken), 'setUrl');
});
suite('list of repo branches', () => {
@@ -2338,7 +2339,7 @@
html`<gr-repo-detail-list></gr-repo-detail-list>`
);
element.detailType = RepoDetailView.TAGS;
- sinon.stub(page, 'show');
+ sinon.stub(testResolver(navigationToken), 'setUrl');
});
suite('list of repo tags', () => {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
index 65f5a8c..906b733 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-list/gr-repo-list_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-repo-list';
import {GrRepoList} from './gr-repo-list';
-import {page} from '../../../utils/page-wrapper-utils';
import {
mockPromise,
queryAndAssert,
@@ -23,6 +22,8 @@
import {GrListView} from '../../shared/gr-list-view/gr-list-view';
import {fixture, html, assert} from '@open-wc/testing';
import {AdminChildView, AdminViewState} from '../../../models/views/admin';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
function createRepo(name: string, counter: number) {
return {
@@ -51,7 +52,7 @@
let repos: ProjectInfoWithName[];
setup(async () => {
- sinon.stub(page, 'show');
+ sinon.stub(testResolver(navigationToken), 'setUrl');
element = await fixture(html`<gr-repo-list></gr-repo-list>`);
});
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 faaee0b..d2ba2c9 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
@@ -6,7 +6,6 @@
import '../gr-change-list/gr-change-list';
import '../gr-repo-header/gr-repo-header';
import '../gr-user-header/gr-user-header';
-import {page} from '../../../utils/page-wrapper-utils';
import {
AccountDetailInfo,
AccountId,
@@ -28,6 +27,7 @@
import {resolve} from '../../../models/dependency';
import {subscribe} from '../../lit/subscription-controller';
import {userModelToken} from '../../../models/user/user-model';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
const LIMIT_OPERATOR_PATTERN = /\blimit:(\d+)/i;
@@ -81,6 +81,8 @@
private readonly getViewModel = resolve(this, searchViewModelToken);
+ private readonly getNavigation = resolve(this, navigationToken);
+
constructor() {
super();
this.addEventListener('next-page', () => this.handleNextPage());
@@ -282,15 +284,13 @@
// 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));
+ this.getNavigation().setUrl(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));
+ this.getNavigation().setUrl(this.computeNavLink(-1));
}
// private but used in test
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
index f4bd8bd..decc253 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view_test.ts
@@ -6,7 +6,6 @@
import '../../../test/common-test-setup';
import './gr-change-list-view';
import {GrChangeListView} from './gr-change-list-view';
-import {page} from '../../../utils/page-wrapper-utils';
import {query, queryAndAssert} from '../../../test/test-utils';
import {createChange} from '../../../test/test-data-generators';
import {ChangeInfo} from '../../../api/rest-api';
@@ -14,6 +13,8 @@
import {GrChangeList} from '../gr-change-list/gr-change-list';
import {GrChangeListSection} from '../gr-change-list-section/gr-change-list-section';
import {GrChangeListItem} from '../gr-change-list-item/gr-change-list-item';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
suite('gr-change-list-view tests', () => {
let element: GrChangeListView;
@@ -158,7 +159,7 @@
});
test('handleNextPage', async () => {
- const showStub = sinon.stub(page, 'show');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
element.changes = Array(25)
.fill(0)
.map(_ => createChange());
@@ -166,7 +167,7 @@
element.loading = false;
await element.updateComplete;
element.handleNextPage();
- assert.isFalse(showStub.called);
+ assert.isFalse(setUrlStub.called);
element.changes = Array(25)
.fill(0)
@@ -174,11 +175,11 @@
element.loading = false;
await element.updateComplete;
element.handleNextPage();
- assert.isTrue(showStub.called);
+ assert.isTrue(setUrlStub.called);
});
test('handlePreviousPage', async () => {
- const showStub = sinon.stub(page, 'show');
+ const setUrlStub = sinon.stub(testResolver(navigationToken), 'setUrl');
element.offset = 0;
element.changes = Array(25)
.fill(0)
@@ -187,11 +188,11 @@
element.loading = false;
await element.updateComplete;
element.handlePreviousPage();
- assert.isFalse(showStub.called);
+ assert.isFalse(setUrlStub.called);
element.offset = 25;
await element.updateComplete;
element.handlePreviousPage();
- assert.isTrue(showStub.called);
+ assert.isTrue(setUrlStub.called);
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
index b0dbda5..6f8bd9a 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
@@ -71,6 +71,9 @@
text = '';
@state()
+ shouldRebaseChain = false;
+
+ @state()
private query: AutocompleteQuery;
@state()
@@ -184,7 +187,7 @@
/>
<label id="rebaseOnTipLabel" for="rebaseOnTipInput">
Rebase on top of the ${this.branch} branch<span
- ?hidden=${!this.hasParent}
+ ?hidden=${!this.hasParent || this.shouldRebaseChain}
>
(breaks relation chain)
</span>
@@ -206,7 +209,9 @@
/>
<label id="rebaseOnOtherLabel" for="rebaseOnOtherInput">
Rebase on a specific change, ref, or commit
- <span ?hidden=${!this.hasParent}> (breaks relation chain) </span>
+ <span ?hidden=${!this.hasParent || this.shouldRebaseChain}>
+ (breaks relation chain)
+ </span>
</label>
</div>
<div class="parentRevisionContainer">
@@ -230,10 +235,17 @@
>
</div>
${when(
- this.flagsService.isEnabled(KnownExperimentId.REBASE_CHAIN),
+ this.flagsService.isEnabled(KnownExperimentId.REBASE_CHAIN) &&
+ this.hasParent,
() =>
html`<div>
- <input id="rebaseChain" type="checkbox" />
+ <input
+ id="rebaseChain"
+ type="checkbox"
+ @change=${() => {
+ this.shouldRebaseChain = !!this.rebaseChain?.checked;
+ }}
+ />
<label for="rebaseChain">Rebase all ancestors</label>
</div>`
)}
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 bcf6937..00cb938 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -49,6 +49,7 @@
AdminChildView,
AdminViewModel,
AdminViewState,
+ PLUGIN_LIST_ROUTE,
} from '../../../models/views/admin';
import {
AgreementViewModel,
@@ -99,6 +100,8 @@
isInBaseOfPatchRange,
} from '../../../utils/comment-util';
import {isFileUnchanged} from '../../../embed/diff/gr-diff/gr-diff-utils';
+import {Route, ViewState} from '../../../models/views/base';
+import {Model} from '../../../models/model';
const RoutePattern = {
ROOT: '/',
@@ -184,8 +187,6 @@
PLUGINS: /^\/plugins\/(.+)$/,
- PLUGIN_LIST: /^\/admin\/plugins(\/)?$/,
-
// Matches /admin/plugins[,<offset>][/].
PLUGIN_LIST_OFFSET: /^\/admin\/plugins(,(\d+))?(\/)?$/,
PLUGIN_LIST_FILTER: '/admin/plugins/q/filter::filter',
@@ -306,6 +307,8 @@
private view?: GerritView;
+ readonly page = page.create();
+
constructor(
private readonly reporting: ReportingService,
private readonly routerModel: RouterModel,
@@ -342,7 +345,7 @@
}
if (browserUrl.toString() !== stateUrl.toString()) {
- page.replace(
+ this.page.replace(
stateUrl.toString(),
null,
/* init: */ false,
@@ -359,6 +362,7 @@
subscription.unsubscribe();
}
this.subscriptions = [];
+ this.page.stop();
}
start() {
@@ -369,6 +373,7 @@
}
setState(state: AppElementParams) {
+ // TODO: Move this logic into the change model.
if ('repo' in state && state.repo !== undefined && 'changeNum' in state)
this.restApiService.setInProjectLookup(state.changeNum, state.repo);
@@ -400,7 +405,7 @@
redirect(url: string) {
this._isRedirecting = true;
- page.redirect(url);
+ this.page.redirect(url);
}
/**
@@ -429,7 +434,9 @@
*/
redirectToLogin(returnUrl: string) {
const basePath = getBaseUrl() || '';
- page('/login/' + encodeURIComponent(returnUrl.substring(basePath.length)));
+ this.page(
+ '/login/' + encodeURIComponent(returnUrl.substring(basePath.length))
+ );
}
/**
@@ -488,6 +495,9 @@
* route is matched, the handler will be executed with `this` referring
* to the component. Its return value will be discarded so that it does
* not interfere with page.js.
+ * TODO: Get rid of this parameter. This is really not something that the
+ * router wants to be concerned with. The reporting service and the view
+ * models should figure that out between themselves.
* @param authRedirect If true, then auth is checked before
* executing the handler. If the user is not logged in, it will redirect
* to the login flow and the handler will not be executed. The login
@@ -499,7 +509,7 @@
handler: (ctx: PageContext) => void,
authRedirect?: boolean
) {
- page(
+ this.page(
pattern,
(ctx, next) => this.loadUserMiddleware(ctx, next),
ctx => {
@@ -515,6 +525,32 @@
}
/**
+ * Convenience wrapper of `mapRoute()` for when you have a `Route` object that
+ * can deal with state creation. Takes care of setting the view model state,
+ * which is currently duplicated lots of times for direct callers of
+ * `mapRoute()`.
+ */
+ mapRouteState<T extends ViewState>(
+ route: Route<T>,
+ viewModel: Model<T | undefined>,
+ handlerName: string,
+ authRedirect?: boolean
+ ) {
+ const handler = (ctx: PageContext) => {
+ const state = route.createState(ctx);
+ // Note that order is important: `this.setState()` must be called before
+ // `viewModel.setState()`. Otherwise the chain of model subscriptions
+ // would be very different. Some views may want app element to swap the
+ // top level view first. Also, `this.setState()` has some special change
+ // view model resetting logic. Eventually the order might not be important
+ // anymore, but be careful! :-)
+ this.setState(state as AppElementParams);
+ viewModel.setState(state);
+ };
+ this.mapRoute(route.urlPattern, handlerName, handler, authRedirect);
+ }
+
+ /**
* This is similar to letting the browser navigate to this URL when the user
* clicks it, or to just setting `window.location.href` directly.
*
@@ -524,14 +560,14 @@
* page.show() eventually just calls `window.history.pushState()`.
*/
setUrl(url: string) {
- page.show(url);
+ this.page.show(url);
}
/**
* Navigate to this URL, but replace the current URL in the history instead of
* adding a new one (which is what `setUrl()` would do).
*
- * page.redirect() eventually just calls `window.history.replaceState()`.
+ * this.page.redirect() eventually just calls `window.history.replaceState()`.
*/
replaceUrl(url: string) {
this.redirect(url);
@@ -554,10 +590,10 @@
startRouter() {
const base = getBaseUrl();
if (base) {
- page.base(base);
+ this.page.base(base);
}
- page.exit('*', (_, next) => {
+ this.page.exit('*', (_, next) => {
if (!this._isRedirecting) {
this.reporting.beforeLocationChanged();
}
@@ -568,7 +604,7 @@
// Remove the tracking param 'usp' (User Source Parameter) from the URL,
// just to have users look at cleaner URLs.
- page((ctx, next) => {
+ this.page((ctx, next) => {
if (window.URLSearchParams) {
const pathname = toPathname(ctx.canonicalPath);
const searchParams = toSearchParams(ctx.canonicalPath);
@@ -584,7 +620,7 @@
});
// Middleware
- page((ctx, next) => {
+ this.page((ctx, next) => {
document.body.scrollTop = 0;
if (ctx.hash.match(RoutePattern.PLUGIN_SCREEN)) {
@@ -814,10 +850,10 @@
true
);
- this.mapRoute(
- RoutePattern.PLUGIN_LIST,
+ this.mapRouteState(
+ PLUGIN_LIST_ROUTE,
+ this.adminViewModel,
'handlePluginListRoute',
- ctx => this.handlePluginListRoute(ctx),
true
);
@@ -955,7 +991,7 @@
this.handleDefaultRoute()
);
- page.start();
+ this.page.start();
}
/**
@@ -1122,7 +1158,7 @@
const state: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
- offset: ctx.params[1] || 0,
+ offset: ctx.params[1] ?? '0',
filter: null,
openCreateModal: ctx.hash === 'create',
};
@@ -1137,6 +1173,7 @@
adminView: AdminChildView.GROUPS,
offset: ctx.params['offset'],
filter: ctx.params['filter'],
+ openCreateModal: false,
};
// Note that router model view must be updated before view models.
this.setState(state);
@@ -1147,7 +1184,9 @@
const state: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
+ offset: ctx.params[1] ?? '0',
filter: ctx.params['filter'] || null,
+ openCreateModal: false,
};
// Note that router model view must be updated before view models.
this.setState(state);
@@ -1399,16 +1438,6 @@
this.adminViewModel.setState(state);
}
- handlePluginListRoute(_: PageContext) {
- const state: AdminViewState = {
- view: GerritView.ADMIN,
- adminView: AdminChildView.PLUGINS,
- };
- // Note that router model view must be updated before view models.
- this.setState(state);
- this.adminViewModel.setState(state);
- }
-
handleQueryRoute(ctx: PageContext) {
const state: Partial<SearchViewState> = {
view: GerritView.SEARCH,
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index d8761bf..af44739 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -5,12 +5,13 @@
*/
import '../../../test/common-test-setup';
import './gr-router';
-import {page, PageContext} from '../../../utils/page-wrapper-utils';
+import {Page, PageContext} from '../../../utils/page-wrapper-utils';
import {
stubBaseUrl,
stubRestApi,
addListenerForTest,
waitEventLoop,
+ waitUntilCalled,
} from '../../../test/test-utils';
import {GrRouter, routerToken, _testOnly_RoutePattern} from './gr-router';
import {GerritView} from '../../../services/router/router-model';
@@ -25,7 +26,7 @@
} from '../../../types/common';
import {AppElementParams} from '../../gr-app-types';
import {assert} from '@open-wc/testing';
-import {AdminChildView} from '../../../models/views/admin';
+import {AdminChildView, AdminViewState} from '../../../models/views/admin';
import {RepoDetailView} from '../../../models/views/repo';
import {GroupDetailView} from '../../../models/views/group';
import {ChangeChildView, ChangeViewState} from '../../../models/views/change';
@@ -38,12 +39,15 @@
createRevision,
} from '../../../test/test-data-generators';
import {ParsedChangeInfo} from '../../../types/types';
+import {ViewState} from '../../../models/views/base';
suite('gr-router tests', () => {
let router: GrRouter;
+ let page: Page;
setup(() => {
router = testResolver(routerToken);
+ page = router.page;
});
test('getHashFromCanonicalPath', () => {
@@ -289,6 +293,19 @@
assert.deepEqual(setStateStub.lastCall.args[0], params);
}
+ async function checkUrlToState<T extends ViewState>(url: string, state: T) {
+ setStateStub.reset();
+ router.page.show(url);
+ await waitUntilCalled(setStateStub, 'setState');
+ assert.deepEqual(setStateStub.lastCall.firstArg, state);
+ }
+
+ async function checkUrlNotMatched(url: string) {
+ handlePassThroughRoute.reset();
+ router.page.show(url);
+ await waitUntilCalled(handlePassThroughRoute, 'handlePassThroughRoute');
+ }
+
function createPageContext(): PageContext {
return {
canonicalPath: '',
@@ -304,6 +321,7 @@
redirectStub = sinon.stub(router, 'redirect');
setStateStub = sinon.stub(router, 'setState');
handlePassThroughRoute = sinon.stub(router, 'handlePassThroughRoute');
+ router.startRouter();
});
test('handleLegacyProjectDashboardRoute', () => {
@@ -735,55 +753,55 @@
});
});
- test('handleGroupListOffsetRoute', () => {
- const ctx = createPageContext();
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
+ test('list of groups', async () => {
+ const defaultState: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
- offset: 0,
- filter: null,
+ offset: '0',
openCreateModal: false,
- });
-
- ctx.params[1] = '42';
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
filter: null,
- openCreateModal: false,
- });
+ };
- ctx.hash = 'create';
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
- filter: null,
+ await checkUrlToState('/admin/groups', defaultState);
+ await checkUrlToState('/admin/groups/', defaultState);
+ await checkUrlToState('/admin/groups#create', {
+ ...defaultState,
openCreateModal: true,
});
- });
-
- test('handleGroupListFilterOffsetRoute', () => {
- const ctx = {
- ...createPageContext(),
- params: {filter: 'foo', offset: '42'},
- };
- assertctxToParams(ctx, 'handleGroupListFilterOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
- filter: 'foo',
+ await checkUrlToState('/admin/groups,123', {
+ ...defaultState,
+ offset: '123',
});
- });
-
- test('handleGroupListFilterRoute', () => {
- const ctx = {...createPageContext(), params: {filter: 'foo'}};
- assertctxToParams(ctx, 'handleGroupListFilterRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- filter: 'foo',
+ await checkUrlToState('/admin/groups,123#create', {
+ ...defaultState,
+ offset: '123',
+ openCreateModal: true,
});
+
+ await checkUrlToState('/admin/groups/q/filter:asdf', {
+ ...defaultState,
+ filter: 'asdf',
+ });
+ await checkUrlToState('/admin/groups/q/filter:asdf,123', {
+ ...defaultState,
+ filter: 'asdf',
+ offset: '123',
+ });
+ // #create is ignored when filtering
+ await checkUrlToState('/admin/groups/q/filter:asdf,123#create', {
+ ...defaultState,
+ filter: 'asdf',
+ offset: '123',
+ });
+ // filter is decoded (twice)
+ await checkUrlToState(
+ '/admin/groups/q/filter:XX%20XX%2520XX%252FXX%3FXX',
+ {...defaultState, filter: 'XX XX XX/XX?XX'}
+ );
+
+ // Slash must be double encoded in `filter` param.
+ await checkUrlNotMatched('/admin/groups/q/filter:asdf/qwer,11');
+ await checkUrlNotMatched('/admin/groups/q/filter:asdf%2Fqwer,11');
});
test('handleGroupRoute', () => {
@@ -1065,14 +1083,6 @@
filter: 'foo',
});
});
-
- test('handlePluginListRoute', () => {
- const ctx = createPageContext();
- assertctxToParams(ctx, 'handlePluginListRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.PLUGINS,
- });
- });
});
suite('change/diff routes', () => {
diff --git a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
index 0092193..ea5e9f3 100644
--- a/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
+++ b/polygerrit-ui/app/elements/documentation/gr-documentation-search/gr-documentation-search_test.ts
@@ -6,10 +6,11 @@
import '../../../test/common-test-setup';
import './gr-documentation-search';
import {GrDocumentationSearch} from './gr-documentation-search';
-import {page} from '../../../utils/page-wrapper-utils';
import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {DocResult} from '../../../types/common';
import {fixture, html, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
function documentationGenerator(counter: number) {
return {
@@ -31,7 +32,7 @@
let documentationSearches: DocResult[];
setup(async () => {
- sinon.stub(page, 'show');
+ sinon.stub(testResolver(navigationToken), 'setUrl');
element = await fixture(
html`<gr-documentation-search></gr-documentation-search>`
);
diff --git a/polygerrit-ui/app/elements/gr-app-types.ts b/polygerrit-ui/app/elements/gr-app-types.ts
index 0261992..68e7309 100644
--- a/polygerrit-ui/app/elements/gr-app-types.ts
+++ b/polygerrit-ui/app/elements/gr-app-types.ts
@@ -28,6 +28,7 @@
justRegistered: boolean;
}
+// TODO: Get rid of this type. <gr-app-element> needs to be refactored for that.
export type AppElementParams =
| DashboardViewState
| GroupViewState
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 dad802a..f5837e3 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
@@ -7,13 +7,14 @@
import '../gr-button/gr-button';
import '../gr-icon/gr-icon';
import {encodeURL, getBaseUrl} from '../../../utils/url-util';
-import {page} from '../../../utils/page-wrapper-utils';
import {fireEvent} from '../../../utils/event-util';
import {debounce, DelayedTask} from '../../../utils/async-util';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, css, html} from 'lit';
import {customElement, property} from 'lit/decorators.js';
import {BindValueChangeEvent} from '../../../types/events';
+import {resolve} from '../../../models/dependency';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
const REQUEST_DEBOUNCE_INTERVAL_MS = 200;
@@ -48,6 +49,8 @@
private reloadTask?: DelayedTask;
+ private readonly getNavigation = resolve(this, navigationToken);
+
override disconnectedCallback() {
this.reloadTask?.cancel();
super.disconnectedCallback();
@@ -165,12 +168,12 @@
() => {
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)}`);
+ this.getNavigation().setUrl(
+ `${this.path}/q/filter:${encodeURL(filter, false)}`
+ );
return;
}
- // TODO: Use navigation service instead of `page.show()` directly.
- page.show(this.path);
+ this.getNavigation().setUrl(this.path);
},
REQUEST_DEBOUNCE_INTERVAL_MS
);
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
index bf94e8f..ecc1c25 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.ts
@@ -6,10 +6,11 @@
import '../../../test/common-test-setup';
import './gr-list-view';
import {GrListView} from './gr-list-view';
-import {page} from '../../../utils/page-wrapper-utils';
import {queryAndAssert, stubBaseUrl} from '../../../test/test-utils';
import {GrButton} from '../gr-button/gr-button';
import {fixture, html, assert} from '@open-wc/testing';
+import {testResolver} from '../../../test/common-test-setup';
+import {navigationToken} from '../../core/gr-navigation/gr-navigation';
suite('gr-list-view tests', () => {
let element: GrListView;
@@ -84,7 +85,9 @@
let resolve: (url: string) => void;
const promise = new Promise(r => (resolve = r));
element.path = '/admin/projects';
- sinon.stub(page, 'show').callsFake(r => resolve(r));
+ sinon
+ .stub(testResolver(navigationToken), 'setUrl')
+ .callsFake(r => resolve(r));
element.filter = 'test';
await element.updateComplete;
diff --git a/polygerrit-ui/app/models/views/admin.ts b/polygerrit-ui/app/models/views/admin.ts
index 3380637..de9ec5d 100644
--- a/polygerrit-ui/app/models/views/admin.ts
+++ b/polygerrit-ui/app/models/views/admin.ts
@@ -7,7 +7,18 @@
import {getBaseUrl} from '../../utils/url-util';
import {define} from '../dependency';
import {Model} from '../model';
-import {ViewState} from './base';
+import {Route, ViewState} from './base';
+
+export const PLUGIN_LIST_ROUTE: Route<AdminViewState> = {
+ urlPattern: /^\/admin\/plugins(\/)?$/,
+ createState: () => {
+ const state: AdminViewState = {
+ view: GerritView.ADMIN,
+ adminView: AdminChildView.PLUGINS,
+ };
+ return state;
+ },
+};
export enum AdminChildView {
REPOS = 'gr-repo-list',
diff --git a/polygerrit-ui/app/models/views/admin_test.ts b/polygerrit-ui/app/models/views/admin_test.ts
new file mode 100644
index 0000000..c9b7801
--- /dev/null
+++ b/polygerrit-ui/app/models/views/admin_test.ts
@@ -0,0 +1,29 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {assert} from '@open-wc/testing';
+import {GerritView} from '../../services/router/router-model';
+import '../../test/common-test-setup';
+import {AdminChildView, PLUGIN_LIST_ROUTE} from './admin';
+
+suite('admin view model', () => {
+ suite('routes', () => {
+ test('PLUGIN_LIST', () => {
+ const {urlPattern: pattern, createState} = PLUGIN_LIST_ROUTE;
+
+ assert.isTrue(pattern.test('/admin/plugins'));
+ assert.isTrue(pattern.test('/admin/plugins/'));
+ assert.isFalse(pattern.test('admin/plugins'));
+ assert.isFalse(pattern.test('//admin/plugins'));
+ assert.isFalse(pattern.test('//admin/plugins?'));
+ assert.isFalse(pattern.test('/admin/plugins//'));
+
+ assert.deepEqual(createState({}), {
+ view: GerritView.ADMIN,
+ adminView: AdminChildView.PLUGINS,
+ });
+ });
+ });
+});
diff --git a/polygerrit-ui/app/models/views/base.ts b/polygerrit-ui/app/models/views/base.ts
index 065495d..72bec33 100644
--- a/polygerrit-ui/app/models/views/base.ts
+++ b/polygerrit-ui/app/models/views/base.ts
@@ -8,3 +8,24 @@
export interface ViewState {
view: GerritView;
}
+
+/**
+ * While we are using page.js this interface will normally be implemented by
+ * PageContext, but it helps testing and independence to have our own type
+ * here.
+ */
+export interface UrlInfo {
+ querystring?: string;
+ hash?: string;
+ /** What the regular expression matching returns. */
+ params?: {[paramIndex: string]: string};
+}
+
+/**
+ * Based on `urlPattern` knows whether a URL matches and if so, then
+ * `createState()` can produce a `ViewState` from the matched URL.
+ */
+export interface Route<T extends ViewState> {
+ urlPattern: RegExp;
+ createState: (info: UrlInfo) => T;
+}
diff --git a/polygerrit-ui/app/services/router/router-model.ts b/polygerrit-ui/app/services/router/router-model.ts
index c3c1cb6..5e2cc10 100644
--- a/polygerrit-ui/app/services/router/router-model.ts
+++ b/polygerrit-ui/app/services/router/router-model.ts
@@ -21,12 +21,17 @@
SETTINGS = 'settings',
}
+// TODO: Consider renaming this to AppElementState or something similar.
+// Or maybe RootViewState. This class does *not* model the state of the router.
export interface RouterState {
// Note that this router model view must be updated before view model state.
view?: GerritView;
}
export const routerModelToken = define<RouterModel>('router-model');
+
+// TODO: Consider renaming this to AppElementViewModel or something similar.
+// Or maybe RootViewModel. This class is *not* a view model of the router.
export class RouterModel extends Model<RouterState> {
readonly routerView$: Observable<GerritView | undefined> = select(
this.state$,
diff --git a/polygerrit-ui/app/utils/page-wrapper-utils.ts b/polygerrit-ui/app/utils/page-wrapper-utils.ts
index 78e78ed..2e5c7b42 100644
--- a/polygerrit-ui/app/utils/page-wrapper-utils.ts
+++ b/polygerrit-ui/app/utils/page-wrapper-utils.ts
@@ -17,6 +17,7 @@
replace(path: string, state: null, init: boolean, dispatch: boolean): void;
base(url: string): void;
start(): void;
+ stop(): void;
exit(pattern: string | RegExp, ...pageCallback: PageCallback[]): void;
}
@@ -37,6 +38,7 @@
next: PageNextCallback
) => void;
-// TODO: Convert page usages to the real types and remove this file of wrapper
-// types. Also remove workarounds in rollup config.
-export const page = pagejs as unknown as Page;
+// Must only be used by gr-router!
+// TODO: Move this into gr-router. Note that there is a Google import rule
+// that would need to be modified.
+export const page = pagejs as unknown as {create(): Page};
diff --git a/tools/deps.bzl b/tools/deps.bzl
index ed8d65f5..28904a9 100644
--- a/tools/deps.bzl
+++ b/tools/deps.bzl
@@ -2,7 +2,7 @@
CAFFEINE_VERS = "2.9.2"
ANTLR_VERS = "3.5.2"
-COMMONMARK_VERS = "0.10.0"
+COMMONMARK_VERSION = "0.21.0"
FLEXMARK_VERS = "0.50.50"
GREENMAIL_VERS = "1.5.5"
MAIL_VERS = "1.6.0"
@@ -14,7 +14,7 @@
AUTO_VALUE_GSON_VERSION = "1.3.1"
PROLOG_VERS = "1.4.4"
PROLOG_REPO = GERRIT
-GITILES_VERS = "1.0.0"
+GITILES_VERS = "1.1.0"
GITILES_REPO = GERRIT
# When updating Bouncy Castle, also update it in bazlets.
@@ -172,26 +172,26 @@
# commonmark must match the version used in Gitiles
maven_jar(
name = "commonmark",
- artifact = "com.atlassian.commonmark:commonmark:" + COMMONMARK_VERS,
- sha1 = "119cb7bedc3570d9ecb64ec69ab7686b5c20559b",
+ artifact = "org.commonmark:commonmark:" + COMMONMARK_VERSION,
+ sha1 = "c98f0473b17c87fe4fa2fc62a7c6523a2fe018f0",
)
maven_jar(
name = "cm-autolink",
- artifact = "com.atlassian.commonmark:commonmark-ext-autolink:" + COMMONMARK_VERS,
- sha1 = "a6056a5efbd68f57d420bc51bbc54b28a5d3c56b",
+ artifact = "org.commonmark:commonmark-ext-autolink:" + COMMONMARK_VERSION,
+ sha1 = "55c0312cf443fa3d5af0daeeeca00d6deee3cf90",
)
maven_jar(
name = "gfm-strikethrough",
- artifact = "com.atlassian.commonmark:commonmark-ext-gfm-strikethrough:" + COMMONMARK_VERS,
- sha1 = "40837da951b421b545edddac57012e15fcc9e63c",
+ artifact = "org.commonmark:commonmark-ext-gfm-strikethrough:" + COMMONMARK_VERSION,
+ sha1 = "953f4b71e133a98fcca93f3c3f4e58b895b76d1f",
)
maven_jar(
name = "gfm-tables",
- artifact = "com.atlassian.commonmark:commonmark-ext-gfm-tables:" + COMMONMARK_VERS,
- sha1 = "c075db2a3301100cf70c7dced8ecf86b494458a2",
+ artifact = "org.commonmark:commonmark-ext-gfm-tables:" + COMMONMARK_VERSION,
+ sha1 = "fb7d65fa89a4cfcd2f51535d2549b570cf1dbd1a",
)
maven_jar(
@@ -347,8 +347,8 @@
# Transitive dependency of flexmark and gitiles
maven_jar(
name = "autolink",
- artifact = "org.nibor.autolink:autolink:0.7.0",
- sha1 = "649f9f13422cf50c926febe6035662ae25dc89b2",
+ artifact = "org.nibor.autolink:autolink:0.10.0",
+ sha1 = "6579ea7079be461e5ffa99f33222a632711cc671",
)
maven_jar(
@@ -527,14 +527,14 @@
artifact = "com.google.gitiles:blame-cache:" + GITILES_VERS,
attach_source = False,
repository = GITILES_REPO,
- sha1 = "f46833f8aa6f33ce3e443c8a414c295559eaf43e",
+ sha1 = "31c1a6e5d92b57bb2f9db24e1032145961c09a8d",
)
maven_jar(
name = "gitiles-servlet",
artifact = "com.google.gitiles:gitiles-servlet:" + GITILES_VERS,
repository = GITILES_REPO,
- sha1 = "90e107da00c2cd32490dd9ae8e3fb1ee095ea675",
+ sha1 = "c6550362c5c22d8e07edd4e2151ee12594082e76",
)
# prettify must match the version used in Gitiles
