Fix share expiration date change

apps/files/src/components/Collaborators/EditCollaborator.vue

@@ -1,6 +1,14 @@
 <template>
   <div class="files-collaborators-collaborator-edit-dialog">
-    <div v-if="user.id !== collaborator.owner.name" class="uk-text-meta uk-flex uk-flex-middle uk-margin-small-bottom"><oc-icon name="repeat" class="uk-margin-small-right" /> {{ collaborator.owner.displayName }}</div>
+    <transition enter-active-class="uk-animation-slide-top-small" leave-active-class="uk-animation-slide-top-small uk-animation-reverse"
+                name="custom-classes-transition">
+      <oc-alert v-if="errors" class="oc-files-collaborators-collaborator-error-alert" variation="danger">
+        {{ errors }}
+      </oc-alert>
+    </transition>
+    <div v-if="user.id !== collaborator.owner.name" class="uk-text-meta uk-flex uk-flex-middle uk-margin-small-bottom">
+      <oc-icon name="repeat" class="uk-margin-small-right" /> {{ collaborator.owner.displayName }}
+    </div>
     <collaborator class="uk-width-expand" :collaborator="collaborator" :first-column="false" />
     <collaborators-edit-options
       :existingRole="$_originalRole"
@@ -60,7 +68,8 @@ export default {
       selectedRole: null,
       additionalPermissions: null,
       saving: false,
-      expirationDate: null
+      expirationDate: null,
+      errors: false
     }
   },
   computed: {
@@ -147,7 +156,8 @@ export default {
         expirationDate: this.expirationDate
       })
         .then(() => this.$_ocCollaborators_cancelChanges())
-        .catch(() => {
+        .catch(errors => {
+          this.errors = errors
           this.saving = false
         })
     },
@@ -156,6 +166,7 @@ export default {
       this.selectedRole = null
       this.additionalPermissions = null
       this.expirationDate = this.originalExpirationDate
+      this.errors = false
       this.saving = false
       this.close()
     },

apps/files/src/components/PublicLinksSidebar/EditPublicLink.vue

@@ -3,10 +3,9 @@
     <form @submit.prevent>
       <transition enter-active-class="uk-animation-slide-top-small" leave-active-class="uk-animation-slide-top-small uk-animation-reverse"
                   name="custom-classes-transition">
-        <div class="uk-alert-danger" uk-alert v-if="errors">
-          <a class="uk-alert-close" uk-close/>
-          <p v-text="errors"/>
-        </div>
+        <oc-alert v-if="errors" class="oc-files-file-link-error-alert" variation="danger">
+          {{ errors }}
+        </oc-alert>
       </transition>
       <div class="uk-margin">
         <label class="oc-label"><span v-translate>Name:</span></label>
@@ -18,7 +17,8 @@
       <div class="uk-margin uk-grid-small uk-flex uk-flex-middle" uk-grid>
         <div class="uk-width-1-1 uk-width-2-5@m" v-if="$_expirationDate">
           <label class="oc-label" for="oc-files-file-link-expire-date">
-            <span v-translate>Expiration date:</span><em class="uk-margin-small-left" v-if="$_expirationDate.enforced">(<span v-translate>required</span>)</em>
+            <span v-translate>Expiration date:</span>
+            <translate v-if="$_expirationDate.enforced" tag="em">(required)</translate>
           </label>
           <div class="uk-position-relative">
             <oc-datepicker :class="{ 'uk-form-danger': !$_expirationIsValid }" :date="expireDate" :key="'oc-datepicker-' + expireDate"
@@ -174,7 +174,7 @@ export default {
       return {
         enabled: !!expireDate.enabled,
         days: (expireDate.days) ? expireDate.days : false,
-        enforced: expireDate.enforced === '1'
+        enforced: !!expireDate.enforced
       }
     },
 

apps/files/src/store/actions.js

@@ -671,13 +671,17 @@ export default {
       })
     }
 
