opendev/gerrit
Code Issues Proposed changes

Create gr-apply-fix-dialog and show fix preview.

Browse Source 
in this change:
- add "Show Fix" option to gr-comment when fixes exist
- add GET /preview and POST /apply endpoints to gr-rest-api-interface
- create gr-apply-fix-dialog that fetch preview and supports showing
multiple diffs for a single fix, apply fix button that navigate to
edit-change view, and cancel that close dialog

Screenshots: https://imgur.com/a/6xQIoz8

next:
- support switching between multiple fixes

Change-Id: I9ba5121dd3b5b51791540ac9758f98c8669b44b2
This commit is contained in:
Julie Pan
2019-10-07 07:37:02 -07:00
parent 96c56f5180
commit 49d0a6e10d
13 changed files with 463 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
View File

@@ -51,6 +51,7 @@ limitations under the License.
<link rel="import" href="../gr-included-in-dialog/gr-included-in-dialog.html">
<link rel="import" href="../gr-messages-list/gr-messages-list.html">
<link rel="import" href="../gr-related-changes-list/gr-related-changes-list.html">
<link rel="import" href="../../diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html">
<link rel="import" href="../gr-reply-dialog/gr-reply-dialog.html">
<link rel="import" href="../gr-thread-list/gr-thread-list.html">
<link rel="import" href="../gr-upload-help-dialog/gr-upload-help-dialog.html">
@@ -635,6 +636,11 @@ limitations under the License.
on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
</template>
</div>
<gr-apply-fix-dialog
id="applyFixDialog"
prefs="[[_diffPrefs]]"
change="[[_change]]"
change-num="[[_changeNum]]"></gr-apply-fix-dialog>
<gr-overlay id="downloadOverlay" with-backdrop>
<gr-download-dialog
id="downloadDialog"

14
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
View File

