feat: remove internal link type

Removes the internal link type since the permanent link makes it obsolete. Existing internal links still resolve correctly to ensure backwards compatibility.

changelog/unreleased/enhancement-internal-link-removal

@@ -0,0 +1,6 @@
+Enhancement: Internal link removal
+
+The internal link type has been removed because the permanent link makes it obsolete. Existing internal links still resolve correctly to ensure backwards compatibility.
+
+https://github.com/owncloud/web/pull/11553
+https://github.com/owncloud/web/issues/11544

packages/web-app-files/src/components/SideBar/Shares/FileLinks.vue

@@ -5,17 +5,14 @@
       <oc-contextual-helper v-if="helpersEnabled" class="oc-pl-xs" v-bind="viaLinkHelp" />
     </div>
     <p
-      v-if="!canCreateLinks({ space, resource })"
+      v-if="!canCreateLinks"
       data-testid="files-links-no-share-permissions-message"
       class="oc-mt-m"
       v-text="$gettext('You do not have permission to create links')"
     />
-    <div v-if="quicklink || canCreateLinks({ space, resource })" class="oc-mt-m">
+    <div v-if="quicklink || canCreateLinks" class="oc-mt-m">
       <name-and-copy v-if="quicklink" :link-share="quicklink" />
-      <create-quick-link
-        v-else-if="canCreateLinks({ space, resource })"
-        @create-public-link="addNewLink(true)"
-      />
+      <create-quick-link v-else-if="canCreateLinks" @create-public-link="addNewLink(true)" />
       <details-and-edit
         v-if="quicklink"
         :can-rename="false"
@@ -29,7 +26,7 @@
       />
       <hr class="oc-my-m" />
       <oc-button
-        v-if="canCreateLinks({ space, resource })"
+        v-if="canCreateLinks"
         id="files-file-link-add"
         variation="primary"
         appearance="raw"
