Add "Start review" workflow using work_in_progress
Feature: Issue 3799
Feature: Issue 5310
Change-Id: I262c156b0155d1b64b84a9df91279979b41daede
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 8807917..1c3642d 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
@@ -17,6 +17,17 @@
(function(window) {
'use strict';
+ // Tags identifying ChangeMessages that move change into WIP state.
+ var WIP_TAGS = [
+ 'autogenerated:gerrit:newWipPatchSet',
+ 'autogenerated:gerrit:setWorkInProgress',
+ ];
+
+ // Tags identifying ChangeMessages that move change out of WIP state.
+ var READY_TAGS = [
+ 'autogenerated:gerrit:setReadyForReview',
+ ];
+
/** @polymerBehavior Gerrit.PatchSetBehavior */
var PatchSetBehavior = {
/**
@@ -37,7 +48,23 @@
}
},
+ /**
+ * Construct a chronological list of patch sets derived from change details.
+ * Each element of this list is an object with the following properties:
+ *
+ * * num {number} The number identifying the patch set
+ * * desc {!string} Optional patch set description
+ * * wip {boolean} If true, this patch set was never subject to review.
+ *
+ * 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
+ * above
+ */
computeAllPatchSets: function(change) {
+ if (!change) { return []; }
var patchNums = [];
for (var commit in change.revisions) {
if (change.revisions.hasOwnProperty(commit)) {
@@ -47,10 +74,45 @@
});
}
}
- return patchNums.sort(function(a, b) { return a.num - b.num; });
+ patchNums.sort(function(a, b) { return a.num - b.num; });
+ return this._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
+ * generated by computeAllPatchSets
+ * @return {Array<Object>} The given list of patch set objects, with the
+ * wip property set on each of them
+ */
+ _computeWipForPatchSets: function(change, patchNums) {
+ if (!change.messages || !change.messages.length) {
+ return patchNums;
+ }
+ var psWip = {};
+ var wip = change.work_in_progress;
+ for (var i = 0; i < change.messages.length; i++) {
+ var msg = change.messages[i];
+ if (WIP_TAGS.indexOf(msg.tag) != -1) {
+ wip = true;
+ } else if (READY_TAGS.indexOf(msg.tag) != -1) {
+ wip = false;
+ }
+ if (psWip[msg._revision_number] !== false) {
+ psWip[msg._revision_number] = wip;
+ }
+ }
+
+ for (var i = 0; i < patchNums.length; i++) {
+ patchNums[i].wip = psWip[patchNums[i].num];
+ }
+ return patchNums;
},
computeLatestPatchNum: function(allPatchSets) {
+ if (!allPatchSets || !allPatchSets.length) { return undefined; }
return allPatchSets[allPatchSets.length - 1].num;
},
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 892d94b..87e6455 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
@@ -79,5 +79,87 @@
done();
});
});
+
+ test('_computeWipForPatchSets', function() {
+ // Compute patch sets for a given timeline on a change. The initial WIP
+ // property of the change can be true or false. The map of tags by
+ // revision is keyed by patch set number. Each value is a list of change
+ // message tags in the order that they occurred in the timeline. These
+ // indicate actions that modify the WIP property of the change and/or
+ // create new patch sets.
+ //
+ // Returns the actual results with an assertWip method that can be used
+ // to compare against an expected value for a particular patch set.
+ function compute(initialWip, tagsByRevision) {
+ var change = {
+ messages: [],
+ work_in_progress: initialWip,
+ };
+ var revs = Object.keys(tagsByRevision).sort(function(a, b) {
+ return a - b;
+ });
+ revs.forEach(function(rev) {
+ tagsByRevision[rev].forEach(function(tag) {
+ change.messages.push({
+ tag: tag,
+ _revision_number: rev,
+ });
+ });
+ });
+ var patchNums = revs.map(function(rev) { return {num: rev}; });
+ patchNums = Gerrit.PatchSetBehavior._computeWipForPatchSets(
+ change, patchNums);
+ var actualWipsByRevision = {};
+ patchNums.forEach(function(patchNum) {
+ actualWipsByRevision[patchNum.num] = patchNum.wip;
+ });
+ var verifier = {
+ assertWip: function(revision, expectedWip) {
+ var patchNum = patchNums.find(function(patchNum) {
+ return patchNum.num == revision;
+ });
+ if (!patchNum) {
+ assert.fail('revision ' + revision + ' not found');
+ }
+ assert.equal(patchNum.wip, expectedWip,
+ 'wip state for ' + revision + ' is ' +
+ patchNum.wip + '; expected ' + expectedWip);
+ return verifier;
+ },
+ };
+ return verifier;
+ }
+
+ compute(false, {1: ['upload']}).assertWip(1, false);
+ compute(true, {1: ['upload']}).assertWip(1, true);
+
+ var setWip = 'autogenerated:gerrit:setWorkInProgress';
+ var uploadInWip = 'autogenerated:gerrit:newWipPatchSet';
+ var clearWip = 'autogenerated:gerrit:setReadyForReview';
+
+ compute(false, {
+ 1: ['upload', setWip],
+ 2: ['upload'],
+ 3: ['upload', clearWip],
+ 4: ['upload', setWip],
+ }).assertWip(1, false) // Change was created with PS1 ready for review
+ .assertWip(2, true) // PS2 was uploaded during WIP
+ .assertWip(3, false) // PS3 was marked ready for review after upload
+ .assertWip(4, false); // PS4 was uploaded ready for review
+
+ compute(false, {
+ 1: [uploadInWip, null, 'addReviewer'],
+ 2: ['upload'],
+ 3: ['upload', clearWip, setWip],
+ 4: ['upload'],
+ 5: ['upload', clearWip],
+ 6: [uploadInWip],
+ }).assertWip(1, true) // Change was created in WIP
+ .assertWip(2, true) // PS2 was uploaded during WIP
+ .assertWip(3, false) // PS3 was marked ready for review
+ .assertWip(4, true) // PS4 was uploaded during WIP
+ .assertWip(5, false) // PS5 was marked ready for review
+ .assertWip(6, true); // PS6 was uploaded with WIP option
+ });
});
</script>
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 31721a0..bef92ed 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,6 +51,7 @@
RESTORE: 'restore',
REVERT: 'revert',
UNIGNORE: 'unignore',
+ WIP: 'wip',
};
// TODO(andybons): Add the rest of the revision actions.
@@ -596,6 +597,9 @@
case ChangeActions.DELETE:
this._handleDeleteTap();
break;
+ case ChangeActions.WIP:
+ this._handleWipTap();
+ break;
default:
this._fireAction(this._prependSlash(key), this.actions[key], false);
}
@@ -786,6 +790,9 @@
page.show(this.changePath(this.changeNum));
}
break;
+ case ChangeActions.WIP:
+ page.show(this.changePath(this.changeNum));
+ break;
default:
this.dispatchEvent(new CustomEvent('reload-change',
{detail: {action: action.__key}, bubbles: false}));
@@ -850,6 +857,10 @@
this._showActionDialog(this.$.confirmDeleteDialog);
},
+ _handleWipTap: function() {
+ this._fireAction('/wip', this.actions.wip, false);
+ },
+
/**
* Merge sources of change actions into a single ordered array of action
* values.
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index 778b09c..7408e04 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -255,6 +255,9 @@
<section class="labelStatus">
<span class="title">Label Status</span>
<span class="value">
+ <div hidden$="[[!change.work_in_progress]]">
+ Work in progress
+ </div>
<div hidden$="[[!_showMissingLabels(change.labels)]]">
[[_computeMissingLabelsHeader(change.labels)]]
<ul id="missingLabels">
@@ -265,7 +268,7 @@
</template>
</ul>
</div>
- <div hidden$="[[_showMissingLabels(change.labels)]]">
+ <div hidden$="[[_showMissingRequirements(change.labels, change.work_in_progress)]]">
Ready to submit
</div>
</span>
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index 58938af..9046d1b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -242,6 +242,10 @@
return !!this._computeMissingLabels(labels).length;
},
+ _showMissingRequirements: function(labels, workInProgress) {
+ return workInProgress || this._showMissingLabels(labels);
+ },
+
_computeProjectURL: function(project) {
return this.getBaseUrl() + '/q/project:' +
this.encodeURL(project, false);
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
index c531c79..0ac8531 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -88,6 +88,17 @@
});
test('computes submit status', function() {
+ var showMissingLabels = false;
+ sandbox.stub(element, '_showMissingLabels', function() {
+ return showMissingLabels;
+ });
+ assert.isFalse(element._showMissingRequirements(null, false));
+ assert.isTrue(element._showMissingRequirements(null, true));
+ showMissingLabels = true;
+ assert.isTrue(element._showMissingRequirements(null, false));
+ });
+
+ test('show missing labels', function() {
var labels = {};
assert.isFalse(element._showMissingLabels(labels));
labels = {test: {}};
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 ff3f8b5..776ace2 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
@@ -68,6 +68,11 @@
transition: box-shadow 250ms linear;
width: 100%;
}
+ .header.wip {
+ background-color: #fcfad6;
+ border-bottom: 1px solid #ddd;
+ margin-bottom: .5em;
+ }
.header-title {
flex: 1;
font-size: 1.2em;
@@ -277,7 +282,7 @@
</style>
<div class="container loading" hidden$="[[!_loading]]">Loading...</div>
<div class="container" hidden$="{{_loading}}">
- <div class="header">
+ <div class$="[[_computeHeaderClass(_change)]]">
<span class="header-title">
<gr-change-star
id="changeStar"
@@ -301,6 +306,7 @@
--></template><!--
-->)<!--
--></template><!--
+ --><span hidden$="[[!_change.work_in_progress]]"> (Work in progress)</span><!--
-->: [[_change.subject]]
</span>
</div>
@@ -501,10 +507,12 @@
diff-drafts="[[_diffDrafts]]"
server-config="[[serverConfig]]"
project-config="[[_projectConfig]]"
+ can-be-started="[[_canStartReview]]"
on-send="_handleReplySent"
on-cancel="_handleReplyCancel"
on-autogrow="_handleReplyAutogrow"
- hidden$="[[!_loggedIn]]">Reply</gr-reply-dialog>
+ hidden$="[[!_loggedIn]]">
+ </gr-reply-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-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 488a20d..112b2a7 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
@@ -81,6 +81,10 @@
type: Object,
value: {},
},
+ _canStartReview: {
+ type: Boolean,
+ computed: '_computeCanStartReview(_loggedIn, _change, _account)',
+ },
_comments: Object,
_change: {
type: Object,
@@ -135,7 +139,7 @@
_replyButtonLabel: {
type: String,
value: 'Reply',
- computed: '_computeReplyButtonLabel(_diffDrafts.*)',
+ computed: '_computeReplyButtonLabel(_diffDrafts.*, _canStartReview)',
},
_selectedPatchSet: String,
_initialLoadComplete: {
@@ -562,7 +566,7 @@
},
_changeChanged: function(change) {
- if (!change) { return; }
+ if (!change || !this._patchRange || !this._allPatchSets) { return; }
this.set('_patchRange.basePatchNum',
this._patchRange.basePatchNum || 'PARENT');
this.set('_patchRange.patchNum',
@@ -718,7 +722,11 @@
return result;
},
- _computeReplyButtonLabel: function(changeRecord) {
+ _computeReplyButtonLabel: function(changeRecord, canStartReview) {
+ if (canStartReview) {
+ return 'Start review';
+ }
+
var drafts = (changeRecord && changeRecord.base) || {};
var draftCount = Object.keys(drafts).reduce(function(count, file) {
return count + drafts[file].length;
@@ -1084,6 +1092,11 @@
}
},
+ _computeCanStartReview: function(loggedIn, change, account) {
+ return loggedIn && change.work_in_progress &&
+ change.owner._account_id === account._account_id;
+ },
+
_computeDescriptionReadOnly: function(loggedIn, change, account) {
return !(loggedIn && (account._account_id === change.owner._account_id));
},
@@ -1230,5 +1243,9 @@
this._startUpdateCheckTimer();
}
},
+
+ _computeHeaderClass: function(change) {
+ return change.work_in_progress ? 'header wip' : 'header';
+ },
});
})();
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 cec1c23..b7c848c 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
@@ -98,6 +98,7 @@
test('A toggles overlay when logged in', function(done) {
sandbox.stub(element, '_getLoggedIn').returns(Promise.resolve(true));
+ element._change = {labels: {}};
MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a');
flush(function() {
assert.isTrue(element.$.replyOverlay.opened);
@@ -348,21 +349,28 @@
});
test('reply button has updated count when there are drafts', function() {
- var replyButton = element.$$('gr-button.reply');
- assert.ok(replyButton);
- assert.equal(replyButton.textContent, 'Reply');
+ var getLabel = element._computeReplyButtonLabel;
- element._diffDrafts = null;
- assert.equal(replyButton.textContent, 'Reply');
+ assert.equal(getLabel(null, false), 'Reply');
+ assert.equal(getLabel(null, true), 'Start review');
- element._diffDrafts = {};
- assert.equal(replyButton.textContent, 'Reply');
+ var changeRecord = {base: null};
+ assert.equal(getLabel(changeRecord, false), 'Reply');
- element._diffDrafts = {
+ changeRecord.base = {};
+ assert.equal(getLabel(changeRecord, false), 'Reply');
+
+ changeRecord.base = {
'file1.txt': [{}],
'file2.txt': [{}, {}],
};
- assert.equal(replyButton.textContent, 'Reply (3)');
+ assert.equal(getLabel(changeRecord, false), 'Reply (3)');
+ });
+
+ test('start review button when owner of WIP change', function() {
+ assert.equal(
+ element._computeReplyButtonLabel(null, true),
+ 'Start review');
});
test('comment events properly update diff drafts', function() {
@@ -979,6 +987,7 @@
suite('reply dialog tests', function() {
setup(function() {
sandbox.stub(element.$.replyDialog, '_draftChanged');
+ element._change = {labels: {}};
});
test('reply from comment adds quote text', function() {
@@ -1183,6 +1192,26 @@
element.serverConfig = {change: {update_delay: 12345}};
});
});
+
+ test('canStartReview computation', function() {
+ var account1 = {_account_id: 1};
+ var account2 = {_account_id: 2};
+ var change = {
+ owner: {_account_id: 1},
+ work_in_progress: false,
+ };
+ assert.isFalse(element._computeCanStartReview(true, change, account1));
+ change.work_in_progress = true;
+ assert.isTrue(element._computeCanStartReview(true, change, account1));
+ assert.isFalse(element._computeCanStartReview(false, change, account1));
+ assert.isFalse(element._computeCanStartReview(true, change, account2));
+ });
+
+ test('header class computation', function() {
+ assert.equal(element._computeHeaderClass({}), 'header');
+ assert.equal(element._computeHeaderClass({work_in_progress: true}),
+ 'header wip');
+ });
});
});
</script>
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 9b64809..6f8568c 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
@@ -223,10 +223,11 @@
<option value="PARENT">Base</option>
<template
is="dom-repeat"
- items="[[_computePatchSets(revisions.*, patchRange.*)]]"
+ items="[[computeAllPatchSets(change)]]"
as="patchNum">
- <option value$="[[patchNum.num]]"
- disabled$="[[_computePatchSetDisabled(patchNum.num, patchRange.patchNum)]]">
+ <option
+ disabled$="[[_computePatchSetDisabled(patchNum.num, patchRange.patchNum)]]"
+ value$="[[patchNum.num]]">
[[patchNum.num]]
[[patchNum.desc]]
</option>
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 91d5eca..705a8f1 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
@@ -206,20 +206,6 @@
return this.$.restAPI.getPreferences();
},
- _computePatchSets: function(revisionRecord) {
- var revisions = revisionRecord.base;
- var patchNums = [];
- for (var commit in revisions) {
- if (revisions.hasOwnProperty(commit)) {
- patchNums.push({
- num: revisions[commit]._number,
- desc: revisions[commit].description,
- });
- }
- }
- return patchNums.sort(function(a, b) { return a.num - b.num; });
- },
-
_computePatchSetDisabled: function(patchNum, currentPatchNum) {
return parseInt(patchNum, 10) >= parseInt(currentPatchNum, 10);
},
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 e51c33a..a34463b 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
@@ -594,8 +594,8 @@
{num: 3, desc: 'test'},
{num: 4, desc: 'test'},
];
- var patchNums = element._computePatchSets({
- base: {
+ var patchNums = element.computeAllPatchSets({
+ revisions: {
rev3: {_number: 3, description: 'test'},
rev1: {_number: 1, description: 'test'},
rev4: {_number: 4, description: 'test'},
@@ -624,10 +624,12 @@
basePatchNum: 'PARENT',
patchNum: '3',
};
- element.revisions = {
- rev1: {_number: 1},
- rev2: {_number: 2},
- rev3: {_number: 3},
+ element.change = {
+ revisions: {
+ rev1: {_number: 1},
+ rev2: {_number: 2},
+ rev3: {_number: 3},
+ },
};
flush(function() {
var selectEl = element.$.patchChange;
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
index d08961c..d9cdcc0 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.html
@@ -204,7 +204,7 @@
id="textarea"
class="message"
autocomplete="on"
- placeholder="Say something nice..."
+ placeholder=[[_messagePlaceholder]]
disabled="{{disabled}}"
rows="4"
max-rows="15"
@@ -248,7 +248,14 @@
primary
disabled="[[!_isState(knownLatestState, 'latest')]]"
class="action send"
- on-tap="_sendTapHandler">Send</gr-button>
+ on-tap="_sendTapHandler">[[_sendButtonLabel]]</gr-button>
+ </gr-button>
+ <template is="dom-if" if="[[canBeStarted]]">
+ <gr-button
+ disabled="[[!_isState(knownLatestState, 'latest')]]"
+ class="action save"
+ on-tap="_saveTapHandler">Save</gr-button>
+ </template>
<span
id="checkingStatusLabel"
hidden$="[[!_isState(knownLatestState, 'checking')]]">
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index 60d3c4f..79a1f34 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -65,6 +65,10 @@
properties: {
change: Object,
patchNum: String,
+ canBeStarted: {
+ type: Boolean,
+ value: false,
+ },
disabled: {
type: Boolean,
value: false,
@@ -90,6 +94,10 @@
serverConfig: Object,
projectConfig: Object,
knownLatestState: String,
+ underReview: {
+ type: Boolean,
+ value: true,
+ },
_account: Object,
_ccs: Array,
@@ -97,6 +105,10 @@
type: Object,
observer: '_reviewerPendingConfirmationUpdated',
},
+ _messagePlaceholder: {
+ type: String,
+ computed: '_computeMessagePlaceholder(canBeStarted)',
+ },
_owner: Object,
_pendingConfirmationDetails: Object,
_includeComments: {
@@ -120,6 +132,10 @@
REVIEWER: [],
},
},
+ _sendButtonLabel: {
+ type: String,
+ computed: '_computeSendButtonLabel(canBeStarted)',
+ },
},
FocusTarget: FocusTarget,
@@ -417,6 +433,12 @@
if (total > 1) { return total + ' Drafts'; }
},
+ _computeMessagePlaceholder: function(canBeStarted) {
+ return canBeStarted ?
+ 'Add a note for your reviewers...' :
+ 'Say something nice...';
+ },
+
_changeUpdated: function(changeRecord, owner, serverConfig) {
this._rebuildReviewerArrays(changeRecord.base, owner, serverConfig);
},
@@ -499,18 +521,39 @@
this.serverConfig);
},
- _sendTapHandler: function(e) {
+ _saveTapHandler: function(e) {
e.preventDefault();
this.send(this._includeComments).then(function(keepReviewers) {
this._purgeReviewersPendingRemove(false, keepReviewers);
}.bind(this));
},
+ _sendTapHandler: function(e) {
+ e.preventDefault();
+ if (this.canBeStarted) {
+ this._startReview()
+ .then(function() {
+ return this.send(this._includeComments);
+ }.bind(this))
+ .then(this._purgeReviewersPendingRemove.bind(this));
+ return;
+ }
+ this.send(this._includeComments)
+ .then(this._purgeReviewersPendingRemove.bind(this));
+ },
+
_saveReview: function(review, opt_errFn) {
return this.$.restAPI.saveChangeReview(this.change._number, this.patchNum,
review, opt_errFn);
},
+ _startReview: function() {
+ if (!this.canBeStarted) {
+ return Promise.resolve();
+ }
+ return this.$.restAPI.startReview(this.change._number);
+ },
+
_reviewerPendingConfirmationUpdated: function(reviewer) {
if (reviewer === null) {
this.$.reviewerConfirmationOverlay.close();
@@ -583,5 +626,9 @@
// Load the current change without any patch range.
location.href = this.getBaseUrl() + '/c/' + this.change._number;
},
+
+ _computeSendButtonLabel: function(canBeStarted) {
+ return canBeStarted ? "Start review" : "Send";
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 34ac167..55a2c9c 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -676,5 +676,23 @@
assert.isTrue(cancelHandler.called);
});
+
+ test('_computeMessagePlaceholder', function() {
+ assert.equal(
+ element._computeMessagePlaceholder(false),
+ 'Say something nice...');
+ assert.equal(
+ element._computeMessagePlaceholder(true),
+ 'Add a note for your reviewers...');
+ });
+
+ test('_computeSendButtonLabel', function() {
+ assert.equal(
+ element._computeSendButtonLabel(false),
+ 'Send');
+ assert.equal(
+ element._computeSendButtonLabel(true),
+ 'Start review');
+ });
});
</script>
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 d7e3328..ad79a2c 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
@@ -1131,5 +1131,24 @@
return response.ok;
});
},
+
+ startWorkInProgress: function(changeNum, opt_message) {
+ var payload = {};
+ if (opt_message) {
+ payload.message = opt_message;
+ }
+ var url = this.getChangeActionURL(changeNum, null, '/wip');
+ return this.send('POST', url, payload)
+ .then(function(response) {
+ if (response.status === 204) {
+ return 'Change marked as Work In Progress.';
+ }
+ });
+ },
+
+ startReview: function(changeNum, review) {
+ return this.send(
+ 'POST', this.getChangeActionURL(changeNum, null, '/ready'), review);
+ },
});
})();
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 f2f41b8..933b4f4 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
@@ -613,5 +613,22 @@
}
);
});
+
+ test('startWorkInProgress', function() {
+ sandbox.stub(element, 'send').returns(Promise.resolve('ok'));
+ element.startWorkInProgress('42');
+ assert.isTrue(element.send.calledWith(
+ 'POST', '/changes/42/wip', {}));
+ element.startWorkInProgress('42', 'revising...');
+ assert.isTrue(element.send.calledWith(
+ 'POST', '/changes/42/wip', {message: 'revising...'}));
+ });
+
+ test('startReview', function() {
+ sandbox.stub(element, 'send').returns(Promise.resolve({}));
+ element.startReview('42', {message: 'Please review.'});
+ assert.isTrue(element.send.calledWith(
+ 'POST', '/changes/42/ready', {message: 'Please review.'}));
+ });
});
</script>