-    return client.shares.updateShare(share.id, params)
-      .then((updatedShare) => {
-        commit('CURRENT_FILE_OUTGOING_SHARES_UPDATE', _buildCollaboratorShare(updatedShare.shareInfo, getters.highlightedFile))
-      })
-      .catch(e => {
-        console.log(e)
-      })
+    return new Promise((resolve, reject) => {
+      client.shares.updateShare(share.id, params)
+        .then((updatedShare) => {
+          const share = _buildCollaboratorShare(updatedShare.shareInfo, getters.highlightedFile)
+          commit('CURRENT_FILE_OUTGOING_SHARES_UPDATE', share)
+          resolve(share)
+        })
+        .catch(e => {
+          reject(e)
+        })
+    })
   },
   addShare (context, { client, path, $gettext, shareWith, shareType, permissions, expirationDate }) {
     if (shareType === shareTypes.group) {

changelog/unreleased/3176

@@ -0,0 +1,7 @@
+Bugfix: Display errors when saving collaborator fails
+
+When saving a collaborator has failed, the UI was still behaving like it saved everything successfully. This has been
+fixed by displaying the errors at the top of the collaborator editing form and staying in the editing view.
+
+https://github.com/owncloud/phoenix/issues/3176
+https://github.com/owncloud/phoenix/pull/3241

tests/acceptance/features/webUISharingInternalUsers/shareWithUsers.feature

@@ -663,6 +663,51 @@ Feature: Sharing files and folders with internal users
       | lorem.txt       |
       | simple-folder   |
 
+  Scenario: user cannot concurrently update the role and date of a share after the system maximum expiry date has been reduced
+    Given the setting "shareapi_default_expire_date_user_share" of app "core" has been set to "yes"
+    And the setting "shareapi_enforce_expire_date_user_share" of app "core" has been set to "yes"
+    And the setting "shareapi_expire_after_n_days_user_share" of app "core" has been set to "30"
+    And user "user1" has created a new share with following settings
+      | path             | lorem.txt |
+      | shareWith        | user2     |
+      | expireDate       | +30       |
+      | permissionString | read      |
+    And user "user1" has logged in using the webUI
+    And the setting "shareapi_expire_after_n_days_user_share" of app "core" has been set to "10"
+    When the user tries to edit the collaborator "User Two" of file "lorem.txt" changing following
+      | expireDate       | +15    |
+      | permissionString | Editor |
+    Then the user should see an error message on the collaborator share dialog saying "Cannot set expiration date more than 10 days in the future"
+    And user "user1" should have a share with these details:
+      | field       | value      |
+      | path        | /lorem.txt |
+      | share_type  | user       |
+      | uid_owner   | user1      |
+      | share_with  | user2      |
+      | expiration  | +30        |
+      | permissions | read       |
+
+  Scenario: user cannot update the date of a share after the system maximum expiry date has been reduced
+    Given the setting "shareapi_default_expire_date_user_share" of app "core" has been set to "yes"
+    And the setting "shareapi_enforce_expire_date_user_share" of app "core" has been set to "yes"
+    And the setting "shareapi_expire_after_n_days_user_share" of app "core" has been set to "30"
+    And user "user1" has created a new share with following settings
+      | path             | lorem.txt |
+      | shareWith        | user2     |
+      | expireDate       | +30       |
+    And user "user1" has logged in using the webUI
+    And the setting "shareapi_expire_after_n_days_user_share" of app "core" has been set to "10"
+    When the user tries to edit the collaborator "User Two" of file "lorem.txt" changing following
+      | expireDate       | +15    |
+    Then the user should see an error message on the collaborator share dialog saying "Cannot set expiration date more than 10 days in the future"
+    And user "user1" should have a share with these details:
+      | field       | value      |
+      | path        | /lorem.txt |
+      | share_type  | user       |
+      | uid_owner   | user1      |
+      | share_with  | user2      |
+      | expiration  | +30        |
+
   @issue-3174
   Scenario Outline: enforced expiry date for group affect user shares
     Given the setting "shareapi_default_expire_date_user_share" of app "core" has been set to "yes"

tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature

@@ -426,13 +426,13 @@ Feature: Share by public link
       | lorem.txt       |
       | simple-folder   |
 
-  Scenario: user cannot set an expiry date when creating a public link to a date that is past the enforced max expiry date
+  Scenario: expiry date is set to enforced max expiry date when creating a public link to a date that is past the enforced max expiry date
     Given the setting "shareapi_default_expire_date" of app "core" has been set to "yes"
+    And the setting "shareapi_expire_after_n_days" of app "core" has been set to "7"
     And the setting "shareapi_enforce_expire_date" of app "core" has been set to "yes"
     And user "user1" has logged in using the webUI
-    When the user tries to create a new public link for resource "simple-folder" using the webUI with
-      | expireDate | +8 |
-    Then the user should see an error message on the public link share dialog saying "Cannot set expiration date more than 7 days in the future"
+    When the user tries to create a new public link for resource "simple-folder" which expires in "+15" days using the webUI
+    Then the expiration date shown on the webUI should be "+7" days
     And user "user1" should not have created any shares
 
   Scenario: user cannot change the expiry date of an existing public link to a date that is past the enforced max expiry date
@@ -443,10 +443,8 @@ Feature: Share by public link
       | name       | Public link |
       | expireDate | +6          |
     And user "user1" has logged in using the webUI
-    When the user edits the public link named "Public link" of file "lorem.txt" changing following
-      | expireDate | +8 |
-    Then the user should see an error message on the public link share dialog saying "Cannot set expiration date more than 7 days in the future"
-    And user "user1" should have a share with these details:
+    When the user tries to edit expiration of the public link named "Public link" of file "lorem.txt" to past date "+15 days"
+    Then user "user1" should have a share with these details:
       | field       | value       |
       | share_type  | public_link |
       | uid_owner   | user1       |
@@ -455,6 +453,28 @@ Feature: Share by public link
       | name        | Public link |
       | expiration  | +6          |
 
+  Scenario: user cannot change the expiry date of an existing public link to a date that is past the enforced max expiry date
+    Given the setting "shareapi_default_expire_date" of app "core" has been set to "yes"
+    And the setting "shareapi_expire_after_n_days" of app "core" has been set to "16"
+    And the setting "shareapi_enforce_expire_date" of app "core" has been set to "yes"
+    And user "user1" has created a public link with following settings
+      | path       | lorem.txt   |
+      | name       | Public link |
+      | expireDate | +16          |
+    And user "user1" has logged in using the webUI
+    And the setting "shareapi_expire_after_n_days" of app "core" has been set to "7"
+    When the user edits the public link named "Public link" of file "lorem.txt" changing following
+      | expireDate | +15 |
+    Then the user should see an error message on the public link share dialog saying "Cannot set expiration date more than 7 days in the future"
+    Then user "user1" should have a share with these details:
+      | field       | value       |
+      | share_type  | public_link |
+      | uid_owner   | user1       |
+      | permissions | read        |
+      | path        | /lorem.txt  |
+      | name        | Public link |
+      | expiration  | +16         |
+
   Scenario: user can set an expiry date when creating a public link to a date that is before the enforced max expiry date
     Given the setting "shareapi_default_expire_date" of app "core" has been set to "yes"
     And the setting "shareapi_enforce_expire_date" of app "core" has been set to "yes"

tests/acceptance/pageObjects/FilesPageElement/expirationDatePicker.js

@@ -154,7 +154,7 @@ module.exports = {
         return this.click('@publicLinkDeleteExpirationDateButton')
       }
       const dateToSet = new Date(Date.parse(value))
-      if (shareType === 'collaborator') {
+      if (shareType === 'collaborator' || shareType === 'link') {
         const disabled = await this.isExpiryDateDisabled(dateToSet)
         if (disabled) {
           console.log('WARNING: Cannot change expiration date to disabled value!')

tests/acceptance/pageObjects/FilesPageElement/publicLinksDialog.js

@@ -186,6 +186,31 @@ module.exports = {
         .waitForElementNotPresent('@publicLinkCreateButton')
         .waitForOutstandingAjaxCalls()
     },
+    /**
+     * tries to create a new public link in specified date
+     *
+     * @param {string} settings.expireDate - Expire date for a public link share
+     * @returns {*}
+     */
+    setPublicLinkDate: async function (days) {
+      await this
+        .waitForElementVisible('@publicLinkAddButton')
+        .click('@publicLinkAddButton')
+        .waitForElementVisible('@publicLinkCreateButton')
+      if (days) {
+        const isExpiryDateChanged = await this.setPublicLinkForm('expireDate', days)
+        if (!isExpiryDateChanged) {
+          console.log('WARNING: Cannot create share with disabled expiration date!')
+          return
+        }
+      }
+      return this
+        .initAjaxCounters()
+        .waitForElementVisible('@publicLinkCreateButton')
+        .click('@publicLinkCreateButton')
+        .waitForElementNotPresent('@publicLinkCreateButton')
+        .waitForOutstandingAjaxCalls()
+    },
     /**
      * Gets the data of all public links of the currently open public link tab
      *

tests/acceptance/pageObjects/FilesPageElement/sharingDialog.js

@@ -532,9 +532,55 @@ module.exports = {
           disabled = result.value
         })
       return disabled
+    },
+    /**
+     * tries to change the settings of collaborator
+     * @param {string} collaborator Name of the collaborator
+     * @return {*}
+     */
+    changeCollaboratorSettings: async function (collaborator, editData) {
+      await collaboratorDialog.clickEditShare(collaborator)
+      for (const [key, value] of Object.entries(editData)) {
+        await this.setCollaboratorForm(key, value)
+      }
+      return this.waitForElementVisible('@saveShareButton')
+        .click('@saveShareButton')
+    },
+    /**
+     * function sets different fields for collaborator form
+     *
+     * @param key fields like permissionString, expireDate
+     * @param value values for the different fields to be set
+     * @returns {*|Promise<void>|exports}
+     */
+    setCollaboratorForm: function (key, value) {
+      if (key === 'permissionString') {
+        return this.changeCollaboratorRoleInDropdown(value)
+      } else if (key === 'expireDate') {
+        const dateToSet = calculateDate(value)
+        return this.openExpirationDatePicker()
+          .setExpirationDate(dateToSet)
+      }
+      return this
+    },
+    /**
+     *
+     * @returns {Promise<string>}
+     */
+    getErrorMessage: async function () {
+      let message
+      await this.waitForElementVisible('@collaboratorErrorAlert')
+        .getText('xpath', this.elements.collaboratorErrorAlert.selector, function (result) {
+          message = result.value
+        })
+      return message
     }
   },
   elements: {
+    collaboratorErrorAlert: {
+      selector: '//div[contains(@class, "collaborator-error-alert")]',
+      locateStrategy: 'xpath'
+    },
     sharingSidebarRoot: {
       selector: '//*[@id="oc-files-sharing-sidebar"]',
       locateStrategy: 'xpath'

tests/acceptance/stepDefinitions/publicLinkContext.js

@@ -30,6 +30,17 @@ When(
   }
 )
 
+When(
+  'the user (tries to) create a new public link for file/folder/resource {string} which expires in {string} day/days using the webUI',
+  async function (resource, days) {
+    await client.page.FilesPageElement
+      .appSideBar()
+      .closeSidebar(100)
+      .openPublicLinkDialog(resource)
+    return client.page.FilesPageElement.publicLinksDialog().setPublicLinkDate(days)
+  }
+)
+
 When('the public uses the webUI to access the last public link created by user {string}', async function (linkCreator) {
   const lastShare = await sharingHelper.fetchLastPublicLinkShare(linkCreator)
   if (lastShare.permissions === sharingHelper.PERMISSION_TYPES.create) {

tests/acceptance/stepDefinitions/sharingContext.js

@@ -155,6 +155,24 @@ Given('user {string} has created a new share with following settings',
     )
   })
 
+When('the user tries to edit the collaborator {string} of file/folder/resource {string} changing following',
+  async function (collaborator, resource, dataTable) {
+    const settings = dataTable.rowsHash()
+    const api = client.page.FilesPageElement
+    await api
+      .appSideBar()
+      .closeSidebar(100)
+      .openSharingDialog(resource)
+    return api.sharingDialog().changeCollaboratorSettings(collaborator, settings)
+  })
+
+Then('the user should see an error message on the collaborator share dialog saying {string}', async function (expectedMessage) {
+  const actualMessage = await client.page.FilesPageElement
+    .sharingDialog()
+    .getErrorMessage()
+  return client.assert.strictEqual(actualMessage, expectedMessage)
+})
+
 /**
  * sets up data into a standard format for creating new public link share
  *