From ce7775acd04a00db92604827df1ea503c551f588 Mon Sep 17 00:00:00 2001 From: Jan-Christoph Borchardt Date: Thu, 16 Nov 2017 17:27:20 +0100 Subject: [PATCH 01/17] Replace information icon with confirmation button in share input The confirmation button right now is just an icon; its behaviour will be added in the following commits. Signed-off-by: Jan-Christoph Borchardt --- apps/files_sharing/css/sharetabview.scss | 12 ++++- core/js/sharedialogview.js | 53 ++++------------------ core/js/tests/specs/sharedialogviewSpec.js | 16 ------- 3 files changed, 19 insertions(+), 62 deletions(-) diff --git a/apps/files_sharing/css/sharetabview.scss b/apps/files_sharing/css/sharetabview.scss index f3bbd952c2d24..b4b64daff2b0b 100644 --- a/apps/files_sharing/css/sharetabview.scss +++ b/apps/files_sharing/css/sharetabview.scss @@ -13,15 +13,23 @@ top: 0px; } -.shareTabView .shareWithRemoteInfo, +.shareTabView .shareWithConfirm, .shareTabView .clipboardButton, .shareTabView .linkPass .icon-loading-small { position: absolute; right: -7px; - top: -4px; + top: -2px; padding: 14px; } +.shareTabView .shareWithConfirm { + opacity: .5; +} + +.shareTabView .shareWithField:focus ~ .shareWithConfirm { + opacity: 1; +} + .shareTabView .linkMore { position: absolute; right: -7px; diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index d2c6349014158..f8f3efce2c5f3 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -22,7 +22,7 @@ '
' + ' ' + ' '+ - '{{{shareInfo}}}' + + ' ' + '
' + '{{/if}}' + '
' + @@ -30,10 +30,6 @@ '
' + ''; - var TEMPLATE_SHARE_INFO = - ''; - /** * @class OCA.Share.ShareDialogView * @member {OC.Share.ShareItemModel} model @@ -142,7 +138,7 @@ var $shareWithField = $('.shareWithField'), view = this, $loading = this.$el.find('.shareWithLoading'), - $shareInfo = this.$el.find('.shareWithRemoteInfo'); + $confirm = this.$el.find('.shareWithConfirm'); var count = oc_config['sharing.minSearchStringLength']; if (search.term.trim().length < count) { @@ -167,7 +163,7 @@ $loading.removeClass('hidden'); $loading.addClass('inlineblock'); - $shareInfo.addClass('hidden'); + $confirm.addClass('hidden'); $shareWithField.removeClass('error') .tooltip('hide'); @@ -184,7 +180,7 @@ function (result) { $loading.addClass('hidden'); $loading.removeClass('inlineblock'); - $shareInfo.removeClass('hidden'); + $confirm.removeClass('hidden'); if (result.ocs.meta.statuscode === 100) { var users = result.ocs.data.exact.users.concat(result.ocs.data.users); var groups = result.ocs.data.exact.groups.concat(result.ocs.data.groups); @@ -318,7 +314,7 @@ ).fail(function() { $loading.addClass('hidden'); $loading.removeClass('inlineblock'); - $shareInfo.removeClass('hidden'); + $confirm.removeClass('hidden'); OC.Notification.show(t('core', 'An error occurred. Please try again')); window.setTimeout(OC.Notification.hide, 5000); }); @@ -363,22 +359,22 @@ var $loading = this.$el.find('.shareWithLoading'); $loading.removeClass('hidden') .addClass('inlineblock'); - var $shareInfo = this.$el.find('.shareWithRemoteInfo'); - $shareInfo.addClass('hidden'); + var $confirm = this.$el.find('.shareWithConfirm'); + $confirm.addClass('hidden'); this.model.addShare(s.item.value, {success: function() { $(e.target).val('') .attr('disabled', false); $loading.addClass('hidden') .removeClass('inlineblock'); - $shareInfo.removeClass('hidden'); + $confirm.removeClass('hidden'); }, error: function(obj, msg) { OC.Notification.showTemporary(msg); $(e.target).attr('disabled', false) .autocomplete('search', $(e.target).val()); $loading.addClass('hidden') .removeClass('inlineblock'); - $shareInfo.removeClass('hidden'); + $confirm.removeClass('hidden'); }}); }, @@ -416,7 +412,6 @@ cid: this.cid, shareLabel: t('core', 'Share'), sharePlaceholder: this._renderSharePlaceholderPart(), - shareInfo: this._renderShareInfoPart(), isSharingAllowed: this.model.sharePermissionPossible() })); @@ -461,27 +456,6 @@ this.linkShareView.showLink = this._showLink; }, - _renderShareInfoPart: function() { - var shareInfo = ''; - var infoTemplate = this._getShareInfoTemplate(); - - if(this.configModel.get('isMailShareAllowed') && this.configModel.get('isRemoteShareAllowed')) { - shareInfo = infoTemplate({ - tooltip: t('core', 'Share with other people by entering a user or group, a federated cloud ID or an email address.') - }); - } else if(this.configModel.get('isRemoteShareAllowed')) { - shareInfo = infoTemplate({ - tooltip: t('core', 'Share with other people by entering a user or group or a federated cloud ID.') - }); - } else if(this.configModel.get('isMailShareAllowed')) { - shareInfo = infoTemplate({ - tooltip: t('core', 'Share with other people by entering a user or group or an email address.') - }); - } - - return shareInfo; - }, - _renderSharePlaceholderPart: function () { var allowRemoteSharing = this.configModel.get('isRemoteShareAllowed'); var allowMailSharing = this.configModel.get('isMailShareAllowed'); @@ -513,15 +487,6 @@ return this._templates[key]; }, - /** - * returns the info template for remote sharing - * - * @returns {Function} - * @private - */ - _getShareInfoTemplate: function() { - return this._getTemplate('shareInfo', TEMPLATE_SHARE_INFO); - } }); OC.Share.ShareDialogView = ShareDialogView; diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index c6d5793623cfe..6fd8fa7031f5d 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -472,22 +472,6 @@ describe('OC.Share.ShareDialogView', function() { }); }); }); - describe('remote sharing', function() { - it('shows remote share info when allowed', function() { - configModel.set({ - isRemoteShareAllowed: true - }); - dialog.render(); - expect(dialog.$el.find('.shareWithRemoteInfo').length).toEqual(1); - }); - it('does not show remote share info when not allowed', function() { - configModel.set({ - isRemoteShareAllowed: false - }); - dialog.render(); - expect(dialog.$el.find('.shareWithRemoteInfo').length).toEqual(0); - }); - }); describe('autocompletion of users', function() { it('triggers autocomplete display and focus with data when ajax search succeeds', function () { dialog.render(); From 6fef01c481bb436ed6811097d41ba260007fcf38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 16 Mar 2018 14:59:01 +0100 Subject: [PATCH 02/17] Adjust search term to test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the server response is faked the search term is ignored in the tests. However, it is clearer to use a search term that would make the server return what the faked response contains. Signed-off-by: Daniel Calviño Sánchez --- core/js/tests/specs/sharedialogviewSpec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index 6fd8fa7031f5d..c014dfc15d52c 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -650,7 +650,7 @@ describe('OC.Share.ShareDialogView', function() { it('users', function () { dialog.render(); var response = sinon.stub(); - dialog.autocompleteHandler({term: 'bob'}, response); + dialog.autocompleteHandler({term: 'bo'}, response); var jsonData = JSON.stringify({ 'ocs': { 'meta': { @@ -701,7 +701,7 @@ describe('OC.Share.ShareDialogView', function() { it('groups', function () { dialog.render(); var response = sinon.stub(); - dialog.autocompleteHandler({term: 'group'}, response); + dialog.autocompleteHandler({term: 'grou'}, response); var jsonData = JSON.stringify({ 'ocs': { 'meta': { @@ -752,7 +752,7 @@ describe('OC.Share.ShareDialogView', function() { it('remotes', function () { dialog.render(); var response = sinon.stub(); - dialog.autocompleteHandler({term: 'bob'}, response); + dialog.autocompleteHandler({term: 'foo'}, response); var jsonData = JSON.stringify({ 'ocs': { 'meta': { From 375eab9df3997480ae56eea233e6eec2c3fdf82c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 16 Mar 2018 17:19:22 +0100 Subject: [PATCH 03/17] Add tests for emails and circles already shared with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/js/tests/specs/sharedialogviewSpec.js | 121 ++++++++++++++++++++- 1 file changed, 120 insertions(+), 1 deletion(-) diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index c014dfc15d52c..684c2bea45a46 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -642,7 +642,20 @@ describe('OC.Share.ShareDialogView', function() { share_type: OC.Share.SHARE_TYPE_REMOTE, share_with: 'foo@bar.com/baz', share_with_displayname: 'foo@bar.com/baz' - + },{ + id: 103, + item_source: 123, + permissions: 31, + share_type: OC.Share.SHARE_TYPE_EMAIL, + share_with: 'foo@bar.com', + share_with_displayname: 'foo@bar.com' + },{ + id: 104, + item_source: 123, + permissions: 31, + share_type: OC.Share.SHARE_TYPE_CIRCLE, + share_with: 'shortId', + share_with_displayname: 'CircleName (type, owner)' }] }); }); @@ -799,6 +812,112 @@ describe('OC.Share.ShareDialogView', function() { }])).toEqual(true); expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); }); + + it('emails', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'foo'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [], + 'emails': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [], + 'emails': [ + { + 'label': 'foo@bar.com', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_EMAIL, + 'shareWith': 'foo@bar.com' + } + }, + { + 'label': 'foo2@bar.com', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_EMAIL, + 'shareWith': 'foo2@bar.com' + } + } + ] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'foo2@bar.com', + 'value': {'shareType': OC.Share.SHARE_TYPE_EMAIL, 'shareWith': 'foo2@bar.com'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); + + it('circles', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'CircleNam'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [], + 'circles': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [], + 'circles': [ + { + 'label': 'CircleName (type, owner)', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_CIRCLE, + 'shareWith': 'shortId' + } + }, + { + 'label': 'CircleName (type2, owner)', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_CIRCLE, + 'shareWith': 'shortId2' + } + } + ] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'CircleName (type2, owner)', + 'value': {'shareType': OC.Share.SHARE_TYPE_CIRCLE, 'shareWith': 'shortId2'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); }); }); From 8af9c553e6fac750d47585ecbdc0b468bb36a0c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 16 Mar 2018 17:19:29 +0100 Subject: [PATCH 04/17] Add tests for exact search results already shared with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/js/tests/specs/sharedialogviewSpec.js | 274 +++++++++++++++++++++ 1 file changed, 274 insertions(+) diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index 684c2bea45a46..a5e82727a142a 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -711,6 +711,58 @@ describe('OC.Share.ShareDialogView', function() { expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); }); + it('users (exact)', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'bob'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [], + 'remotes': [] + }, + 'users': [ + { + 'label': 'bobby', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'imbob' + } + } + ], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'bobby', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'imbob'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); + it('groups', function () { dialog.render(); var response = sinon.stub(); @@ -762,6 +814,58 @@ describe('OC.Share.ShareDialogView', function() { expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); }); + it('groups (exact)', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'group'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [ + { + 'label': 'group', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_GROUP, + 'shareWith': 'group' + } + } + ], + 'remotes': [] + }, + 'users': [], + 'groups': [ + { + 'label': 'group2', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_GROUP, + 'shareWith': 'group2' + } + } + ], + 'remotes': [], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'group2', + 'value': {'shareType': OC.Share.SHARE_TYPE_GROUP, 'shareWith': 'group2'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); + it('remotes', function () { dialog.render(); var response = sinon.stub(); @@ -813,6 +917,58 @@ describe('OC.Share.ShareDialogView', function() { expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); }); + it('remotes (exact)', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'foo@bar.com/baz'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [ + { + 'label': 'foo@bar.com/baz', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_REMOTE, + 'shareWith': 'foo@bar.com/baz' + } + } + ] + }, + 'users': [], + 'groups': [], + 'remotes': [ + { + 'label': 'foo@bar.com/baz2', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_REMOTE, + 'shareWith': 'foo@bar.com/baz2' + } + } + ], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'foo@bar.com/baz2', + 'value': {'shareType': OC.Share.SHARE_TYPE_REMOTE, 'shareWith': 'foo@bar.com/baz2'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); + it('emails', function () { dialog.render(); var response = sinon.stub(); @@ -866,6 +1022,60 @@ describe('OC.Share.ShareDialogView', function() { expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); }); + it('emails (exact)', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'foo@bar.com'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [], + 'emails': [ + { + 'label': 'foo@bar.com', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_EMAIL, + 'shareWith': 'foo@bar.com' + } + } + ] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [], + 'emails': [ + { + 'label': 'foo@bar.com2', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_EMAIL, + 'shareWith': 'foo@bar.com2' + } + } + ] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'foo@bar.com2', + 'value': {'shareType': OC.Share.SHARE_TYPE_EMAIL, 'shareWith': 'foo@bar.com2'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); + it('circles', function () { dialog.render(); var response = sinon.stub(); @@ -918,6 +1128,70 @@ describe('OC.Share.ShareDialogView', function() { }])).toEqual(true); expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); }); + + it('circles (exact)', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'CircleName'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [], + 'circles': [ + { + 'label': 'CircleName (type, owner)', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_CIRCLE, + 'shareWith': 'shortId' + } + }, + { + 'label': 'CircleName (type2, owner)', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_CIRCLE, + 'shareWith': 'shortId2' + } + } + ] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [], + 'circles': [ + { + 'label': 'CircleName2 (type, owner)', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_CIRCLE, + 'shareWith': 'shortId3' + } + } + ] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'CircleName (type2, owner)', + 'value': {'shareType': OC.Share.SHARE_TYPE_CIRCLE, 'shareWith': 'shortId2'} + }, { + 'label': 'CircleName2 (type, owner)', + 'value': {'shareType': OC.Share.SHARE_TYPE_CIRCLE, 'shareWith': 'shortId3'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); }); }); From 3980364b6d95feebd1ac8a4c0c9cb5773c9e5fd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 16 Mar 2018 17:19:34 +0100 Subject: [PATCH 05/17] Add autocompletion tests for each type of share MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/js/tests/specs/sharedialogviewSpec.js | 308 +++++++++++++++++++-- 1 file changed, 283 insertions(+), 25 deletions(-) diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index a5e82727a142a..491a52851b287 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -473,37 +473,295 @@ describe('OC.Share.ShareDialogView', function() { }); }); describe('autocompletion of users', function() { - it('triggers autocomplete display and focus with data when ajax search succeeds', function () { - dialog.render(); - var response = sinon.stub(); - dialog.autocompleteHandler({term: 'bob'}, response); - var jsonData = JSON.stringify({ - 'ocs' : { - 'meta' : { - 'status' : 'success', - 'statuscode' : 100, - 'message' : null - }, - 'data' : { - 'exact' : { - 'users' : [], - 'groups' : [], - 'remotes': [] + describe('triggers autocomplete display and focus with data when ajax search succeeds', function () { + it('users', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'bob'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null }, - 'users' : [{'label': 'bob', 'value': {'shareType': 0, 'shareWith': 'test'}}], - 'groups' : [], - 'remotes': [], - 'lookup': [] + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [], + 'remotes': [] + }, + 'users': [ + { + 'label': 'bobby', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'imbob' + } + } + ], + 'groups': [], + 'remotes': [], + 'lookup': [] + } } - } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }, { + 'label': 'bobby', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'imbob'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); }); - fakeServer.requests[0].respond( + + it('groups', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'group'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [ + { + 'label': 'group', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_GROUP, + 'shareWith': 'group' + } + } + ], + 'remotes': [] + }, + 'users': [], + 'groups': [ + { + 'label': 'group2', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_GROUP, + 'shareWith': 'group2' + } + } + ], + 'remotes': [], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( 200, {'Content-Type': 'application/json'}, jsonData - ); - expect(response.calledWithExactly(JSON.parse(jsonData).ocs.data.users)).toEqual(true); - expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + ); + expect(response.calledWithExactly([{ + 'label': 'group', + 'value': {'shareType': OC.Share.SHARE_TYPE_GROUP, 'shareWith': 'group'} + }, { + 'label': 'group2', + 'value': {'shareType': OC.Share.SHARE_TYPE_GROUP, 'shareWith': 'group2'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); + + it('remotes', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'foo@bar.com/baz'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [ + { + 'label': 'foo@bar.com/baz', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_REMOTE, + 'shareWith': 'foo@bar.com/baz' + } + } + ] + }, + 'users': [], + 'groups': [], + 'remotes': [ + { + 'label': 'foo@bar.com/baz2', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_REMOTE, + 'shareWith': 'foo@bar.com/baz2' + } + } + ], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'foo@bar.com/baz', + 'value': {'shareType': OC.Share.SHARE_TYPE_REMOTE, 'shareWith': 'foo@bar.com/baz'} + }, { + 'label': 'foo@bar.com/baz2', + 'value': {'shareType': OC.Share.SHARE_TYPE_REMOTE, 'shareWith': 'foo@bar.com/baz2'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); + + it('emails', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'foo@bar.com'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [], + 'emails': [ + { + 'label': 'foo@bar.com', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_EMAIL, + 'shareWith': 'foo@bar.com' + } + } + ] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [], + 'emails': [ + { + 'label': 'foo@bar.com2', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_EMAIL, + 'shareWith': 'foo@bar.com2' + } + } + ] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'foo@bar.com', + 'value': {'shareType': OC.Share.SHARE_TYPE_EMAIL, 'shareWith': 'foo@bar.com'} + }, { + 'label': 'foo@bar.com2', + 'value': {'shareType': OC.Share.SHARE_TYPE_EMAIL, 'shareWith': 'foo@bar.com2'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); + + it('circles', function () { + dialog.render(); + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'CircleName'}, response); + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [], + 'circles': [ + { + 'label': 'CircleName (type, owner)', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_CIRCLE, + 'shareWith': 'shortId' + } + }, + { + 'label': 'CircleName (type2, owner)', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_CIRCLE, + 'shareWith': 'shortId2' + } + } + ] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [], + 'circles': [ + { + 'label': 'CircleName2 (type, owner)', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_CIRCLE, + 'shareWith': 'shortId3' + } + } + ] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + expect(response.calledWithExactly([{ + 'label': 'CircleName (type, owner)', + 'value': {'shareType': OC.Share.SHARE_TYPE_CIRCLE, 'shareWith': 'shortId'} + }, { + 'label': 'CircleName (type2, owner)', + 'value': {'shareType': OC.Share.SHARE_TYPE_CIRCLE, 'shareWith': 'shortId2'} + }, { + 'label': 'CircleName2 (type, owner)', + 'value': {'shareType': OC.Share.SHARE_TYPE_CIRCLE, 'shareWith': 'shortId3'} + }])).toEqual(true); + expect(autocompleteStub.calledWith("option", "autoFocus", true)).toEqual(true); + }); }); describe('filter out', function() { From d6062195766fc21b53be1990e6633da7da58d732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 16 Mar 2018 17:19:47 +0100 Subject: [PATCH 06/17] Extract code to get suggestions to its own method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 165 +++++++++++++++++++++---------------- 1 file changed, 92 insertions(+), 73 deletions(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index f8f3efce2c5f3..fc867e3196803 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -134,53 +134,18 @@ this.$el.find('.shareWithField').autocomplete("search"); }, - autocompleteHandler: function (search, response) { - var $shareWithField = $('.shareWithField'), - view = this, - $loading = this.$el.find('.shareWithLoading'), - $confirm = this.$el.find('.shareWithConfirm'); - - var count = oc_config['sharing.minSearchStringLength']; - if (search.term.trim().length < count) { - var title = n('core', - 'At least {count} character is needed for autocompletion', - 'At least {count} characters are needed for autocompletion', - count, - { count: count } - ); - $shareWithField.addClass('error') - .attr('data-original-title', title) - .tooltip('hide') - .tooltip({ - placement: 'bottom', - trigger: 'manual' - }) - .tooltip('fixTitle') - .tooltip('show'); - response(); - return; - } - - $loading.removeClass('hidden'); - $loading.addClass('inlineblock'); - $confirm.addClass('hidden'); - - $shareWithField.removeClass('error') - .tooltip('hide'); + _getSuggestions: function(searchTerm, perPage, model) { + var deferred = $.Deferred(); - var perPage = 200; $.get( OC.linkToOCS('apps/files_sharing/api/v1') + 'sharees', { format: 'json', - search: search.term.trim(), + search: searchTerm, perPage: perPage, - itemType: view.model.get('itemType') + itemType: model.get('itemType') }, function (result) { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); if (result.ocs.meta.statuscode === 100) { var users = result.ocs.data.exact.users.concat(result.ocs.data.users); var groups = result.ocs.data.exact.groups.concat(result.ocs.data.groups); @@ -213,17 +178,17 @@ } // Filter out the owner of the share - if (view.model.hasReshare()) { + if (model.hasReshare()) { usersLength = users.length; for (i = 0 ; i < usersLength; i++) { - if (users[i].value.shareWith === view.model.getReshareOwner()) { + if (users[i].value.shareWith === model.getReshareOwner()) { users.splice(i, 1); break; } } } - var shares = view.model.get('shares'); + var shares = model.get('shares'); var sharesLength = shares.length; // Now filter out all sharees that are already shared with @@ -275,43 +240,97 @@ var suggestions = users.concat(groups).concat(remotes).concat(emails).concat(circles).concat(lookup); - if (suggestions.length > 0) { - $shareWithField - .autocomplete("option", "autoFocus", true); + deferred.resolve(suggestions); + } else { + deferred.resolve(null); + } + } + ).fail(function() { + deferred.reject(); + }); - response(suggestions); + return deferred.promise(); + }, - // show a notice that the list is truncated - // this is the case if one of the search results is at least as long as the max result config option - if(oc_config['sharing.maxAutocompleteResults'] > 0 && - Math.min(perPage, oc_config['sharing.maxAutocompleteResults']) - <= Math.max(users.length, groups.length, remotes.length, emails.length, lookup.length)) { + autocompleteHandler: function (search, response) { + var $shareWithField = $('.shareWithField'), + view = this, + $loading = this.$el.find('.shareWithLoading'), + $confirm = this.$el.find('.shareWithConfirm'); - var message = t('core', 'This list is maybe truncated - please refine your search term to see more results.'); - $('.ui-autocomplete').append('
  • ' + message + '
    • '); - } + var count = oc_config['sharing.minSearchStringLength']; + if (search.term.trim().length < count) { + var title = n('core', + 'At least {count} character is needed for autocompletion', + 'At least {count} characters are needed for autocompletion', + count, + { count: count } + ); + $shareWithField.addClass('error') + .attr('data-original-title', title) + .tooltip('hide') + .tooltip({ + placement: 'bottom', + trigger: 'manual' + }) + .tooltip('fixTitle') + .tooltip('show'); + response(); + return; + } - } else { - var title = t('core', 'No users or groups found for {search}', {search: $shareWithField.val()}); - if (!view.configModel.get('allowGroupSharing')) { - title = t('core', 'No users found for {search}', {search: $('.shareWithField').val()}); - } - $shareWithField.addClass('error') - .attr('data-original-title', title) - .tooltip('hide') - .tooltip({ - placement: 'bottom', - trigger: 'manual' - }) - .tooltip('fixTitle') - .tooltip('show'); - response(); - } - } else { - response(); + $loading.removeClass('hidden'); + $loading.addClass('inlineblock'); + $confirm.addClass('hidden'); + + $shareWithField.removeClass('error') + .tooltip('hide'); + + var perPage = 200; + this._getSuggestions( + search.term.trim(), + perPage, + view.model + ).done(function(suggestions) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + + if (suggestions && suggestions.length > 0) { + $shareWithField + .autocomplete("option", "autoFocus", true); + + response(suggestions); + + // show a notice that the list is truncated + // this is the case if one of the search results is at least as long as the max result config option + if(oc_config['sharing.maxAutocompleteResults'] > 0 && + Math.min(perPage, oc_config['sharing.maxAutocompleteResults']) + <= Math.max(users.length, groups.length, remotes.length, emails.length, lookup.length)) { + + var message = t('core', 'This list is maybe truncated - please refine your search term to see more results.'); + $('.ui-autocomplete').append('
  • ' + message + '
    • '); } + + } else if (suggestions) { + var title = t('core', 'No users or groups found for {search}', {search: $shareWithField.val()}); + if (!view.configModel.get('allowGroupSharing')) { + title = t('core', 'No users found for {search}', {search: $('.shareWithField').val()}); + } + $shareWithField.addClass('error') + .attr('data-original-title', title) + .tooltip('hide') + .tooltip({ + placement: 'bottom', + trigger: 'manual' + }) + .tooltip('fixTitle') + .tooltip('show'); + response(); + } else { + response(); } - ).fail(function() { + }).fail(function() { $loading.addClass('hidden'); $loading.removeClass('inlineblock'); $confirm.removeClass('hidden'); From fcef15af801b4e88f857b5621ad028abdeebec28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 19 Mar 2018 14:12:06 +0100 Subject: [PATCH 07/17] Move stub setup outside the test method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stubs should be restored outside the test method in which they are used to ensure that they are properly restored no matter the result of the test (for example, if an exception is thrown). Besides that, this will make possible to reuse the stub in other sibling tests without having to explicitly setup it in them. Signed-off-by: Daniel Calviño Sánchez --- core/js/tests/specs/sharedialogviewSpec.js | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index 491a52851b287..63434c3b1ee15 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -30,7 +30,6 @@ describe('OC.Share.ShareDialogView', function() { var saveLinkShareStub; var fetchStub; - var notificationStub; var configModel; var shareModel; @@ -473,6 +472,16 @@ describe('OC.Share.ShareDialogView', function() { }); }); describe('autocompletion of users', function() { + var showNotificationStub; + + beforeEach(function() { + showNotificationStub = sinon.stub(OC.Notification, 'show'); + }); + + afterEach(function() { + showNotificationStub.restore(); + }); + describe('triggers autocomplete display and focus with data when ajax search succeeds', function () { it('users', function () { dialog.render(); @@ -1474,12 +1483,10 @@ describe('OC.Share.ShareDialogView', function() { }); it('throws a notification when the ajax search lookup fails', function () { - notificationStub = sinon.stub(OC.Notification, 'show'); dialog.render(); dialog.autocompleteHandler({term: 'bob'}, sinon.stub()); fakeServer.requests[0].respond(500); - expect(notificationStub.calledOnce).toEqual(true); - notificationStub.restore(); + expect(showNotificationStub.calledOnce).toEqual(true); }); describe('renders the autocomplete elements', function() { From ed1452d7a067d383371dcd165ad86e01f174840b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 19 Mar 2018 15:28:15 +0100 Subject: [PATCH 08/17] Use "showTemporary" instead of explicitly hiding the notification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "OC.Notification.hide" expects the notification to be hidden to be passed as an argument. As it was being used to show a temporary notification the combination of "OC.Notification.show" and "OC.Notification.hide" was replaced by a single call to "OC.Notification.showTemporary". The timeout could have been specified in the options of the call, but it was left to the default value (7 seconds) for consistency with other notifications. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 3 +-- core/js/tests/specs/sharedialogviewSpec.js | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index fc867e3196803..9e3de2636ce2c 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -334,8 +334,7 @@ $loading.addClass('hidden'); $loading.removeClass('inlineblock'); $confirm.removeClass('hidden'); - OC.Notification.show(t('core', 'An error occurred. Please try again')); - window.setTimeout(OC.Notification.hide, 5000); + OC.Notification.showTemporary(t('core', 'An error occurred. Please try again')); }); }, diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index 63434c3b1ee15..c12c734a5df70 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -472,14 +472,14 @@ describe('OC.Share.ShareDialogView', function() { }); }); describe('autocompletion of users', function() { - var showNotificationStub; + var showTemporaryNotificationStub; beforeEach(function() { - showNotificationStub = sinon.stub(OC.Notification, 'show'); + showTemporaryNotificationStub = sinon.stub(OC.Notification, 'show'); }); afterEach(function() { - showNotificationStub.restore(); + showTemporaryNotificationStub.restore(); }); describe('triggers autocomplete display and focus with data when ajax search succeeds', function () { @@ -1486,7 +1486,7 @@ describe('OC.Share.ShareDialogView', function() { dialog.render(); dialog.autocompleteHandler({term: 'bob'}, sinon.stub()); fakeServer.requests[0].respond(500); - expect(showNotificationStub.calledOnce).toEqual(true); + expect(showTemporaryNotificationStub.calledOnce).toEqual(true); }); describe('renders the autocomplete elements', function() { From 1c440519c2653323dd49fb4ec3a06851d114c4fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 19 Mar 2018 15:36:08 +0100 Subject: [PATCH 09/17] Show an error when getting the suggestions succeeds with failure content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of silently failing now an error is shown to the user when the ajax call to get the suggestions succeeds yet it returns failure content (for example, if an "OCSBadRequestException" was thrown in the server). Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 17 ++++++++++------- core/js/tests/specs/sharedialogviewSpec.js | 9 ++++++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 9e3de2636ce2c..7c31239d4a1be 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -242,7 +242,7 @@ deferred.resolve(suggestions); } else { - deferred.resolve(null); + deferred.reject(result.ocs.meta.message); } } ).fail(function() { @@ -296,7 +296,7 @@ $loading.removeClass('inlineblock'); $confirm.removeClass('hidden'); - if (suggestions && suggestions.length > 0) { + if (suggestions.length > 0) { $shareWithField .autocomplete("option", "autoFocus", true); @@ -312,7 +312,7 @@ $('.ui-autocomplete').append('
  • ' + message + '
    • '); } - } else if (suggestions) { + } else { var title = t('core', 'No users or groups found for {search}', {search: $shareWithField.val()}); if (!view.configModel.get('allowGroupSharing')) { title = t('core', 'No users found for {search}', {search: $('.shareWithField').val()}); @@ -327,14 +327,17 @@ .tooltip('fixTitle') .tooltip('show'); response(); - } else { - response(); } - }).fail(function() { + }).fail(function(message) { $loading.addClass('hidden'); $loading.removeClass('inlineblock'); $confirm.removeClass('hidden'); - OC.Notification.showTemporary(t('core', 'An error occurred. Please try again')); + + if (message) { + OC.Notification.showTemporary(t('core', 'An error occurred ("{message}"). Please try again', { message: message })); + } else { + OC.Notification.showTemporary(t('core', 'An error occurred. Please try again')); + } }); }, diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index c12c734a5df70..be1aa4c9a8eee 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -1462,7 +1462,7 @@ describe('OC.Share.ShareDialogView', function() { }); }); - it('gracefully handles successful ajax call with failure content', function () { + it('throws a notification for a successful ajax call with failure content', function () { dialog.render(); var response = sinon.stub(); dialog.autocompleteHandler({term: 'bob'}, response); @@ -1470,7 +1470,8 @@ describe('OC.Share.ShareDialogView', function() { 'ocs' : { 'meta' : { 'status': 'failure', - 'statuscode': 400 + 'statuscode': 400, + 'message': 'error message' } } }); @@ -1479,7 +1480,9 @@ describe('OC.Share.ShareDialogView', function() { {'Content-Type': 'application/json'}, jsonData ); - expect(response.calledWithExactly()).toEqual(true); + expect(response.called).toEqual(false); + expect(showTemporaryNotificationStub.calledOnce).toEqual(true); + expect(showTemporaryNotificationStub.firstCall.args[0]).toContain('error message'); }); it('throws a notification when the ajax search lookup fails', function () { From 89b0e34d9b0d80315839ce022439b58466b3d5db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 19 Mar 2018 16:57:16 +0100 Subject: [PATCH 10/17] Extract code to filter suggestions to its own function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 124 +++++++++++++++++++------------------ 1 file changed, 64 insertions(+), 60 deletions(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 7c31239d4a1be..bfa63a63bd1f5 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -160,83 +160,87 @@ circles = result.ocs.data.exact.circles.concat(result.ocs.data.circles); } - var usersLength; - var groupsLength; - var remotesLength; - var emailsLength; - var circlesLength; - - var i, j; - - //Filter out the current user - usersLength = users.length; - for (i = 0; i < usersLength; i++) { - if (users[i].value.shareWith === OC.currentUser) { - users.splice(i, 1); - break; - } - } + var filter = function(users, groups, remotes, emails, circles) { + var usersLength; + var groupsLength; + var remotesLength; + var emailsLength; + var circlesLength; + + var i, j; - // Filter out the owner of the share - if (model.hasReshare()) { + //Filter out the current user usersLength = users.length; - for (i = 0 ; i < usersLength; i++) { - if (users[i].value.shareWith === model.getReshareOwner()) { + for (i = 0; i < usersLength; i++) { + if (users[i].value.shareWith === OC.currentUser) { users.splice(i, 1); break; } } - } - - var shares = model.get('shares'); - var sharesLength = shares.length; - // Now filter out all sharees that are already shared with - for (i = 0; i < sharesLength; i++) { - var share = shares[i]; - - if (share.share_type === OC.Share.SHARE_TYPE_USER) { + // Filter out the owner of the share + if (model.hasReshare()) { usersLength = users.length; - for (j = 0; j < usersLength; j++) { - if (users[j].value.shareWith === share.share_with) { - users.splice(j, 1); + for (i = 0 ; i < usersLength; i++) { + if (users[i].value.shareWith === model.getReshareOwner()) { + users.splice(i, 1); break; } } - } else if (share.share_type === OC.Share.SHARE_TYPE_GROUP) { - groupsLength = groups.length; - for (j = 0; j < groupsLength; j++) { - if (groups[j].value.shareWith === share.share_with) { - groups.splice(j, 1); - break; + } + + var shares = model.get('shares'); + var sharesLength = shares.length; + + // Now filter out all sharees that are already shared with + for (i = 0; i < sharesLength; i++) { + var share = shares[i]; + + if (share.share_type === OC.Share.SHARE_TYPE_USER) { + usersLength = users.length; + for (j = 0; j < usersLength; j++) { + if (users[j].value.shareWith === share.share_with) { + users.splice(j, 1); + break; + } } - } - } else if (share.share_type === OC.Share.SHARE_TYPE_REMOTE) { - remotesLength = remotes.length; - for (j = 0; j < remotesLength; j++) { - if (remotes[j].value.shareWith === share.share_with) { - remotes.splice(j, 1); - break; + } else if (share.share_type === OC.Share.SHARE_TYPE_GROUP) { + groupsLength = groups.length; + for (j = 0; j < groupsLength; j++) { + if (groups[j].value.shareWith === share.share_with) { + groups.splice(j, 1); + break; + } } - } - } else if (share.share_type === OC.Share.SHARE_TYPE_EMAIL) { - emailsLength = emails.length; - for (j = 0; j < emailsLength; j++) { - if (emails[j].value.shareWith === share.share_with) { - emails.splice(j, 1); - break; + } else if (share.share_type === OC.Share.SHARE_TYPE_REMOTE) { + remotesLength = remotes.length; + for (j = 0; j < remotesLength; j++) { + if (remotes[j].value.shareWith === share.share_with) { + remotes.splice(j, 1); + break; + } } - } - } else if (share.share_type === OC.Share.SHARE_TYPE_CIRCLE) { - circlesLength = circles.length; - for (j = 0; j < circlesLength; j++) { - if (circles[j].value.shareWith === share.share_with) { - circles.splice(j, 1); - break; + } else if (share.share_type === OC.Share.SHARE_TYPE_EMAIL) { + emailsLength = emails.length; + for (j = 0; j < emailsLength; j++) { + if (emails[j].value.shareWith === share.share_with) { + emails.splice(j, 1); + break; + } + } + } else if (share.share_type === OC.Share.SHARE_TYPE_CIRCLE) { + circlesLength = circles.length; + for (j = 0; j < circlesLength; j++) { + if (circles[j].value.shareWith === share.share_with) { + circles.splice(j, 1); + break; + } } } } - } + }; + + filter(users, groups, remotes, emails, circles); var suggestions = users.concat(groups).concat(remotes).concat(emails).concat(circles).concat(lookup); From 5e2a8cca1b79d7ab5574bcc4d7309bcf0f7a2522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 19 Mar 2018 17:16:11 +0100 Subject: [PATCH 11/17] Return also exact matches besides all suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "_getSuggestions" returned all the suggestions from the server, which are composed by exact matches and partial matches. Now the exact matches are also returned on their own parameter. This will be used by the button to confirm a share. Note that until now the order of the suggestions was "exact users, partial users, exact groups, partial groups, exact..."; this commit also changes that order to become "exact users, exact groups, exact..., partial users, partial groups, partial...". This is not a problem, as the suggestions were used in the autocomplete dropdown, and this new order is arguably better than the old one, as all exact matches appear now at the beginning. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 67 ++++-- core/js/tests/specs/sharedialogviewSpec.js | 248 +++++++++++++++++++++ 2 files changed, 299 insertions(+), 16 deletions(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index bfa63a63bd1f5..d67ff89e28136 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -147,20 +147,14 @@ }, function (result) { if (result.ocs.meta.statuscode === 100) { - var users = result.ocs.data.exact.users.concat(result.ocs.data.users); - var groups = result.ocs.data.exact.groups.concat(result.ocs.data.groups); - var remotes = result.ocs.data.exact.remotes.concat(result.ocs.data.remotes); - var lookup = result.ocs.data.lookup; - var emails = [], - circles = []; - if (typeof(result.ocs.data.emails) !== 'undefined') { - emails = result.ocs.data.exact.emails.concat(result.ocs.data.emails); - } - if (typeof(result.ocs.data.circles) !== 'undefined') { - circles = result.ocs.data.exact.circles.concat(result.ocs.data.circles); - } - var filter = function(users, groups, remotes, emails, circles) { + if (typeof(emails) === 'undefined') { + emails = []; + } + if (typeof(circles) === 'undefined') { + circles = []; + } + var usersLength; var groupsLength; var remotesLength; @@ -240,11 +234,52 @@ } }; - filter(users, groups, remotes, emails, circles); + filter( + result.ocs.data.exact.users, + result.ocs.data.exact.groups, + result.ocs.data.exact.remotes, + result.ocs.data.exact.emails, + result.ocs.data.exact.circles + ); + + var exactUsers = result.ocs.data.exact.users; + var exactGroups = result.ocs.data.exact.groups; + var exactRemotes = result.ocs.data.exact.remotes; + var exactEmails = []; + if (typeof(result.ocs.data.emails) !== 'undefined') { + exactEmails = result.ocs.data.exact.emails; + } + var exactCircles = []; + if (typeof(result.ocs.data.circles) !== 'undefined') { + exactCircles = result.ocs.data.exact.circles; + } + + var exactMatches = exactUsers.concat(exactGroups).concat(exactRemotes).concat(exactEmails).concat(exactCircles); + + filter( + result.ocs.data.users, + result.ocs.data.groups, + result.ocs.data.remotes, + result.ocs.data.emails, + result.ocs.data.circles + ); + + var users = result.ocs.data.users; + var groups = result.ocs.data.groups; + var remotes = result.ocs.data.remotes; + var lookup = result.ocs.data.lookup; + var emails = []; + if (typeof(result.ocs.data.emails) !== 'undefined') { + emails = result.ocs.data.emails; + } + var circles = []; + if (typeof(result.ocs.data.circles) !== 'undefined') { + circles = result.ocs.data.circles; + } - var suggestions = users.concat(groups).concat(remotes).concat(emails).concat(circles).concat(lookup); + var suggestions = exactMatches.concat(users).concat(groups).concat(remotes).concat(emails).concat(circles).concat(lookup); - deferred.resolve(suggestions); + deferred.resolve(suggestions, exactMatches); } else { deferred.reject(result.ocs.meta.message); } diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index be1aa4c9a8eee..e35e8df15a1e9 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -471,6 +471,254 @@ describe('OC.Share.ShareDialogView', function() { }); }); }); + describe('get suggestions', function() { + it('no matches', function() { + var doneStub = sinon.stub(); + var failStub = sinon.stub(); + + dialog._getSuggestions('bob', 42, shareModel).done(doneStub).fail(failStub); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + + expect(doneStub.called).toEqual(false); + expect(failStub.called).toEqual(false); + + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(doneStub.calledOnce).toEqual(true); + expect(doneStub.calledWithExactly([], [])).toEqual(true); + expect(failStub.called).toEqual(false); + }); + it('single partial match', function() { + var doneStub = sinon.stub(); + var failStub = sinon.stub(); + + dialog._getSuggestions('bob', 42, shareModel).done(doneStub).fail(failStub); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [] + }, + 'users': [ + { + 'label': 'bobby', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'imbob' + } + } + ], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + + expect(doneStub.called).toEqual(false); + expect(failStub.called).toEqual(false); + + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(doneStub.calledOnce).toEqual(true); + expect(doneStub.calledWithExactly([{ + 'label': 'bobby', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'imbob'} + }], [ + ])).toEqual(true); + expect(failStub.called).toEqual(false); + }); + it('single exact match', function() { + var doneStub = sinon.stub(); + var failStub = sinon.stub(); + + dialog._getSuggestions('bob', 42, shareModel).done(doneStub).fail(failStub); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + + expect(doneStub.called).toEqual(false); + expect(failStub.called).toEqual(false); + + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(doneStub.calledOnce).toEqual(true); + expect(doneStub.calledWithExactly([{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }], [{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }])).toEqual(true); + expect(failStub.called).toEqual(false); + }); + it('mixed matches', function() { + var doneStub = sinon.stub(); + var failStub = sinon.stub(); + + dialog._getSuggestions('bob', 42, shareModel).done(doneStub).fail(failStub); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_GROUP, + 'shareWith': 'group1' + } + } + ], + 'remotes': [] + }, + 'users': [ + { + 'label': 'bobby', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'imbob' + } + }, + { + 'label': 'bob the second', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user2' + } + } + ], + 'groups': [ + { + 'label': 'bobfans', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_GROUP, + 'shareWith': 'fans' + } + } + ], + 'remotes': [], + 'lookup': [] + } + } + }); + + expect(doneStub.called).toEqual(false); + expect(failStub.called).toEqual(false); + + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(doneStub.calledOnce).toEqual(true); + expect(doneStub.calledWithExactly([{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }, { + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_GROUP, 'shareWith': 'group1'} + }, { + 'label': 'bobby', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'imbob'} + }, { + 'label': 'bob the second', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user2'} + }, { + 'label': 'bobfans', + 'value': {'shareType': OC.Share.SHARE_TYPE_GROUP, 'shareWith': 'fans'} + }], [{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }, { + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_GROUP, 'shareWith': 'group1'} + }])).toEqual(true); + expect(failStub.called).toEqual(false); + }); + }); describe('autocompletion of users', function() { var showTemporaryNotificationStub; From 9371b61c4df9eb9dc6285e988431ac9cae228952 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 19 Mar 2018 18:54:27 +0100 Subject: [PATCH 12/17] Add a share when clicking on the confirm button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking on the confirm button now adds a share, but only if there is just a single exact match. If there are no exact matches or there is more than one exact match no share is added, and the autocomplete dropdown is shown again with all the suggestions. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 90 +++++- core/js/tests/specs/sharedialogviewSpec.js | 358 +++++++++++++++++++++ 2 files changed, 447 insertions(+), 1 deletion(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index d67ff89e28136..89bd2e2dbfc39 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -67,7 +67,8 @@ events: { 'focus .shareWithField': 'onShareWithFieldFocus', - 'input .shareWithField': 'onShareWithFieldChanged' + 'input .shareWithField': 'onShareWithFieldChanged', + 'click .shareWithConfirm': '_confirmShare' }, initialize: function(options) { @@ -438,6 +439,93 @@ }}); }, + _confirmShare: function() { + var self = this; + var $shareWithField = $('.shareWithField'); + var $loading = this.$el.find('.shareWithLoading'); + var $confirm = this.$el.find('.shareWithConfirm'); + + $loading.removeClass('hidden'); + $loading.addClass('inlineblock'); + $confirm.addClass('hidden'); + + $shareWithField.prop('disabled', true); + + var perPage = 200; + var onlyExactMatches = true; + this._getSuggestions( + $shareWithField.val(), + perPage, + this.model, + onlyExactMatches + ).done(function(suggestions, exactMatches) { + if (suggestions.length === 0) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + + $shareWithField.prop('disabled', false); + $shareWithField.focus(); + + // There is no need to show an error message here; it will + // be automatically shown when the autocomplete is activated + // again (due to the focus on the field) and it finds no + // matches. + + return; + } + + if (exactMatches.length !== 1) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + + $shareWithField.prop('disabled', false); + $shareWithField.focus(); + + return; + } + + var actionSuccess = function() { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + + $shareWithField.val(''); + $shareWithField.prop('disabled', false); + $shareWithField.focus(); + }; + + var actionError = function(obj, msg) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + + $shareWithField.prop('disabled', false); + $shareWithField.focus(); + + OC.Notification.showTemporary(msg); + }; + + self.model.addShare(exactMatches[0].value, { + success: actionSuccess, + error: actionError + }); + }).fail(function(message) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + + $shareWithField.prop('disabled', false); + $shareWithField.focus(); + + // There is no need to show an error message here; it will be + // automatically shown when the autocomplete is activated again + // (due to the focus on the field) and getting the suggestions + // fail. + }); + }, + _toggleLoading: function(state) { this._loading = state; this.$el.find('.subView').toggleClass('hidden', state); diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index e35e8df15a1e9..76a214f6705e4 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -1836,6 +1836,364 @@ describe('OC.Share.ShareDialogView', function() { addShareStub.restore(); }); }); + describe('confirm share', function() { + var addShareStub; + var tooltipStub; + var showTemporaryNotificationStub; + + beforeEach(function() { + addShareStub = sinon.stub(shareModel, 'addShare'); + + tooltipStub = sinon.stub($.fn, 'tooltip').callsFake(function() { + return $('
    '); + }); + + showTemporaryNotificationStub = sinon.stub(OC.Notification, 'showTemporary'); + + dialog.render(); + }); + + afterEach(function() { + addShareStub.restore(); + tooltipStub.restore(); + showTemporaryNotificationStub.restore(); + }); + + it('sets the appropriate UI state while waiting to get the suggestions', function() { + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(autocompleteStub.callCount).toEqual(1); + expect(typeof autocompleteStub.firstCall.args[0]).toEqual('object'); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); + + dialog.$el.find('.shareWithField').val('bob'); + + dialog._confirmShare(); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(true); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); + }); + + it('calls addShare with the only suggestion', function() { + dialog.$el.find('.shareWithField').val('bob'); + + dialog._confirmShare(); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + // Ensure that the UI is not restored before adding the share + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(true); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); + + expect(addShareStub.calledOnce).toEqual(true); + expect(addShareStub.firstCall.args[0]).toEqual({ + shareType: OC.Share.SHARE_TYPE_USER, + shareWith: 'user1' + }); + + // "yield" and "callArg" from SinonJS can not be used, as the + // callback is a property not in the first argument. + addShareStub.firstCall.args[1]['success'].apply(shareModel); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); + expect(dialog.$el.find('.shareWithField').val()).toEqual(''); + }); + + it('handles a failure to share', function() { + expect(showTemporaryNotificationStub.called).toEqual(false); + + dialog.$el.find('.shareWithField').val('bob'); + + dialog._confirmShare(); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + // Ensure that the UI is not restored before adding the share + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(true); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); + + expect(addShareStub.calledOnce).toEqual(true); + expect(addShareStub.firstCall.args[0]).toEqual({ + shareType: OC.Share.SHARE_TYPE_USER, + shareWith: 'user1' + }); + + // "yield" and "callArg" from SinonJS can not be used, as the + // callback is a property not in the first argument. + addShareStub.firstCall.args[1]['error'].apply(shareModel); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); + + expect(showTemporaryNotificationStub.calledOnce).toEqual(true); + }); + + it('restores UI if there are no matches at all', function() { + dialog.$el.find('.shareWithField').val('bob'); + + dialog._confirmShare(); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(addShareStub.called).toEqual(false); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); + + // No explicit tooltip is shown; it is automatically shown when the + // autocomplete is activated again and it finds no matches. + expect(tooltipStub.lastCall.args[0]).not.toEqual('show'); + }); + + it('shows tooltip if there are matches but no exact matches', function() { + dialog.$el.find('.shareWithField').val('bo'); + + dialog._confirmShare(); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [] + }, + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(addShareStub.called).toEqual(false); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bo'); + }); + + it('shows tooltip if there is more than one exact match', function() { + dialog.$el.find('.shareWithField').val('bob'); + + dialog._confirmShare(); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_GROUP, + 'shareWith': 'group1' + } + } + ], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(addShareStub.called).toEqual(false); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); + }); + + it('throws a notification for a successful ajax call with failure content', function () { + dialog.$el.find('.shareWithField').val('bob'); + + dialog._confirmShare(); + + var jsonData = JSON.stringify({ + 'ocs' : { + 'meta' : { + 'status': 'failure', + 'statuscode': 400, + 'message': 'error message' + } + } + }); + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(addShareStub.called).toEqual(false); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); + + expect(showTemporaryNotificationStub.called).toEqual(false); + }); + + it('throws a notification when the ajax search lookup fails', function () { + dialog.$el.find('.shareWithField').val('bob'); + + dialog._confirmShare(); + + fakeServer.requests[0].respond(500); + + expect(addShareStub.called).toEqual(false); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); + expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); + + expect(showTemporaryNotificationStub.called).toEqual(false); + }); + }); describe('reshare permissions', function() { it('does not show sharing options when sharing not allowed', function() { shareModel.set({ From 10a4f8e45ee1e3b6a8f2b6c9348271724eb3371b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 20 Mar 2018 12:02:49 +0100 Subject: [PATCH 13/17] Confirm a share also by pressing enter on the input field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Besides confirming a share by clicking on the confirm button now it is possible to do it by pressing enter on the input field. Clicking on the confirm button implicitly hides the autocomplete dropdown. On the other hand, pressing enter on the input field does not, so the autocompletion must be disabled and closed when the confirmation begins and then enabled again once it finishes. Otherwise the autocomplete dropdown would be visible and it would be possible to interact with it while the share is being confirmed. The order in which the input field and the autompletion are disabled is important. Internally, the autocompletion sets a timeout when the input field is modified that requests the suggestions to the server and then shows them in the dropdown. That timeout is not cancelled when the autocompletion is disabled, but when the input field loses its focus and the autocompletion is not disabled. Therefore, the input field has to be disabled (which causes it to lose the focus) before the autocompletion is disabled. Otherwise it could happen that while a share is being confirmed the timeout ends, so an autocompletion request is sent and then, once the share is successfully confirmed and thus the autocompletion is enabled again, the request is received and the autocomplete dropdown is shown with the old suggestions. Strange, but possible nevertheless ;-) Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 36 ++++++++++++++++++++++ core/js/tests/specs/sharedialogviewSpec.js | 11 +++++++ 2 files changed, 47 insertions(+) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 89bd2e2dbfc39..0f1d8f395506b 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -415,6 +415,10 @@ _onSelectRecipient: function(e, s) { e.preventDefault(); + // Ensure that the keydown handler for the input field is not + // called; otherwise it would try to add the recipient again, which + // would fail. + e.stopImmediatePropagation(); $(e.target).attr('disabled', true) .val(s.item.label); var $loading = this.$el.find('.shareWithLoading'); @@ -451,6 +455,15 @@ $shareWithField.prop('disabled', true); + // Disabling the autocompletion does not clear its search timeout; + // removing the focus from the input field does, but only if the + // autocompletion is not disabled when the field loses the focus. + // Thus, the field has to be disabled before disabling the + // autocompletion to prevent an old pending search result from + // appearing once the field is enabled again. + $shareWithField.autocomplete('close'); + $shareWithField.autocomplete('disable'); + var perPage = 200; var onlyExactMatches = true; this._getSuggestions( @@ -467,6 +480,8 @@ $shareWithField.prop('disabled', false); $shareWithField.focus(); + $shareWithField.autocomplete('enable'); + // There is no need to show an error message here; it will // be automatically shown when the autocomplete is activated // again (due to the focus on the field) and it finds no @@ -483,6 +498,8 @@ $shareWithField.prop('disabled', false); $shareWithField.focus(); + $shareWithField.autocomplete('enable'); + return; } @@ -494,6 +511,8 @@ $shareWithField.val(''); $shareWithField.prop('disabled', false); $shareWithField.focus(); + + $shareWithField.autocomplete('enable'); }; var actionError = function(obj, msg) { @@ -504,6 +523,8 @@ $shareWithField.prop('disabled', false); $shareWithField.focus(); + $shareWithField.autocomplete('enable'); + OC.Notification.showTemporary(msg); }; @@ -519,6 +540,8 @@ $shareWithField.prop('disabled', false); $shareWithField.focus(); + $shareWithField.autocomplete('enable'); + // There is no need to show an error message here; it will be // automatically shown when the autocomplete is activated again // (due to the focus on the field) and getting the suggestions @@ -554,6 +577,7 @@ }, render: function() { + var self = this; var baseTemplate = this._getTemplate('base', TEMPLATE_BASE); this.$el.html(baseTemplate({ @@ -565,6 +589,16 @@ var $shareField = this.$el.find('.shareWithField'); if ($shareField.length) { + var shareFieldKeydownHandler = function(event) { + if (event.keyCode !== 13) { + return true; + } + + self._confirmShare(); + + return false; + }; + $shareField.autocomplete({ minLength: 1, delay: 750, @@ -574,6 +608,8 @@ source: this.autocompleteHandler, select: this._onSelectRecipient }).data('ui-autocomplete')._renderItem = this.autocompleteRenderItem; + + $shareField.on('keydown', null, shareFieldKeydownHandler); } this.resharerInfoView.$el = this.$el.find('.resharerInfoView'); diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index 76a214f6705e4..7ef38c022dd7a 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -1872,6 +1872,8 @@ describe('OC.Share.ShareDialogView', function() { expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(false); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(true); + expect(autocompleteStub.lastCall.args[0]).toEqual('disable'); + expect(autocompleteStub.calledWith('close')).toEqual(true); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(true); expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); }); @@ -1918,6 +1920,7 @@ describe('OC.Share.ShareDialogView', function() { // Ensure that the UI is not restored before adding the share expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(false); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(true); + expect(autocompleteStub.lastCall.args[0]).toEqual('disable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(true); expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); @@ -1933,6 +1936,7 @@ describe('OC.Share.ShareDialogView', function() { expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(autocompleteStub.lastCall.args[0]).toEqual('enable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); expect(dialog.$el.find('.shareWithField').val()).toEqual(''); }); @@ -1981,6 +1985,7 @@ describe('OC.Share.ShareDialogView', function() { // Ensure that the UI is not restored before adding the share expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(false); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(true); + expect(autocompleteStub.lastCall.args[0]).toEqual('disable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(true); expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); @@ -1996,6 +2001,7 @@ describe('OC.Share.ShareDialogView', function() { expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(autocompleteStub.lastCall.args[0]).toEqual('enable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); @@ -2037,6 +2043,7 @@ describe('OC.Share.ShareDialogView', function() { expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(autocompleteStub.lastCall.args[0]).toEqual('enable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); @@ -2088,6 +2095,7 @@ describe('OC.Share.ShareDialogView', function() { expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(autocompleteStub.lastCall.args[0]).toEqual('enable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); expect(dialog.$el.find('.shareWithField').val()).toEqual('bo'); }); @@ -2143,6 +2151,7 @@ describe('OC.Share.ShareDialogView', function() { expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(autocompleteStub.lastCall.args[0]).toEqual('enable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); }); @@ -2171,6 +2180,7 @@ describe('OC.Share.ShareDialogView', function() { expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(autocompleteStub.lastCall.args[0]).toEqual('enable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); @@ -2188,6 +2198,7 @@ describe('OC.Share.ShareDialogView', function() { expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + expect(autocompleteStub.lastCall.args[0]).toEqual('enable'); expect(dialog.$el.find('.shareWithField').prop('disabled')).toEqual(false); expect(dialog.$el.find('.shareWithField').val()).toEqual('bob'); From 6eb5cc54120168351286572b18bd1e07dc3bbc5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 20 Mar 2018 13:14:26 +0100 Subject: [PATCH 14/17] Reuse last suggestions if the same parameters are used MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a share is confirmed the suggestions are got to check if there is an exact match. Usually the suggestions were already got with the same parameters in order to fill the autocomplete dropdown, so to avoid a superfluous request now the last suggestions are reused when got again, although only if the same parameters as the last time are used. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 19 ++- core/js/tests/specs/sharedialogviewSpec.js | 174 +++++++++++++++++++++ 2 files changed, 192 insertions(+), 1 deletion(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 0f1d8f395506b..96f076c6a47a1 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -65,6 +65,9 @@ /** @type {object} **/ shareeListView: undefined, + /** @type {object} **/ + _lastSuggestions: undefined, + events: { 'focus .shareWithField': 'onShareWithFieldFocus', 'input .shareWithField': 'onShareWithFieldChanged', @@ -136,6 +139,13 @@ }, _getSuggestions: function(searchTerm, perPage, model) { + if (this._lastSuggestions && + this._lastSuggestions.searchTerm === searchTerm && + this._lastSuggestions.perPage === perPage && + this._lastSuggestions.model === model) { + return this._lastSuggestions.promise; + } + var deferred = $.Deferred(); $.get( @@ -289,7 +299,14 @@ deferred.reject(); }); - return deferred.promise(); + this._lastSuggestions = { + searchTerm: searchTerm, + perPage: perPage, + model: model, + promise: deferred.promise() + }; + + return this._lastSuggestions.promise; }, autocompleteHandler: function (search, response) { diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index 7ef38c022dd7a..1b4c19bbb4440 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -718,6 +718,180 @@ describe('OC.Share.ShareDialogView', function() { }])).toEqual(true); expect(failStub.called).toEqual(false); }); + + it('does not send a request to the server again for the same parameters', function() { + var doneStub = sinon.stub(); + var failStub = sinon.stub(); + + dialog._getSuggestions('bob', 42, shareModel).done(doneStub).fail(failStub); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + + expect(doneStub.called).toEqual(false); + expect(failStub.called).toEqual(false); + + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(doneStub.calledOnce).toEqual(true); + expect(doneStub.calledWithExactly([{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }], [{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }])).toEqual(true); + expect(failStub.called).toEqual(false); + + var done2Stub = sinon.stub(); + var fail2Stub = sinon.stub(); + + dialog._getSuggestions('bob', 42, shareModel).done(done2Stub).fail(fail2Stub); + + expect(doneStub.calledOnce).toEqual(true); + expect(failStub.called).toEqual(false); + + expect(done2Stub.calledOnce).toEqual(true); + expect(done2Stub.calledWithExactly([{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }], [{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }])).toEqual(true); + expect(fail2Stub.called).toEqual(false); + }); + + it('sends a request to the server again for the same parameters if the calls are not consecutive', function() { + var doneStub = sinon.stub(); + var failStub = sinon.stub(); + + dialog._getSuggestions('bob', 42, shareModel).done(doneStub).fail(failStub); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [ + { + 'label': 'bob', + 'value': { + 'shareType': OC.Share.SHARE_TYPE_USER, + 'shareWith': 'user1' + } + } + ], + 'groups': [], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + + expect(doneStub.called).toEqual(false); + expect(failStub.called).toEqual(false); + + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(doneStub.calledOnce).toEqual(true); + expect(doneStub.calledWithExactly([{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }], [{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }])).toEqual(true); + expect(failStub.called).toEqual(false); + + var done2Stub = sinon.stub(); + var fail2Stub = sinon.stub(); + + dialog._getSuggestions('bob', 108, shareModel).done(done2Stub).fail(fail2Stub); + + expect(done2Stub.called).toEqual(false); + expect(fail2Stub.called).toEqual(false); + + fakeServer.requests[1].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(done2Stub.calledOnce).toEqual(true); + expect(fail2Stub.called).toEqual(false); + + var done3Stub = sinon.stub(); + var fail3Stub = sinon.stub(); + + dialog._getSuggestions('bob', 42, shareModel).done(done3Stub).fail(fail3Stub); + + expect(done3Stub.called).toEqual(false); + expect(fail3Stub.called).toEqual(false); + + fakeServer.requests[2].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(doneStub.calledOnce).toEqual(true); + expect(failStub.called).toEqual(false); + expect(done2Stub.calledOnce).toEqual(true); + expect(fail2Stub.called).toEqual(false); + + expect(done3Stub.calledOnce).toEqual(true); + expect(done3Stub.calledWithExactly([{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }], [{ + 'label': 'bob', + 'value': {'shareType': OC.Share.SHARE_TYPE_USER, 'shareWith': 'user1'} + }])).toEqual(true); + expect(fail3Stub.called).toEqual(false); + }); }); describe('autocompletion of users', function() { var showTemporaryNotificationStub; From 9a0fbe307d1eb8583117e5c38ec1f4ae2b5e0f9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Wed, 21 Mar 2018 03:57:14 +0100 Subject: [PATCH 15/17] Discard cached suggestions when adding a share MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suggestions depend on the results returned by the server, but also on the sharees already shared with. Due to that adding a share changes the suggestions, so now the cached suggestions are discarded when a share is added. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 96f076c6a47a1..21fb03c7b0509 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -445,6 +445,9 @@ $confirm.addClass('hidden'); this.model.addShare(s.item.value, {success: function() { + // Adding a share changes the suggestions. + self._lastSuggestions = undefined; + $(e.target).val('') .attr('disabled', false); $loading.addClass('hidden') @@ -525,6 +528,9 @@ $loading.removeClass('inlineblock'); $confirm.removeClass('hidden'); + // Adding a share changes the suggestions. + self._lastSuggestions = undefined; + $shareWithField.val(''); $shareWithField.prop('disabled', false); $shareWithField.focus(); From a2c52cd6a513d697a888687db5842265af726d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 20 Mar 2018 13:44:28 +0100 Subject: [PATCH 16/17] Extract code to restore the UI after confirming a share MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 45 +++++++++++++------------------------- 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 21fb03c7b0509..0b36db6511611 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -484,6 +484,15 @@ $shareWithField.autocomplete('close'); $shareWithField.autocomplete('disable'); + var restoreUI = function() { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + + $shareWithField.prop('disabled', false); + $shareWithField.focus(); + }; + var perPage = 200; var onlyExactMatches = true; this._getSuggestions( @@ -493,12 +502,7 @@ onlyExactMatches ).done(function(suggestions, exactMatches) { if (suggestions.length === 0) { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); - - $shareWithField.prop('disabled', false); - $shareWithField.focus(); + restoreUI(); $shareWithField.autocomplete('enable'); @@ -511,12 +515,7 @@ } if (exactMatches.length !== 1) { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); - - $shareWithField.prop('disabled', false); - $shareWithField.focus(); + restoreUI(); $shareWithField.autocomplete('enable'); @@ -524,27 +523,18 @@ } var actionSuccess = function() { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); - // Adding a share changes the suggestions. self._lastSuggestions = undefined; $shareWithField.val(''); - $shareWithField.prop('disabled', false); - $shareWithField.focus(); + + restoreUI(); $shareWithField.autocomplete('enable'); }; var actionError = function(obj, msg) { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); - - $shareWithField.prop('disabled', false); - $shareWithField.focus(); + restoreUI(); $shareWithField.autocomplete('enable'); @@ -556,12 +546,7 @@ error: actionError }); }).fail(function(message) { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); - - $shareWithField.prop('disabled', false); - $shareWithField.focus(); + restoreUI(); $shareWithField.autocomplete('enable'); From 203bf51543f7d1866efeaa9decd2817cb41b3c3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 20 Mar 2018 16:58:04 +0100 Subject: [PATCH 17/17] Keep showing the working icon while there are pending operations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before, whenever a pending operation (getting the suggestions, confirming a share or selecting a recipient) finished the working icon was hidden and the confirm button was shown again, even if there were other pending operations (the most common case is typing slowly on the input field, as several operations to get the suggestions could pile if the server response is not received fast enough). Now, the working icon is not hidden until the last pending operation finishes. Signed-off-by: Daniel Calviño Sánchez --- core/js/sharedialogview.js | 61 ++++++++++++++++------ core/js/tests/specs/sharedialogviewSpec.js | 50 ++++++++++++++++++ 2 files changed, 94 insertions(+), 17 deletions(-) diff --git a/core/js/sharedialogview.js b/core/js/sharedialogview.js index 0b36db6511611..dede768fad5a7 100644 --- a/core/js/sharedialogview.js +++ b/core/js/sharedialogview.js @@ -68,6 +68,9 @@ /** @type {object} **/ _lastSuggestions: undefined, + /** @type {int} **/ + _pendingOperationsCount: 0, + events: { 'focus .shareWithField': 'onShareWithFieldFocus', 'input .shareWithField': 'onShareWithFieldChanged', @@ -339,6 +342,7 @@ $loading.removeClass('hidden'); $loading.addClass('inlineblock'); $confirm.addClass('hidden'); + this._pendingOperationsCount++; $shareWithField.removeClass('error') .tooltip('hide'); @@ -349,9 +353,12 @@ perPage, view.model ).done(function(suggestions) { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); + view._pendingOperationsCount--; + if (view._pendingOperationsCount === 0) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + } if (suggestions.length > 0) { $shareWithField @@ -386,9 +393,12 @@ response(); } }).fail(function(message) { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); + view._pendingOperationsCount--; + if (view._pendingOperationsCount === 0) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + } if (message) { OC.Notification.showTemporary(t('core', 'An error occurred ("{message}"). Please try again', { message: message })); @@ -431,6 +441,8 @@ }, _onSelectRecipient: function(e, s) { + var self = this; + e.preventDefault(); // Ensure that the keydown handler for the input field is not // called; otherwise it would try to add the recipient again, which @@ -438,11 +450,14 @@ e.stopImmediatePropagation(); $(e.target).attr('disabled', true) .val(s.item.label); + var $loading = this.$el.find('.shareWithLoading'); - $loading.removeClass('hidden') - .addClass('inlineblock'); var $confirm = this.$el.find('.shareWithConfirm'); + + $loading.removeClass('hidden'); + $loading.addClass('inlineblock'); $confirm.addClass('hidden'); + this._pendingOperationsCount++; this.model.addShare(s.item.value, {success: function() { // Adding a share changes the suggestions. @@ -450,16 +465,24 @@ $(e.target).val('') .attr('disabled', false); - $loading.addClass('hidden') - .removeClass('inlineblock'); - $confirm.removeClass('hidden'); + + self._pendingOperationsCount--; + if (self._pendingOperationsCount === 0) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + } }, error: function(obj, msg) { OC.Notification.showTemporary(msg); $(e.target).attr('disabled', false) .autocomplete('search', $(e.target).val()); - $loading.addClass('hidden') - .removeClass('inlineblock'); - $confirm.removeClass('hidden'); + + self._pendingOperationsCount--; + if (self._pendingOperationsCount === 0) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + } }}); }, @@ -472,6 +495,7 @@ $loading.removeClass('hidden'); $loading.addClass('inlineblock'); $confirm.addClass('hidden'); + this._pendingOperationsCount++; $shareWithField.prop('disabled', true); @@ -485,9 +509,12 @@ $shareWithField.autocomplete('disable'); var restoreUI = function() { - $loading.addClass('hidden'); - $loading.removeClass('inlineblock'); - $confirm.removeClass('hidden'); + self._pendingOperationsCount--; + if (self._pendingOperationsCount === 0) { + $loading.addClass('hidden'); + $loading.removeClass('inlineblock'); + $confirm.removeClass('hidden'); + } $shareWithField.prop('disabled', false); $shareWithField.focus(); diff --git a/core/js/tests/specs/sharedialogviewSpec.js b/core/js/tests/specs/sharedialogviewSpec.js index 1b4c19bbb4440..d363915984987 100644 --- a/core/js/tests/specs/sharedialogviewSpec.js +++ b/core/js/tests/specs/sharedialogviewSpec.js @@ -2009,6 +2009,56 @@ describe('OC.Share.ShareDialogView', function() { addShareStub.restore(); }); + + it('hides the loading icon when all the pending operations finish', function() { + dialog.render(); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + + var response = sinon.stub(); + dialog.autocompleteHandler({term: 'bob'}, response); + dialog.autocompleteHandler({term: 'bobby'}, response); + + var jsonData = JSON.stringify({ + 'ocs': { + 'meta': { + 'status': 'success', + 'statuscode': 100, + 'message': null + }, + 'data': { + 'exact': { + 'users': [], + 'groups': [], + 'remotes': [] + }, + 'users': [], + 'groups': [], + 'remotes': [], + 'lookup': [] + } + } + }); + + fakeServer.requests[0].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(false); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(true); + + fakeServer.requests[1].respond( + 200, + {'Content-Type': 'application/json'}, + jsonData + ); + + expect(dialog.$el.find('.shareWithLoading').hasClass('hidden')).toEqual(true); + expect(dialog.$el.find('.shareWithConfirm').hasClass('hidden')).toEqual(false); + }); }); describe('confirm share', function() { var addShareStub;