Fix assignee autocomplete behavior The assignee autocomplete box uses a different API endpoint than the reviewer/CC suggestion autocomplete box. This change introduces a prop to gr-account-entry (and by extension, gr-account-list) that allows toggling between the two endpoints, and utilizes it in gr-change-metadata. Bug: Issue 5245 Change-Id: I1d9f41656ba172a82cc8929be40e1e77b59b04b5
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js index c5827d0..5dea33d 100644 --- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js +++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
@@ -28,6 +28,16 @@ change: Object, filter: Function, placeholder: String, + /** + * When true, account-entry uses the account suggest API endpoint, which + * suggests any account in that Gerrit instance (and does not suggest + * groups). + * + * When false/undefined, account-entry uses the suggest_reviewers API + * endpoint, which suggests any account or group in that Gerrit instance + * that is not already a reviewer (or is not CCed) on that change. + */ + allowAnyUser: Boolean, suggestFrom: { type: Number, @@ -60,21 +70,34 @@ _makeSuggestion: function(reviewer) { if (reviewer.account) { + // Reviewer is an account suggestion from getChangeSuggestedReviewers. return { name: reviewer.account.name + ' (' + reviewer.account.email + ')', value: reviewer, }; } else if (reviewer.group) { + // Reviewer is a group suggestion from getChangeSuggestedReviewers. return { name: reviewer.group.name + ' (group)', value: reviewer, }; + } else if (reviewer._account_id) { + // Reviewer is an account suggestion from getSuggestedAccounts. + return { + name: reviewer.name + ' (' + reviewer.email + ')', + value: { + account: reviewer, + count: 1, + }, + }; } }, _getReviewerSuggestions: function(input) { - var xhr = this.$.restAPI.getChangeSuggestedReviewers( - this.change._number, input); + var api = this.$.restAPI; + var xhr = this.allowAnyUser ? + api.getSuggestedAccounts(input) : + api.getChangeSuggestedReviewers(this.change._number, input); return xhr.then(function(reviewers) { if (!reviewers) { return []; }
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html index 94db890..a4635d5 100644 --- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html +++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
@@ -33,6 +33,7 @@ <script> suite('gr-account-entry tests', function() { + var sandbox; var _nextAccountId = 0; var makeAccount = function() { var accountId = ++_nextAccountId; @@ -72,49 +73,85 @@ REVIEWER: [existingReviewer2], }, }; - - stub('gr-rest-api-interface', { - getChangeSuggestedReviewers: function() { - var redundantSuggestion1 = {account: existingReviewer1}; - var redundantSuggestion2 = {account: existingReviewer2}; - var redundantSuggestion3 = {account: owner}; - return Promise.resolve([redundantSuggestion1, redundantSuggestion2, - redundantSuggestion3, suggestion1, suggestion2, suggestion3]); - }, - }); + sandbox = sinon.sandbox.create(); }); - test('_makeSuggestion formats account or group accordingly', function() { - var account = makeAccount(); - var suggestion = element._makeSuggestion({account: account}); - assert.deepEqual(suggestion, { - name: account.name + ' (' + account.email + ')', - value: {account: account}, - }); - - var group = {name: 'test'}; - suggestion = element._makeSuggestion({group: group}); - assert.deepEqual(suggestion, { - name: group.name + ' (group)', - value: {group: group}, - }); + teardown(function() { + sandbox.restore(); }); - test('_getReviewerSuggestions excludes owner+reviewers', function(done) { - element._getReviewerSuggestions().then(function(reviewers) { - // Default is no filtering. - assert.equal(reviewers.length, 6); + suite('stubbed values for _getReviewerSuggestions', function() { + setup(function() { + stub('gr-rest-api-interface', { + getChangeSuggestedReviewers: function() { + var redundantSuggestion1 = {account: existingReviewer1}; + var redundantSuggestion2 = {account: existingReviewer2}; + var redundantSuggestion3 = {account: owner}; + return Promise.resolve([redundantSuggestion1, redundantSuggestion2, + redundantSuggestion3, suggestion1, suggestion2, suggestion3]); + }, + }); + }); - // Set up filter that only accepts suggestion1. - var accountId = suggestion1.account._account_id; - element.filter = function(suggestion) { - return suggestion.account && - suggestion.account._account_id === accountId; - }; + test('_makeSuggestion formats account or group accordingly', function() { + var account = makeAccount(); + var suggestion = element._makeSuggestion({account: account}); + assert.deepEqual(suggestion, { + name: account.name + ' (' + account.email + ')', + value: {account: account}, + }); + var group = {name: 'test'}; + suggestion = element._makeSuggestion({group: group}); + assert.deepEqual(suggestion, { + name: group.name + ' (group)', + value: {group: group}, + }); + + suggestion = element._makeSuggestion(account); + assert.deepEqual(suggestion, { + name: account.name + ' (' + account.email + ')', + value: {account: account, count: 1}, + }); + }); + + test('_getReviewerSuggestions excludes owner+reviewers', function(done) { element._getReviewerSuggestions().then(function(reviewers) { - assert.deepEqual(reviewers, [element._makeSuggestion(suggestion1)]); - }).then(done); + // Default is no filtering. + assert.equal(reviewers.length, 6); + + // Set up filter that only accepts suggestion1. + var accountId = suggestion1.account._account_id; + element.filter = function(suggestion) { + return suggestion.account && + suggestion.account._account_id === accountId; + }; + + element._getReviewerSuggestions().then(function(reviewers) { + assert.deepEqual(reviewers, [element._makeSuggestion(suggestion1)]); + }).then(done); + }); + }); + }); + + test('allowAnyUser', function(done) { + var suggestReviewerStub = + sandbox.stub(element.$.restAPI, 'getChangeSuggestedReviewers') + .returns(Promise.resolve([])); + var suggestAccountStub = + sandbox.stub(element.$.restAPI, 'getSuggestedAccounts') + .returns(Promise.resolve([])); + + element._getReviewerSuggestions('').then(function() { + assert.isTrue(suggestReviewerStub.calledOnce); + assert.isFalse(suggestAccountStub.called); + element.allowAnyUser = true; + + element._getReviewerSuggestions('').then(function() { + assert.isTrue(suggestReviewerStub.calledOnce); + assert.isTrue(suggestAccountStub.calledOnce); + done(); + }); }); }); });
diff --git a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html index 4ad9053..810658c 100644 --- a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html +++ b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.html
@@ -55,7 +55,8 @@ filter="[[filter]]" placeholder="[[placeholder]]" on-add="_handleAdd" - on-input-keydown="_handleInputKeydown"> + on-input-keydown="_handleInputKeydown" + allow-any-user="[[allowAnyUser]]"> </gr-account-entry> </template> <script src="gr-account-list.js"></script>
diff --git a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.js b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.js index f7e8fde..35311f9 100644 --- a/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.js +++ b/polygerrit-ui/app/elements/change/gr-account-list/gr-account-list.js
@@ -36,6 +36,19 @@ value: false, }, /** + * When true, the account-entry autocomplete uses the account suggest API + * endpoint, which suggests any account in that Gerrit instance (and does + * not suggest groups). + * + * When false/undefined, account-entry uses the suggest_reviewers API + * endpoint, which suggests any account or group in that Gerrit instance + * that is not already a reviewer (or is not CCed) on that change. + */ + allowAnyUser: { + type: Boolean, + value: false, + }, + /** * Array of values (groups/accounts) that are removable. When this prop is * undefined, all values are removable. */
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 4cb2d19..95a1497 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
@@ -114,9 +114,9 @@ id="assigneeValue" placeholder="Add assignee..." accounts="{{_assignee}}" - filter="[[_filterAssigneeSuggestion]]" change="[[change]]" - readonly="[[!mutable]]"></gr-account-list> + readonly="[[!mutable]]" + allow-any-user></gr-account-list> </span> </section> <section> @@ -147,9 +147,9 @@ id="assigneeValue" placeholder="Add assignee..." accounts="{{_assignee}}" - filter="[[_filterAssigneeSuggestion]]" change="[[change]]" - readonly="[[!mutable]]"></gr-account-list> + readonly="[[!mutable]]" + allow-any-user></gr-account-list> </span> </section> <section>
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 16915ab..f16dc8a 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
@@ -41,15 +41,6 @@ type: Boolean, computed: '_computeShowLabelStatus(change)', }, - /** - * Groups are not valid assignees. - */ - _filterAssigneeSuggestion: { - type: Function, - value: function() { - return function(suggestion) { return suggestion.account; }; - }, - }, _assignee: Array, },