Merge "Specify project in URLs"
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java
index bd9c98d..861a22c 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java
@@ -212,6 +212,11 @@
Header allowOrigin = r.getFirstHeader(ACCESS_CONTROL_ALLOW_ORIGIN);
assertThat(allowOrigin).named(ACCESS_CONTROL_ALLOW_ORIGIN).isNotNull();
assertThat(allowOrigin.getValue()).named(ACCESS_CONTROL_ALLOW_ORIGIN).isEqualTo(origin);
+
+ Header allowAuth = r.getFirstHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS);
+ assertThat(allowAuth).named(ACCESS_CONTROL_ALLOW_CREDENTIALS).isNotNull();
+ assertThat(allowAuth.getValue()).named(ACCESS_CONTROL_ALLOW_CREDENTIALS).isEqualTo("true");
+
checkTopic(change, "test-xd");
}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 8adf5f5..5b1045a 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -554,6 +554,7 @@
}
res.addHeader(VARY, ORIGIN);
res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, origin);
+ res.setHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS, "true");
} else if (!Strings.isNullOrEmpty(origin)) {
// All other requests must be processed, but conditionally set CORS headers.
if (globals.allowOrigin != null) {
@@ -591,7 +592,6 @@
String headers = req.getHeader(ACCESS_CONTROL_REQUEST_HEADERS);
if (headers != null) {
- res.addHeader(VARY, ACCESS_CONTROL_REQUEST_HEADERS);
for (String reqHdr : Splitter.on(',').trimResults().split(headers)) {
if (!ALLOWED_CORS_REQUEST_HEADERS.contains(reqHdr.toLowerCase(Locale.US))) {
throw new BadRequestException(reqHdr + " not allowed in CORS");
@@ -1148,7 +1148,6 @@
CurrentUser user = globals.currentUser.get();
if (isRead(req)) {
user.setAccessPath(AccessPath.REST_API);
- user.setLastLoginExternalIdKey(globals.webSession.get().getLastLoginExternalId());
} else if (user instanceof AnonymousUser) {
throw new AuthException("Authentication required");
} else if (!globals.webSession.get().isAccessPathOk(AccessPath.REST_API)) {
@@ -1156,6 +1155,9 @@
"Invalid authentication method. In order to authenticate, "
+ "prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/).");
}
+ if (user.isIdentifiedUser()) {
+ user.setLastLoginExternalIdKey(globals.webSession.get().getLastLoginExternalId());
+ }
}
private static boolean isRead(HttpServletRequest req) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexDefinition.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexDefinition.java
index 340e35e..0d42ee5 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexDefinition.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexDefinition.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.index;
import com.google.common.collect.ImmutableSortedMap;
-import com.google.inject.Provider;
+import com.google.gerrit.common.Nullable;
/**
* Definition of an index over a Gerrit data type.
@@ -33,13 +33,13 @@
private final SchemaDefinitions<V> schemaDefs;
private final IndexCollection<K, V, I> indexCollection;
private final IndexFactory<K, V, I> indexFactory;
- private final Provider<SiteIndexer<K, V, I>> siteIndexer;
+ private final SiteIndexer<K, V, I> siteIndexer;
protected IndexDefinition(
SchemaDefinitions<V> schemaDefs,
IndexCollection<K, V, I> indexCollection,
IndexFactory<K, V, I> indexFactory,
- Provider<SiteIndexer<K, V, I>> siteIndexer) {
+ @Nullable SiteIndexer<K, V, I> siteIndexer) {
this.schemaDefs = schemaDefs;
this.indexCollection = indexCollection;
this.indexFactory = indexFactory;
@@ -66,7 +66,8 @@
return indexFactory;
}
+ @Nullable
public final SiteIndexer<K, V, I> getSiteIndexer() {
- return siteIndexer.get();
+ return siteIndexer;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexDefinition.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexDefinition.java
index 72f23be..25bf541 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexDefinition.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AccountIndexDefinition.java
@@ -19,7 +19,6 @@
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.index.IndexDefinition;
import com.google.inject.Inject;
-import com.google.inject.util.Providers;
public class AccountIndexDefinition
extends IndexDefinition<Account.Id, AccountState, AccountIndex> {
@@ -29,10 +28,6 @@
AccountIndexCollection indexCollection,
AccountIndex.Factory indexFactory,
@Nullable AllAccountsIndexer allAccountsIndexer) {
- super(
- AccountSchemaDefinitions.INSTANCE,
- indexCollection,
- indexFactory,
- Providers.of(allAccountsIndexer));
+ super(AccountSchemaDefinitions.INSTANCE, indexCollection, indexFactory, allAccountsIndexer);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexDefinition.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexDefinition.java
index 4404298..8b63a1d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexDefinition.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeIndexDefinition.java
@@ -19,7 +19,6 @@
import com.google.gerrit.server.index.IndexDefinition;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
-import com.google.inject.util.Providers;
public class ChangeIndexDefinition extends IndexDefinition<Change.Id, ChangeData, ChangeIndex> {
@@ -28,10 +27,6 @@
ChangeIndexCollection indexCollection,
ChangeIndex.Factory indexFactory,
@Nullable AllChangesIndexer allChangesIndexer) {
- super(
- ChangeSchemaDefinitions.INSTANCE,
- indexCollection,
- indexFactory,
- Providers.of(allChangesIndexer));
+ super(ChangeSchemaDefinitions.INSTANCE, indexCollection, indexFactory, allChangesIndexer);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupIndexDefinition.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupIndexDefinition.java
index 0dbea79..8e15b5e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupIndexDefinition.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupIndexDefinition.java
@@ -18,7 +18,6 @@
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.index.IndexDefinition;
import com.google.inject.Inject;
-import com.google.inject.util.Providers;
public class GroupIndexDefinition
extends IndexDefinition<AccountGroup.UUID, AccountGroup, GroupIndex> {
@@ -28,10 +27,6 @@
GroupIndexCollection indexCollection,
GroupIndex.Factory indexFactory,
@Nullable AllGroupsIndexer allGroupsIndexer) {
- super(
- GroupSchemaDefinitions.INSTANCE,
- indexCollection,
- indexFactory,
- Providers.of(allGroupsIndexer));
+ super(GroupSchemaDefinitions.INSTANCE, indexCollection, indexFactory, allGroupsIndexer);
}
}
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 ec7541b..faf1489 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
@@ -30,6 +30,8 @@
/** @polymerBehavior Gerrit.PatchSetBehavior */
const PatchSetBehavior = {
+ // Corresponds to the patchNum for an edit patchset.
+ EDIT_NAME: 'edit',
/**
* As patchNum can be either a string (e.g. 'edit', 'PARENT') OR a number,
@@ -58,6 +60,55 @@
},
/**
+ * Find change edit base revision if change edit exists.
+ *
+ * @param {!Array<!Object>} revisions The revisions array.
+ * @return {Object} change edit parent revision or null if change edit
+ * doesn't exist.
+ */
+ findEditParentRevision(revisions) {
+ const editInfo =
+ revisions.find(info => info._number === PatchSetBehavior.EDIT_NAME);
+
+ if (!editInfo) { return null; }
+
+ return revisions.find(info => info._number === editInfo.basePatchNum) ||
+ null;
+ },
+
+ /**
+ * Find change edit base patch set number if change edit exists.
+ *
+ * @param {!Array<!Object>} revisions The revisions array.
+ * @return {number} Change edit patch set number or -1.
+ */
+ findEditParentPatchNum(revisions) {
+ const revisionInfo = PatchSetBehavior.findEditParentRevision(revisions);
+ return revisionInfo ? revisionInfo._number : -1;
+ },
+
+ /**
+ * Sort given revisions array according to the patch set number. The sort
+ * algorithm is change edit aware. Change edit has patch set number equals
+ * 0, but must appear after the patch set it was based on. Example: change
+ * edit is based on patch set 2, and another patch set was uploaded after
+ * change edit creation, the sorted order should be: 1, 2, (0|edit), 3.
+ *
+ * @param {!Array<!Object>} revisions The revisions array
+ * @return {!Array<!Object>} The sorted {revisions} array
+ */
+ sortRevisions(revisions) {
+ const editParent = PatchSetBehavior.findEditParentPatchNum(revisions);
+ // Map a normal patchNum to 2 * (patchNum - 1) + 1... I.e. 1 -> 1,
+ // 2 -> 3, 3 -> 5, etc.
+ // Map an edit to the patchNum of parent*2... I.e. edit on 2 -> 4.
+ const num = r => r._number === PatchSetBehavior.EDIT_NAME ?
+ 2 * editParent :
+ 2 * (r._number - 1) + 1;
+ return revisions.sort((a, b) => num(a) - num(b));
+ },
+
+ /**
* Construct a chronological list of patch sets derived from change details.
* Each element of this list is an object with the following properties:
*
@@ -68,32 +119,37 @@
* The wip property is determined by the change's current work_in_progress
* property and its log of change messages.
*
- * @param {Object} change The change details
- * @return {Array<Object>} Sorted list of patch set objects, as described
+ * @param {!Object} change The change details
+ * @return {!Array<!Object>} Sorted list of patch set objects, as described
* above
*/
computeAllPatchSets(change) {
if (!change) { return []; }
- const patchNums = [];
- for (const commit in change.revisions) {
- if (change.revisions.hasOwnProperty(commit)) {
- patchNums.push({
- num: change.revisions[commit]._number,
- desc: change.revisions[commit].description,
- });
- }
+ let patchNums = [];
+ if (change.revisions &&
+ Object.keys(change.revisions).length) {
+ patchNums =
+ PatchSetBehavior.sortRevisions(Object.values(change.revisions))
+ .map(e => {
+ // TODO(kaspern): Mark which patchset an edit was made on, if an
+ // edit exists -- perhaps with a temporary description.
+ return {
+ num: e._number,
+ desc: e.description,
+ };
+ });
}
- patchNums.sort((a, b) => { return a.num - b.num; });
+ patchNums.sort((a, b) => a.num - b.num);
return PatchSetBehavior._computeWipForPatchSets(change, patchNums);
},
/**
* Populate the wip properties of the given list of patch sets.
*
- * @param {Object} change The change details
- * @param {Array<Object>} patchNums Sorted list of patch set objects, as
+ * @param {!Object} change The change details
+ * @param {!Array<!Object>} patchNums Sorted list of patch set objects, as
* generated by computeAllPatchSets
- * @return {Array<Object>} The given list of patch set objects, with the
+ * @return {!Array<!Object>} The given list of patch set objects, with the
* wip property set on each of them
*/
_computeWipForPatchSets(change, patchNums) {
@@ -122,15 +178,20 @@
computeLatestPatchNum(allPatchSets) {
if (!allPatchSets || !allPatchSets.length) { return undefined; }
+ if (allPatchSets[allPatchSets.length - 1].num ===
+ PatchSetBehavior.EDIT_NAME) {
+ return allPatchSets[allPatchSets.length - 2].num;
+ }
return allPatchSets[allPatchSets.length - 1].num;
},
/**
* Check whether there is no newer patch than the latest patch that was
* available when this change was loaded.
- * @return {Promise} A promise that yields true if the latest patch has been
- * loaded, and false if a newer patch has been uploaded in the meantime.
- * The promise is rejected on network error.
+ *
+ * @return {Promise<boolean>} A promise that yields true if the latest patch
+ * has been loaded, and false if a newer patch has been uploaded in the
+ * meantime. The promise is rejected on network error.
*/
fetchIsLatestKnown(change, restAPI) {
const knownLatest = PatchSetBehavior.computeLatestPatchNum(
@@ -145,6 +206,18 @@
return actualLatest <= knownLatest;
});
},
+
+ /**
+ * @param {number|string} patchNum
+ * @param {!Array<!Object>} revisions A sorted array of revisions.
+ *
+ * @return {number} The index of the revision with the given patchNum.
+ */
+ findSortedIndex(patchNum, revisions) {
+ revisions = revisions || [];
+ const findNum = rev => rev._number + '' === patchNum + '';
+ return revisions.findIndex(findNum);
+ },
};
window.Gerrit = window.Gerrit || {};
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 2a5caed..c2ccccd 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
@@ -173,5 +173,63 @@
assert.isTrue(equals('edit', 'edit'));
assert.isTrue(equals('PARENT', 'PARENT'));
});
+
+ test('findEditParentRevision', () => {
+ const findParent = Gerrit.PatchSetBehavior.findEditParentRevision;
+ let revisions = [
+ {_number: 0},
+ {_number: 1},
+ {_number: 2},
+ ];
+ assert.strictEqual(findParent(revisions), null);
+
+ revisions = [...revisions, {_number: 'edit', basePatchNum: 3}];
+ assert.strictEqual(findParent(revisions), null);
+
+ revisions = [...revisions, {_number: 3}];
+ assert.deepEqual(findParent(revisions), {_number: 3});
+ });
+
+ test('findEditParentPatchNum', () => {
+ const findNum = Gerrit.PatchSetBehavior.findEditParentPatchNum;
+ let revisions = [
+ {_number: 0},
+ {_number: 1},
+ {_number: 2},
+ ];
+ assert.equal(findNum(revisions), -1);
+
+ revisions =
+ [...revisions, {_number: 'edit', basePatchNum: 3}, {_number: 3}];
+ assert.deepEqual(findNum(revisions), 3);
+ });
+
+ test('sortRevisions', () => {
+ const sort = Gerrit.PatchSetBehavior.sortRevisions;
+ const revisions = [
+ {_number: 0},
+ {_number: 2},
+ {_number: 1},
+ ];
+ const sorted = [
+ {_number: 0},
+ {_number: 1},
+ {_number: 2},
+ ];
+
+ assert.deepEqual(sort(revisions), sorted);
+
+ // Edit patchset should follow directly after its basePatchNum.
+ revisions.push({_number: 'edit', basePatchNum: 2});
+ sorted.push({_number: 'edit', basePatchNum: 2});
+ assert.deepEqual(sort(revisions), sorted);
+
+ revisions[3].basePatchNum = 0;
+ const edit = sorted.pop();
+ edit.basePatchNum = 0;
+ // Edit patchset should be at index 1.
+ sorted.splice(1, 0, edit);
+ assert.deepEqual(sort(revisions), sorted);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.js b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.js
index 12b401f..00aeb91 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.js
@@ -69,6 +69,7 @@
attached() {
this._getCreateGroupCapability();
+ this.fire('title-change', {title: 'Groups'});
},
_paramsChanged(params) {
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-project-list/gr-admin-project-list.js b/polygerrit-ui/app/elements/admin/gr-admin-project-list/gr-admin-project-list.js
index 4b51df5..4f93e98 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-project-list/gr-admin-project-list.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-project-list/gr-admin-project-list.js
@@ -69,7 +69,7 @@
attached() {
this._getCreateProjectCapability();
- this.fire('title-change', {title: 'Project List'});
+ this.fire('title-change', {title: 'Projects'});
},
_paramsChanged(params) {
diff --git a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.js b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.js
index a18a862..624ff57 100644
--- a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.js
+++ b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.js
@@ -59,7 +59,7 @@
],
attached() {
- this.fire('title-change', {title: 'Plugin List'});
+ this.fire('title-change', {title: 'Plugins'});
},
_paramsChanged(params) {
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 329a343..f8dad01 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
@@ -817,8 +817,8 @@
_setLabelValuesOnRevert(newChangeId) {
const labels = this.$.jsAPI.getLabelValuesPostRevert(this.change);
if (labels) {
- const url = `/changes/${newChangeId}/revisions/current/review`;
- this.$.restAPI.send(this.actions.revert.method, url, {labels});
+ this.$.restAPI.getChangeURLAndSend(newChangeId,
+ this.actions.revert.method, 'current', '/review', {labels});
}
},
@@ -878,11 +878,10 @@
cleanupFn();
return Promise.resolve();
}
-
- const url = this.$.restAPI.getChangeActionURL(this.changeNum,
- revisionAction ? this.patchNum : null, actionEndpoint);
- return this.$.restAPI.send(method, url, payload,
- this._handleResponseError, this).then(response => {
+ const patchNum = revisionAction ? this.patchNum : null;
+ return this.$.restAPI.getChangeURLAndSend(this.changeNum, method,
+ patchNum, actionEndpoint, payload, this._handleResponseError,
+ this).then(response => {
cleanupFn.call(this);
return response;
});
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 1f96c31..7e305bb 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
@@ -71,12 +71,12 @@
send(method, url, payload) {
if (method !== 'POST') { return Promise.reject('bad method'); }
- if (url === '/changes/42/revisions/2/submit') {
+ if (url === '/changes/test~42/revisions/2/submit') {
return Promise.resolve({
ok: true,
text() { return Promise.resolve(')]}\'\n{}'); },
});
- } else if (url === '/changes/42/revisions/2/rebase') {
+ } else if (url === '/changes/test~42/revisions/2/rebase') {
return Promise.resolve({
ok: true,
text() { return Promise.resolve(')]}\'\n{}'); },
@@ -228,6 +228,8 @@
});
test('submit change', done => {
+ sandbox.stub(element.$.restAPI, '_getFromProjectLookup')
+ .returns(Promise.resolve('test'));
sandbox.stub(element, 'fetchIsLatestKnown',
() => { return Promise.resolve(true); });
element.change = {
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 b1cbe3e..f9f1d03 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
@@ -459,8 +459,9 @@
<select>
<template is="dom-repeat" items="[[_allPatchSets]]"
as="patchNum">
- <option value$="[[patchNum.num]]"
- disabled$="[[_computePatchSetDisabled(patchNum.num, _patchRange.basePatchNum)]]">
+ <option
+ value$="[[patchNum.num]]"
+ disabled$="[[_computePatchSetDisabled(patchNum.num, _patchRange.basePatchNum, _sortedRevisions)]]">
[[patchNum.num]]
/
[[computeLatestPatchNum(_allPatchSets)]]
@@ -511,10 +512,11 @@
patch-range="{{_patchRange}}"
comments="[[_comments]]"
drafts="[[_diffDrafts]]"
- revisions="[[_change.revisions]]"
+ revisions="[[_sortedRevisions]]"
project-config="[[_projectConfig]]"
selected-index="{{viewState.selectedFileIndex}}"
diff-view-mode="{{viewState.diffMode}}"
+ edit-loaded="[[_editLoaded]]"
num-files-shown="{{_numFilesShown}}"
file-list-increment="{{_numFilesShown}}"></gr-file-list>
</section>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 1a20e7c..1d8025e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -181,6 +181,11 @@
value: true,
},
_updateCheckTimerHandle: Number,
+ _sortedRevisions: Array,
+ _editLoaded: {
+ type: Boolean,
+ computed: '_computeEditLoaded(_patchRange.*)',
+ },
},
behaviors: [
@@ -195,6 +200,7 @@
observers: [
'_labelsChanged(_change.labels.*)',
'_paramsAndChangeChanged(params, _change)',
+ '_updateSortedRevisions(_change.revisions.*)',
],
keyBindings: {
@@ -243,6 +249,11 @@
}
},
+ _updateSortedRevisions(revisionsRecord) {
+ const revisions = revisionsRecord.base;
+ this._sortedRevisions = this.sortRevisions(Object.values(revisions));
+ },
+
_computePrefsButtonHidden(prefs, loggedIn) {
return !loggedIn || !prefs;
},
@@ -684,13 +695,15 @@
/**
* Determines if a patch number should be disabled based on value of the
* basePatchNum from gr-file-list.
- * @param {Number} patchNum Patch number available in dropdown
- * @param {Number|String} basePatchNum Base patch number from file list
- * @return {Boolean}
+ * @param {number} patchNum Patch number available in dropdown
+ * @param {number|string} basePatchNum Base patch number from file list
+ * @return {boolean}
*/
_computePatchSetDisabled(patchNum, basePatchNum) {
- basePatchNum = basePatchNum === 'PARENT' ? 0 : basePatchNum;
- return parseInt(patchNum, 10) <= parseInt(basePatchNum, 10);
+ if (basePatchNum === 'PARENT') { return false; }
+
+ return this.findSortedIndex(patchNum, this._sortedRevisions) <=
+ this.findSortedIndex(basePatchNum, this._sortedRevisions);
},
_computeLabelNames(labels) {
@@ -901,12 +914,24 @@
},
_getChangeDetail() {
- return this.$.restAPI.getChangeDetail(this._changeNum,
- this._handleGetChangeDetailError.bind(this)).then(change => {
+ const detailCompletes = this.$.restAPI.getChangeDetail(
+ this._changeNum, this._handleGetChangeDetailError.bind(this));
+ const editCompletes = this._getEdit();
+
+ return Promise.all([detailCompletes, editCompletes])
+ .then(([change, edit]) => {
if (!change) {
return '';
}
this._upgradeUrl(change, this.params);
+ if (edit) {
+ change.revisions[edit.commit.commit] = {
+ _number: this.EDIT_NAME,
+ basePatchNum: edit.base_patch_set_number,
+ commit: edit.commit,
+ fetch: edit.fetch,
+ };
+ }
// Issue 4190: Coalesce missing topics to null.
if (!change.topic) { change.topic = null; }
if (!change.reviewer_updates) {
@@ -952,6 +977,10 @@
});
},
+ _getEdit() {
+ return this.$.restAPI.getChangeEdit(this._changeNum, true);
+ },
+
_getLatestCommitMessage() {
return this.$.restAPI.getChangeCommitInfo(this._changeNum,
this.computeLatestPatchNum(this._allPatchSets)).then(commitInfo => {
@@ -1294,5 +1323,10 @@
_computeHeaderClass(change) {
return change.work_in_progress ? 'header wip' : 'header';
},
+
+ _computeEditLoaded(patchRangeRecord) {
+ const patchRange = patchRangeRecord.base || {};
+ return this.patchNumEquals(patchRange.patchNum, this.EDIT_NAME);
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 31d1591..ddbbd5f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -58,6 +58,10 @@
});
suite('keyboard shortcuts', () => {
+ setup(() => {
+ sandbox.stub(element, '_updateSortedRevisions');
+ });
+
test('S should toggle the CL star', () => {
const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
MockInteractions.pressAndReleaseKeyOn(element, 83, null, 's');
@@ -233,6 +237,12 @@
});
test('_computePatchSetDisabled', () => {
+ element._sortedRevisions = [
+ {_number: 1},
+ {_number: 2},
+ {_number: element.EDIT_NAME, basePatchNum: 2},
+ {_number: 3},
+ ];
let basePatchNum = 'PARENT';
let patchNum = 1;
assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
@@ -243,6 +253,12 @@
patchNum = 2;
assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
false);
+ basePatchNum = element.EDIT_NAME;
+ assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
+ true);
+ patchNum = '3';
+ assert.equal(element._computePatchSetDisabled(patchNum, basePatchNum),
+ false);
});
test('_prepareCommitMsgForLinkify', () => {
@@ -470,6 +486,7 @@
});
test('change num change', () => {
+ sandbox.stub(element, '_updateSortedRevisions');
element._changeNum = null;
element._patchRange = {
basePatchNum: 'PARENT',
@@ -852,6 +869,37 @@
});
});
+ test('edit is added to change', () => {
+ sandbox.stub(element, '_changeChanged');
+ sandbox.stub(element, '_upgradeUrl');
+ sandbox.stub(element.$.restAPI, 'getChangeDetail', () => {
+ return Promise.resolve({
+ id: '123456789',
+ labels: {},
+ current_revision: 'foo',
+ revisions: {foo: {commit: {}}},
+ });
+ });
+ sandbox.stub(element, '_getEdit', () => {
+ return Promise.resolve({
+ base_patch_set_number: 1,
+ commit: {commit: 'bar'},
+ });
+ });
+
+ return element._getChangeDetail().then(() => {
+ const revs = element._change.revisions;
+ assert.equal(Object.keys(revs).length, 2);
+ assert.deepEqual(revs['foo'], {commit: {commit: 'foo'}});
+ assert.deepEqual(revs['bar'], {
+ _number: element.EDIT_NAME,
+ basePatchNum: 1,
+ commit: {commit: 'bar'},
+ fetch: undefined,
+ });
+ });
+ });
+
test('reply dialog focus can be controlled', () => {
const FocusTarget = element.$.replyDialog.FocusTarget;
const openStub = sandbox.stub(element, '_openReplyDialog');
@@ -966,6 +1014,7 @@
suite('reply dialog tests', () => {
setup(() => {
sandbox.stub(element.$.replyDialog, '_draftChanged');
+ sandbox.stub(element, '_updateSortedRevisions');
sandbox.stub(element.$.replyDialog, 'fetchIsLatestKnown',
() => { return Promise.resolve(true); });
element._change = {labels: {}};
@@ -1239,6 +1288,14 @@
assert.isTrue(element.$.relatedChanges.reload.calledOnce);
});
+ test('_computeEditLoaded', () => {
+ const callCompute = range => element._computeEditLoaded({base: range});
+ assert.isFalse(callCompute({}));
+ assert.isFalse(callCompute({basePatchNum: 'PARENT', patchNum: 1}));
+ assert.isFalse(callCompute({basePatchNum: 'edit', patchNum: 1}));
+ assert.isTrue(callCompute({basePatchNum: 1, patchNum: 'edit'}));
+ });
+
suite('_upgradeUrl calls', () => {
let upgradeStub;
const mockChange = {project: 'test'};
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index dc64042..b218f02 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -168,6 +168,9 @@
.mobile {
display: none;
}
+ #container.editLoaded .hideOnEdit {
+ display: none;
+ }
@media screen and (max-width: 50em) {
.desktop {
display: none;
@@ -234,7 +237,7 @@
items="[[computeAllPatchSets(change)]]"
as="patchNum">
<option
- disabled$="[[_computePatchSetDisabled(patchNum.num, patchRange.patchNum)]]"
+ disabled$="[[_computePatchSetDisabled(patchNum.num, patchRange.patchNum, revisions)]]"
value$="[[patchNum.num]]">
[[patchNum.num]]
[[patchNum.desc]]
@@ -245,7 +248,10 @@
</label>
</div>
</header>
- <div on-tap="_handleFileListTap">
+ <div
+ id="container"
+ class$="[[_computeContainerClass(editLoaded)]]"
+ on-tap="_handleFileListTap">
<template is="dom-repeat"
items="[[_shownFiles]]"
id="files"
@@ -253,7 +259,7 @@
initial-count="[[fileListIncrement]]"
target-framerate="1">
<div class="file-row row" data-path$="[[file.__path]]" tabindex="-1">
- <div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
+ <div class="reviewed hideOnEdit" hidden$="[[!_loggedIn]]" hidden>
<input
type="checkbox"
checked="[[file.isReviewed]]"
@@ -267,9 +273,9 @@
[[_computeFileStatus(file.status)]]
</div>
<span
- data-url="[[_computeDiffURL(change, patchRange, file.__path)]]"
+ data-url="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path)]]"
class$="[[_computePathClass(file.__path, _expandedFilePaths.*)]]">
- <a href$="[[_computeDiffURL(change, patchRange, file.__path)]]">
+ <a href$="[[_computeDiffURL(change, patchRange.patchNum, patchRange.basePatchNum, file.__path)]]">
<span title$="[[_computeFileDisplayName(file.__path)]]"
class="fullFileName">
[[_computeFileDisplayName(file.__path)]]
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 8560717..851f6ef 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -14,6 +14,8 @@
(function() {
'use strict';
+ const ERR_EDIT_LOADED = 'You cannot change the review status of an edit.';
+
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
const WARN_SHOW_ALL_THRESHOLD = 1000;
@@ -41,7 +43,8 @@
changeNum: String,
comments: Object,
drafts: Object,
- revisions: Object,
+ // Already sorted by the change-view.
+ revisions: Array,
projectConfig: Object,
selectedIndex: {
type: Number,
@@ -57,6 +60,7 @@
notify: true,
observer: '_updateDiffPreferences',
},
+ editLoaded: Boolean,
_files: {
type: Array,
observer: '_filesChanged',
@@ -116,6 +120,7 @@
type: Boolean,
observer: '_loadingChanged',
},
+ _sortedRevisions: Array,
},
behaviors: [
@@ -231,7 +236,8 @@
},
_computePatchSetDisabled(patchNum, currentPatchNum) {
- return parseInt(patchNum, 10) >= parseInt(currentPatchNum, 10);
+ return this.findSortedIndex(patchNum, this.revisions) >=
+ this.findSortedIndex(currentPatchNum, this.revisions);
},
_togglePathExpanded(path) {
@@ -384,6 +390,10 @@
},
_reviewFile(path) {
+ if (this.editLoaded) {
+ this.fire('show-alert', {message: ERR_EDIT_LOADED});
+ return;
+ }
const index = this._reviewed.indexOf(path);
const reviewed = index !== -1;
if (reviewed) {
@@ -662,9 +672,8 @@
return status || 'M';
},
- _computeDiffURL(change, patchRange, path) {
- return Gerrit.Nav.getUrlForDiff(change, path, patchRange.patchNum,
- patchRange.basePatchNum);
+ _computeDiffURL(change, patchNum, basePatchNum, path) {
+ return Gerrit.Nav.getUrlForDiff(change, path, patchNum, basePatchNum);
},
_computeFileDisplayName(path) {
@@ -940,5 +949,9 @@
this.classList.toggle('loading', loading && this._files.length);
}, LOADING_DEBOUNCE_INTERVAL);
},
+
+ _computeContainerClass(editLoaded) {
+ return editLoaded ? 'editLoaded' : '';
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 9be19a7..e8123a7 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -664,21 +664,29 @@
revisions: {
rev1: {_number: 1},
rev2: {_number: 2},
- rev3: {_number: 3},
+ rev3: {_number: 'edit', basePatchNum: 2},
+ rev4: {_number: 3},
},
};
+ element.revisions = [
+ {_number: 1},
+ {_number: 2},
+ {_number: 'edit', basePatchNum: 2},
+ {_number: 3},
+ ];
+
flush(() => {
const selectEl = element.$.patchChange;
assert.equal(selectEl.nativeSelect.value, 'PARENT');
assert.isTrue(element.$$('option[value="3"]').hasAttribute('disabled'));
selectEl.addEventListener('change', () => {
- assert.equal(selectEl.nativeSelect.value, '2');
- assert(navStub.lastCall.calledWithExactly(element.change, '3', '2'),
- 'Should navigate to /c/42/2..3');
+ assert.equal(selectEl.nativeSelect.value, 'edit');
+ assert(navStub.lastCall.calledWithExactly(element.change, '3', 'edit'),
+ 'Should navigate to /c/42/edit..3');
navStub.restore();
done();
});
- selectEl.nativeSelect.value = '2';
+ selectEl.nativeSelect.value = 'edit';
element.fire('change', {}, {node: selectEl.nativeSelect});
});
});
@@ -988,6 +996,21 @@
element.flushDebouncer('loading-change');
assert.isFalse(element.classList.contains('loading'));
});
+
+ test('no execute _computeDiffURL before patchNum is knwon', done => {
+ const urlStub = sandbox.stub(element, '_computeDiffURL');
+ element.change = {_number: 123};
+ element.patchRange = {patchNum: undefined, basePatchNum: 'PARENT'};
+ element._files = [{__path: 'foo/bar.cpp'}];
+ flush(() => {
+ assert.isFalse(urlStub.called);
+ element.set('patchRange.patchNum', 4);
+ flush(() => {
+ assert.isTrue(urlStub.called);
+ done();
+ });
+ });
+ });
});
suite('gr-file-list inline diff tests', () => {
@@ -1259,7 +1282,30 @@
element._handleEscKey(mockEvent);
assert.isFalse(element._displayLine);
});
- });
+ suite('editLoaded behavior', () => {
+ const isVisible = el => {
+ assert.ok(el);
+ return getComputedStyle(el).getPropertyValue('display') !== 'none';
+ };
+
+ test('reviewed checkbox', () => {
+ const alertStub = sandbox.stub();
+ element.addEventListener('show-alert', alertStub);
+ element.editLoaded = false;
+ // Reviewed checkbox should be shown.
+ assert.isTrue(isVisible(element.$$('.reviewed')));
+ MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
+ assert.isFalse(alertStub.called);
+
+ element.editLoaded = true;
+ flushAsynchronousOperations();
+
+ assert.isFalse(isVisible(element.$$('.reviewed')));
+ MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
+ assert.isTrue(alertStub.called);
+ });
+ });
+ });
a11ySuite('basic');
</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 6e1cb64..67a8110 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -39,6 +39,7 @@
const encode = window.Gerrit.URLEncodingBehavior.encodeURL;
const patchNumEquals = window.Gerrit.PatchSetBehavior.patchNumEquals;
+ const EDIT_NAME = window.Gerrit.PatchSetBehavior.EDIT_NAME;
function startRouter(generateUrl) {
const base = window.Gerrit.BaseUrlBehavior.getBaseUrl();
@@ -419,18 +420,27 @@
if (params.basePatchNum &&
patchNumEquals(params.basePatchNum, params.patchNum)) {
params.basePatchNum = null;
- upgradeUrl(params);
} else if (params.basePatchNum && !params.patchNum) {
params.patchNum = params.basePatchNum;
params.basePatchNum = null;
}
+ // In GWTUI, edits are represented in URLs with either 0 or 'edit'.
+ // TODO(kaspern): Remove this normalization when GWT UI is gone.
+ if (patchNumEquals(params.basePatchNum, 0)) {
+ params.basePatchNum = EDIT_NAME;
+ }
+ if (patchNumEquals(params.patchNum, 0)) {
+ params.patchNum = EDIT_NAME;
+ }
+ upgradeUrl(params);
};
// Matches
- // /c/<project>/+/<changeNum>/[<basePatchNum>..][<patchNum>]/[path].
+ // /c/<project>/+/<changeNum>/[<basePatchNum|edit>..][<patchNum|edit>]/[path].
// TODO(kaspern): Migrate completely to project based URLs, with backwards
// compatibility for change-only.
- page(/^\/c\/(.+)\/\+\/(\d+)(\/?((\d+)(\.\.(\d+))?(\/(.+))?))?\/?$/,
+ // eslint-disable-next-line max-len
+ page(/^\/c\/(.+)\/\+\/(\d+)(\/?((\d+|edit)(\.\.(\d+|edit))?(\/(.+))?))?\/?$/,
ctx => {
// Parameter order is based on the regex group number matched.
const params = {
@@ -448,7 +458,7 @@
});
// Matches /c/<changeNum>/[<basePatchNum>..][<patchNum>][/].
- page(/^\/c\/(\d+)\/?(((\d+)(\.\.(\d+))?))?\/?$/, ctx => {
+ page(/^\/c\/(\d+)\/?(((\d+|edit)(\.\.(\d+|edit))?))?\/?$/, ctx => {
// Parameter order is based on the regex group number matched.
const params = {
changeNum: ctx.params[0],
@@ -462,7 +472,7 @@
});
// Matches /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
- page(/^\/c\/(\d+)\/((\d+)(\.\.(\d+))?)\/(.+)/, ctx => {
+ page(/^\/c\/(\d+)\/((\d+|edit)(\.\.(\d+|edit))?)\/(.+)/, ctx => {
// Check if path has an '@' which indicates it was using GWT style line
// numbers. Even if the filename had an '@' in it, it would have already
// been URI encoded. Redirect to hash version of path.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 70d19e5..2908f85 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -271,7 +271,7 @@
change-num="[[_changeNum]]"
patch-range="[[_patchRange]]"
files-weblinks="[[_filesWeblinks]]"
- available-patches="[[_computeAvailablePatches(_change.revisions)]]"
+ available-patches="[[_computeAvailablePatches(_change.revisions, _change.revisions.*)]]"
revisions="[[_change.revisions]]">
</gr-patch-range-select>
<span class="download desktop">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 5574fd6..0f6a07d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -176,6 +176,10 @@
}
},
+ _getChangeEdit(changeNum) {
+ return this.$.restAPI.getChangeEdit(this._changeNum);
+ },
+
_getFiles(changeNum, patchRangeRecord) {
const patchRange = patchRangeRecord.base;
return this.$.restAPI.getChangeFilePathsAsSpeciallySortedArray(
@@ -495,7 +499,17 @@
promises.push(this._loadComments());
- Promise.all(promises).then(() => {
+ promises.push(this._getChangeEdit(this._changeNum));
+
+ Promise.all(promises).then(r => {
+ const edit = r[4];
+ if (edit) {
+ this.set('_change.revisions.' + edit.commit.commit, {
+ _number: this.EDIT_NAME,
+ basePatchNum: edit.base_patch_set_number,
+ commit: edit.commit,
+ });
+ }
this._loading = false;
this.$.diff.comments = this._commentsForDiff;
this.$.diff.reload();
@@ -562,13 +576,8 @@
return patchStr;
},
- _computeAvailablePatches(revisions) {
- const patchNums = [];
- if (!revisions) { return patchNums; }
- for (const rev of Object.values(revisions)) {
- patchNums.push(rev._number);
- }
- return patchNums.sort((a, b) => { return a - b; });
+ _computeAvailablePatches(revs) {
+ return this.sortRevisions(Object.values(revs)).map(e => e._number);
},
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 7135601..7ef5c60 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -15,6 +15,8 @@
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+
+<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-diff-builder/gr-diff-builder.html">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 94f60d9..728a766 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -14,6 +14,9 @@
(function() {
'use strict';
+ const ERR_COMMENT_ON_EDIT = 'You cannot comment on an edit.';
+ const ERR_INVALID_LINE = 'Invalid line number: ';
+
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
UNIFIED: 'UNIFIED_DIFF',
@@ -79,10 +82,6 @@
},
noRenderOnPrefsChange: Boolean,
comments: Object,
- _loggedIn: {
- type: Boolean,
- value: false,
- },
lineWrapping: {
type: Boolean,
value: false,
@@ -93,6 +92,10 @@
value: DiffViewMode.SIDE_BY_SIDE,
observer: '_viewModeObserver',
},
+ _loggedIn: {
+ type: Boolean,
+ value: false,
+ },
_diff: Object,
_diffHeaderItems: {
type: Array,
@@ -120,6 +123,10 @@
_showWarning: Boolean,
},
+ behaviors: [
+ Gerrit.PatchSetBehavior,
+ ],
+
listeners: {
'thread-discard': '_handleThreadDiscard',
'comment-discard': '_handleCommentDiscard',
@@ -169,27 +176,6 @@
return Polymer.dom(this.root).querySelectorAll('.diff-row');
},
- addDraftAtLine(el) {
- this._selectLine(el);
- this._getLoggedIn().then(loggedIn => {
- if (!loggedIn) {
- this.fire('show-auth-required');
- return;
- }
-
- const value = el.getAttribute('data-value');
- if (value === GrDiffLine.FILE) {
- this._addDraft(el);
- return;
- }
- const lineNum = parseInt(value, 10);
- if (isNaN(lineNum)) {
- throw Error('Invalid line number: ' + value);
- }
- this._addDraft(el, lineNum);
- });
- },
-
isRangeSelected() {
return this.$.highlights.isRangeSelected();
},
@@ -254,34 +240,65 @@
});
},
- _handleCreateComment(e) {
- const range = e.detail.range;
- const diffSide = e.detail.side;
- const line = range.endLine;
- const lineEl = this.$.diffBuilder.getLineElByNumber(line, diffSide);
- const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
- const contentEl = contentText.parentElement;
- const patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
- const isOnParent =
- this._getIsParentCommentByLineAndContent(lineEl, contentEl);
- const threadEl = this._getOrCreateThreadAtLineRange(contentEl, patchNum,
- diffSide, isOnParent, range);
+ addDraftAtLine(el) {
+ this._selectLine(el);
+ this._isValidElForComment(el).then(valid => {
+ if (!valid) { return; }
- threadEl.addOrEditDraft(line, range);
+ const value = el.getAttribute('data-value');
+ let lineNum;
+ if (value !== GrDiffLine.FILE) {
+ lineNum = parseInt(value, 10);
+ if (isNaN(lineNum)) {
+ this.fire('show-alert', {message: ERR_INVALID_LINE + value});
+ return;
+ }
+ }
+ this._createComment(el, lineNum);
+ });
},
- _addDraft(lineEl, opt_lineNum) {
+ _handleCreateComment(e) {
+ const range = e.detail.range;
+ const side = e.detail.side;
+ const lineNum = range.endLine;
+ const lineEl = this.$.diffBuilder.getLineElByNumber(lineNum, side);
+ this._isValidElForComment(el).then(valid => {
+ if (!valid) { return; }
+
+ this._createComment(lineEl, lineNum, side, range);
+ });
+ },
+
+ _isValidElForComment(el) {
+ return this._getLoggedIn().then(loggedIn => {
+ if (!loggedIn) {
+ this.fire('show-auth-required');
+ return false;
+ }
+ const patchNum = el.classList.contains(DiffSide.LEFT) ?
+ this.patchRange.basePatchNum :
+ this.patchRange.patchNum;
+
+ if (this.patchNumEquals(patchNum, this.EDIT_NAME)) {
+ this.fire('show-alert', {message: ERR_COMMENT_ON_EDIT});
+ return false;
+ }
+ return true;
+ });
+ },
+
+ _createComment(lineEl, opt_lineNum, opt_side, opt_range) {
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
- const patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
- const commentSide =
+ const side = opt_side ||
this._getCommentSideByLineAndContent(lineEl, contentEl);
+ const patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
const isOnParent =
- this._getIsParentCommentByLineAndContent(lineEl, contentEl);
+ this._getIsParentCommentByLineAndContent(lineEl, contentEl);
const threadEl = this._getOrCreateThreadAtLineRange(contentEl, patchNum,
- commentSide, isOnParent);
-
- threadEl.addOrEditDraft(opt_lineNum);
+ side, isOnParent);
+ threadEl.addOrEditDraft(opt_lineNum, opt_range);
},
_getThreadForRange(threadGroupEl, rangeToCheck) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 8077e3a..8e790a0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -713,6 +713,7 @@
});
suite('logged in', () => {
+ let fakeLineEl;
setup(() => {
stub('gr-rest-api-interface', {
getLoggedIn() { return Promise.resolve(true); },
@@ -721,18 +722,43 @@
},
});
element = fixture('basic');
+ element.patchRange = {};
+
+ fakeLineEl = {
+ getAttribute: sandbox.stub().returns(42),
+ classList: {
+ contains: sandbox.stub().returns(true),
+ },
+ };
});
test('addDraftAtLine', done => {
- const fakeLineEl = {getAttribute: sandbox.stub().returns(42)};
sandbox.stub(element, '_selectLine');
- sandbox.stub(element, '_addDraft');
+ sandbox.stub(element, '_createComment');
const loggedInErrorSpy = sandbox.spy();
element.addEventListener('show-auth-required', loggedInErrorSpy);
element.addDraftAtLine(fakeLineEl);
flush(() => {
assert.isFalse(loggedInErrorSpy.called);
- assert.isTrue(element._addDraft.calledWithExactly(fakeLineEl, 42));
+ assert.isTrue(element._createComment
+ .calledWithExactly(fakeLineEl, 42));
+ done();
+ });
+ });
+
+ test.only('addDraftAtLine on an edit', done => {
+ element.patchRange.basePatchNum = element.EDIT_NAME;
+ sandbox.stub(element, '_selectLine');
+ sandbox.stub(element, '_createComment');
+ const loggedInErrorSpy = sandbox.spy();
+ const alertSpy = sandbox.spy();
+ element.addEventListener('show-auth-required', loggedInErrorSpy);
+ element.addEventListener('show-alert', alertSpy);
+ element.addDraftAtLine(fakeLineEl);
+ flush(() => {
+ assert.isFalse(loggedInErrorSpy.called);
+ assert.isTrue(alertSpy.called);
+ assert.isFalse(element._createComment.called);
done();
});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
index e9437bd..9a4000f 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.js
@@ -30,10 +30,13 @@
observer: '_updateSelected',
},
revisions: Object,
+ _sortedRevisions: Array,
_rightSelected: String,
_leftSelected: String,
},
+ observers: ['_updateSortedRevisions(revisions.*)'],
+
behaviors: [Gerrit.PatchSetBehavior],
_updateSelected() {
@@ -41,6 +44,11 @@
this._leftSelected = this.patchRange.basePatchNum;
},
+ _updateSortedRevisions(revisionsRecord) {
+ const revisions = revisionsRecord.base;
+ this._sortedRevisions = this.sortRevisions(Object.values(revisions));
+ },
+
_handlePatchChange(e) {
const leftPatch = this._leftSelected;
const rightPatch = this._rightSelected;
@@ -53,12 +61,15 @@
},
_computeLeftDisabled(patchNum, patchRange) {
- return parseInt(patchNum, 10) >= parseInt(patchRange.patchNum, 10);
+ return this.findSortedIndex(patchNum, this._sortedRevisions) >=
+ this.findSortedIndex(patchRange.patchNum, this._sortedRevisions);
},
_computeRightDisabled(patchNum, patchRange) {
if (patchRange.basePatchNum == 'PARENT') { return false; }
- return parseInt(patchNum, 10) <= parseInt(patchRange.basePatchNum, 10);
+
+ return this.findSortedIndex(patchNum, this._sortedRevisions) <=
+ this.findSortedIndex(patchRange.basePatchNum, this._sortedRevisions);
},
// On page load, the dom-if for options getting added occurs after
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
index abe3e5d..2fe5986 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select_test.html
@@ -36,16 +36,26 @@
<script>
suite('gr-patch-range-select tests', () => {
let element;
+ let sandbox;
setup(() => {
element = fixture('basic');
+ sandbox = sinon.sandbox.create();
});
+ teardown(() => sandbox.restore());
+
test('enabled/disabled options', () => {
const patchRange = {
basePatchNum: 'PARENT',
patchNum: '3',
};
+ element._sortedRevisions = [
+ {_number: 1},
+ {_number: 2},
+ {_number: element.EDIT_NAME, basePatchNum: 2},
+ {_number: 3},
+ ];
for (const patchNum of ['1', '2', '3']) {
assert.isFalse(element._computeRightDisabled(patchNum, patchRange));
}
@@ -54,18 +64,21 @@
}
assert.isTrue(element._computeLeftDisabled('3', patchRange));
- patchRange.basePatchNum = '2';
+ patchRange.basePatchNum = element.EDIT_NAME;
assert.isTrue(element._computeLeftDisabled('3', patchRange));
assert.isTrue(element._computeRightDisabled('1', patchRange));
assert.isTrue(element._computeRightDisabled('2', patchRange));
assert.isFalse(element._computeRightDisabled('3', patchRange));
+ assert.isTrue(element._computeRightDisabled(element.EDIT_NAME, patchRange));
});
test('navigation', done => {
- const showStub = sinon.stub(page, 'show');
+ sandbox.stub(element, '_computeLeftDisabled').returns(false);
+ sandbox.stub(element, '_computeRightDisabled').returns(false);
+ const showStub = sandbox.stub(page, 'show');
const leftSelectEl = element.$.leftPatchSelect;
const rightSelectEl = element.$.rightPatchSelect;
- const blurSpy = sinon.spy(leftSelectEl, 'blur');
+ const blurSpy = sandbox.spy(leftSelectEl, 'blur');
element.changeNum = '42';
element.path = 'path/to/file.txt';
element.availablePatches = ['1', '2', '3'];
@@ -89,7 +102,6 @@
assert(showStub.lastCall.calledWithExactly(
'/c/42/1..3/path/to/file.txt'),
'Should navigate to /c/42/1..3/path/to/file.txt');
- showStub.restore();
done();
}
});
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 9317425..5ddd0fe 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -596,7 +596,8 @@
},
getChangeActionURL(changeNum, opt_patchNum, endpoint) {
- return this._changeBaseURL(changeNum, opt_patchNum) + endpoint;
+ return this._changeBaseURL(changeNum, opt_patchNum)
+ .then(url => url + endpoint);
},
getChangeDetail(changeNum, opt_errFn, opt_cancelCondition) {
@@ -624,34 +625,34 @@
_getChangeDetail(changeNum, params, opt_errFn,
opt_cancelCondition) {
- const url = this.getChangeActionURL(changeNum, null, '/detail');
- const urlWithParams = this._urlWithParams(url, params);
- return this._fetchRawJSON(
- url,
- opt_errFn,
- opt_cancelCondition,
- {O: params},
- this._etags.getOptions(urlWithParams))
- .then(response => {
- if (response && response.status === 304) {
- return Promise.resolve(
- this._etags.getCachedPayload(urlWithParams));
- } else {
- const payloadPromise = response ?
- this.getResponseObject(response) :
- Promise.resolve();
- payloadPromise.then(payload => {
- this._etags.collect(urlWithParams, response, payload);
- this._maybeInsertInLookup(payload);
- });
- return payloadPromise;
- }
- });
+ return this.getChangeActionURL(changeNum, null, '/detail').then(url => {
+ const urlWithParams = this._urlWithParams(url, params);
+ return this._fetchRawJSON(
+ url,
+ opt_errFn,
+ opt_cancelCondition,
+ {O: params},
+ this._etags.getOptions(urlWithParams))
+ .then(response => {
+ if (response && response.status === 304) {
+ return Promise.resolve(
+ this._etags.getCachedPayload(urlWithParams));
+ } else {
+ const payloadPromise = response ?
+ this.getResponseObject(response) :
+ Promise.resolve();
+ payloadPromise.then(payload => {
+ this._etags.collect(urlWithParams, response, payload);
+ this._maybeInsertInLookup(payload);
+ });
+ return payloadPromise;
+ }
+ });
+ });
},
getChangeCommitInfo(changeNum, patchNum) {
- return this.fetchJSON(
- this.getChangeActionURL(changeNum, patchNum, '/commit?links'));
+ return this._getChangeURLAndFetch(changeNum, '/commit?links', patchNum);
},
getChangeFiles(changeNum, patchRange) {
@@ -659,8 +660,8 @@
if (patchRange.basePatchNum !== 'PARENT') {
endpoint += '?base=' + encodeURIComponent(patchRange.basePatchNum);
}
- return this.fetchJSON(
- this.getChangeActionURL(changeNum, patchRange.patchNum, endpoint));
+ return this._getChangeURLAndFetch(changeNum, endpoint,
+ patchRange.patchNum);
},
getChangeFilesAsSpeciallySortedArray(changeNum, patchRange) {
@@ -689,26 +690,23 @@
},
getChangeRevisionActions(changeNum, patchNum) {
- return this.fetchJSON(
- this.getChangeActionURL(changeNum, patchNum, '/actions')).then(
- revisionActions => {
- // The rebase button on change screen is always enabled.
+ return this._getChangeURLAndFetch(changeNum, '/actions', patchNum)
+ .then(revisionActions => {
+ // The rebase button on change screen is always enabled.
if (revisionActions.rebase) {
revisionActions.rebase.rebaseOnCurrent =
- !!revisionActions.rebase.enabled;
+ !!revisionActions.rebase.enabled;
revisionActions.rebase.enabled = true;
}
return revisionActions;
});
},
- getChangeSuggestedReviewers(changeNum, inputVal, opt_errFn,
- opt_ctx) {
- const url =
- this.getChangeActionURL(changeNum, null, '/suggest_reviewers');
- const req = {n: 10};
- if (inputVal) { req.q = inputVal; }
- return this.fetchJSON(url, opt_errFn, opt_ctx, req);
+ getChangeSuggestedReviewers(changeNum, inputVal, opt_errFn) {
+ const params = {n: 10};
+ if (inputVal) { params.q = inputVal; }
+ return this._getChangeURLAndFetch(changeNum, '/suggest_reviewers', null,
+ opt_errFn, null, params);
},
_computeFilter(filter) {
@@ -808,30 +806,30 @@
},
_sendChangeReviewerRequest(method, changeNum, reviewerID) {
- let url = this.getChangeActionURL(changeNum, null, '/reviewers');
- let body;
- switch (method) {
- case 'POST':
- body = {reviewer: reviewerID};
- break;
- case 'DELETE':
- url += '/' + encodeURIComponent(reviewerID);
- break;
- default:
- throw Error('Unsupported HTTP method: ' + method);
- }
+ return this.getChangeActionURL(changeNum, null, '/reviewers')
+ .then(url => {
+ let body;
+ switch (method) {
+ case 'POST':
+ body = {reviewer: reviewerID};
+ break;
+ case 'DELETE':
+ url += '/' + encodeURIComponent(reviewerID);
+ break;
+ default:
+ throw Error('Unsupported HTTP method: ' + method);
+ }
- return this.send(method, url, body);
+ return this.send(method, url, body);
+ });
},
getRelatedChanges(changeNum, patchNum) {
- return this.fetchJSON(
- this.getChangeActionURL(changeNum, patchNum, '/related'));
+ return this._getChangeURLAndFetch(changeNum, '/related', patchNum);
},
getChangesSubmittedTogether(changeNum) {
- return this.fetchJSON(
- this.getChangeActionURL(changeNum, null, '/submitted_together'));
+ return this._getChangeURLAndFetch(changeNum, '/submitted_together', null);
},
getChangeConflicts(changeNum) {
@@ -879,90 +877,84 @@
},
getReviewedFiles(changeNum, patchNum) {
- return this.fetchJSON(
- this.getChangeActionURL(changeNum, patchNum, '/files?reviewed'));
+ return this._getChangeURLAndFetch(changeNum, '/files?reviewed', patchNum);
},
saveFileReviewed(changeNum, patchNum, path, reviewed, opt_errFn, opt_ctx) {
const method = reviewed ? 'PUT' : 'DELETE';
- const url = this.getChangeActionURL(changeNum, patchNum,
- '/files/' + encodeURIComponent(path) + '/reviewed');
-
- return this.send(method, url, null, opt_errFn, opt_ctx);
+ const e = `/files/${encodeURIComponent(path)}/reviewed`;
+ return this.getChangeURLAndSend(changeNum, method, patchNum, e, null,
+ opt_errFn, opt_ctx);
},
saveChangeReview(changeNum, patchNum, review, opt_errFn, opt_ctx) {
- const url = this.getChangeActionURL(changeNum, patchNum, '/review');
- return this.awaitPendingDiffDrafts()
- .then(() => this.send('POST', url, review, opt_errFn, opt_ctx));
+ const promises = [
+ this.awaitPendingDiffDrafts(),
+ this.getChangeActionURL(changeNum, patchNum, '/review'),
+ ];
+ return Promise.all(promises).then(([, url]) => {
+ return this.send('POST', url, review, opt_errFn, opt_ctx);
+ });
+ },
+
+ getChangeEdit(changeNum, opt_download_commands) {
+ const params = opt_download_commands ? {'download-commands': true} : null;
+ return this.getLoggedIn().then(loggedIn => {
+ return loggedIn ?
+ this._getChangeURLAndFetch(changeNum, '/edit/', null, null, null,
+ params) :
+ false;
+ });
},
getFileInChangeEdit(changeNum, path) {
- return this.send('GET',
- this.getChangeActionURL(changeNum, null,
- '/edit/' + encodeURIComponent(path)
- ));
+ const e = '/edit/' + encodeURIComponent(path);
+ return this.getChangeURLAndSend(changeNum, 'GET', null, e);
},
rebaseChangeEdit(changeNum) {
- return this.send('POST',
- this.getChangeActionURL(changeNum, null,
- '/edit:rebase'
- ));
+ return this.getChangeURLAndSend(changeNum, 'POST', null, '/edit:rebase');
},
deleteChangeEdit(changeNum) {
- return this.send('DELETE',
- this.getChangeActionURL(changeNum, null,
- '/edit'
- ));
+ return this.getChangeURLAndSend(changeNum, 'DELETE', null, '/edit');
},
restoreFileInChangeEdit(changeNum, restore_path) {
- return this.send('POST',
- this.getChangeActionURL(changeNum, null, '/edit'),
- {restore_path}
- );
+ const p = {restore_path};
+ return this.getChangeURLAndSend(changeNum, 'POST', null, '/edit', p);
},
renameFileInChangeEdit(changeNum, old_path, new_path) {
- return this.send('POST',
- this.getChangeActionURL(changeNum, null, '/edit'),
- {old_path},
- {new_path}
- );
+ const p = {old_path, new_path};
+ return this.getChangeURLAndSend(changeNum, 'POST', null, '/edit', p);
},
deleteFileInChangeEdit(changeNum, path) {
- return this.send('DELETE',
- this.getChangeActionURL(changeNum, null,
- '/edit/' + encodeURIComponent(path)
- ));
+ const e = '/edit/' + encodeURIComponent(path);
+ return this.getChangeURLAndSend(changeNum, 'DELETE', null, e);
},
saveChangeEdit(changeNum, path, contents) {
- return this.send('PUT',
- this.getChangeActionURL(changeNum, null,
- '/edit/' + encodeURIComponent(path)
- ),
- contents
- );
+ const e = '/edit/' + encodeURIComponent(path);
+ return this.getChangeURLAndSend(changeNum, 'PUT', null, e, contents);
},
// Deprecated, prefer to use putChangeCommitMessage instead.
saveChangeCommitMessageEdit(changeNum, message) {
- const url = this.getChangeActionURL(changeNum, null, '/edit:message');
- return this.send('PUT', url, {message});
+ const p = {message};
+ return this.getChangeURLAndSend(changeNum, 'PUT', null, '/edit:message',
+ p);
},
publishChangeEdit(changeNum) {
- return this.send('POST',
- this.getChangeActionURL(changeNum, null, '/edit:publish'));
+ return this.getChangeURLAndSend(changeNum, 'POST', null,
+ '/edit:publish');
},
putChangeCommitMessage(changeNum, message) {
- const url = this.getChangeActionURL(changeNum, null, '/message');
- return this.send('PUT', url, {message});
+ const p = {message};
+ return this.getChangeURLAndSend(changeNum, 'PUT', null, '/message', p);
},
saveChangeStarred(changeNum, starred) {
@@ -1004,7 +996,6 @@
getDiff(changeNum, basePatchNum, patchNum, path,
opt_errFn, opt_cancelCondition) {
- const url = this._getDiffFetchURL(changeNum, patchNum, path);
const params = {
context: 'ALL',
intraline: null,
@@ -1013,13 +1004,10 @@
if (basePatchNum != PARENT_PATCH_NUM) {
params.base = basePatchNum;
}
+ const endpoint = `/files/${encodeURIComponent(path)}/diff`;
- return this.fetchJSON(url, opt_errFn, opt_cancelCondition, params);
- },
-
- _getDiffFetchURL(changeNum, patchNum, path) {
- return this._changeBaseURL(changeNum, patchNum) + '/files/' +
- encodeURIComponent(path) + '/diff';
+ return this._getChangeURLAndFetch(changeNum, endpoint, patchNum,
+ opt_errFn, opt_cancelCondition, params);
},
getDiffComments(changeNum, opt_basePatchNum, opt_patchNum, opt_path) {
@@ -1076,11 +1064,20 @@
_getDiffComments(changeNum, endpoint, opt_basePatchNum,
opt_patchNum, opt_path) {
- if (!opt_basePatchNum && !opt_patchNum && !opt_path) {
- return this.fetchJSON(
- this._getDiffCommentsFetchURL(changeNum, endpoint));
- }
+ /**
+ * Fetches the comments for a given patchNum.
+ * Helper function to make promises more legible.
+ *
+ * @param {string|number} patchNum
+ * @return {!Object} Diff comments response.
+ */
+ const fetchComments = patchNum => {
+ return this._getChangeURLAndFetch(changeNum, endpoint, patchNum);
+ };
+ if (!opt_basePatchNum && !opt_patchNum && !opt_path) {
+ return fetchComments();
+ }
function onlyParent(c) { return c.side == PARENT_PATCH_NUM; }
function withoutParent(c) { return c.side != PARENT_PATCH_NUM; }
function setPath(c) { c.path = opt_path; }
@@ -1088,16 +1085,13 @@
const promises = [];
let comments;
let baseComments;
- const url =
- this._getDiffCommentsFetchURL(changeNum, endpoint, opt_patchNum);
- promises.push(this.fetchJSON(url).then(response => {
+ let fetchPromise;
+ fetchPromise = fetchComments(opt_patchNum).then(response => {
comments = response[opt_path] || [];
-
- // TODO(kaspern): Implement this on in the backend so this can be
- // removed.
-
- // Sort comments by date so that parent ranges can be propagated in a
- // single pass.
+ // TODO(kaspern): Implement this on in the backend so this can
+ // be removed.
+ // Sort comments by date so that parent ranges can be propagated
+ // in a single pass.
comments = this._setRanges(comments);
if (opt_basePatchNum == PARENT_PATCH_NUM) {
@@ -1107,18 +1101,17 @@
comments = comments.filter(withoutParent);
comments.forEach(setPath);
- }));
+ });
+ promises.push(fetchPromise);
if (opt_basePatchNum != PARENT_PATCH_NUM) {
- const baseURL = this._getDiffCommentsFetchURL(changeNum, endpoint,
- opt_basePatchNum);
- promises.push(this.fetchJSON(baseURL).then(response => {
- baseComments = (response[opt_path] || []).filter(withoutParent);
-
+ fetchPromise = fetchComments(opt_basePatchNum).then(response => {
+ baseComments = (response[opt_path] || [])
+ .filter(withoutParent);
baseComments = this._setRanges(baseComments);
-
baseComments.forEach(setPath);
- }));
+ });
+ promises.push(fetchPromise);
}
return Promise.all(promises).then(() => {
@@ -1130,7 +1123,8 @@
},
_getDiffCommentsFetchURL(changeNum, endpoint, opt_patchNum) {
- return this._changeBaseURL(changeNum, opt_patchNum) + endpoint;
+ return this._changeBaseURL(changeNum, opt_patchNum)
+ .then(url => url + endpoint);
},
saveDiffDraft(changeNum, patchNum, draft) {
@@ -1161,9 +1155,9 @@
},
_sendDiffDraftRequest(method, changeNum, patchNum, draft) {
- let url = this.getChangeActionURL(changeNum, patchNum, '/drafts');
+ let endpoint = '/drafts';
if (draft.id) {
- url += '/' + draft.id;
+ endpoint += '/' + draft.id;
}
let body;
if (method === 'PUT') {
@@ -1174,7 +1168,8 @@
this._pendingRequests[Requests.SEND_DIFF_DRAFT] = [];
}
- const promise = this.send(method, url, body);
+ const promise = this.getChangeURLAndSend(changeNum, method, patchNum,
+ endpoint, body);
this._pendingRequests[Requests.SEND_DIFF_DRAFT].push(promise);
return promise;
},
@@ -1200,11 +1195,10 @@
getChangeFileContents(changeId, patchNum, path, opt_parentIndex) {
const parent = typeof opt_parentIndex === 'number' ?
'?parent=' + opt_parentIndex : '';
- return this._fetchB64File(
- '/changes/' + encodeURIComponent(changeId) +
- '/revisions/' + encodeURIComponent(patchNum) +
- '/files/' + encodeURIComponent(path) +
- '/content' + parent);
+ return this._changeBaseUrl(changeId, patchNum).then(url => {
+ url = `${url}/files/${encodeURIComponent(path)}/content${parent}`;
+ return this._fetchB64File(url);
+ });
},
getImagesForDiff(changeNum, diff, patchRange) {
@@ -1249,22 +1243,36 @@
});
},
- _changeBaseURL(changeNum, opt_patchNum) {
- let v = '/changes/' + changeNum;
- if (opt_patchNum) {
- v += '/revisions/' + opt_patchNum;
- }
- return v;
+ /**
+ * @param {string} changeNum
+ * @param {number|string=} opt_patchNum
+ * @param {string=} opt_project
+ * @return {!Promise<string>}
+ */
+ _changeBaseURL(changeNum, opt_patchNum, opt_project) {
+ // TODO(kaspern): For full slicer migration, app should warn with a call
+ // stack every time _changeBaseURL is called without a project.
+ const projectPromise = opt_project ?
+ Promise.resolve(opt_project) :
+ this._getFromProjectLookup(changeNum);
+ return projectPromise.then(project => {
+ let url = `/changes/${encodeURIComponent(project)}~${changeNum}`;
+ if (opt_patchNum) {
+ url += `/revisions/${opt_patchNum}`;
+ }
+ return url;
+ });
},
setChangeTopic(changeNum, topic) {
- return this.send('PUT', '/changes/' + encodeURIComponent(changeNum) +
- '/topic', {topic}).then(this.getResponseObject);
+ const p = {topic};
+ return this.getChangeURLAndSend(changeNum, 'PUT', null, '/topic', p)
+ .then(this.getResponseObject);
},
setChangeHashtag(changeNum, hashtag) {
- return this.send('POST', '/changes/' + encodeURIComponent(changeNum) +
- '/hashtags', hashtag).then(this.getResponseObject);
+ return this.getChangeURLAndSend(changeNum, 'POST', null, '/hashtags',
+ hashtag).then(this.getResponseObject);
},
deleteAccountHttpPassword() {
@@ -1299,15 +1307,15 @@
return this.send('DELETE', '/accounts/self/sshkeys/' + id);
},
- deleteVote(changeID, account, label) {
- return this.send('DELETE', '/changes/' + changeID +
- '/reviewers/' + account + '/votes/' + encodeURIComponent(label));
+ deleteVote(changeNum, account, label) {
+ const e = `/reviewers/${account}/votes/${encodeURIComponent(label)}`;
+ return this.getChangeURLAndSend(changeNum, 'DELETE', null, e);
},
setDescription(changeNum, patchNum, desc) {
- return this.send('PUT',
- this.getChangeActionURL(changeNum, patchNum, '/description'),
- {description: desc});
+ const p = {description: desc};
+ return this.getChangeURLAndSend(changeNum, 'PUT', patchNum,
+ '/description', p);
},
confirmEmail(token) {
@@ -1321,14 +1329,12 @@
},
setAssignee(changeNum, assignee) {
- return this.send('PUT',
- this.getChangeActionURL(changeNum, null, '/assignee'),
- {assignee});
+ const p = {assignee};
+ return this.getChangeURLAndSend(changeNum, 'PUT', null, '/assignee', p);
},
deleteAssignee(changeNum) {
- return this.send('DELETE',
- this.getChangeActionURL(changeNum, null, '/assignee'));
+ return this.getChangeURLAndSend(changeNum, 'DELETE', null, '/assignee');
},
probePath(path) {
@@ -1343,8 +1349,7 @@
if (opt_message) {
payload.message = opt_message;
}
- const url = this.getChangeActionURL(changeNum, null, '/wip');
- return this.send('POST', url, payload)
+ return this.getChangeURLAndSend(changeNum, 'POST', null, '/wip', payload)
.then(response => {
if (response.status === 204) {
return 'Change marked as Work In Progress.';
@@ -1353,16 +1358,15 @@
},
startReview(changeNum, opt_body, opt_errFn) {
- return this.send(
- 'POST', this.getChangeActionURL(changeNum, null, '/ready'),
+ return this.getChangeURLAndSend(changeNum, 'POST', null, '/ready',
opt_body, opt_errFn);
},
deleteComment(changeNum, patchNum, commentID, reason) {
- const url = this.changeBaseURL(changeNum, patchNum) +
- '/comments/' + commentID + '/delete';
- return this.send('POST', url, {reason}).then(response =>
- this.getResponseObject(response));
+ const endpoint = `/comments/${commentID}/delete`;
+ const payload = {reason};
+ return this.getChangeURLAndSend(changeNum, 'POST', patchNum, endpoint,
+ payload).then(this.getResponseObject);
},
/**
@@ -1372,6 +1376,7 @@
* @return {Promise<Object>} The change
*/
getChange(changeNum) {
+ // Cannot use _changeBaseURL, as this function is used by _projectLookup.
return this.fetchJSON(`/changes/${changeNum}`);
},
@@ -1405,5 +1410,44 @@
return change.project;
});
},
+
+ /**
+ * Alias for _changeBaseURL.then(send).
+ *
+ * @param {string|number} changeNum
+ * @param {string} method
+ * @param {string} endpoint
+ * @param {string|number=} opt_patchNum
+ * @param {!Object=} opt_payload
+ * @param {function(?Response, string)=} opt_errFn
+ * @return {!Promise<!Object>}
+ */
+ getChangeURLAndSend(changeNum, method, opt_patchNum, endpoint, opt_payload,
+ opt_errFn, opt_ctx, opt_contentType) {
+ return this._changeBaseURL(changeNum, opt_patchNum).then(url => {
+ return this.send(method, url + endpoint, opt_payload, opt_errFn,
+ opt_ctx, opt_contentType);
+ });
+ },
+
+ /**
+ * Alias for _changeBaseURL.then(fetchJSON).
+ *
+ * @param {string|number} changeNum
+ * @param {string} endpoint
+ * @param {string|number=} opt_patchNum
+ * @param {function(?Response, string)=} opt_errFn
+ * @param {!function()=} opt_cancelCondition
+ * @param {!Object=} opt_params
+ * @param {!Object=} opt_options
+ * @return {!Promise<!Object>}
+ */
+ _getChangeURLAndFetch(changeNum, endpoint, opt_patchNum, opt_errFn,
+ opt_cancelCondition, opt_params, opt_options) {
+ return this._changeBaseURL(changeNum, opt_patchNum).then(url => {
+ return this.fetchJSON(url + endpoint, opt_errFn, opt_cancelCondition,
+ opt_params, opt_options);
+ });
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index 2cda640..eac758f 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -269,8 +269,10 @@
});
test('differing patch diff comments are properly grouped', done => {
+ sandbox.stub(element, '_getFromProjectLookup')
+ .returns(Promise.resolve('test'));
sandbox.stub(element, 'fetchJSON', url => {
- if (url == '/changes/42/revisions/1') {
+ if (url === '/changes/test~42/revisions/1') {
return Promise.resolve({
'/COMMIT_MSG': [],
'sieve.go': [
@@ -285,7 +287,7 @@
},
],
});
- } else if (url == '/changes/42/revisions/2') {
+ } else if (url === '/changes/test~42/revisions/2') {
return Promise.resolve({
'/COMMIT_MSG': [],
'sieve.go': [
@@ -606,8 +608,7 @@
test('_sendDiffDraft pending requests tracked', () => {
const obj = element._pendingRequests;
- sandbox.stub(element, 'send', () => mockPromise());
- sandbox.stub(element, 'getChangeActionURL');
+ sandbox.stub(element, 'getChangeURLAndSend', () => mockPromise());
assert.notOk(element.hasPendingDiffDrafts());
element._sendDiffDraftRequest(null, null, null, {});
@@ -627,6 +628,7 @@
});
test('saveChangeEdit', done => {
+ element._projectLookup = {1: 'test'};
const change_num = '1';
const file_name = 'index.php';
const file_contents = '<?php';
@@ -636,17 +638,16 @@
sandbox.stub(element, 'getResponseObject')
.returns(Promise.resolve([change_num, file_name, file_contents]));
element._cache['/changes/' + change_num + '/edit/' + file_name] = {};
- element.saveChangeEdit(change_num, file_name, file_contents).then(
- () => {
- assert.isTrue(element.send.calledWith('PUT',
- '/changes/' + change_num + '/edit/' + file_name,
- file_contents));
- done();
- }
- );
+ element.saveChangeEdit(change_num, file_name, file_contents).then(() => {
+ assert.isTrue(element.send.calledWith('PUT',
+ '/changes/test~1/edit/' + file_name,
+ file_contents));
+ done();
+ });
});
test('putChangeCommitMessage', done => {
+ element._projectLookup = {1: 'test'};
const change_num = '1';
const message = 'this is a commit message';
sandbox.stub(element, 'send').returns(
@@ -655,42 +656,42 @@
sandbox.stub(element, 'getResponseObject')
.returns(Promise.resolve([change_num, message]));
element._cache['/changes/' + change_num + '/message'] = {};
- element.putChangeCommitMessage(change_num, message).then(
- () => {
- assert.isTrue(element.send.calledWith('PUT',
- '/changes/' + change_num + '/message', {message}));
- done();
- }
- );
+ element.putChangeCommitMessage(change_num, message).then(() => {
+ assert.isTrue(element.send.calledWith('PUT',
+ '/changes/test~1/message', {message}));
+ done();
+ });
});
test('startWorkInProgress', () => {
- sandbox.stub(element, 'send').returns(Promise.resolve('ok'));
+ sandbox.stub(element, 'getChangeURLAndSend')
+ .returns(Promise.resolve('ok'));
element.startWorkInProgress('42');
- assert.isTrue(element.send.calledWith(
- 'POST', '/changes/42/wip', {}));
+ assert.isTrue(element.getChangeURLAndSend.calledWith(
+ '42', 'POST', null, '/wip', {}));
element.startWorkInProgress('42', 'revising...');
- assert.isTrue(element.send.calledWith(
- 'POST', '/changes/42/wip', {message: 'revising...'}));
+ assert.isTrue(element.getChangeURLAndSend.calledWith(
+ '42', 'POST', null, '/wip', {message: 'revising...'}));
});
test('startReview', () => {
- sandbox.stub(element, 'send').returns(Promise.resolve({}));
+ sandbox.stub(element, 'getChangeURLAndSend')
+ .returns(Promise.resolve({}));
element.startReview('42', {message: 'Please review.'});
- assert.isTrue(element.send.calledWith(
- 'POST', '/changes/42/ready', {message: 'Please review.'}));
+ assert.isTrue(element.getChangeURLAndSend.calledWith(
+ '42', 'POST', null, '/ready', {message: 'Please review.'}));
});
test('deleteComment', done => {
- sandbox.stub(element, 'send').returns(Promise.resolve());
+ sandbox.stub(element, 'getChangeURLAndSend').returns(Promise.resolve());
sandbox.stub(element, 'getResponseObject').returns('some response');
element.deleteComment('foo', 'bar', '01234', 'removal reason')
.then(response => {
assert.equal(response, 'some response');
done();
});
- assert.isTrue(element.send.calledWith(
- 'POST', '/changes/foo/revisions/bar/comments/01234/delete',
+ assert.isTrue(element.getChangeURLAndSend.calledWith(
+ 'foo', 'POST', 'bar', '/comments/01234/delete',
{reason: 'removal reason'}));
});
@@ -763,10 +764,9 @@
test('_getChangeDetail passes params to ETags decorator', () => {
const changeNum = 4321;
+ element._projectLookup[changeNum] = 'test';
const params = {foo: 'bar'};
- const expectedUrl = element._urlWithParams(
- element.getChangeActionURL(changeNum, null, '/detail'),
- params);
+ const expectedUrl = '/changes/test~4321/detail?foo=bar';
sandbox.stub(element._etags, 'getOptions');
sandbox.stub(element._etags, 'collect');
return element._getChangeDetail(changeNum, params).then(() => {
@@ -858,5 +858,23 @@
assert.equal(element._projectLookup[1], 'test');
});
});
+
+ test('_getChangeURLAndFetch', () => {
+ element._projectLookup = {1: 'test'};
+ const fetchStub = sandbox.stub(element, 'fetchJSON')
+ .returns(Promise.resolve());
+ return element._getChangeURLAndFetch(1, '/test', 1).then(() => {
+ assert.isTrue(fetchStub.calledWith('/changes/test~1/revisions/1/test'));
+ });
+ });
+
+ test('getChangeURLAndSend', () => {
+ element._projectLookup = {1: 'test'};
+ const sendStub = sandbox.stub(element, 'send').returns(Promise.resolve());
+ return element.getChangeURLAndSend(1, 'POST', 1, '/test').then(() => {
+ assert.isTrue(sendStub.calledWith('POST',
+ '/changes/test~1/revisions/1/test'));
+ });
+ });
});
</script>
