Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect archiver limits on BatchAction download #9055

Merged
merged 2 commits into from
May 19, 2023
Merged

Respect archiver limits on BatchAction download #9055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8cfa4ff
Respect archiver limits for batch download action (#8456)
pascalwengerter May 16, 2023
950f2a8
Apply suggestions from code review
pascalwengerter May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog/unreleased/enhancement-respect-archiver-limits
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Enhancement: Respect archiver limits

The archiver service announces a limit for the accumulated filesize of the currently selected resources.
The web UI now respects those limits and shows a disabled download button once the limit has been reached.

https://github.com/owncloud/web/pull/9055
https://github.com/owncloud/web/issues/8456
2 changes: 1 addition & 1 deletion packages/design-system/changelog/7.0.0_2021-05-31/bugfix-interactive-oc-tooltip-texts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Bugfix: Interactive texts in OcTooltips

The new tooltip texts would only be initialized once and then didn't offer
the possibility of chaging reactively. By updating the directive, we've changed that.
the possibility of changing reactively. By updating the directive, we've changed that.

https://github.com/owncloud/owncloud-design-system/pull/1288
28 changes: 27 additions & 1 deletion packages/web-app-files/src/composables/actions/files/useFileActionsDownloadArchive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ import {
import { FileAction, FileActionOptions } from 'web-pkg/src/composables/actions'
import { useGettext } from 'vue3-gettext'
import { useArchiverService } from 'web-app-files/src/composables/archiverService'
import { formatFileSize } from 'web-pkg/src/helpers/filesize'

export const useFileActionsDownloadArchive = ({ store }: { store?: Store<any> } = {}) => {
store = store || useStore()
const router = useRouter()
const loadingService = useLoadingService()
const archiverService = useArchiverService()
const { $ngettext, $gettext } = useGettext()
const { $ngettext, $gettext, interpolate: $gettextInterpolate, current } = useGettext()
const publicLinkPassword = usePublicLinkPassword({ store })
const isFilesAppActive = useIsFilesAppActive()

Expand Down Expand Up @@ -59,6 +60,20 @@ export const useFileActionsDownloadArchive = ({ store }: { store?: Store<any> }
})
}

const areArchiverLimitsExceeded = (resources: Resource[]) => {
const archiverCapabilities = archiverService.capability
if (!archiverCapabilities) {
return
}

const selectedFilesSize = resources.reduce(
(accumulator, currentValue) => accumulator + parseInt(`${currentValue.size}`),
0
)

return selectedFilesSize > parseInt(archiverCapabilities.max_size)
}

const actions = computed((): FileAction[] => {
return [
{
Expand All @@ -68,6 +83,17 @@ export const useFileActionsDownloadArchive = ({ store }: { store?: Store<any> }
label: () => {
return $gettext('Download')
},
disabledTooltip: ({ resources }) => {
return areArchiverLimitsExceeded(resources)
? $gettextInterpolate(
$gettext('The selection exceeds the allowed archive size (max. %{maxSize})'),
{
maxSize: formatFileSize(archiverService.capability.max_size, current)
}
)
: ''
},
isDisabled: ({ resources }) => areArchiverLimitsExceeded(resources),
isEnabled: ({ resources }) => {
if (
unref(isFilesAppActive) &&
Expand Down
2 changes: 2 additions & 0 deletions packages/web-app-files/src/services/archiver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export interface ArchiverCapability {
formats: string[]
// eslint-disable-next-line camelcase
archiver_url: string
max_num_files: string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are those strings ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just using what oCIS/reva provide on my recently pulled docker image ;) see https://github.com/cs3org/reva/blob/2be8a61879b22823c852757497c81f2d544f6f9d/internal/http/services/owncloud/ocs/data/capabilities.go#L114

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't we parse them beforehand?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this only reflects the raw capabilities coming from the backend. Parsing makes sense in the capability composables though.

max_size: string
}

interface TriggerDownloadOptions {
Expand Down
6 changes: 3 additions & 3 deletions ...web-app-files/tests/unit/components/Spaces/__snapshots__/SpaceContextActions.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ exports[`SpaceContextActions action handlers renders actions that are always ava
<div id="oc-files-context-menu">
<ul class="oc-list oc-my-rm oc-mx-rm oc-files-context-actions oc-pb-s oc-files-context-actions-border" id="oc-files-context-actions-members">
<li class="context-menu oc-files-context-action oc-px-s oc-rounded oc-menu-item-hover">
<button class="oc-button oc-rounded oc-button-s oc-button-justify-content-left oc-button-gap-m oc-button-passive oc-button-passive-raw oc-files-actions-show-details-trigger action-menu-item oc-py-s oc-px-m oc-width-1-1" type="button">
<button aria-label="" class="oc-button oc-rounded oc-button-s oc-button-justify-content-left oc-button-gap-m oc-button-passive oc-button-passive-raw oc-files-actions-show-details-trigger action-menu-item oc-py-s oc-px-m oc-width-1-1" type="button">
<!-- @slot Content of the button -->
<span class="oc-icon oc-icon-m oc-icon-passive">
<!---->
Expand All @@ -18,7 +18,7 @@ exports[`SpaceContextActions action handlers renders actions that are always ava
</ul>
<ul class="oc-list oc-my-rm oc-mx-rm oc-files-context-actions oc-pb-s oc-pt-s oc-files-context-actions-border" id="oc-files-context-actions-secondaryActions">
<li class="context-menu oc-files-context-action oc-px-s oc-rounded oc-menu-item-hover">
<button class="oc-button oc-rounded oc-button-s oc-button-justify-content-left oc-button-gap-m oc-button-passive oc-button-passive-raw oc-files-actions-edit-quota-trigger action-menu-item oc-py-s oc-px-m oc-width-1-1" type="button">
<button aria-label="" class="oc-button oc-rounded oc-button-s oc-button-justify-content-left oc-button-gap-m oc-button-passive oc-button-passive-raw oc-files-actions-edit-quota-trigger action-menu-item oc-py-s oc-px-m oc-width-1-1" type="button">
<!-- @slot Content of the button -->
<span class="oc-icon oc-icon-m oc-icon-passive">
<!---->
Expand All @@ -31,7 +31,7 @@ exports[`SpaceContextActions action handlers renders actions that are always ava
</ul>
<ul class="oc-list oc-my-rm oc-mx-rm oc-files-context-actions oc-pt-s" id="oc-files-context-actions-sidebar">
<li class="context-menu oc-files-context-action oc-px-s oc-rounded oc-menu-item-hover">
<button class="oc-button oc-rounded oc-button-s oc-button-justify-content-left oc-button-gap-m oc-button-passive oc-button-passive-raw oc-files-actions-show-details-trigger action-menu-item oc-py-s oc-px-m oc-width-1-1" type="button">
<button aria-label="" class="oc-button oc-rounded oc-button-s oc-button-justify-content-left oc-button-gap-m oc-button-passive oc-button-passive-raw oc-files-actions-show-details-trigger action-menu-item oc-py-s oc-px-m oc-width-1-1" type="button">
<!-- @slot Content of the button -->
<span class="oc-icon oc-icon-m oc-icon-passive">
<!---->
Expand Down
20 changes: 15 additions & 5 deletions packages/web-app-files/tests/unit/services/archiver.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ describe('archiver', () => {
enabled: true,
version: 'v2.3.5',
archiver_url: archiverUrl,
formats: []
formats: [],
max_num_files: '42',
max_size: '1073741824'
}
]

Expand Down Expand Up @@ -72,7 +74,9 @@ describe('archiver', () => {
enabled: true,
version: 'v1.2.3',
archiver_url: archiverUrl,
formats: []
formats: [],
max_num_files: '42',
max_size: '1073741824'
}
]
it('is announcing itself as not supporting fileIds', () => {
Expand Down Expand Up @@ -105,19 +109,25 @@ describe('archiver', () => {
enabled: true,
version: 'v1.2.3',
archiver_url: archiverUrl + '/v1',
formats: []
formats: [],
max_num_files: '42',
max_size: '1073741824'
}
const capabilityV2 = {
enabled: true,
version: 'v2.3.5',
archiver_url: archiverUrl + '/v2',
formats: []
formats: [],
max_num_files: '42',
max_size: '1073741824'
}
const capabilityV3 = {
enabled: false,
version: 'v3.2.5',
archiver_url: archiverUrl + '/v3',
formats: []
formats: [],
max_num_files: '42',
max_size: '1073741824'
}

it('uses the highest major version', async () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/web-pkg/src/components/ContextActions/ActionMenuItem.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
<template>
<li>
<li v-oc-tooltip="componentProps.disabled ? action.disabledTooltip(actionOptions) : ''">
<oc-button
v-oc-tooltip="showTooltip || action.hideLabel ? action.label(actionOptions) : ''"
:type="action.componentType"
v-bind="componentProps"
:class="[action.class, 'action-menu-item', 'oc-py-s', 'oc-px-m', 'oc-width-1-1']"
:aria-label="componentProps.disabled ? action.disabledTooltip(actionOptions) : ''"
data-testid="action-handler"
size="small"
justify-content="left"
Expand Down Expand Up @@ -35,8 +36,9 @@
v-if="!action.hideLabel"
class="oc-files-context-action-label"
data-testid="action-label"
>{{ action.label(actionOptions) }}</span
>
{{ action.label(actionOptions) }}
</span>
<span
v-if="action.shortcut && shortcutHint"
class="oc-files-context-action-shortcut"
Expand Down
1 change: 1 addition & 0 deletions packages/web-pkg/src/composables/actions/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface Action<T = ActionOptions> {

// can be used to display the action in a disabled state in the UI
isDisabled?(options?: T): boolean
disabledTooltip?(options?: T): string
}

export type FileActionOptions = {
Expand Down
12 changes: 11 additions & 1 deletion packages/web-pkg/tests/unit/components/ContextActions/ActionMenuItem.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ const selectors = {
icon: '[data-testid="action-icon"]',
img: '[data-testid="action-img"]',
label: '[data-testid="action-label"]',
srHint: '[data-testid="action-sr-hint"]'
srHint: '[data-testid="action-sr-hint"]',
ariaLabel: '[aria-label="foo"]'
}

const fileActions = {
Expand Down Expand Up @@ -50,6 +51,15 @@ describe('ActionMenuItem component', () => {
expect(wrapper.find(selectors.label).exists()).toBeTruthy()
expect(wrapper.find(selectors.label).text()).toBe(action.label())
})
it('renders a tooltip for a disabled action', () => {
const action = { ...fileActions.download, disabledTooltip: () => 'Foo', isDisabled: () => true }
const { wrapper } = getWrapper(action)

expect(wrapper.find(fileActions.download.selector).attributes().arialabel).toBe(
action.disabledTooltip()
)
expect(wrapper.find(fileActions.download.selector).attributes().disabled).toBeTruthy()
})
describe('component is of type oc-button', () => {
it('calls the action handler on button click', async () => {
const action = fileActions.download
Expand Down