@@ -381,6 +381,10 @@
this._handleCommitMessageSave.bind(this));
this.addEventListener('editable-content-cancel',
this._handleCommitMessageCancel.bind(this));
this.addEventListener('open-fix-preview',
this._onOpenFixPreview.bind(this));
this.addEventListener('close-fix-preview',
this._onCloseFixPreview.bind(this));
this.listen(window, 'scroll', '_handleScroll');
this.listen(document, 'visibilitychange', '_handleVisibilityChange');
}
@@ -420,6 +424,14 @@
});
}
_onOpenFixPreview(e) {
this.$.applyFixDialog.open(e);
}
_onCloseFixPreview(e) {
this._reload();
}
_handleToggleDiffMode(e) {
if (this.shouldSuppressKeyboardShortcut(e) ||
this.modifierPressed(e)) { return; }
@@ -1461,7 +1473,7 @@
let coreDataPromise;
// If the patch number is specified
if (this._patchRange.patchNum) {
if (this._patchRange && this._patchRange.patchNum) {
// Because a specific patchset is specified, reload the resources that
// are keyed by patch number or patch range.
const patchResourcesLoaded = this._reloadPatchNumDependentResources();

73
polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.html Normal file
View File

@@ -0,0 +1,73 @@
<!--
@license
Copyright (C) 2019 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="/bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/fire-behavior/fire-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-dialog/gr-dialog.html">
<link rel="import" href="../../shared/gr-overlay/gr-overlay.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-apply-fix-dialog">
<template>
<style include="shared-styles">
gr-diff {
--content-width: 90vw;
}
.diffContainer {
padding: var(--spacing-l) 0;
border-bottom: 1px solid var(--border-color);
}
.file-name {
display: block;
padding: var(--spacing-s) var(--spacing-l);
background-color: var(--background-color-secondary);
border-bottom: 1px solid var(--border-color);
}
.fixActions {
display: flex;
justify-content: flex-end;
}
</style>
<gr-overlay id="applyFixOverlay" with-backdrop>
<gr-dialog
id="applyFixDialog"
on-confirm="_handleApplyFix"
confirm-label="[[_getApplyFixButtonLabel(_isApplyFixLoading)]]"
disabled="[[_isApplyFixLoading]]"
on-cancel="onCancel">
<div slot="header">[[_robotId]] - [[getFixDescription(_currentFix)]]</div>
<div slot="main">
<template is="dom-repeat" items="[[_currentPreviews]]">
<div class="file-name">
<span>[[item.filepath]]</span>
</div>
<div class="diffContainer">
<gr-diff
prefs="[[overridePartialPrefs(prefs)]]"
change-num="[[changeNum]]"
path="[[item.filepath]]"
diff="[[item.preview]]"></gr-diff>
</div>
</template>
</div>
</gr-dialog>
</gr-overlay>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
</template>
<script src="gr-apply-fix-dialog.js"></script>
</dom-module>

153
polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog.js Normal file
View File

@@ -0,0 +1,153 @@
/**
* @license
* Copyright (C) 2019 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
(function() {
'use strict';
Polymer({
is: 'gr-apply-fix-dialog',
properties: {
// Diff rendering preference API response.
prefs: Array,
// ChangeInfo API response object.
change: Object,
changeNum: String,
_patchNum: Number,
// robot ID associated with a robot comment.
_robotId: String,
// Selected FixSuggestionInfo entity from robot comment API response.
_currentFix: Object,
// Flattened /preview API response DiffInfo map object.
_currentPreviews: {type: Array, value: () => []},
// FixSuggestionInfo entities from robot comment API response.
_fixSuggestions: Array,
_isApplyFixLoading: {
type: Boolean,
value: false,
},
},
behaviors: [
Gerrit.FireBehavior,
],
/**
* Given robot comment CustomEvent objevt, fetch diffs associated
* with first robot comment suggested fix and open dialog.
*
* @param {*} e CustomEvent to be passed from gr-comment with
* robot comment detail.
* @return {Promise<undefined>} Promise that resolves either when all
* preview diffs are fetched or no fix suggestions in custom event detail.
*/
open(e) {
this._patchNum = e.detail.patchNum;
this._fixSuggestions = e.detail.comment.fix_suggestions;
this._robotId = e.detail.comment.robot_id;
if (this._fixSuggestions == null || this._fixSuggestions.length == 0) {
return Promise.resolve();
}
const promises = [];
promises.push(
this._showSelectedFixSuggestion(this._fixSuggestions[0]),
this.$.applyFixOverlay.open()
);
return Promise.all(promises)
.then(() => {
// ensures gr-overlay repositions overlay in center
this.$.applyFixOverlay.fire('iron-resize');
});
},
_showSelectedFixSuggestion(fixSuggestion) {
this._currentFix = fixSuggestion;
return this._fetchFixPreview(fixSuggestion.fix_id);
},
_fetchFixPreview(fixId) {
return this.$.restAPI
.getRobotCommentFixPreview(this.changeNum, this._patchNum, fixId)
.then(res => {
if (res != null) {
const previews = Object.keys(res).map(key =>
({filepath: key, preview: res[key]}));
this._currentPreviews = previews;
}
}).catch(err => {
this._close();
this.dispatchEvent(new CustomEvent('show-error', {
bubbles: true,
composed: true,
detail: {message: `Error generating fix preview: ${err}`},
}));
});
},
overridePartialPrefs(prefs) {
// generate a smaller gr-diff than fullscreen for dialog
return Object.assign({}, prefs, {line_length: 50});
},
onCancel(e) {
if (e) {
e.stopPropagation();
}
this._close();
},
_close() {
this._currentFix = {};
this._currentPreviews = [];
this._isApplyFixLoading = false;
this.dispatchEvent(new CustomEvent('close-fix-preview', {
bubbles: true,
composed: true,
}));
this.$.applyFixOverlay.close();
},
_getApplyFixButtonLabel(isLoading) {
return isLoading ? 'Saving...' : 'Apply Fix';
},
_handleApplyFix(e) {
if (e) {
e.stopPropagation();
}
if (this._currentFix == null || this._currentFix.fix_id == null) {
return;
}
this._isApplyFixLoading = true;
return this.$.restAPI.applyFixSuggestion(this.changeNum, this._patchNum,
this._currentFix.fix_id).then(res => {
Gerrit.Nav.navigateToChange(this.change, 'edit', this._patchNum);
this._close();
}).catch(err => {
this.dispatchEvent(new CustomEvent('show-error', {
bubbles: true,
composed: true,
detail: {message: `Error applying fix suggestion: ${err}`},
}));
});
},
getFixDescription(currentFix) {
return currentFix != null && currentFix.description ?
currentFix.description : '';
},
});
})();

143
polygerrit-ui/app/elements/diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html Normal file
View File

