Edit button in diff-view must be visible only for open changes

Issue: 12690
Change-Id: I2321eea9f0f43a15f5bf61674fa408dcba682125
This commit is contained in:
Dmitrii Filippov
2020-05-06 14:06:27 +02:00
parent e903bbfd00
commit 4022a64ce3
3 changed files with 92 additions and 30 deletions

View File

@@ -1327,6 +1327,11 @@ class GrDiffView extends mixinBehaviors( [
_computeIsLoggedIn(loggedIn) { _computeIsLoggedIn(loggedIn) {
return loggedIn ? true : false; return loggedIn ? true : false;
} }
_computeCanEdit(loggedIn, changeChangeRecord) {
return this._computeIsLoggedIn(loggedIn) &&
this.changeIsOpen(changeChangeRecord.base);
}
} }
customElements.define(GrDiffView.is, GrDiffView); customElements.define(GrDiffView.is, GrDiffView);

View File

@@ -310,7 +310,7 @@ export const htmlTemplate = html`
_isBlameLoading)]]</gr-button _isBlameLoading)]]</gr-button
> >
</span> </span>
<template is="dom-if" if="[[_computeIsLoggedIn(_loggedIn)]]"> <template is="dom-if" if="[[_computeCanEdit(_loggedIn, _change.*)]]">
<span class="separator"></span> <span class="separator"></span>
<span class="editButton"> <span class="editButton">
<gr-button <gr-button

View File

@@ -44,6 +44,7 @@ import './gr-diff-view.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js'; import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {KeyboardShortcutBinder} from '../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js'; import {KeyboardShortcutBinder} from '../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js'; import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {ChangeStatus} from '../../../constants/constants.js';
suite('gr-diff-view tests', () => { suite('gr-diff-view tests', () => {
suite('basic tests', () => { suite('basic tests', () => {
@@ -94,16 +95,36 @@ suite('gr-diff-view tests', () => {
sandbox = sinon.sandbox.create(); sandbox = sinon.sandbox.create();
stub('gr-rest-api-interface', { stub('gr-rest-api-interface', {
getConfig() { return Promise.resolve({change: {}}); }, getConfig() {
getLoggedIn() { return Promise.resolve(false); }, return Promise.resolve({change: {}});
getProjectConfig() { return Promise.resolve({}); }, },
getDiffChangeDetail() { return Promise.resolve({}); }, getLoggedIn() {
getChangeFiles() { return Promise.resolve({}); }, return Promise.resolve(false);
saveFileReviewed() { return Promise.resolve(); }, },
getDiffComments() { return Promise.resolve({}); }, getProjectConfig() {
getDiffRobotComments() { return Promise.resolve({}); }, return Promise.resolve({});
getDiffDrafts() { return Promise.resolve({}); }, },
getReviewedFiles() { return Promise.resolve([]); }, getDiffChangeDetail() {
return Promise.resolve({});
},
getChangeFiles() {
return Promise.resolve({});
},
saveFileReviewed() {
return Promise.resolve();
},
getDiffComments() {
return Promise.resolve({});
},
getDiffRobotComments() {
return Promise.resolve({});
},
getDiffDrafts() {
return Promise.resolve({});
},
getReviewedFiles() {
return Promise.resolve([]);
},
}); });
element = fixture('basic'); element = fixture('basic');
return element._loadComments(); return element._loadComments();
@@ -383,6 +404,7 @@ suite('gr-diff-view tests', () => {
}; };
element._change = { element._change = {
_number: 42, _number: 42,
status: ChangeStatus.NEW,
revisions: { revisions: {
a: {_number: 1, commit: {parents: []}}, a: {_number: 1, commit: {parents: []}},
b: {_number: 2, commit: {parents: []}}, b: {_number: 2, commit: {parents: []}},
@@ -399,8 +421,9 @@ suite('gr-diff-view tests', () => {
}); });
}); });
test('edit hidden when not logged in', done => { function isEditVisibile({loggedIn, changeStatus}) {
element._loggedIn = false; return new Promise(resolve => {
element._loggedIn = loggedIn;
element._path = 't.txt'; element._path = 't.txt';
element._patchRange = { element._patchRange = {
basePatchNum: PARENT, basePatchNum: PARENT,
@@ -408,6 +431,7 @@ suite('gr-diff-view tests', () => {
}; };
element._change = { element._change = {
_number: 42, _number: 42,
status: changeStatus,
revisions: { revisions: {
a: {_number: 1, commit: {parents: []}}, a: {_number: 1, commit: {parents: []}},
b: {_number: 2, commit: {parents: []}}, b: {_number: 2, commit: {parents: []}},
@@ -416,10 +440,43 @@ suite('gr-diff-view tests', () => {
flush(() => { flush(() => {
const editBtn = element.shadowRoot const editBtn = element.shadowRoot
.querySelector('.editButton gr-button'); .querySelector('.editButton gr-button');
assert.isFalse(!!editBtn); resolve(!!editBtn);
done();
}); });
}); });
}
test('edit visible only when logged and status NEW', async () => {
for (const changeStatus in ChangeStatus) {
if (!ChangeStatus.hasOwnProperty(changeStatus)) {
continue;
}
assert.isFalse(await isEditVisibile({loggedIn: false, changeStatus}),
`loggedIn: false, changeStatus: ${changeStatus}`);
if (changeStatus !== ChangeStatus.NEW) {
assert.isFalse(await isEditVisibile({loggedIn: true, changeStatus}),
`loggedIn: true, changeStatus: ${changeStatus}`);
} else {
assert.isTrue(await isEditVisibile({loggedIn: true, changeStatus}),
`loggedIn: true, changeStatus: ${changeStatus}`);
}
}
});
test('edit visible when logged and status NEW', async () => {
assert.isTrue(await isEditVisibile(
{loggedIn: true, changeStatus: ChangeStatus.NEW}));
});
test('edit hidden when logged and status ABANDONED', async () => {
assert.isFalse(await isEditVisibile(
{loggedIn: true, changeStatus: ChangeStatus.ABANDONED}));
});
test('edit hidden when logged and status MERGED', async () => {
assert.isFalse(await isEditVisibile(
{loggedIn: true, changeStatus: ChangeStatus.MERGED}));
});
suite('diff prefs hidden', () => { suite('diff prefs hidden', () => {
test('when no prefs or logged out', () => { test('when no prefs or logged out', () => {