Request /submitted_together?o=NON_VISIBLE_CHANGES

This prevents a 403 error if any changes to be submitted together are
not visible to the viewing user.

If the server reports non-zero non-visible changes, display a note in
the related changes section.

Change-Id: I570019a74ff63c23f884dc50504fcfa3a7c8dc8b
(cherry picked from commit 9e45abed9d)
This commit is contained in:
Logan Hanks
2018-10-12 12:30:05 -07:00
committed by Paladox none
parent 9381a76239
commit c59c96503a
4 changed files with 111 additions and 8 deletions

View File

@@ -61,6 +61,9 @@ limitations under the License.
flex-shrink: 0;
width: 1.2em;
}
.note {
color: var(--error-text-color);
}
.relatedChanges a {
display: inline-block;
}
@@ -118,9 +121,11 @@ limitations under the License.
</div>
</template>
</section>
<section hidden$="[[!_submittedTogether.length]]" hidden>
<section
id="submittedTogether"
class$="[[_computeSubmittedTogetherClass(_submittedTogether)]]">
<h4>Submitted together</h4>
<template is="dom-repeat" items="[[_submittedTogether]]" as="related">
<template is="dom-repeat" items="[[_submittedTogether.changes]]" as="related">
<div class$="[[_computeChangeContainerClass(change, related)]]">
<a href$="[[_computeChangeURL(related._number, related.project)]]"
class$="[[_computeLinkClass(related)]]"
@@ -133,6 +138,11 @@ limitations under the License.
class$="submittableCheck [[_computeLinkClass(related)]]"></span>
</div>
</template>
<template is="dom-if" if="[[_submittedTogether.non_visible_changes]]">
<div class="note">
[[_computeNonVisibleChangesNote(_submittedTogether.non_visible_changes)]]
</div>
</template>
</section>
<section hidden$="[[!_sameTopic.length]]" hidden>
<h4>Same topic</h4>

View File

@@ -57,9 +57,10 @@
type: Object,
value() { return {changes: []}; },
},
/** @type {?} */
_submittedTogether: {
type: Array,
value() { return []; },
type: Object,
value() { return {changes: []}; },
},
_conflicts: {
type: Array,
@@ -90,7 +91,7 @@
this.hidden = true;
this._relatedResponse = {changes: []};
this._submittedTogether = [];
this._submittedTogether = {changes: []};
this._conflicts = [];
this._cherryPicks = [];
this._sameTopic = [];
@@ -339,5 +340,19 @@
}
return connected;
},
_computeSubmittedTogetherClass(submittedTogether) {
if (!submittedTogether || (
submittedTogether.changes.length === 0 &&
!submittedTogether.non_visible_changes)) {
return 'hidden';
}
return '';
},
_computeNonVisibleChangesNote(n) {
const noun = n === 1 ? 'change' : 'changes';
return `(+ ${n} non-visible ${noun})`;
},
});
})();

View File

@@ -398,7 +398,7 @@ limitations under the License.
status: 'NEW',
}];
element._relatedResponse = {changes};
element._submittedTogether = changes;
element._submittedTogether = {changes};
element._conflicts = changes;
element._cherryPicks = changes;
element._sameTopic = changes;
@@ -407,7 +407,7 @@ limitations under the License.
element.clear();
assert.isTrue(element.hidden);
assert.equal(element._relatedResponse.changes.length, 0);
assert.equal(element._submittedTogether.length, 0);
assert.equal(element._submittedTogether.changes.length, 0);
assert.equal(element._conflicts.length, 0);
assert.equal(element._cherryPicks.length, 0);
assert.equal(element._sameTopic.length, 0);
@@ -431,5 +431,83 @@ limitations under the License.
element._computeChangeURL(123, 'abc/def', 12);
assert.isTrue(getUrlStub.called);
});
suite('submitted together changes', () => {
const change = {
project: 'foo/bar',
change_id: 'Ideadbeef',
commit: {
commit: 'deadbeef',
parents: [{commit: 'abc123'}],
author: {},
subject: 'do that thing',
},
_change_number: 12345,
_revision_number: 1,
_current_revision_number: 1,
status: 'NEW',
};
test('_computeSubmittedTogetherClass', () => {
assert.strictEqual(
element._computeSubmittedTogetherClass(undefined),
'hidden');
assert.strictEqual(
element._computeSubmittedTogetherClass({changes: []}),
'hidden');
assert.strictEqual(
element._computeSubmittedTogetherClass({changes: [{}]}),
'');
assert.strictEqual(
element._computeSubmittedTogetherClass({
changes: [],
non_visible_changes: 0,
}),
'hidden');
assert.strictEqual(
element._computeSubmittedTogetherClass({
changes: [],
non_visible_changes: 1,
}),
'');
assert.strictEqual(
element._computeSubmittedTogetherClass({
changes: [{}],
non_visible_changes: 1,
}),
'');
});
test('no submitted together changes', () => {
flushAsynchronousOperations();
assert.include(element.$.submittedTogether.className, 'hidden');
});
test('no non-visible submitted together changes', () => {
element._submittedTogether = {changes: [change]};
flushAsynchronousOperations();
assert.notInclude(element.$.submittedTogether.className, 'hidden');
assert.isNull(element.$$('.note'));
});
test('no visible submitted together changes', () => {
// Technically this should never happen, but worth asserting the logic.
element._submittedTogether = {changes: [], non_visible_changes: 1};
flushAsynchronousOperations();
assert.notInclude(element.$.submittedTogether.className, 'hidden');
assert.isNotNull(element.$$('.note'));
assert.strictEqual(
element.$$('.note').innerText, '(+ 1 non-visible change)');
});
test('visible and non-visible submitted together changes', () => {
element._submittedTogether = {changes: [change], non_visible_changes: 2};
flushAsynchronousOperations();
assert.notInclude(element.$.submittedTogether.className, 'hidden');
assert.isNotNull(element.$$('.note'));
assert.strictEqual(
element.$$('.note').innerText, '(+ 2 non-visible changes)');
});
});
});
</script>

View File

@@ -1747,7 +1747,7 @@
getChangesSubmittedTogether(changeNum) {
return this._getChangeURLAndFetch({
changeNum,
endpoint: '/submitted_together',
endpoint: '/submitted_together?o=NON_VISIBLE_CHANGES',
reportEndpointAsIs: true,
});
},