@@ -0,0 +1,143 @@
<!DOCTYPE html>
<!--
@license
Copyright (C) 2019 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the 'License');
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an 'AS IS' BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<meta name='viewport' content='width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes'>
<title>gr-apply-fix-dialog</title>
<script src='/test/common-test-setup.js'></script>
<script src='/bower_components/webcomponentsjs/custom-elements-es5-adapter.js'></script>
<script src='/bower_components/webcomponentsjs/webcomponents-lite.js'></script>
<script src='/bower_components/web-component-tester/browser.js'></script>
<link rel='import' href='../../../test/common-test-setup.html' />
<link rel='import' href='./gr-apply-fix-dialog.html'>
<script>void (0);</script>
<test-fixture id='basic'>
<template>
<gr-apply-fix-dialog></gr-apply-fix-dialog>
</template>
</test-fixture>
<script>
suite('gr-apply-fix-dialog tests', () => {
let element;
let sandbox;
const ROBOT_COMMENT = {
robot_id: 'robot_1',
fix_suggestions: [{fix_id: 'fix_1'}],
};
setup(() => {
sandbox = sinon.sandbox.create();
element = fixture('basic');
element.changeNum = '1';
element._patchNum = 2;
element.change = {
_number: '1',
project: 'project',
};
element.prefs = {
font_size: 12,
line_length: 100,
tab_size: 4,
};
});
teardown(() => { sandbox.restore(); });
test('dialog opens fetch and set previews', done => {
sandbox.stub(element.$.restAPI, 'getRobotCommentFixPreview')
.returns(Promise.resolve({
f1: {
meta_a: {},
meta_b: {},
content: [
{
ab: ['loqlwkqll'],
},
{
b: ['qwqqsqw'],
},
{
ab: ['qwqqsqw', 'qweqeqweqeq', 'qweqweq'],
},
],
},
f2: {
meta_a: {},
meta_b: {},
content: [
{
ab: ['eqweqweqwex'],
},
{
b: ['zassdasd'],
},
{
ab: ['zassdasd', 'dasdasda', 'asdasdad'],
},
],
},
}));
sandbox.stub(element.$.applyFixOverlay, 'open').returns(Promise.resolve());
element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT}})
.then(() => {
assert.equal(element._currentFix.fix_id, 'fix_1');
assert.equal(element._currentPreviews.length, 2);
assert.equal(element._robotId, 'robot_1');
done();
});
});
test('preview endpoint throws error should reset dialog', done => {
element.addEventListener('show-error', () => {
assert.deepEqual(element._currentFix, {});
assert.equal(element._currentPreviews.length, 0);
done();
});
sandbox.stub(element.$.restAPI, 'getRobotCommentFixPreview',
() => Promise.reject(new Error('backend error')));
element.open({detail: {patchNum: 2, comment: ROBOT_COMMENT}});
});
test('apply fix button should call apply' +
'and navigate to change view', done => {
sandbox.stub(element.$.restAPI, 'applyFixSuggestion')
.returns(Promise.resolve());
sandbox.stub(Gerrit.Nav, 'navigateToChange');
element._currentFix = {fix_id: '123'};
element._handleApplyFix().then(() => {
assert.isTrue(element.$.restAPI.applyFixSuggestion
.calledWithExactly('1', 2, '123'));
assert.isTrue(Gerrit.Nav.navigateToChange.calledWithExactly({
_number: '1',
project: 'project',
}, 'edit', 2));
// reset gr-apply-fix-dialog and close
assert.deepEqual(element._currentFix, {});
assert.equal(element._currentPreviews.length, 0);
done();
});
});
});
</script>

4
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
View File

@@ -357,7 +357,9 @@
}
for (const layer of this.layers) {
layer.annotate(contentText, lineNumberEl, line);
if (typeof layer.annotate == 'function') {
layer.annotate(contentText, lineNumberEl, line);
}
}
td.appendChild(contentText);

7
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
View File

