Merge "Make IllegalLabelException a checked Exception"
diff --git a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
index 5110a28..9936730 100644
--- a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
+++ b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html
@@ -46,6 +46,17 @@
patchNumEquals(a, b) {
return a + '' === b + '';
},
+
+ /**
+ * Whether the given patch is a numbered parent of a merge (i.e. a negative
+ * number).
+ * @param {string|number} n
+ * @return {Boolean}
+ */
+ isMergeParent(n) {
+ return (n + '')[0] === '-';
+ },
+
/**
* Given an object of revisions, get a particular revision based on patch
* num.
@@ -192,6 +203,13 @@
return allPatchSets[allPatchSets.length - 1].num;
},
+ /** @return {Boolean} */
+ hasEditBasedOnCurrentPatchSet(allPatchSets) {
+ if (!allPatchSets || !allPatchSets.length) { return false; }
+ return allPatchSets[allPatchSets.length - 1].num ===
+ Gerrit.PatchSetBehavior.EDIT_NAME;
+ },
+
/**
* Check whether there is no newer patch than the latest patch that was
* available when this change was loaded.
diff --git a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior_test.html b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior_test.html
index c2ccccd..54c1355 100644
--- a/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/gr-patch-set-behavior/gr-patch-set-behavior_test.html
@@ -174,6 +174,20 @@
assert.isTrue(equals('PARENT', 'PARENT'));
});
+ test('isMergeParent', () => {
+ const isParent = Gerrit.PatchSetBehavior.isMergeParent;
+ assert.isFalse(isParent(1));
+ assert.isFalse(isParent(4321));
+ assert.isFalse(isParent('52'));
+ assert.isFalse(isParent('edit'));
+ assert.isFalse(isParent('PARENT'));
+ assert.isFalse(isParent(0));
+
+ assert.isTrue(isParent(-23));
+ assert.isTrue(isParent(-1));
+ assert.isTrue(isParent('-42'));
+ });
+
test('findEditParentRevision', () => {
const findParent = Gerrit.PatchSetBehavior.findEditParentRevision;
let revisions = [
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index f2ed33a..b7a048b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -157,6 +157,19 @@
Do you really want to delete the change?
</div>
</gr-confirm-dialog>
+ <gr-confirm-dialog
+ id="confirmDeleteEditDialog"
+ class="confirmDialog"
+ confirm-label="Delete"
+ on-cancel="_handleConfirmDialogCancel"
+ on-confirm="_handleDeleteEditConfirm">
+ <div class="header">
+ Delete Change Edit
+ </div>
+ <div class="main">
+ Do you really want to delete the edit?
+ </div>
+ </gr-confirm-dialog>
</gr-overlay>
<gr-js-api-interface id="jsAPI"></gr-js-api-interface>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index ff77142..5657cbe 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -51,11 +51,14 @@
const ChangeActions = {
ABANDON: 'abandon',
DELETE: '/',
+ DELETE_EDIT: 'deleteEdit',
IGNORE: 'ignore',
MOVE: 'move',
MUTE: 'mute',
PRIVATE: 'private',
PRIVATE_DELETE: 'private.delete',
+ PUBLISH_EDIT: 'publishEdit',
+ REBASE_EDIT: 'rebaseEdit',
RESTORE: 'restore',
REVERT: 'revert',
UNIGNORE: 'unignore',
@@ -118,6 +121,36 @@
__type: 'revision',
};
+ const REBASE_EDIT = {
+ enabled: true,
+ label: 'Rebase Edit',
+ title: 'Rebase change edit',
+ __key: 'rebaseEdit',
+ __primary: false,
+ __type: 'change',
+ method: 'POST',
+ };
+
+ const PUBLISH_EDIT = {
+ enabled: true,
+ label: 'Publish Edit',
+ title: 'Publish change edit',
+ __key: 'publishEdit',
+ __primary: false,
+ __type: 'change',
+ method: 'POST',
+ };
+
+ const DELETE_EDIT = {
+ enabled: true,
+ label: 'Delete Edit',
+ title: 'Delete change edit',
+ __key: 'deleteEdit',
+ __primary: false,
+ __type: 'change',
+ method: 'DELETE',
+ };
+
const AWAIT_CHANGE_ATTEMPTS = 5;
const AWAIT_CHANGE_TIMEOUT_MS = 1000;
@@ -276,6 +309,14 @@
type: Array,
value() { return []; },
},
+ editLoaded: {
+ type: Boolean,
+ value: false,
+ },
+ editBasedOnCurrentPatchSet: {
+ type: Boolean,
+ value: true,
+ },
},
ActionType,
@@ -288,7 +329,8 @@
],
observers: [
- '_actionsChanged(actions.*, revisionActions.*, _additionalActions.*)',
+ '_actionsChanged(actions.*, revisionActions.*, _additionalActions.*, ' +
+ 'editLoaded, editBasedOnCurrentPatchSet, change)',
],
listeners: {
@@ -422,7 +464,8 @@
},
_actionsChanged(actionsChangeRecord, revisionActionsChangeRecord,
- additionalActionsChangeRecord) {
+ additionalActionsChangeRecord, editLoaded, editBasedOnCurrentPatchSet,
+ change) {
const additionalActions = (additionalActionsChangeRecord &&
additionalActionsChangeRecord.base) || [];
this.hidden = this._keyCount(actionsChangeRecord) === 0 &&
@@ -436,6 +479,47 @@
!revisionActions.download) {
this.set('revisionActions.download', DOWNLOAD_ACTION);
}
+
+ const changeActions = actionsChangeRecord.base || {};
+ if (Object.keys(changeActions).length !== 0) {
+ if (editLoaded) {
+ if (this.changeIsOpen(change.status)) {
+ if (editBasedOnCurrentPatchSet) {
+ if (!changeActions.publishEdit) {
+ this.set('actions.publishEdit', PUBLISH_EDIT);
+ }
+ if (changeActions.rebaseEdit) {
+ delete this.actions.rebaseEdit;
+ this.notifyPath('actions.rebaseEdit');
+ }
+ } else {
+ if (!changeActions.rebasEdit) {
+ this.set('actions.rebaseEdit', REBASE_EDIT);
+ }
+ if (changeActions.publishEdit) {
+ delete this.actions.publishEdit;
+ this.notifyPath('actions.publishEdit');
+ }
+ }
+ }
+ if (!changeActions.deleteEdit) {
+ this.set('actions.deleteEdit', DELETE_EDIT);
+ }
+ } else {
+ if (changeActions.publishEdit) {
+ delete this.actions.publishEdit;
+ this.notifyPath('actions.publishEdit');
+ }
+ if (changeActions.rebaseEdit) {
+ delete this.actions.rebaseEdit;
+ this.notifyPath('actions.rebaseEdit');
+ }
+ if (changeActions.deleteEdit) {
+ delete this.actions.deleteEdit;
+ this.notifyPath('actions.deleteEdit');
+ }
+ }
+ }
},
_getValuesFor(obj) {
@@ -638,12 +722,21 @@
case ChangeActions.DELETE:
this._handleDeleteTap();
break;
+ case ChangeActions.DELETE_EDIT:
+ this._handleDeleteEditTap();
+ break;
case ChangeActions.WIP:
this._handleWipTap();
break;
case ChangeActions.MOVE:
this._handleMoveTap();
break;
+ case ChangeActions.PUBLISH_EDIT:
+ this._handlePublishEditTap();
+ break;
+ case ChangeActions.REBASE_EDIT:
+ this._handleRebaseEditTap();
+ break;
default:
this._fireAction(this._prependSlash(key), this.actions[key], false);
}
@@ -775,6 +868,12 @@
this._fireAction('/', this.actions[ChangeActions.DELETE], false);
},
+ _handleDeleteEditConfirm() {
+ this._hideAllDialogs();
+
+ this._fireAction('/edit', this.actions.deleteEdit, false);
+ },
+
_getActionOverflowIndex(type, key) {
return this._overflowActions.findIndex(action => {
return action.type === type && action.key === key;
@@ -862,6 +961,9 @@
}
break;
case ChangeActions.WIP:
+ case ChangeActions.DELETE_EDIT:
+ case ChangeActions.PUBLISH_EDIT:
+ case ChangeActions.REBASE_EDIT:
page.show(this.changePath(this.changeNum));
break;
default:
@@ -949,10 +1051,22 @@
this._showActionDialog(this.$.confirmDeleteDialog);
},
+ _handleDeleteEditTap() {
+ this._showActionDialog(this.$.confirmDeleteEditDialog);
+ },
+
_handleWipTap() {
this._fireAction('/wip', this.actions.wip, false);
},
+ _handlePublishEditTap() {
+ this._fireAction('/edit:publish', this.actions.publishEdit, false);
+ },
+
+ _handleRebaseEditTap() {
+ this._fireAction('/edit:rebase', this.actions.rebaseEdit, false);
+ },
+
_handleHideBackgroundContent() {
this.$.mainContent.classList.add('overlayOpen');
},
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 11a2569..2c252eb 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -360,6 +360,137 @@
assert.isFalse(element.$.mainContent.classList.contains('overlayOpen'));
});
+ suite('change edits', () => {
+ let fireActionStub;
+ const deleteEditAction = {
+ enabled: true,
+ label: 'Delete Edit',
+ title: 'Delete change edit',
+ __key: 'deleteEdit',
+ __primary: false,
+ __type: 'change',
+ method: 'DELETE',
+ };
+ const publishEditAction = {
+ enabled: true,
+ label: 'Publish Edit',
+ title: 'Publish change edit',
+ __key: 'publishEdit',
+ __primary: false,
+ __type: 'change',
+ method: 'POST',
+ };
+ const rebaseEditAction = {
+ enabled: true,
+ label: 'Rebase Edit',
+ title: 'Rebase change edit',
+ __key: 'rebaseEdit',
+ __primary: false,
+ __type: 'change',
+ method: 'POST',
+ };
+
+ setup(() => {
+ fireActionStub = sandbox.stub(element, '_fireAction');
+ element.patchNum = 'edit';
+ element.editLoaded = true;
+ });
+
+ test('does not delete edit on action', () => {
+ element._handleDeleteEditTap();
+ assert.isFalse(fireActionStub.called);
+ });
+
+ test('shows confirm dialog for delete edit', () => {
+ element._handleDeleteEditTap();
+ assert.isFalse(element.$$('#confirmDeleteEditDialog').hidden);
+ assert.ok(element.$$('gr-button[data-action-key="deleteEdit"]'));
+ MockInteractions.tap(
+ element.$$('#confirmDeleteEditDialog').$$('gr-button[primary]'));
+ flushAsynchronousOperations();
+ assert.isTrue(
+ fireActionStub.calledWith('/edit', deleteEditAction, false));
+ });
+
+ test('show publish edit but rebaseEdit is hidden', () => {
+ element.change = {
+ status: 'NEW',
+ };
+ const rebaseEditButton =
+ element.$$('gr-button[data-action-key="rebaseEdit"]');
+ assert.isNotOk(rebaseEditButton);
+
+ const publishEditButton =
+ element.$$('gr-button[data-action-key="publishEdit"]');
+ assert.ok(publishEditButton);
+ MockInteractions.tap(publishEditButton);
+ element._handlePublishEditTap();
+ flushAsynchronousOperations();
+
+ assert.isTrue(
+ fireActionStub.calledWith('/edit:publish', publishEditAction, false));
+ });
+
+ test('show rebase edit but publishEdit is hidden', () => {
+ element.change = {
+ status: 'NEW',
+ };
+ element.editBasedOnCurrentPatchSet = false;
+
+ const publishEditButton =
+ element.$$('gr-button[data-action-key="publishEdit"]');
+ assert.isNotOk(publishEditButton);
+
+ const rebaseEditButton =
+ element.$$('gr-button[data-action-key="rebaseEdit"]');
+ assert.ok(rebaseEditButton);
+ MockInteractions.tap(rebaseEditButton);
+ element._handleRebaseEditTap();
+ flushAsynchronousOperations();
+
+ assert.isTrue(
+ fireActionStub.calledWith('/edit:rebase', rebaseEditAction, false));
+ });
+
+ test('hide publishEdit and rebaseEdit if change is not open', () => {
+ element.change = {
+ status: 'MERGED',
+ };
+ flushAsynchronousOperations();
+
+ const publishEditButton =
+ element.$$('gr-button[data-action-key="publishEdit"]');
+ assert.isNotOk(publishEditButton);
+
+ const rebaseEditButton =
+ element.$$('gr-button[data-action-key="rebaseEdit"]');
+ assert.isNotOk(rebaseEditButton);
+
+ const deleteEditButton =
+ element.$$('gr-button[data-action-key="deleteEdit"]');
+ assert.ok(deleteEditButton);
+ });
+
+ test('do not show delete edit on a non change edit', () => {
+ element.editLoaded = false;
+ flushAsynchronousOperations();
+ const deleteEditButton =
+ element.$$('gr-button[data-action-key="deleteEdit"]');
+ assert.isNotOk(deleteEditButton);
+ });
+
+ test('do not show publish edit on a non change edit', () => {
+ element.change = {
+ status: 'NEW',
+ };
+ element.editLoaded = false;
+ flushAsynchronousOperations();
+ const publishEditButton =
+ element.$$('gr-button[data-action-key="publishEdit"]');
+ assert.isNotOk(publishEditButton);
+ });
+ });
+
suite('cherry-pick', () => {
let fireActionStub;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index dbba7ad..031acda 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -351,6 +351,8 @@
commit-num="[[_commitInfo.commit]]"
patch-num="[[computeLatestPatchNum(_allPatchSets)]]"
commit-message="[[_latestCommitMessage]]"
+ edit-loaded="[[_editLoaded]]"
+ edit-based-on-current-patch-set="[[hasEditBasedOnCurrentPatchSet(_allPatchSets)]]"
on-reload-change="_handleReloadChange"
on-download-tap="_handleOpenDownloadDialog"></gr-change-actions>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
index b4ae93f..5c2422d 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.html
@@ -44,13 +44,15 @@
display: initial;
}
.patchInfo-header {
- padding: .5em calc(var(--default-horizontal-margin) / 2);
- }
- .patchInfo-header {
background-color: #f6f6f6;
border-bottom: 1px solid #ebebeb;
display: flex;
- justify-content: space-between;
+ padding: .5em calc(var(--default-horizontal-margin) / 2);
+ }
+ .patchInfo-header-wrapper {
+ align-items: center;
+ display: flex;
+ width: 100%;
}
.latestPatchContainer {
display: none;
@@ -63,11 +65,9 @@
}
#diffPrefsContainer,
.rightControls {
+ align-self: flex-end;
margin: auto 0 auto auto;
}
- .patchInfo-header-wrapper {
- width: 100%;
- }
.showOnEdit {
display: none;
}
@@ -78,12 +78,13 @@
display: initial;
}
.fileList-header {
+ align-items: center;
display: flex;
font-weight: bold;
- justify-content: space-between;
- margin: .5em calc(var(--default-horizontal-margin) / 2);
+ margin: .5em calc(var(--default-horizontal-margin) / 2) 0;
}
.rightControls {
+ align-items: center;
display: flex;
flex-wrap: wrap;
font-weight: normal;
@@ -109,40 +110,42 @@
</style>
<div class$="patchInfo-header [[_computeEditLoadedClass(editLoaded)]] [[_computePatchInfoClass(patchRange.patchNum, allPatchSets)]]">
<div class="patchInfo-header-wrapper">
- <gr-patch-range-select
- id="rangeSelect"
- comments="[[comments]]"
- change-num="[[changeNum]]"
- patch-range="[[patchRange]]"
- available-patches="[[allPatchSets]]"
- revisions="[[change.revisions]]"
- on-patch-range-change="_handlePatchChange">
- </gr-patch-range-select>
- /
- <gr-commit-info
- change="[[change]]"
- server-config="[[serverConfig]]"
- commit-info="[[commitInfo]]"></gr-commit-info>
- <span class="latestPatchContainer">
+ <div>
+ <gr-patch-range-select
+ id="rangeSelect"
+ comments="[[comments]]"
+ change-num="[[changeNum]]"
+ patch-range="[[patchRange]]"
+ available-patches="[[allPatchSets]]"
+ revisions="[[change.revisions]]"
+ on-patch-range-change="_handlePatchChange">
+ </gr-patch-range-select>
/
- <a href$="[[changeUrl]]">Go to latest patch set</a>
- </span>
- <span class="downloadContainer desktop">
- /
- <gr-button link
- class="download"
- on-tap="_handleDownloadTap">Download</gr-button>
- </span>
- <span class="descriptionContainer hideOnEdit">
- /
- <gr-editable-label
- id="descriptionLabel"
- class="descriptionLabel"
- value="[[_computePatchSetDescription(change, patchRange.patchNum)]]"
- placeholder="[[_computeDescriptionPlaceholder(_descriptionReadOnly)]]"
- read-only="[[_descriptionReadOnly]]"
- on-changed="_handleDescriptionChanged"></gr-editable-label>
- </span>
+ <gr-commit-info
+ change="[[change]]"
+ server-config="[[serverConfig]]"
+ commit-info="[[commitInfo]]"></gr-commit-info>
+ <span class="latestPatchContainer">
+ /
+ <a href$="[[changeUrl]]">Go to latest patch set</a>
+ </span>
+ <span class="downloadContainer desktop">
+ /
+ <gr-button link
+ class="download"
+ on-tap="_handleDownloadTap">Download</gr-button>
+ </span>
+ <span class="descriptionContainer hideOnEdit">
+ /
+ <gr-editable-label
+ id="descriptionLabel"
+ class="descriptionLabel"
+ value="[[_computePatchSetDescription(change, patchRange.patchNum)]]"
+ placeholder="[[_computeDescriptionPlaceholder(_descriptionReadOnly)]]"
+ read-only="[[_descriptionReadOnly]]"
+ on-changed="_handleDescriptionChanged"></gr-editable-label>
+ </span>
+ </div>
<span id="diffPrefsContainer"
class="hideOnEdit"
hidden$="[[_computePrefsButtonHidden(diffPrefs, loggedIn)]]"
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js
index 0a2b974..ac74ee5 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js
@@ -126,7 +126,9 @@
*/
_showAlert(text, opt_actionText, opt_actionCallback,
opt_dismissOnNavigation) {
- if (this._alertElement) { return; }
+ if (this._alertElement) {
+ this._hideAlert();
+ }
this._clearHideAlertHandle();
if (opt_dismissOnNavigation) {
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html
index 5d1c461..17fa746 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html
@@ -272,20 +272,11 @@
});
});
- test('dismissOnNavigation respected', () => {
- const asyncStub = sandbox.stub(element, 'async');
- const hideSpy = sandbox.spy(element, '_hideAlert');
- // No async call when dismissOnNavigation supplied.
- element._showAlert('test', null, null, true);
- assert.isFalse(asyncStub.called);
-
- // When page nav happens, clear alert.
- document.dispatchEvent(new CustomEvent('location-change'));
- assert.isTrue(hideSpy.called);
-
- // When timeout is not supplied, use HIDE_ALERT_TIMEOUT_MS.
- element._showAlert('test');
- assert.isTrue(asyncStub.called);
+ test('_showAlert hides existing alerts', () => {
+ element._alertElement = element._createToastAlert();
+ const hideStub = sandbox.stub(element, '_hideAlert');
+ element._showAlert();
+ assert.isTrue(hideStub.calledOnce);
});
});
</script>
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index a2861c6..acc9d4f 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -91,7 +91,7 @@
QUERY_OFFSET: '/q/:query,:offset',
// Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
- CHANGE_LEGACY: /^\/c\/(\d+)\/?(((\d+|edit)(\.\.(\d+|edit))?))?\/?$/,
+ CHANGE_LEGACY: /^\/c\/(\d+)\/?(((-?\d+|edit)(\.\.(\d+|edit))?))?\/?$/,
CHANGE_NUMBER_LEGACY: /^\/(\d+)\/?/,
// Matches
@@ -100,15 +100,15 @@
// TODO(kaspern): Migrate completely to project based URLs, with backwards
// compatibility for change-only.
// eslint-disable-next-line max-len
- CHANGE_OR_DIFF: /^\/c\/(.+)\/\+\/(\d+)(\/?((\d+|edit)(\.\.(\d+|edit))?(\/(.+))?))?\/?$/,
+ CHANGE_OR_DIFF: /^\/c\/(.+)\/\+\/(\d+)(\/?((-?\d+|edit)(\.\.(\d+|edit))?(\/(.+))?))?\/?$/,
// Matches
// /c/<project>/+/<changeNum>/[<basePatchNum>..]<patchNum>/<path>,edit
// eslint-disable-next-line max-len
- DIFF_EDIT: /^\/c\/(.+)\/\+\/(\d+)(\/(((\d+|edit)\.\.)?(edit)(\/(.+)))),edit$/,
+ DIFF_EDIT: /^\/c\/(.+)\/\+\/(\d+)(\/(((-?\d+|edit)\.\.)?(edit)(\/(.+)))),edit$/,
// Matches /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
- DIFF_LEGACY: /^\/c\/(\d+)\/((\d+|edit)(\.\.(\d+|edit))?)\/(.+)/,
+ DIFF_LEGACY: /^\/c\/(\d+)\/((-?\d+|edit)(\.\.(\d+|edit))?)\/(.+)/,
SETTINGS: /^\/settings\/?/,
SETTINGS_LEGACY: /^\/settings\/VE\/(\S+)/,
@@ -298,8 +298,17 @@
const hasPatchNum = params.patchNum !== null &&
params.patchNum !== undefined;
let needsRedirect = false;
+
+ // Diffing a patch against itself is invalid, so if the base and revision
+ // patches are equal clear the base.
+ // NOTE: while selecting numbered parents of a merge is not yet
+ // implemented, normalize parent base patches to be un-selected parents in
+ // the same way.
+ // TODO(issue 4760): Remove the isMergeParent check when PG supports
+ // diffing against numbered parents of a merge.
if (hasBasePatchNum &&
- this.patchNumEquals(params.basePatchNum, params.patchNum)) {
+ (this.patchNumEquals(params.basePatchNum, params.patchNum) ||
+ this.isMergeParent(params.basePatchNum))) {
needsRedirect = true;
params.basePatchNum = null;
} else if (hasBasePatchNum && !hasPatchNum) {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
index df46a38..a9231b6 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.html
@@ -376,6 +376,16 @@
assert.isNotOk(params.basePatchNum);
assert.equal(params.patchNum, 'edit');
});
+
+ // TODO(issue 4760): Remove when PG supports diffing against numbered
+ // parents of a merge.
+ test('range -n..m normalizes to m', () => {
+ const params = {basePatchNum: -2, patchNum: 4};
+ const needsRedirect = element._normalizePatchRangeParams(params);
+ assert.isTrue(needsRedirect);
+ assert.isNotOk(params.basePatchNum);
+ assert.equal(params.patchNum, 4);
+ });
});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 4f0ebcc..bda9a35 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -15,6 +15,12 @@
'use strict';
const STORAGE_DEBOUNCE_INTERVAL = 400;
+ const TOAST_DEBOUNCE_INTERVAL = 200;
+
+ const SAVING_MESSAGE = 'Saving';
+ const DRAFT_SINGULAR = 'draft...';
+ const DRAFT_PLURAL = 'drafts...';
+ const SAVED_MESSAGE = 'All changes saved';
Polymer({
is: 'gr-diff-comment',
@@ -109,6 +115,11 @@
type: Boolean,
observer: '_toggleResolved',
},
+
+ _numPendingDiffRequests: {
+ type: Object,
+ value: {number: 0}, // Intentional to share the object across instances.
+ },
},
observers: [
@@ -424,13 +435,52 @@
});
},
+ _getSavingMessage(numPending) {
+ if (numPending === 0) { return SAVED_MESSAGE; }
+ return [
+ SAVING_MESSAGE,
+ numPending,
+ numPending === 1 ? DRAFT_SINGULAR : DRAFT_PLURAL,
+ ].join(' ');
+ },
+
+ _showStartRequest() {
+ const numPending = ++this._numPendingDiffRequests.number;
+ this._updateRequestToast(numPending);
+ },
+
+ _showEndRequest() {
+ const numPending = --this._numPendingDiffRequests.number;
+ this._updateRequestToast(numPending);
+ },
+
+ _updateRequestToast(numPending) {
+ const message = this._getSavingMessage(numPending);
+ this.debounce('draft-toast', () => {
+ // Note: the event is fired on the body rather than this element because
+ // this element may not be attached by the time this executes, in which
+ // case the event would not bubble.
+ document.body.dispatchEvent(new CustomEvent('show-alert',
+ {detail: {message}, bubbles: true}));
+ }, TOAST_DEBOUNCE_INTERVAL);
+ },
+
_saveDraft(draft) {
- return this.$.restAPI.saveDiffDraft(this.changeNum, this.patchNum, draft);
+ this._showStartRequest();
+ return this.$.restAPI.saveDiffDraft(this.changeNum, this.patchNum, draft)
+ .then(result => {
+ this._showEndRequest();
+ return result;
+ });
},
_deleteDraft(draft) {
+ this._showStartRequest();
return this.$.restAPI.deleteDiffDraft(this.changeNum, this.patchNum,
- draft);
+ draft).then(result => {
+ this._showEndRequest();
+ return result;
+ });
},
_getPatchNum() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index bdbf4f9..2816647 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -602,5 +602,34 @@
element.comment = {unresolved: true};
assert.isFalse(element.$$('.resolve input').checked);
});
+
+ suite('draft saving messages', () => {
+ test('_getSavingMessage', () => {
+ assert.equal(element._getSavingMessage(0), 'All changes saved');
+ assert.equal(element._getSavingMessage(1), 'Saving 1 draft...');
+ assert.equal(element._getSavingMessage(2), 'Saving 2 drafts...');
+ assert.equal(element._getSavingMessage(3), 'Saving 3 drafts...');
+ });
+
+ test('_show{Start,End}Request', () => {
+ const updateStub = sandbox.stub(element, '_updateRequestToast');
+ element._numPendingDiffRequests.number = 1;
+
+ element._showStartRequest();
+ assert.isTrue(updateStub.calledOnce);
+ assert.equal(updateStub.lastCall.args[0], 2);
+ assert.equal(element._numPendingDiffRequests.number, 2);
+
+ element._showEndRequest();
+ assert.isTrue(updateStub.calledTwice);
+ assert.equal(updateStub.lastCall.args[0], 1);
+ assert.equal(element._numPendingDiffRequests.number, 1);
+
+ element._showEndRequest();
+ assert.isTrue(updateStub.calledThrice);
+ assert.equal(updateStub.lastCall.args[0], 0);
+ assert.equal(element._numPendingDiffRequests.number, 0);
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
index 5c56826..c3332b3 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js
@@ -29,6 +29,8 @@
const GERRIT_DOCS_BASE_URL = 'https://gerrit-review.googlesource.com/' +
'Documentation';
const GERRIT_DOCS_FILTER_PATH = '/user-notify.html';
+ const ABSOLUTE_URL_PATTERN = /^https?:/;
+ const TRAILING_SLASH_PATTERN = /\/$/;
Polymer({
is: 'gr-settings-view',
@@ -366,9 +368,13 @@
_getFilterDocsLink(docsBaseUrl) {
let base = docsBaseUrl;
- if (!base || !base.startsWith('http')) {
+ if (!base || !ABSOLUTE_URL_PATTERN.test(base)) {
base = GERRIT_DOCS_BASE_URL;
}
+
+ // Remove any trailing slash, since it is in the GERRIT_DOCS_FILTER_PATH.
+ base = base.replace(TRAILING_SLASH_PATTERN, '');
+
return base + GERRIT_DOCS_FILTER_PATH;
},
});
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
index a8585da..e1182c1 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html
@@ -386,6 +386,39 @@
assert.isTrue(element.$.emailEditor.loadData.calledOnce);
});
+ suite('_getFilterDocsLink', () => {
+ test('with http: docs base URL', () => {
+ const base = 'http://example.com/';
+ const result = element._getFilterDocsLink(base);
+ assert.equal(result, 'http://example.com/user-notify.html');
+ });
+
+ test('with http: docs base URL without slash', () => {
+ const base = 'http://example.com';
+ const result = element._getFilterDocsLink(base);
+ assert.equal(result, 'http://example.com/user-notify.html');
+ });
+
+ test('with https: docs base URL', () => {
+ const base = 'https://example.com/';
+ const result = element._getFilterDocsLink(base);
+ assert.equal(result, 'https://example.com/user-notify.html');
+ });
+
+ test('without docs base URL', () => {
+ const result = element._getFilterDocsLink(null);
+ assert.equal(result, 'https://gerrit-review.googlesource.com/' +
+ 'Documentation/user-notify.html');
+ });
+
+ test('ignores non HTTP links', () => {
+ const base = 'javascript://alert("evil");';
+ const result = element._getFilterDocsLink(base);
+ assert.equal(result, 'https://gerrit-review.googlesource.com/' +
+ 'Documentation/user-notify.html');
+ });
+ });
+
suite('when email verification token is provided', () => {
let resolveConfirm;
