Merge pull request #4462 from danxuliu/fix-sharing-password-protected-link

Fix sharing a password protected link

4 files changed, 327 insertions, 33 deletions

diff --git a/core/js/sharedialoglinkshareview.js b/core/js/sharedialoglinkshareview.js
index 4904ed493cc..9368982d916 100644
--- a/core/js/sharedialoglinkshareview.js
+++ b/core/js/sharedialoglinkshareview.js
@@ -307,10 +307,12 @@
 			this.model.saveLinkShare({
 				password: password
 			}, {
+				complete: function(model) {
+					$loading.removeClass('inlineblock').addClass('hidden');
+				},
 				error: function(model, msg) {
 					// destroy old tooltips
 					$input.tooltip('destroy');
-					$loading.removeClass('inlineblock').addClass('hidden');
 					$input.addClass('error');
 					$input.attr('title', msg);
 					$input.tooltip({placement: 'bottom', trigger: 'manual'});
diff --git a/core/js/shareitemmodel.js b/core/js/shareitemmodel.js
index bc3ea88aa56..41f9eb5e0aa 100644
--- a/core/js/shareitemmodel.js
+++ b/core/js/shareitemmodel.js
@@ -104,7 +104,14 @@
 		/**
 		 * Saves the current link share information.
 		 *
-		 * This will trigger an ajax call and refetch the model afterwards.
+		 * This will trigger an ajax call and, if successful, refetch the model
+		 * afterwards. Callbacks "success", "error" and "complete" can be given
+		 * in the options object; "success" is called after a successful save
+		 * once the model is refetch, "error" is called after a failed save, and
+		 * "complete" is called both after a successful save and after a failed
+		 * save. Note that "complete" is called before "success" and "error" are
+		 * called (unlike in jQuery, in which it is called after them); this
+		 * ensures that "complete" is called even if refetching the model fails.
 		 *
 		 * TODO: this should be a separate model
 		 */
@@ -149,7 +156,6 @@
 
 		addShare: function(attributes, options) {
 			var shareType = attributes.shareType;
-			options = options || {};
 			attributes = _.extend({}, attributes);
 
 			// Default permissions are Edit (CRUD) and Share
@@ -173,53 +179,43 @@
 				attributes.path = this.fileInfoModel.getFullPath();
 			}
 
-			var self = this;
-			return $.ajax({
+			return this._addOrUpdateShare({
 				type: 'POST',
 				url: this._getUrl('shares'),
 				data: attributes,
 				dataType: 'json'
-			}).done(function() {
-				self.fetch().done(function() {
-					if (_.isFunction(options.success)) {
-						options.success(self);
-					}
-				});
-			}).fail(function(xhr) {
-				var msg = t('core', 'Error');
-				var result = xhr.responseJSON;
-				if (result && result.ocs && result.ocs.meta) {
-					msg = result.ocs.meta.message;
-				}
-
-				if (_.isFunction(options.error)) {
-					options.error(self, msg);
-				} else {
-					OC.dialogs.alert(msg, t('core', 'Error while sharing'));
-				}
-			});
+			}, options);
 		},
 
 		updateShare: function(shareId, attrs, options) {
-			var self = this;
-			options = options || {};
-			return $.ajax({
+			return this._addOrUpdateShare({
 				type: 'PUT',
 				url: this._getUrl('shares/' + encodeURIComponent(shareId)),
 				data: attrs,
 				dataType: 'json'
+			}, options);
+		},
+
+		_addOrUpdateShare: function(ajaxSettings, options) {
+			var self = this;
+			options = options || {};
+
+			return $.ajax(
+				ajaxSettings
+			).always(function() {
+				if (_.isFunction(options.complete)) {
+					options.complete(self);
+				}
 			}).done(function() {
-				self.fetch({
-					success: function() {
-						if (_.isFunction(options.success)) {
-							options.success(self);
-						}
+				self.fetch().done(function() {
+					if (_.isFunction(options.success)) {
+						options.success(self);
 					}
 				});
 			}).fail(function(xhr) {
 				var msg = t('core', 'Error');
 				var result = xhr.responseJSON;
-				if (result.ocs && result.ocs.meta) {
+				if (result && result.ocs && result.ocs.meta) {
 					msg = result.ocs.meta.message;
 				}
 
diff --git a/core/js/tests/specs/sharedialoglinkshareview.js b/core/js/tests/specs/sharedialoglinkshareview.js
new file mode 100644
index 00000000000..811919b5603
--- /dev/null
+++ b/core/js/tests/specs/sharedialoglinkshareview.js
@@ -0,0 +1,143 @@
+/**
+ *
+ * @copyright Copyright (c) 2015, Tom Needham (tom@owncloud.com)
+ * @copyright Copyright (c) 2017, Daniel Calviño Sánchez (danxuliu@gmail.com)
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+describe('OC.Share.ShareDialogLinkShareView', function () {
+
+	var configModel;
+	var shareModel;
+	var view;
+
+	beforeEach(function () {
+
+		var fileInfoModel = new OCA.Files.FileInfoModel({
+			id: 123,
+			name: 'shared_file_name.txt',
+			path: '/subdir',
+			size: 100,
+			mimetype: 'text/plain',
+			permissions: OC.PERMISSION_ALL,
+			sharePermissions: OC.PERMISSION_ALL
+		});
+
+		var attributes = {
+			itemType: fileInfoModel.isDirectory() ? 'folder' : 'file',
+			itemSource: fileInfoModel.get('id'),
+			possiblePermissions: OC.PERMISSION_ALL,
+			permissions: OC.PERMISSION_ALL
+		};
+
+		configModel = new OC.Share.ShareConfigModel({
+			enforcePasswordForPublicLink: false,
+			isResharingAllowed: true,
+			enforcePasswordForPublicLink: false,
+			isDefaultExpireDateEnabled: false,
+			isDefaultExpireDateEnforced: false,
+			defaultExpireDate: 7
+		});
+
+		sinon.stub(configModel, 'isShareWithLinkAllowed');
+
+		shareModel = new OC.Share.ShareItemModel(attributes, {
+			configModel: configModel,
+			fileInfoModel: fileInfoModel
+		});
+
+		view = new OC.Share.ShareDialogLinkShareView({
+			configModel: configModel,
+			model: shareModel
+		});
+
+	});
+
+	afterEach(function () {
+		view.remove();
+		configModel.isShareWithLinkAllowed.restore();
+	});
+
+	describe('onPasswordEntered', function () {
+
+		var $passwordText;
+		var $workingIcon;
+
+		beforeEach(function () {
+
+			// Needed to render the view
+			configModel.isShareWithLinkAllowed.returns(true);
+
+			// Setting the share also triggers the rendering
+			shareModel.set({
+				linkShare: {
+					isLinkShare: true,
+					password: 'password'
+				}
+			});
+
+			var $passwordDiv = view.$el.find('#linkPass');
+			$passwordText = view.$el.find('.linkPassText');
+			$workingIcon = view.$el.find('.linkPass .icon-loading-small');
+
+			sinon.stub(shareModel, 'saveLinkShare');
+
+			expect($passwordDiv.hasClass('hidden')).toBeFalsy();
+			expect($passwordText.hasClass('hidden')).toBeFalsy();
+			expect($workingIcon.hasClass('hidden')).toBeTruthy();
+
+			$passwordText.val('myPassword');
+		});
+
+		afterEach(function () {
+			shareModel.saveLinkShare.restore();
+		});
+
+		it('shows the working icon when called', function () {
+			view.onPasswordEntered();
+
+			expect($workingIcon.hasClass('hidden')).toBeFalsy();
+			expect(shareModel.saveLinkShare.withArgs({ password: 'myPassword' }).calledOnce).toBeTruthy();
+		});
+
+		it('hides the working icon when saving the password succeeds', function () {
+			view.onPasswordEntered();
+
+			expect($workingIcon.hasClass('hidden')).toBeFalsy();
+			expect(shareModel.saveLinkShare.withArgs({ password: 'myPassword' }).calledOnce).toBeTruthy();
+
+			shareModel.saveLinkShare.yieldTo("complete", [shareModel]);
+
+			expect($workingIcon.hasClass('hidden')).toBeTruthy();
+		});
+
+		it('hides the working icon when saving the password fails', function () {
+			view.onPasswordEntered();
+
+			expect($workingIcon.hasClass('hidden')).toBeFalsy();
+			expect(shareModel.saveLinkShare.withArgs({ password: 'myPassword' }).calledOnce).toBeTruthy();
+
+			shareModel.saveLinkShare.yieldTo("complete", [shareModel]);
+			shareModel.saveLinkShare.yieldTo("error", [shareModel, "The error message"]);
+
+			expect($workingIcon.hasClass('hidden')).toBeTruthy();
+		});
+
+	});
+
+});
diff --git a/core/js/tests/specs/shareitemmodelSpec.js b/core/js/tests/specs/shareitemmodelSpec.js
index 3d3baf75d15..771a9263709 100644
--- a/core/js/tests/specs/shareitemmodelSpec.js
+++ b/core/js/tests/specs/shareitemmodelSpec.js
@@ -670,6 +670,83 @@ describe('OC.Share.ShareItemModel', function() {
 				shareWith: 'group1'
 			});
 		});
+		it('calls complete handler before refreshing the model', function() {
+			var completeStub = sinon.stub();
+			model.addShare({
+				shareType: OC.Share.SHARE_TYPE_GROUP,
+				shareWith: 'group1'
+			}, {
+				complete: completeStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				200,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({ })
+			);
+
+			expect(completeStub.calledOnce).toEqual(true);
+			expect(completeStub.lastCall.args[0]).toEqual(model);
+
+			fetchReshareDeferred.resolve(makeOcsResponse([]));
+			fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+			expect(completeStub.calledOnce).toEqual(true);
+		});
+		it('calls success handler after refreshing the model', function() {
+			var successStub = sinon.stub();
+			model.addShare({
+				shareType: OC.Share.SHARE_TYPE_GROUP,
+				shareWith: 'group1'
+			}, {
+				success: successStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				200,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({ })
+			);
+
+			expect(successStub.called).toEqual(false);
+
+			fetchReshareDeferred.resolve(makeOcsResponse([]));
+			fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+			expect(successStub.calledOnce).toEqual(true);
+			expect(successStub.lastCall.args[0]).toEqual(model);
+		});
+		it('calls complete handler before error handler', function() {
+			var completeStub = sinon.stub();
+			var errorStub = sinon.stub();
+			model.addShare({
+				shareType: OC.Share.SHARE_TYPE_GROUP,
+				shareWith: 'group1'
+			}, {
+				complete: completeStub,
+				error: errorStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				400,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({
+					ocs: {
+						meta: {
+							message: 'Some error message'
+						}
+					}
+				})
+			);
+
+			expect(completeStub.calledOnce).toEqual(true);
+			expect(completeStub.lastCall.args[0]).toEqual(model);
+			expect(errorStub.calledOnce).toEqual(true);
+			expect(completeStub.calledBefore(errorStub)).toEqual(true);
+		});
 		it('calls error handler with error message', function() {
 			var errorStub = sinon.stub();
 			model.addShare({
@@ -693,6 +770,7 @@ describe('OC.Share.ShareItemModel', function() {
 			);
 
 			expect(errorStub.calledOnce).toEqual(true);
+			expect(errorStub.lastCall.args[0]).toEqual(model);
 			expect(errorStub.lastCall.args[1]).toEqual('Some error message');
 		});
 	});
@@ -712,6 +790,80 @@ describe('OC.Share.ShareItemModel', function() {
 				permissions: '' + (OC.PERMISSION_READ | OC.PERMISSION_SHARE)
 			});
 		});
+		it('calls complete handler before refreshing the model', function() {
+			var completeStub = sinon.stub();
+			model.updateShare(123, {
+				permissions: OC.PERMISSION_READ | OC.PERMISSION_SHARE
+			}, {
+				complete: completeStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				200,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({ })
+			);
+
+			expect(completeStub.calledOnce).toEqual(true);
+			expect(completeStub.lastCall.args[0]).toEqual(model);
+
+			fetchReshareDeferred.resolve(makeOcsResponse([]));
+			fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+			expect(completeStub.calledOnce).toEqual(true);
+		});
+		it('calls success handler after refreshing the model', function() {
+			var successStub = sinon.stub();
+			model.updateShare(123, {
+				permissions: OC.PERMISSION_READ | OC.PERMISSION_SHARE
+			}, {
+				success: successStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				200,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({ })
+			);
+
+			expect(successStub.called).toEqual(false);
+
+			fetchReshareDeferred.resolve(makeOcsResponse([]));
+			fetchSharesDeferred.resolve(makeOcsResponse([]));
+
+			expect(successStub.calledOnce).toEqual(true);
+			expect(successStub.lastCall.args[0]).toEqual(model);
+		});
+		it('calls complete handler before error handler', function() {
+			var completeStub = sinon.stub();
+			var errorStub = sinon.stub();
+			model.updateShare(123, {
+				permissions: OC.PERMISSION_READ | OC.PERMISSION_SHARE
+			}, {
+				complete: completeStub,
+				error: errorStub
+			});
+
+			expect(fakeServer.requests.length).toEqual(1);
+			fakeServer.requests[0].respond(
+				400,
+				{ 'Content-Type': 'application/json' },
+				JSON.stringify({
+					ocs: {
+						meta: {
+							message: 'Some error message'
+						}
+					}
+				})
+			);
+
+			expect(completeStub.calledOnce).toEqual(true);
+			expect(completeStub.lastCall.args[0]).toEqual(model);
+			expect(errorStub.calledOnce).toEqual(true);
+			expect(completeStub.calledBefore(errorStub)).toEqual(true);
+		});
 		it('calls error handler with error message', function() {
 			var errorStub = sinon.stub();
 			model.updateShare(123, {
@@ -734,6 +886,7 @@ describe('OC.Share.ShareItemModel', function() {
 			);
 
 			expect(errorStub.calledOnce).toEqual(true);
+			expect(errorStub.lastCall.args[0]).toEqual(model);
 			expect(errorStub.lastCall.args[1]).toEqual('Some error message');
 		});
 	});