@@ -37,6 +37,7 @@ limitations under the License.
<link rel="import" href="../../shared/revision-info/revision-info.html">
<link rel="import" href="../gr-comment-api/gr-comment-api.html">
<link rel="import" href="../gr-diff-cursor/gr-diff-cursor.html">
<link rel="import" href="../gr-apply-fix-dialog/gr-apply-fix-dialog.html">
<link rel="import" href="../gr-diff-host/gr-diff-host.html">
<link rel="import" href="../gr-diff-mode-selector/gr-diff-mode-selector.html">
<link rel="import" href="../gr-diff-preferences-dialog/gr-diff-preferences-dialog.html">
@@ -343,6 +344,12 @@ limitations under the License.
on-comment-anchor-tap="_onLineSelected"
on-line-selected="_onLineSelected">
</gr-diff-host>
<gr-apply-fix-dialog
id="applyFixDialog"
prefs="[[_prefs]]"
change="[[_change]]"
change-num="[[_changeNum]]">
</gr-apply-fix-dialog>
<gr-diff-preferences-dialog
id="diffPreferencesDialog"
diff-prefs="{{_prefs}}"

6
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
View File

@@ -250,6 +250,8 @@
this._loggedIn = loggedIn;
});
this.addEventListener('open-fix-preview',
this._onOpenFixPreview.bind(this));
this.$.cursor.push('diffs', this.$.diffHost);
}
@@ -365,6 +367,10 @@
this.$.cursor.moveUp();
}
_onOpenFixPreview(e) {
this.$.applyFixDialog.open(e);
}
_handleNextLineOrFileWithComments(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
if (e.detail.keyboardEvent.shiftKey &&

9
polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
View File

@@ -358,6 +358,15 @@ limitations under the License.
Please Fix
</gr-button>
</template>
<gr-button
link
secondary
class="action show-fix"
hidden$="[[_hasNoFix(comment)]]"
on-click="_handleShowFix"
disabled="[[robotButtonDisabled]]">
Show Fix
</gr-button>
<gr-endpoint-decorator name="robot-comment-controls">
<gr-endpoint-param name="comment" value="[[comment]]">
</gr-endpoint-param>

18
polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
View File

@@ -48,6 +48,12 @@
* @event create-fix-comment
*/
/**
* Fired when the show fix preview action is triggered.
*
* @event open-fix-preview
*/
/**
* Fired when this comment is discarded.
*
@@ -505,6 +511,18 @@
}));
}
_handleShowFix() {
this.dispatchEvent(new CustomEvent('open-fix-preview', {
bubbles: true,
composed: true,
detail: this._getEventPayload(),
}));
}
_hasNoFix(comment) {
return !comment || !comment.fix_suggestions;
}
_handleDiscard(e) {
e.preventDefault();
this.$.reporting.recordDraftInteraction();

12
polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
View File

@@ -1007,5 +1007,17 @@ limitations under the License.
flushAsynchronousOperations();
assert.isNotNull(element.$$('.robotActions gr-button'));
});
test('_handleShowFix fires open-fix-preview event', done => {
element.addEventListener('open-fix-preview', e => {
assert.deepEqual(e.detail, element._getEventPayload());
done();
});
element.comment = {fix_suggestions: [{}]};
element.isRobotComment = true;
flushAsynchronousOperations();
MockInteractions.tap(element.$$('.show-fix'));
});
});
</script>

19
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
View File

@@ -1860,6 +1860,25 @@
});
}
getRobotCommentFixPreview(changeNum, patchNum, fixId) {
return this._getChangeURLAndFetch({
changeNum,
patchNum,
endpoint: `/fixes/${encodeURIComponent(fixId)}/preview`,
reportEndpointAsId: true,
});
}
applyFixSuggestion(changeNum, patchNum, fixId) {
return this._getChangeURLAndSend({
method: 'POST',
changeNum,
patchNum,
endpoint: `/fixes/${encodeURIComponent(fixId)}/apply`,
reportEndpointAsId: true,
});
}
// Deprecated, prefer to use putChangeCommitMessage instead.
saveChangeCommitMessageEdit(changeNum, message) {
return this._getChangeURLAndSend({

1
polygerrit-ui/app/test/index.html
View File

@@ -117,6 +117,7 @@ limitations under the License.
'diff/gr-diff-view/gr-diff-view_test.html',
'diff/gr-diff/gr-diff-group_test.html',
'diff/gr-diff/gr-diff_test.html',
'diff/gr-apply-fix-dialog/gr-apply-fix-dialog_test.html',
'diff/gr-patch-range-select/gr-patch-range-select_test.html',
'diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html',
'diff/gr-selection-action-box/gr-selection-action-box_test.html',