Fix the Talk verification

When enabling or disabling Talk verification in mail shares the server
expects also a new password to be set. As we always just update one
property at a time this means the Talk verification was impossible to
activate or deactivate. With this patch, we send the talk option AND the
new password. If there is no new password, the Talk option is disabled
(in mail shares; in link shares it is possible to enable or disable the
video verification without changing the password).

When we finally have descriptive text on ActionCheckbox'es we should
definitely add some explanatory text for the user. Right now this is as
good as it gets.

We'll have to backport to 18.

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

diff --git a/apps/files_sharing/src/components/SharingEntryLink.vue b/apps/files_sharing/src/components/SharingEntryLink.vue
index 797659c263de3de17f8e0182defd735b64101c6a..8ffe9169c4c4a4ab50acc4de761011c98de2916a 100644 (file)
--- a/apps/files_sharing/src/components/SharingEntryLink.vue
+++ b/apps/files_sharing/src/components/SharingEntryLink.vue
@@ -198,9 +198,9 @@
                                        <!-- password protected by Talk -->
                                        <ActionCheckbox v-if="isPasswordProtectedByTalkAvailable"
                                                :checked.sync="isPasswordProtectedByTalk"
-                                               :disabled="saving"
+                                               :disabled="!canTogglePasswordProtectedByTalkAvailable || saving"
                                                class="share-link-password-talk-checkbox"
-                                               @change="queueUpdate('sendPasswordByTalk')">
+                                               @change="onPasswordProtectedByTalkChange">
                                                {{ t('files_sharing', 'Video verification') }}
                                        </ActionCheckbox>
 
@@ -476,6 +476,20 @@ export default {
                                : false
                },
 
+               canTogglePasswordProtectedByTalkAvailable() {
+                       if (!this.isPasswordProtected) {
+                               // Makes no sense
+                               return false
+                       } else if (this.isEmailShareType && !this.hasUnsavedPassword) {
+                               // For email shares we need a new password in order to enable or
+                               // disable
+                               return false
+                       }
+
+                       // Anything else should be fine
+                       return true
+               },
+
                /**
                 * Pending data.
                 * If the share still doesn't have an id, it is not synced
@@ -787,6 +801,22 @@ export default {
                        }
                },
 
+               /**
+                * Update the password along with "sendPasswordByTalk".
+                *
+                * If the password was modified the new password is sent; otherwise
+                * updating a mail share would fail, as in that case it is required that
+                * a new password is set when enabling or disabling
+                * "sendPasswordByTalk".
+                */
+               onPasswordProtectedByTalkChange() {
+                       if (this.hasUnsavedPassword) {
+                               this.share.password = this.share.newPassword.trim()
+                       }
+
+                       this.queueUpdate('sendPasswordByTalk', 'password')
+               },
+
                /**
                 * Save potential changed data on menu close
                 */

diff --git a/apps/files_sharing/src/mixins/ShareRequests.js b/apps/files_sharing/src/mixins/ShareRequests.js
index bad59da6593a7b20cbbc7b7b02b3ec637b136705..bdc9a566a172129903b201db2a3a811c24e41aeb 100644 (file)
--- a/apps/files_sharing/src/mixins/ShareRequests.js
+++ b/apps/files_sharing/src/mixins/ShareRequests.js
@@ -88,17 +88,11 @@ export default {
                 * Update a share
                 *
                 * @param {number} id share id
-                * @param {Object} data destructuring object
-                * @param {string} data.property property to update
-                * @param {any} data.value value to set
+                * @param {Object} properties key-value object of the properties to update
                 */
-               async updateShare(id, { property, value }) {
+               async updateShare(id, properties) {
                        try {
-                               // ocs api requires x-www-form-urlencoded
-                               const data = new URLSearchParams()
-                               data.append(property, value)
-
-                               const request = await axios.put(shareUrl + `/${id}`, { [property]: value }, headers)
+                               const request = await axios.put(shareUrl + `/${id}`, properties, headers)
                                if (!('ocs' in request.data)) {
                                        throw request
                                }
@@ -107,7 +101,7 @@ export default {
                                console.error('Error while updating share', error)
                                OC.Notification.showTemporary(t('files_sharing', 'Error updating the share'), { type: 'error' })
                                const message = error.response.data.ocs.meta.message
-                               throw new Error(`${property}, ${message}`)
+                               throw new Error(`${Object.keys(properties)}, ${message}`)
                        }
                },
        },

diff --git a/apps/files_sharing/src/mixins/SharesMixin.js b/apps/files_sharing/src/mixins/SharesMixin.js
index 912015af37750f9eae3a6034ee1588331ff02335..c1b8be356a08c08f667b7d31106cb5b149707337 100644 (file)
--- a/apps/files_sharing/src/mixins/SharesMixin.js
+++ b/apps/files_sharing/src/mixins/SharesMixin.js
@@ -224,31 +224,34 @@ export default {
                /**
                 * Send an update of the share to the queue
                 *
-                * @param {string} property the property to sync
+                * @param {string} propertyNames the properties to sync
                 */
-               queueUpdate(property) {
+               queueUpdate(...propertyNames) {
+                       if (propertyNames.length === 0) {
+                               // Nothing to update
+                               return
+                       }
+
                        if (this.share.id) {
+                               const properties = {}
                                // force value to string because that is what our
                                // share api controller accepts
-                               const value = this.share[property].toString()
+                               propertyNames.map(p => (properties[p] = this.share[p].toString()))
 
                                this.updateQueue.add(async() => {
                                        this.saving = true
                                        this.errors = {}
                                        try {
-                                               await this.updateShare(this.share.id, {
-                                                       property,
-                                                       value,
-                                               })
+                                               await this.updateShare(this.share.id, properties)
 
                                                // clear any previous errors
-                                               this.$delete(this.errors, property)
+                                               this.$delete(this.errors, propertyNames[0])
 
                                                // reset password state after sync
                                                this.$delete(this.share, 'newPassword')
                                        } catch ({ message }) {
                                                if (message && message !== '') {
-                                                       this.onSyncError(property, message)
+                                                       this.onSyncError(propertyNames[0], message)
                                                }
                                        } finally {
                                                this.saving = false