@@ -137,7 +134,14 @@ export default defineComponent({
     const { dispatchModal } = useModals()
     const { removeResources } = useResourcesStore()
     const { isPasswordEnforcedForLinkType } = useLinkTypes()
-    const { canShare: canCreateLinks } = useCanShare()
+    const { canShare } = useCanShare()
+
+    const canCreateLinks = computed(() => {
+      if (!ability.can('create-all', 'PublicLink')) {
+        return false
+      }
+      return canShare({ space: unref(space), resource: unref(resource) })
+    })
 
     const sharesStore = useSharesStore()
     const { updateLink, deleteLink } = sharesStore
@@ -182,7 +186,7 @@ export default defineComponent({
 
     const canEditLink = (linkShare: LinkShare) => {
       return (
-        canCreateLinks({ space: unref(space), resource: unref(resource) }) &&
+        unref(canCreateLinks) &&
         (can('create-all', 'PublicLink') || linkShare.type === SharingLinkType.Internal)
       )
     }

packages/web-app-files/src/components/SideBar/Shares/Links/DetailsAndEdit.vue

@@ -149,7 +149,6 @@ import { OcDrop } from 'design-system/src/components'
 import { usePasswordPolicyService } from '@ownclouders/web-pkg'
 import { useGettext } from 'vue3-gettext'
 import SetLinkPasswordModal from '../../../Modals/SetLinkPasswordModal.vue'
-import { storeToRefs } from 'pinia'
 import { SharingLinkType } from '@ownclouders/web-client/graph/generated'
 import DatePickerModal from '../../../Modals/DatePickerModal.vue'
 import ExpirationDateIndicator from '../ExpirationDateIndicator.vue'
@@ -204,7 +203,6 @@ export default defineComponent({
       useLinkTypes()
 
     const resourcesStore = useResourcesStore()
-    const { ancestorMetaData } = storeToRefs(resourcesStore)
 
     const space = inject<Ref<SpaceResource>>('space')
     const resource = inject<Ref<Resource>>('resource')

packages/web-app-files/tests/unit/components/SideBar/Shares/FileLinks.spec.ts

@@ -108,7 +108,7 @@ describe('FileLinks', () => {
 
     describe('when the add-new-link button is clicked', () => {
       it('should call createLink', async () => {
-        const { wrapper, mocks } = getWrapper({ abilities: [] })
+        const { wrapper, mocks } = getWrapper()
         await wrapper.find(selectors.linkAddButton).trigger('click')
         expect(mocks.createLinkMock).toHaveBeenCalledTimes(1)
       })
@@ -129,7 +129,7 @@ describe('FileLinks', () => {
       canShare: () => true
     })
 
-    it('existing viewer link is not modifiable', () => {
+    it('existing link is not modifiable', () => {
       const viewerLink = defaultLinksList[0]
       viewerLink.type = SharingLinkType.View
       const { wrapper } = getWrapper({ resource, abilities: [], links: [viewerLink] })
@@ -139,15 +139,9 @@ describe('FileLinks', () => {
       const isModifiable = detailsAndEdit.props('isModifiable')
       expect(isModifiable).toBeFalsy()
     })
-    it('existing internal link is modifiable', () => {
-      const internalLink = defaultLinksList[0]
-      internalLink.type = SharingLinkType.Internal
-      const { wrapper } = getWrapper({ resource, abilities: [], links: [internalLink] })
-      const detailsAndEdit = wrapper.findComponent<typeof DetailsAndEdit>(
-        linkListItemDetailsAndEdit
-      )
-      const isModifiable = detailsAndEdit.props('isModifiable')
-      expect(isModifiable).toBeTruthy()
+    it('new links cannot be created', () => {
+      const { wrapper } = getWrapper({ resource, abilities: [] })
+      expect(wrapper.find(selectors.linkAddButton).exists()).toBeFalsy()
     })
   })
 })

packages/web-pkg/src/components/CreateLinkModal.vue

@@ -24,23 +24,17 @@
       <span v-text="$gettext('Options')" />
     </oc-button>
   </div>
-  <div v-if="!onlyInternalLinksAllowed" class="link-modal-password oc-mb-m">
+  <div class="link-modal-password oc-mb-m">
     <oc-text-input
       v-if="isAdvancedMode"
       :key="passwordInputKey"
       :model-value="password.value"
       type="password"
-      :password-policy="!selectedLinkTypeIsInternal ? passwordPolicy : null"
+      :password-policy="passwordPolicy"
       :generate-password-method="generatePasswordMethod"
       :error-message="password.error"
-      :description-message="
-        selectedLinkTypeIsInternal
-          ? $gettext('Password cannot be set for internal links')
-          : undefined
-      "
       :label="passwordEnforced ? `${$gettext('Password')}*` : $gettext('Password')"
       class="link-modal-password-input"
-      :disabled="selectedLinkTypeIsInternal"
       @update:model-value="updatePassword"
     />
     <div v-else-if="password.value" class="link-modal-password-text oc-text-small oc-text-muted">
@@ -52,12 +46,6 @@
       class="oc-mt-s"
       :min-date="DateTime.now()"
       :label="$gettext('Expiry date')"
-      :disabled="selectedLinkTypeIsInternal"
-      :description-message="
-        selectedLinkTypeIsInternal
-          ? $gettext('Expiry date cannot be set for internal links')
-          : undefined
-      "
       @date-changed="onExpiryDateChanged"
     />
   </div>
@@ -153,10 +141,6 @@ export default defineComponent({
         return $gettext('Share link(s)')
       }
 
-      if (unref(selectedLinkTypeIsInternal)) {
-        return $gettext('Copy link')
-      }
-
       if (unref(passwordEnforced) || unref(password)) {
         return $gettext('Copy link and password')
       }
@@ -183,12 +167,6 @@ export default defineComponent({
       enforcePassword: unref(passwordEnforced)
     })
 
-    const selectedLinkTypeIsInternal = computed(
-      () => unref(selectedType) === SharingLinkType.Internal
-    )
-    const onlyInternalLinksAllowed = computed(
-      () => unref(availableLinkTypes).length === 1 && unref(selectedLinkTypeIsInternal)
-    )
     const setAdvancedMode = () => {
       isAdvancedMode.value = true
     }
@@ -218,10 +196,8 @@ export default defineComponent({
     }
 
     const passwordPolicyFulfilled = computed(() => {
-      if (!unref(selectedLinkTypeIsInternal)) {
-        if (!passwordPolicy.check(unref(password).value)) {
-          return false
-        }
+      if (!passwordPolicy.check(unref(password).value)) {
+        return false
       }
 
       return true
@@ -284,14 +260,6 @@ export default defineComponent({
 
     const updateSelectedLinkType = (type: SharingLinkType) => {
       selectedType.value = type
-      if (unref(selectedLinkTypeIsInternal)) {
-        password.value = ''
-        password.error = ''
-        selectedExpiry.value = undefined
-
-        // re-render password because it's the only way to remove policy messages
-        passwordInputKey.value = uuidV4()
-      }
     }
 
     onMounted(() => {
@@ -317,8 +285,6 @@ export default defineComponent({
       selectedType,
       selectedTypeIcon,
       selectedTypeDescription,
-      selectedLinkTypeIsInternal,
-      onlyInternalLinksAllowed,
       isSelectedLinkType,
       updateSelectedLinkType,
       updatePassword,

packages/web-pkg/src/composables/actions/files/useFileActionsCopyQuicklink.ts

@@ -1,4 +1,4 @@
-import { isLinkShare } from '@ownclouders/web-client'
+import { isLinkShare, ShareTypes } from '@ownclouders/web-client'
 import { computed, unref } from 'vue'
 import { useClientService } from '../../clientService'
 import { useGettext } from 'vue3-gettext'
@@ -8,6 +8,9 @@ import { useClipboard } from '../../clipboard'
 import { Resource, SpaceResource } from '@ownclouders/web-client'
 import { useFileActionsCreateLink } from './useFileActionsCreateLink'
 import { useMessages, useSharesStore } from '../../piniaStores'
+import { useAbility } from '../../ability'
+import { SideBarEventTopics } from '../../sideBar'
+import { useEventBus } from '../../eventBus'
 
 export const useFileActionsCopyQuickLink = () => {
   const { showMessage, showErrorMessage } = useMessages()
@@ -17,6 +20,8 @@ export const useFileActionsCopyQuickLink = () => {
   const { canShare } = useCanShare()
   const sharesStore = useSharesStore()
   const { copyToClipboard } = useClipboard()
+  const { can } = useAbility()
+  const eventBus = useEventBus()
 
   const { actions: createLinkActions } = useFileActionsCreateLink()
   const createQuicklinkAction = computed<FileAction>(() =>
@@ -56,6 +61,13 @@ export const useFileActionsCopyQuickLink = () => {
       return copyQuickLinkToClipboard(existingQuickLink.webUrl)
     }
 
+    if (!can('create-all', 'PublicLink')) {
+      // this is for handling an edge case where the user cannot create public links
+      // and no quick links exists, but a normal one does
+      eventBus.publish(SideBarEventTopics.openWithPanel, 'sharing')
+      return
+    }
+
     return unref(createQuicklinkAction).handler({ space, resources })
   }
 
@@ -70,6 +82,13 @@ export const useFileActionsCopyQuickLink = () => {
           return false
         }
 
+        if (
+          !can('create-all', 'PublicLink') &&
+          !resources[0].shareTypes.includes(ShareTypes.link.value)
+        ) {
+          return false
+        }
+
         return canShare({ space, resource: resources[0] })
       },
       class: 'oc-files-actions-copy-quicklink-trigger'

packages/web-pkg/src/composables/actions/files/useFileActionsCreateLink.ts

@@ -15,7 +15,6 @@ import {
 } from '../../piniaStores'
 import { useClipboard } from '../../clipboard'
 import { useClientService } from '../../clientService'
-import { SharingLinkType } from '@ownclouders/web-client/graph/generated'
 
 export const useFileActionsCreateLink = ({
   enforceModal = false
@@ -88,7 +87,7 @@ export const useFileActionsCreateLink = ({
     { isQuickLink = false }: { isQuickLink?: boolean } = {}
   ) => {
     const passwordEnforced = capabilityStore.sharingPublicPasswordEnforcedFor.read_only === true
-    if (enforceModal || (passwordEnforced && unref(defaultLinkType) !== SharingLinkType.Internal)) {
+    if (enforceModal || passwordEnforced) {
       dispatchModal({
         title: $ngettext(
           'Copy link for "%{resourceName}"',

packages/web-pkg/src/composables/links/useLinkTypes.ts

@@ -12,19 +12,7 @@ export const useLinkTypes = () => {
 
   const canCreatePublicLinks = computed(() => ability.can('create-all', 'PublicLink'))
 
-  const defaultLinkType = computed<SharingLinkType>(() => {
-    const canCreatePublicLink = ability.can('create-all', 'PublicLink')
-    if (!canCreatePublicLink) {
-      return SharingLinkType.Internal
-    }
-
-    const defaultPermissions = capabilityStore.sharingPublicDefaultPermissions
-    if (defaultPermissions === undefined || defaultPermissions === 1) {
-      return SharingLinkType.View
-    }
-
-    return SharingLinkType.Internal
-  })
+  const defaultLinkType = computed<SharingLinkType>(() => SharingLinkType.View)
 
   const isPasswordEnforcedForLinkType = (type: SharingLinkType) => {
     if (type === SharingLinkType.View) {
@@ -44,20 +32,19 @@ export const useLinkTypes = () => {
 
   const getAvailableLinkTypes = ({ isFolder }: { isFolder: boolean }): SharingLinkType[] => {
     if (!unref(canCreatePublicLinks)) {
-      return [SharingLinkType.Internal]
+      return []
     }
 
     if (isFolder) {
       return [
-        SharingLinkType.Internal,
         SharingLinkType.View,
         SharingLinkType.Upload,
         SharingLinkType.Edit,
         SharingLinkType.CreateOnly
       ]
     }
 
-    return [SharingLinkType.Internal, SharingLinkType.View, SharingLinkType.Edit]
+    return [SharingLinkType.View, SharingLinkType.Edit]
   }
 
   // links don't have roles in graph API, hence we need to define them here

packages/web-pkg/tests/unit/components/CreateLinkModal.spec.ts

@@ -42,17 +42,11 @@ describe('CreateLinkModal', () => {
       await nextTick()
       expect(wrapper.find(selectors.passwordInput).exists()).toBeTruthy()
     })
-    it('should be disabled for internal links', async () => {
-      const { wrapper } = getWrapper({ defaultLinkType: SharingLinkType.Internal })
-      wrapper.vm.isAdvancedMode = true
-      await nextTick()
-      expect(wrapper.find(selectors.passwordInput).attributes('disabled')).toBeTruthy()
-    })
     it('should not be rendered if user cannot create public links', () => {
       const { wrapper } = getWrapper({
         userCanCreatePublicLinks: false,
-        availableLinkTypes: [SharingLinkType.Internal],
-        defaultLinkType: SharingLinkType.Internal
+        availableLinkTypes: [],
+        defaultLinkType: SharingLinkType.View
       })
       expect(wrapper.find(selectors.passwordInput).exists()).toBeFalsy()
     })
@@ -70,17 +64,11 @@ describe('CreateLinkModal', () => {
       await nextTick()
       expect(wrapper.findComponent({ name: 'oc-datepicker' }).exists()).toBeTruthy()
     })
-    it('should be disabled for internal links', async () => {
-      const { wrapper } = getWrapper({ defaultLinkType: SharingLinkType.Internal })
-      wrapper.vm.isAdvancedMode = true
-      await nextTick()
-      expect(wrapper.findComponent({ name: 'oc-datepicker' }).attributes('disabled')).toBeTruthy()
-    })
     it('should not be rendered if user cannot create public links', () => {
       const { wrapper } = getWrapper({
         userCanCreatePublicLinks: false,
-        availableLinkTypes: [SharingLinkType.Internal],
-        defaultLinkType: SharingLinkType.Internal
+        availableLinkTypes: [],
+        defaultLinkType: SharingLinkType.View
       })
       expect(wrapper.findComponent({ name: 'oc-datepicker' }).exists()).toBeFalsy()
     })
@@ -93,11 +81,7 @@ describe('CreateLinkModal', () => {
       expect(wrapper.find(selectors.linkRoleDropDownToggle).exists()).toBeFalsy()
     })
     it('lists all types as roles', async () => {
-      const availableLinkTypes = [
-        SharingLinkType.View,
-        SharingLinkType.Internal,
-        SharingLinkType.Edit
-      ]
+      const availableLinkTypes = [SharingLinkType.View, SharingLinkType.Edit]
       const { wrapper } = getWrapper({ availableLinkTypes })
       wrapper.vm.isAdvancedMode = true
       await nextTick()
@@ -183,7 +167,7 @@ function getWrapper({
   embedModeEnabled = false,
   callbackFn = undefined,
   isQuickLink = false,
-  availableLinkTypes = [SharingLinkType.Internal, SharingLinkType.View]
+  availableLinkTypes = [SharingLinkType.View]
 }: {
   resources?: Resource[]
   defaultLinkType?: SharingLinkType
@@ -204,7 +188,7 @@ function getWrapper({
     mock<ReturnType<typeof useLinkTypes>>({
       defaultLinkType: ref(defaultLinkType),
       getAvailableLinkTypes: () => availableLinkTypes,
-      getLinkRoleByType: () => mock<ShareRole>({ description: 'role' }),
+      getLinkRoleByType: () => mock<ShareRole>({ description: 'role', label: '' }),
       isPasswordEnforcedForLinkType: () => passwordEnforced
     })
   )