Merge "Merge branch 'stable-3.3'"
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index a809eab..068cbea 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1736,7 +1736,9 @@
Creates a new branch.
In the request body additional data for the branch can be provided as
-link:#branch-input[BranchInput].
+link:#branch-input[BranchInput]. The link:#branch-id[\{branch-id\}] in the URL
+should exactly match with the `ref` field of link:#branch-input[BranchInput], or
+otherwise the request would fail with `400 Bad Request`.
.Request
----
diff --git a/java/com/google/gerrit/server/restapi/project/CreateBranch.java b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
index b901057..2fd2d65 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateBranch.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateBranch.java
@@ -109,6 +109,12 @@
+ MagicBranch.getMagicRefNamePrefix(ref)
+ "\"");
}
+ if (!isBranchAllowed(ref)) {
+ throw new BadRequestException(
+ "Cannot create a branch with name \""
+ + ref
+ + "\". Not allowed to create branches under Gerrit internal or tags refs.");
+ }
BranchNameKey name = BranchNameKey.create(rsrc.getNameKey(), ref);
try (Repository repo = repoManager.openRepository(rsrc.getNameKey())) {
@@ -187,4 +193,9 @@
throw new BadRequestException("invalid revision \"" + input.revision + "\"", e);
}
}
+
+ /** Branches cannot be created under any Gerrit internal or tags refs. */
+ private boolean isBranchAllowed(String branch) {
+ return !RefNames.isGerritRef(branch) && !branch.startsWith(RefNames.REFS_TAGS);
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index a539bd5..129b546 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -190,15 +190,10 @@
.add(allow(CREATE).ref("refs/*").group(REGISTERED_USERS))
.update();
- String disallowedRef = "refs/changes/00/1000"; // All Gerrit internal refs behave the same way
- requestScopeOperations.setApiUser(admin.id());
- BranchNameKey branchNameKey = BranchNameKey.create(project, disallowedRef);
- createBranch(branchNameKey);
-
requestScopeOperations.setApiUser(user.id());
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject = "Subject";
- ci.branch = disallowedRef;
+ ci.branch = "refs/changes/00/1000"; // disallowedRef
Throwable thrown = assertThrows(RestApiException.class, () -> gApi.changes().create(ci));
assertThat(thrown).hasMessageThat().contains("Cannot create a change on ref " + ci.branch);
@@ -213,15 +208,10 @@
.add(allow(CREATE).ref("refs/*").group(REGISTERED_USERS))
.update();
- String branchName = "refs/tags/v1.0";
- requestScopeOperations.setApiUser(admin.id());
- BranchNameKey branchNameKey = BranchNameKey.create(project, branchName);
- createBranch(branchNameKey);
-
requestScopeOperations.setApiUser(user.id());
ChangeInput ci = newChangeInput(ChangeStatus.NEW);
ci.subject = "Subject";
- ci.branch = branchName;
+ ci.branch = "refs/tags/v1.0"; // disallowed ref
Throwable thrown = assertThrows(RestApiException.class, () -> gApi.changes().create(ci));
assertThat(thrown).hasMessageThat().contains("Cannot create a change on ref " + ci.branch);
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
index 096c72b..93ce255 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/CreateBranchIT.java
@@ -207,7 +207,7 @@
}
@Test
- public void createUserBranch_Conflict() throws Exception {
+ public void createUserBranch_NotAllowed() throws Exception {
projectOperations
.project(allUsers)
.forUpdate()
@@ -217,12 +217,12 @@
assertCreateFails(
BranchNameKey.create(allUsers, RefNames.refsUsers(Account.id(1))),
RefNames.refsUsers(admin.id()),
- ResourceConflictException.class,
- "Not allowed to create user branch.");
+ BadRequestException.class,
+ "Not allowed to create branches under Gerrit internal or tags refs.");
}
@Test
- public void createGroupBranch_Conflict() throws Exception {
+ public void createGroupBranch_NotAllowed() throws Exception {
projectOperations
.project(allUsers)
.forUpdate()
@@ -232,8 +232,8 @@
assertCreateFails(
BranchNameKey.create(allUsers, RefNames.refsGroups(AccountGroup.uuid("foo"))),
RefNames.refsGroups(adminGroupUuid()),
- ResourceConflictException.class,
- "Not allowed to create group branch.");
+ BadRequestException.class,
+ "Not allowed to create branches under Gerrit internal or tags refs.");
}
@Test
@@ -355,6 +355,22 @@
}
@Test
+ public void cannotCreateBranchInGerritInternalRefsNamespace() throws Exception {
+ assertCreateFails(
+ BranchNameKey.create(project, RefNames.REFS_CHANGES + "00/1000"),
+ BadRequestException.class,
+ "Not allowed to create branches under Gerrit internal or tags refs.");
+ }
+
+ @Test
+ public void cannotCreateBranchInTagsNamespace() throws Exception {
+ assertCreateFails(
+ BranchNameKey.create(project, RefNames.REFS_TAGS + "v1.0"),
+ BadRequestException.class,
+ "Not allowed to create branches under Gerrit internal or tags refs.");
+ }
+
+ @Test
public void cannotCreateBranchWithInvalidName() throws Exception {
assertCreateFails(
BranchNameKey.create(project, RefNames.REFS_HEADS),
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 2f3d03c..c2f5678 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -761,7 +761,10 @@
/** Page.js middleware that try parse the querystring into queryMap. */
_queryStringMiddleware(ctx: PageContext, next: PageNextCallback) {
- let queryMap: Map<string, string> | URLSearchParams = new Map();
+ let queryMap: Map<string, string> | URLSearchParams = new Map<
+ string,
+ string
+ >();
if (ctx.querystring) {
// https://caniuse.com/#search=URLSearchParams
if (window.URLSearchParams) {
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
index 197a0c4..f85667c 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.ts
@@ -299,7 +299,7 @@
lastNotify: {left: 1, right: 1},
};
- const rangesCache = new Map();
+ const rangesCache = new Map<string, SyntaxLayerRange[]>();
this._processPromise = util.makeCancelable(
this._loadHLJS().then(
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
index a6c4201..a6e881f 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -30,6 +30,7 @@
import {ChangeInfo, AccountInfo, ServerInfo} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {fireEvent} from '../../../utils/event-util';
+import {isInvolved} from '../../../utils/change-util';
@customElement('gr-account-label')
export class GrAccountLabel extends GestureEventListeners(
@@ -260,15 +261,39 @@
};
}
+ _computeAttentionButtonEnabled(
+ config: ServerInfo | undefined,
+ highlight: boolean,
+ account: AccountInfo,
+ change: ChangeInfo,
+ selfAccount: AccountInfo
+ ) {
+ return (
+ this._hasUnforcedAttention(config, highlight, account, change) &&
+ isInvolved(change, selfAccount)
+ );
+ }
+
_computeAttentionIconTitle(
config: ServerInfo | undefined,
highlight: boolean,
account: AccountInfo,
- change: ChangeInfo
+ change: ChangeInfo,
+ selfAccount: AccountInfo,
+ force: boolean
) {
- return this._hasUnforcedAttention(config, highlight, account, change)
+ const enabled = this._computeAttentionButtonEnabled(
+ config,
+ highlight,
+ account,
+ change,
+ selfAccount
+ );
+ return enabled
? 'Click to remove the user from the attention set'
- : 'Disabled. Use "Modify" to make changes.';
+ : force
+ ? 'Disabled. Use "Modify" to make changes.'
+ : 'Disabled. Only involved users can change.';
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
index b62df9e..39447ad 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
@@ -109,9 +109,9 @@
link=""
aria-label="Remove user from attention set"
on-click="_handleRemoveAttentionClick"
- disabled="[[!_hasUnforcedAttention(_config, highlightAttention, account, change)]]"
- has-tooltip="[[_hasUnforcedAttention(_config, highlightAttention, account, change)]]"
- title="[[_computeAttentionIconTitle(_config, highlightAttention, account, change)]]"
+ disabled="[[!_computeAttentionButtonEnabled(_config, highlightAttention, account, change, _selfAccount)]]"
+ has-tooltip="[[_computeAttentionButtonEnabled(_config, highlightAttention, account, change, _selfAccount)]]"
+ title="[[_computeAttentionIconTitle(_config, highlightAttention, account, change, _selfAccount, forceAttention)]]"
><iron-icon class="attention" icon="gr-icons:attention"></iron-icon>
</gr-button>
</template>
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.js b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.js
index 42b1dd7..6912776 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.js
@@ -82,16 +82,21 @@
});
suite('attention set', () => {
- setup(() => {
+ setup(async () => {
+ const kermit = createAccount('kermit', 31);
element.highlightAttention = true;
element._config = {
change: {enable_attention_set: true},
user: {anonymous_coward_name: 'Anonymous Coward'},
};
- element._selfAccount = createAccount('kermit', 31);
+ element._selfAccount = kermit;
element.account = createAccount('ernie', 42);
- element.change = {attention_set: {42: {}}};
- flush();
+ element.change = {
+ attention_set: {42: {}},
+ owner: kermit,
+ reviewers: {},
+ };
+ await flush();
});
test('show attention button', () => {
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 3dd851e..644b0ae 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -312,6 +312,14 @@
);
}
+ _handlePortedMessageClick() {
+ if (!this.comment) throw new Error('comment not set');
+ this.reporting.reportInteraction('navigate-to-original-comment', {
+ line: this.comment.line,
+ range: this.comment.range,
+ });
+ }
+
@observe('editing')
_onEditingChange(editing?: boolean) {
this.dispatchEvent(
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
index fbf7f47..02fce5b 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
@@ -265,7 +265,7 @@
</template>
<template is="dom-if" if="[[showPortedComment]]">
<a href="[[_getUrlForComment(comment)]]"
- ><span class="portedMessage"
+ ><span class="portedMessage" on-click="_handlePortedMessageClick"
>Ported from patchset [[comment.patch_set]]</span
></a
>
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
index 761e670..f6c4c1c 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
@@ -43,8 +43,8 @@
isAttentionSetEnabled,
} from '../../../utils/attention-set-util';
import {ReviewerState} from '../../../constants/constants';
-import {isRemovableReviewer} from '../../../utils/change-util';
import {CURRENT} from '../../../utils/patch-set-util';
+import {isInvolved, isRemovableReviewer} from '../../../utils/change-util';
@customElement('gr-hovercard-account')
export class GrHovercardAccount extends GestureEventListeners(
@@ -219,15 +219,13 @@
}
_computeShowActionAddToAttentionSet() {
- return (
- this._selfAccount && this.isAttentionEnabled && !this.hasUserAttention
- );
+ const involved = isInvolved(this.change, this._selfAccount);
+ return involved && this.isAttentionEnabled && !this.hasUserAttention;
}
_computeShowActionRemoveFromAttentionSet() {
- return (
- this._selfAccount && this.isAttentionEnabled && this.hasUserAttention
- );
+ const involved = isInvolved(this.change, this._selfAccount);
+ return involved && this.isAttentionEnabled && this.hasUserAttention;
}
_handleClickAddToAttentionSet() {
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
index 99d3d6c..42bd76c 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
@@ -144,7 +144,7 @@
</template>
<template
is="dom-if"
- if="[[_computeShowActionAddToAttentionSet(_config, highlightAttention, account, change)]]"
+ if="[[_computeShowActionAddToAttentionSet(_config, highlightAttention, account, change, _selfAccount)]]"
>
<div class="action">
<gr-button
@@ -159,7 +159,7 @@
</template>
<template
is="dom-if"
- if="[[_computeShowActionRemoveFromAttentionSet(_config, highlightAttention, account, change)]]"
+ if="[[_computeShowActionRemoveFromAttentionSet(_config, highlightAttention, account, change, _selfAccount)]]"
>
<div class="action">
<gr-button
@@ -172,7 +172,10 @@
</gr-button>
</div>
</template>
- <template is="dom-if" if="[[_showReviewerOrCCActions(account, change)]]">
+ <template
+ is="dom-if"
+ if="[[_showReviewerOrCCActions(account, change, _selfAccount)]]"
+ >
<div class="action">
<gr-button
class="removeReviewerOrCC"
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
index c8b4b42..8d1f499 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
@@ -19,6 +19,7 @@
import './gr-hovercard-account.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {ReviewerState} from '../../../constants/constants.js';
+import {appContext} from '../../../services/app-context.js';
const basicFixture = fixtureFromTemplate(html`
<gr-hovercard-account class="hovered"></gr-hovercard-account>
@@ -34,22 +35,22 @@
_account_id: '31415926535',
};
- setup(() => {
- element = basicFixture.instantiate();
- sinon.stub(element.restApiService, 'getAccount').returns(
- new Promise(resolve => { '2'; })
+ setup(async () => {
+ sinon.stub(appContext.restApiService, 'getAccount').returns(
+ Promise.resolve({...ACCOUNT})
);
-
- element._selfAccount = {...ACCOUNT};
+ sinon.stub(appContext.restApiService, 'getConfig').returns(
+ Promise.resolve({change: {enable_attention_set: true}})
+ );
+ element = basicFixture.instantiate();
element.account = {...ACCOUNT};
- element._config = {
- change: {enable_attention_set: true},
- };
element.change = {
attention_set: {},
+ reviewers: {},
+ owner: {...ACCOUNT},
};
element.show({});
- flush();
+ await flush();
});
teardown(() => {
@@ -242,7 +243,11 @@
sinon.stub(element.restApiService, 'removeFromAttentionSet')
.callsFake(() => apiPromise);
element.highlightAttention = true;
- element.change = {attention_set: {31415926535: {}}};
+ element.change = {
+ attention_set: {31415926535: {}},
+ reviewers: {},
+ owner: {...ACCOUNT},
+ };
element._target = document.createElement('div');
flush();
const showAlertListener = sinon.spy();
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
index 43a6af4..616ec02 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-apis/gr-rest-api-helper.ts
@@ -74,7 +74,10 @@
// Returns the cache for the current canonical path.
_cache(): Map<string, unknown> {
if (!this._data.has(window.CANONICAL_PATH)) {
- this._data.set(window.CANONICAL_PATH, new Map());
+ this._data.set(
+ window.CANONICAL_PATH,
+ new Map<string, ParsedJSON | null>()
+ );
}
return this._data.get(window.CANONICAL_PATH) as Map<
string,
@@ -111,7 +114,7 @@
}
invalidatePrefix(prefix: string) {
- const newMap = new Map();
+ const newMap = new Map<string, unknown>();
for (const [key, value] of this._cache().entries()) {
if (!key.startsWith(prefix)) {
newMap.set(key, value);
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
index a474909..881e102 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
@@ -796,10 +796,10 @@
_shortcut_v_key_last_pressed: number | null = null;
@property({type: Object})
- _shortcut_go_table: Map<string, string> = new Map();
+ _shortcut_go_table: Map<string, string> = new Map<string, string>();
@property({type: Object})
- _shortcut_v_table: Map<string, string> = new Map();
+ _shortcut_v_table: Map<string, string> = new Map<string, string>();
Shortcut = Shortcut;
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 13a9c76..5c5a37c 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -27,7 +27,7 @@
type ServiceCreator<T> = () => T;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
-const initializedServices: Map<ServiceName, any> = new Map();
+const initializedServices: Map<ServiceName, any> = new Map<ServiceName, any>();
function getService<K extends ServiceName>(
serviceName: K,
diff --git a/polygerrit-ui/app/services/gr-event-interface/gr-event-interface_impl.ts b/polygerrit-ui/app/services/gr-event-interface/gr-event-interface_impl.ts
index f1e0990..d8c5d77 100644
--- a/polygerrit-ui/app/services/gr-event-interface/gr-event-interface_impl.ts
+++ b/polygerrit-ui/app/services/gr-event-interface/gr-event-interface_impl.ts
@@ -127,7 +127,7 @@
if (eventName) {
this._listenersMap.set(eventName, []);
} else {
- this._listenersMap = new Map();
+ this._listenersMap = new Map<string, EventCallback[]>();
}
}
}
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index 47924e6..ff9fff8 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -177,6 +177,30 @@
return change.owner?._account_id === account._account_id;
}
+export function isReviewer(change?: ChangeInfo, account?: AccountInfo) {
+ if (!change || !account) return false;
+ const reviewers = change.reviewers.REVIEWER ?? [];
+ return reviewers.some(r => r._account_id === account._account_id);
+}
+
+export function isUploader(change?: ChangeInfo, account?: AccountInfo) {
+ if (!change || !account) return false;
+ const rev = getCurrentRevision(change);
+ return rev?.uploader._account_id === account._account_id;
+}
+
+export function isInvolved(change?: ChangeInfo, account?: AccountInfo) {
+ const owner = isOwner(change, account);
+ const uploader = isUploader(change, account);
+ const reviewer = isReviewer(change, account);
+ return owner || uploader || reviewer;
+}
+
+export function getCurrentRevision(change?: ChangeInfo) {
+ if (!change?.revisions || !change?.current_revision) return undefined;
+ return change.revisions[change.current_revision];
+}
+
export function changeStatusString(change: ChangeInfo) {
return changeStatuses(change).join(', ');
}
diff --git a/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index cb0bdec..320cc80 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -225,7 +225,7 @@
return patchNums;
}
// TODO(TS): replace with Map<PatchNum, boolean>
- const psWip: Map<string, boolean> = new Map();
+ const psWip: Map<string, boolean> = new Map<string, boolean>();
let wip = !!change.work_in_progress;
for (let i = 0; i < change.messages.length; i++) {
const msg = change.messages[i];
