Address code review

owncloud · Apr 25, 2022 · 9e064c0 · 9e064c0
1 parent a5b4684
commit 9e064c0
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 64 deletions.
diff --git a/packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue b/packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue
@@ -19,7 +19,6 @@
             variation="primary"
             appearance="raw"
             data-testid="files-link-add-btn"
-            :aria-label="addButtonAriaLabel"
             @click="addNewLink"
             v-text="addButtonLabel"
           />
@@ -126,21 +125,14 @@ export default defineComponent({
     addButtonLabel() {
       return this.$gettext('Add link')
     },
-    addButtonAriaLabel() {
-      return this.$gettext('Create new public link')
-    },
 
     defaultNewLinkName() {
-      if (this.capabilities?.files_sharing?.public?.defaultPublicLinkShareName) {
-        return this.capabilities?.files_sharing?.public?.defaultPublicLinkShareName
-      }
-      return ''
+      return this.capabilities?.files_sharing?.public?.defaultPublicLinkShareName || ''
     },
 
     globalExpirationDate() {
       const expireDate = this.capabilities.files_sharing.public.expire_date
 
-      console.log('expireDate', expireDate)
       let defaultExpireDate = null
       let maxExpireDateFromCaps = null
 
@@ -153,7 +145,7 @@ export default defineComponent({
       }
 
       if (expireDate.enforced) {
-        const days = parseInt(expireDate.days, 10)
+        const days = parseInt(expireDate.days)
         maxExpireDateFromCaps = DateTime.now()
           .setLocale(this.$language.current)
           .plus({ days })
@@ -180,10 +172,6 @@ export default defineComponent({
         }
       })
 
-      // if (this.capabilities.files.privateLinks) {
-      //   return roles
-      // }
-
       // add empty permission link if oCIS for alias link
       return [
         // { role: null, name: 'Alias link', label: this.$gettext('Only invited people') },
@@ -192,16 +180,13 @@ export default defineComponent({
     },
 
     passwordEnforced() {
-      let passwordEnforcedFor = {
-        read_only: false,
-        upload_only: false,
-        read_write: false
-      }
-
-      if (this.capabilities.files_sharing.public.password.enforced_for) {
-        passwordEnforcedFor = this.capabilities.files_sharing.public.password.enforced_for
-      }
-      return passwordEnforcedFor
+      return (
+        this.capabilities.files_sharing.public.password.enforced_for || {
+          read_only: false,
+          upload_only: false,
+          read_write: false
+        }
+      )
     },
 
     helpersEnabled() {
@@ -277,14 +262,10 @@ export default defineComponent({
     },
 
     currentStorageId() {
-      let storageId
       if (this.resourceIsSpace) {
-        storageId = this.highlightedFile.id
-      } else if (this.$route.params.storageId) {
-        storageId = this.$route.params.storageId
+        return this.highlightedFile.id
       }
-
-      return storageId
+      return this.$route.params.storageId || null
     }
   },
   watch: {
@@ -327,7 +308,6 @@ export default defineComponent({
 
     linksComparator(l1, l2) {
       // sorting priority 1: display name (lower case, ascending), 2: creation time (descending)
-      // reasoning is falty since name can be empty and then we use the token value as "name" to display
       const name1 = l1.name.toLowerCase().trim()
       const name2 = l2.name.toLowerCase().trim()
       const l1Direct = !l1.indirect
@@ -408,7 +388,7 @@ export default defineComponent({
       this.currentView = VIEW_SHOW
 
       this.showMessage({
-        title: this.$gettext('Public link was created successfully')
+        title: this.$gettext('Link was created successfully')
       })
     },
 
@@ -425,20 +405,20 @@ export default defineComponent({
           onError(e)
           console.error(e)
           this.showMessage({
-            title: this.$gettext('Error when updating public link'),
+            title: this.$gettext('Failed to update link'),
             status: 'danger'
           })
         })
 
       this.showMessage({
-        title: this.$gettext('Public link was updated successfully')
+        title: this.$gettext('Link was updated successfully')
       })
     },
 
     deleteLinkConfirmation({ link }) {
       const modal = {
         variation: 'danger',
-        title: this.$gettext('Delete public link'),
+        title: this.$gettext('Delete link'),
         message: this.$gettext(
           'Are you sure you want to delete this link? Recreating the same link again is not possible.'
         ),
@@ -457,18 +437,18 @@ export default defineComponent({
 
     async deleteLink({ client, share, resource }) {
       this.hideModal()
-      // removeLink currently reloads all the shares in order to reload the shares indicators
+      // removeLink currently fetches all shares from the backend in order to reload the shares indicators
       // TODO: Check if to-removed link is last link share and only reload if it's the last link
       await this.removeLink({ client, share, resource, storageId: this.currentStorageId })
         .then(
           this.showMessage({
-            title: this.$gettext('Public link was deleted successfully')
+            title: this.$gettext('Link was deleted successfully')
           })
         )
         .catch((e) => {
           console.error(e)
           this.showMessage({
-            title: this.$gettext('Error deleting public link'),
+            title: this.$gettext('Failed to delete link'),
             status: 'danger'
           })
         })

diff --git a/packages/web-app-files/src/components/SideBar/Shares/PublicLinks/DetailsAndEdit.vue b/packages/web-app-files/src/components/SideBar/Shares/PublicLinks/DetailsAndEdit.vue
@@ -36,8 +36,13 @@
                 })
               "
             >
-              <span class="oc-text-bold oc-display-block oc-width-1-1" v-text="descriptor.label" />
-              <span>{{ roleTexts[descriptor.role.bitmask(false)] }} </span>
+              <span>
+                <span
+                  class="oc-text-bold oc-display-block oc-width-1-1"
+                  v-text="descriptor.label"
+                />
+                <span>{{ descriptor.role.description() }}</span>
+              </span>
               <oc-icon
                 v-if="parseInt(link.permissions) === descriptor.role.bitmask(false)"
                 name="check"
@@ -136,7 +141,7 @@ import { mapActions } from 'vuex'
 import Mixins from '../../../../mixins'
 import { createLocationSpaces, isLocationSpacesActive } from '../../../../router'
 import { DateTime } from 'luxon'
-import { linkRoleDescriptions, LinkShareRoles, SharePermissions } from '../../../../helpers/share'
+import { LinkShareRoles, SharePermissions } from '../../../../helpers/share'
 
 export default {
   name: 'DetailsAndEdit',
@@ -176,19 +181,22 @@ export default {
   },
   computed: {
     visibilityHint() {
-      return linkRoleDescriptions[parseInt(this.link.permissions)]
+      return LinkShareRoles.getByBitmask(
+        parseInt(this.link.permissions),
+        this.isFolderShare
+      ).description()
     },
 
     linkRole() {
       return LinkShareRoles.getByBitmask(parseInt(this.link.permissions), this.isFolderShare).label
     },
 
-    roleTexts() {
-      return linkRoleDescriptions
+    linkShareRoles() {
+      return LinkShareRoles
     },
 
     editButtonLabel() {
-      return this.$gettext('Edit public link')
+      return this.$gettext('Edit link')
     },
 
     editOptions() {
@@ -265,7 +273,7 @@ export default {
         ...result,
         {
           id: 'delete',
-          title: this.$gettext('Delete public link'),
+          title: this.$gettext('Delete link'),
           method: this.deleteLink,
           variation: 'danger'
         }
@@ -359,6 +367,7 @@ export default {
 
   methods: {
     ...mapActions(['createModal', 'hideModal', 'setModalInputErrorMessage']),
+
     updateLink({
       link = this.link,
       dropRef = this.$refs.editPublicLinkDropdown,
@@ -378,7 +387,7 @@ export default {
         cancelText: this.$gettext('Cancel'),
         confirmText: this.$gettext('Save'),
         hasInput: true,
-        inputValue: this.link.name ? this.link.name : this.link.token,
+        inputValue: this.link.name,
         inputLabel: this.$gettext('Link name'),
         onCancel: this.hideModal,
         onConfirm: (name) =>

diff --git a/packages/web-app-files/src/components/SideBar/Shares/PublicLinks/NameAndCopy.vue b/packages/web-app-files/src/components/SideBar/Shares/PublicLinks/NameAndCopy.vue
@@ -1,10 +1,7 @@
 <template>
   <div class="oc-mb-s oc-width-1-1">
-    <h5 class="oc-text-truncate oc-files-file-link-name oc-my-s" v-text="linkName" />
-    <div
-      class="oc-flex oc-flex-middle oc-flex-between oc-width-1-1 oc-p-s"
-      style="background-color: var(--oc-color-input-bg); border: 1px solid white"
-    >
+    <h4 class="oc-text-truncate oc-files-file-link-name oc-my-s" v-text="linkName" />
+    <div class="oc-flex oc-flex-middle oc-flex-between oc-width-1-1 oc-p-s link-name-container">
       <div class="oc-flex oc-flex-middle oc-text-truncate">
         <oc-icon name="link" fill-type="line" />
         <p class="oc-files-file-link-url oc-ml-s oc-text-truncate oc-my-rm" v-text="link.url" />
@@ -34,21 +31,21 @@ export default {
   },
   computed: {
     linkName() {
-      return this.link.name ? this.link.name : this.link.token
+      return this.link.name
     },
 
     copyToClipboardLabel() {
       return this.$gettext('Copy link to clipboard')
     },
 
     copyToClipboardSuccessMsgTitle() {
-      return this.$gettext('Public link copied')
+      return this.$gettext('Link copied')
     }
   },
   methods: {
     getCopyToClipboardSuccessMsgText(linkName) {
       return this.$gettextInterpolate(
-        this.$gettext('The public link "%{linkName}" has been copied to your clipboard.'),
+        this.$gettext('The link "%{linkName}" has been copied to your clipboard.'),
         {
           linkName
         },
@@ -58,3 +55,9 @@ export default {
   }
 }
 </script>
+<style lang="scss" scoped>
+.link-name-container {
+  background-color: var(--oc-color-input-bg);
+  border: 1px solid var(--oc-color-input-border);
+}
+</style>
diff --git a/packages/web-app-files/src/helpers/resources.ts b/packages/web-app-files/src/helpers/resources.ts
@@ -442,7 +442,7 @@ function _buildLink(link): Share {
     permissions: link.permissions,
     description,
     stime: link.stime,
-    name: typeof link.name === 'string' ? link.name : '',
+    name: typeof link.name === 'string' ? link.name : (link.token as string),
     password: !!(link.share_with && link.share_with_displayname),
     expiration:
       typeof link.expiration === 'string'

diff --git a/packages/web-app-files/src/helpers/share/role.ts b/packages/web-app-files/src/helpers/share/role.ts
@@ -320,7 +320,7 @@ const shareRoleDescriptions = {
 /**
  * Maps relevant permission bitmasks of link roles to descriptions
  */
-export const linkRoleDescriptions = {
+const linkRoleDescriptions = {
   [linkRoleViewerFile.bitmask(false)]: $gettext('Recipients can view and download contents.'),
   [linkRoleViewerFolder.bitmask(false)]: $gettext('Recipients can view and download contents.'),
   [linkRoleContributorFolder.bitmask(false)]: $gettext(

diff --git a/packages/web-app-files/src/quickActions.js b/packages/web-app-files/src/quickActions.js
@@ -30,10 +30,8 @@ export function createPublicLink(ctx) {
         ctx.store.dispatch('Files/sidebar/openWithPanel', 'sharing-item').then(() => {
           copyToClipboard(link.url)
           ctx.store.dispatch('showMessage', {
-            title: ctx.$gettext('Public link created'),
-            desc: ctx.$gettext(
-              'Public link was created successfully and copied into your clipboard.'
-            )
+            title: ctx.$gettext('Link created'),
+            desc: ctx.$gettext('Link was created successfully and copied to your clipboard.')
           })
         })
         resolve()

diff --git a/tests/acceptance/features/webUISharingPublicBasic/publicLinkCreate.feature b/tests/acceptance/features/webUISharingPublicBasic/publicLinkCreate.feature
@@ -182,8 +182,8 @@ Feature: Create public link shares
       | name        | Quick action link |
     And the following success message should be displayed on the webUI
       """
-      Public link created
-      Public link was created successfully and copied into your clipboard.
+      Link created
+      Link was created successfully and copied to your clipboard.
       """
 
   # This test is skipped in OCIS as it's starting to fail frequently

diff --git a/tests/acceptance/stepDefinitions/publicLinkContext.js b/tests/acceptance/stepDefinitions/publicLinkContext.js
@@ -240,7 +240,7 @@ async function findMatchingPublicLinkByName(name, role, resource, via = null) {
 
   assert.strictEqual(role, share.role)
   if (via !== null) {
-    assert.ok(share.viaLabel, 'Expected via information to be disabled but could not find')
+    assert.ok(share.viaLabel, 'Expected "shared via" icon to be displayed but was not visible')
   }
 }