f78a45c7eb
The UI does not allow creating multiple comments at the same diff location, but instead encourages contributing to the comment threads already at a location. The uniqueness of thread locations was leveraged to simplify the collection of comments into threads by mapping their locations to keys. However, the REST API does permit multiple threads starting from the same diff location, so this expectation in the key-based UI threading logic can be wrong. In such scenarios, an unresolved thread may be mistakenly inserted into a resolved thread, resulting in a comment that is not resolvable through the UI. With this change, the threading logic in gr-diff-comment-thread-group is updated to associate comments into threads via the `in_reply_to` graph rather than location map in the same way that the `unresolved_comment_count` detail property is determined server-side. Bug: Issue 6779 Change-Id: Ibb29d4ba5c5de9e92dfbfba8ac7ac8037c332028
282 lines
8.0 KiB
HTML
282 lines
8.0 KiB
HTML
<!DOCTYPE html>
|
|
<!--
|
|
Copyright (C) 2017 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-diff-comment-thread-group</title>
|
|
|
|
<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
|
|
<script src="../../../bower_components/web-component-tester/browser.js"></script>
|
|
<link rel="import" href="../../../test/common-test-setup.html"/>
|
|
<script src="../../../scripts/util.js"></script>
|
|
|
|
<link rel="import" href="gr-diff-comment-thread-group.html">
|
|
|
|
<script>void(0);</script>
|
|
|
|
<test-fixture id="basic">
|
|
<template>
|
|
<gr-diff-comment-thread-group></gr-diff-comment-thread-group>
|
|
</template>
|
|
</test-fixture>
|
|
|
|
<script>
|
|
suite('gr-diff-comment-thread-group tests', () => {
|
|
let element;
|
|
let sandbox;
|
|
|
|
setup(() => {
|
|
sandbox = sinon.sandbox.create();
|
|
stub('gr-rest-api-interface', {
|
|
getLoggedIn() { return Promise.resolve(false); },
|
|
});
|
|
element = fixture('basic');
|
|
});
|
|
|
|
teardown(() => {
|
|
sandbox.restore();
|
|
});
|
|
|
|
test('_getThreadGroups', () => {
|
|
element.patchForNewThreads = 3;
|
|
const comments = [
|
|
{
|
|
id: 'sallys_confession',
|
|
message: 'i like you, jack',
|
|
updated: '2015-12-23 15:00:20.396000000',
|
|
__commentSide: 'left',
|
|
}, {
|
|
id: 'jacks_reply',
|
|
message: 'i like you, too',
|
|
updated: '2015-12-24 15:01:20.396000000',
|
|
__commentSide: 'left',
|
|
in_reply_to: 'sallys_confession',
|
|
},
|
|
];
|
|
|
|
let expectedThreadGroups = [
|
|
{
|
|
start_datetime: '2015-12-23 15:00:20.396000000',
|
|
commentSide: 'left',
|
|
comments: [{
|
|
id: 'sallys_confession',
|
|
message: 'i like you, jack',
|
|
updated: '2015-12-23 15:00:20.396000000',
|
|
__commentSide: 'left',
|
|
}, {
|
|
id: 'jacks_reply',
|
|
message: 'i like you, too',
|
|
updated: '2015-12-24 15:01:20.396000000',
|
|
__commentSide: 'left',
|
|
in_reply_to: 'sallys_confession',
|
|
}],
|
|
locationRange: 'line-left',
|
|
patchNum: 3,
|
|
},
|
|
];
|
|
|
|
assert.deepEqual(element._getThreadGroups(comments),
|
|
expectedThreadGroups);
|
|
|
|
// Patch num should get inherited from comment rather
|
|
comments.push({
|
|
id: 'betsys_confession',
|
|
message: 'i like you, jack',
|
|
updated: '2015-12-24 15:00:10.396000000',
|
|
range: {
|
|
start_line: 1,
|
|
start_character: 1,
|
|
end_line: 1,
|
|
end_character: 2,
|
|
},
|
|
__commentSide: 'left',
|
|
});
|
|
|
|
expectedThreadGroups = [
|
|
{
|
|
start_datetime: '2015-12-23 15:00:20.396000000',
|
|
commentSide: 'left',
|
|
comments: [{
|
|
id: 'sallys_confession',
|
|
message: 'i like you, jack',
|
|
updated: '2015-12-23 15:00:20.396000000',
|
|
__commentSide: 'left',
|
|
}, {
|
|
id: 'jacks_reply',
|
|
in_reply_to: 'sallys_confession',
|
|
message: 'i like you, too',
|
|
updated: '2015-12-24 15:01:20.396000000',
|
|
__commentSide: 'left',
|
|
}],
|
|
patchNum: 3,
|
|
locationRange: 'line-left',
|
|
},
|
|
{
|
|
start_datetime: '2015-12-24 15:00:10.396000000',
|
|
commentSide: 'left',
|
|
comments: [{
|
|
id: 'betsys_confession',
|
|
message: 'i like you, jack',
|
|
updated: '2015-12-24 15:00:10.396000000',
|
|
range: {
|
|
start_line: 1,
|
|
start_character: 1,
|
|
end_line: 1,
|
|
end_character: 2,
|
|
},
|
|
__commentSide: 'left',
|
|
}],
|
|
patchNum: 3,
|
|
locationRange: 'range-1-1-1-2-left',
|
|
},
|
|
];
|
|
|
|
assert.deepEqual(element._getThreadGroups(comments),
|
|
expectedThreadGroups);
|
|
});
|
|
|
|
test('multiple comments at same location but not threaded', () => {
|
|
element.patchForNewThreads = 3;
|
|
const comments = [
|
|
{
|
|
id: 'sallys_confession',
|
|
message: 'i like you, jack',
|
|
updated: '2015-12-23 15:00:20.396000000',
|
|
__commentSide: 'left',
|
|
}, {
|
|
id: 'jacks_reply',
|
|
message: 'i like you, too',
|
|
updated: '2015-12-24 15:01:20.396000000',
|
|
__commentSide: 'left',
|
|
},
|
|
];
|
|
assert.equal(element._getThreadGroups(comments).length, 2);
|
|
});
|
|
|
|
test('_sortByDate', () => {
|
|
let threadGroups = [
|
|
{
|
|
start_datetime: '2015-12-23 15:00:20.396000000',
|
|
comments: [],
|
|
locationRange: 'line',
|
|
},
|
|
{
|
|
start_datetime: '2015-12-22 15:00:10.396000000',
|
|
comments: [],
|
|
locationRange: 'range-1-1-1-2',
|
|
},
|
|
];
|
|
|
|
let expectedResult = [
|
|
{
|
|
start_datetime: '2015-12-22 15:00:10.396000000',
|
|
comments: [],
|
|
locationRange: 'range-1-1-1-2',
|
|
}, {
|
|
start_datetime: '2015-12-23 15:00:20.396000000',
|
|
comments: [],
|
|
locationRange: 'line',
|
|
},
|
|
];
|
|
|
|
assert.deepEqual(element._sortByDate(threadGroups), expectedResult);
|
|
|
|
// When a comment doesn't have a date, the one without the date should be
|
|
// last.
|
|
threadGroups = [
|
|
{
|
|
start_datetime: '2015-12-23 15:00:20.396000000',
|
|
comments: [],
|
|
locationRange: 'line',
|
|
},
|
|
{
|
|
comments: [],
|
|
locationRange: 'range-1-1-1-2',
|
|
},
|
|
];
|
|
|
|
expectedResult = [
|
|
{
|
|
start_datetime: '2015-12-23 15:00:20.396000000',
|
|
comments: [],
|
|
locationRange: 'line',
|
|
},
|
|
{
|
|
comments: [],
|
|
locationRange: 'range-1-1-1-2',
|
|
},
|
|
];
|
|
|
|
assert.deepEqual(element._sortByDate(threadGroups), expectedResult);
|
|
});
|
|
|
|
test('_calculateLocationRange', () => {
|
|
const comment = {__commentSide: 'left'};
|
|
const range = {
|
|
start_line: 1,
|
|
start_character: 2,
|
|
end_line: 3,
|
|
end_character: 4,
|
|
};
|
|
assert.equal(
|
|
element._calculateLocationRange(range, comment),
|
|
'range-1-2-3-4-left');
|
|
});
|
|
|
|
test('thread groups are updated when comments change', () => {
|
|
const commentsChangedStub = sandbox.stub(element, '_commentsChanged');
|
|
element.comments = [];
|
|
element.comments.push({
|
|
id: 'sallys_confession',
|
|
message: 'i like you, jack',
|
|
updated: '2015-12-23 15:00:20.396000000',
|
|
});
|
|
assert(commentsChangedStub.called);
|
|
});
|
|
|
|
test('addNewThread', () => {
|
|
const locationRange = 'range-1-2-3-4';
|
|
element._threads = [{locationRange: 'line'}];
|
|
element.addNewThread(locationRange);
|
|
assert(element._threads.length, 2);
|
|
});
|
|
|
|
test('_getPatchNum', () => {
|
|
element.patchForNewThreads = 3;
|
|
const comment = {
|
|
id: 'sallys_confession',
|
|
message: 'i like you, jack',
|
|
updated: '2015-12-23 15:00:20.396000000',
|
|
};
|
|
assert.equal(element._getPatchNum(comment), 3);
|
|
comment.patchNum = 4;
|
|
assert.equal(element._getPatchNum(comment), 4);
|
|
});
|
|
|
|
test('removeThread', () => {
|
|
const locationRange = 'range-1-2-3-4';
|
|
element._threads = [
|
|
{locationRange: 'range-1-2-3-4', comments: []},
|
|
{locationRange: 'line', comments: []},
|
|
];
|
|
flushAsynchronousOperations();
|
|
element.removeThread(locationRange);
|
|
flushAsynchronousOperations();
|
|
assert(element._threads.length, 1);
|
|
});
|
|
});
|
|
</script>
|