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.

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

fix: pass hide download attribute while creating the share to fix github issue 50788 #51250

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions apps/files_sharing/src/components/SharingInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

<template>
<div class="sharing-search">
<label class="hidden-visually" for="sharing-search-input">
<label class="hidden-visually" :for="shareInputId">
{{ isExternal ? t('files_sharing', 'Enter external recipients')
: t('files_sharing', 'Search for internal recipients') }}
</label>
<NcSelect ref="select"
v-model="value"
input-id="sharing-search-input"
:input-id="shareInputId"
class="sharing-search__input"
:disabled="!canReshare"
:loading="loading"
Expand Down Expand Up @@ -87,6 +87,12 @@ export default {
},
},

setup() {
return {
shareInputId: `share-input-${Math.random().toString(36).slice(2, 7)}`,
}
},

data() {
return {
config: new Config(),
Expand Down
1 change: 1 addition & 0 deletions apps/files_sharing/src/mixins/ShareDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export default {
scope: 'permissions',
},
],
hideDownload: false,
share_type: shareRequestObject.shareType,
share_with: shareRequestObject.shareWith,
is_no_user: shareRequestObject.isNoUser,
Expand Down
7 changes: 4 additions & 3 deletions apps/files_sharing/src/mixins/SharesMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,14 +278,16 @@ export default {
// force value to string because that is what our
// share api controller accepts
propertyNames.forEach(name => {
if ((typeof this.share[name]) === 'object') {
if (this.share[name] === null || this.share[name] === undefined) {
properties[name] = ''
} else if ((typeof this.share[name]) === 'object') {
properties[name] = JSON.stringify(this.share[name])
} else {
properties[name] = this.share[name].toString()
}
})

this.updateQueue.add(async () => {
return this.updateQueue.add(async () => {
this.saving = true
this.errors = {}
try {
Expand Down Expand Up @@ -317,7 +319,6 @@ export default {
this.saving = false
}
})
return
}

// This share does not exists on the server yet
Expand Down
7 changes: 6 additions & 1 deletion apps/files_sharing/src/models/Share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ export default class Share {
ocsData = ocsData.ocs.data[0]
}

// string to int
if (typeof ocsData.id === 'string') {
ocsData.id = Number.parseInt(ocsData.id)
}
// convert int into boolean
ocsData.hide_download = !!ocsData.hide_download
ocsData.mail_send = !!ocsData.mail_send
Expand Down Expand Up @@ -77,7 +81,7 @@ export default class Share {
* Get the share attributes
*/
get attributes(): Array<ShareAttribute> {
return this._share.attributes
return this._share.attributes || []
}

/**
Expand Down Expand Up @@ -241,6 +245,7 @@ export default class Share {
*/
get hideDownload(): boolean {
return this._share.hide_download === true
|| this.attributes.find?.(({ scope, key, value }) => scope === 'permissions' && key === 'download' && !value) !== undefined
}

/**
Expand Down
16 changes: 15 additions & 1 deletion apps/files_sharing/src/views/SharingDetailsTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -1005,10 +1005,24 @@ export default {

this.creating = true
const share = await this.addShare(incomingShare)
this.creating = false
// ugly hack to make code work - we need the id to be set but at the same time we need to keep values we want to update
this.share._share.id = share.id
await this.queueUpdate(...permissionsAndAttributes)
// Also a ugly hack to update the updated permissions
for (const prop of permissionsAndAttributes) {
if (prop in share && prop in this.share) {
try {
share[prop] = this.share[prop]
} catch {
share._share[prop] = this.share[prop]
}
}
}
this.share = share
this.creating = false
this.$emit('add:share', this.share)
} else {
// Let's update after creation as some attrs are only available after creation
this.$emit('update:share', this.share)
emit('update:share', this.share)
this.queueUpdate(...permissionsAndAttributes)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
*/
// @ts-expect-error The package is currently broken - but works...
import { deleteDownloadsFolderBeforeEach } from 'cypress-delete-downloads-folder'
import { createShare, getShareUrl, setupPublicShare, type ShareContext } from './setup-public-share.ts'
import { createShare, getShareUrl, openLinkShareDetails, setupPublicShare, type ShareContext } from './setup-public-share.ts'
import { getRowForFile, getRowForFileId, triggerActionForFile, triggerActionForFileId } from '../../files/FilesUtils.ts'
import { zipFileContains } from '../../../support/utils/assertions.ts'
import { openSharingDetails } from '../FilesSharingUtils.ts'
import { c } from 'tar'
import type { User } from '@nextcloud/cypress'

describe('files_sharing: Public share - downloading files', { testIsolation: true }, () => {

Expand Down Expand Up @@ -170,4 +173,106 @@ describe('files_sharing: Public share - downloading files', { testIsolation: tru
})
})
})

describe('download permission - link share', () => {
let context: ShareContext
before(() => {
cy.createRandomUser()
.then((user) => {
cy.mkdir(user, '/test')

context = { user }
createShare(context, 'test')
})
})

beforeEach(() => {
cy.login(context.user)
cy.visit('/apps/files')
})

deleteDownloadsFolderBeforeEach()

it('download permission is retained', () => {
getRowForFile('test').should('be.visible')
triggerActionForFile('test', 'details')

openLinkShareDetails(0)

cy.intercept('PUT', '**/ocs/v2.php/apps/files_sharing/api/v1/shares/*').as('update')

cy.findByRole('checkbox', { name: /hide download/i })
.should('exist')
.and('not.be.checked')
.check({ force: true })
cy.findByRole('checkbox', { name: /hide download/i })
.should('be.checked')
cy.findByRole('button', { name: /update share/i })
.click()

cy.wait('@update')

openLinkShareDetails(0)
cy.findByRole('checkbox', { name: /hide download/i })
.should('be.checked')

cy.reload()

getRowForFile('test').should('be.visible')
triggerActionForFile('test', 'details')
openLinkShareDetails(0)
cy.findByRole('checkbox', { name: /hide download/i })
.should('be.checked')
})
})

describe('download permission - mail share', () => {
let user: User

before(() => {
cy.createRandomUser()
.then(($user) => {
user = $user
cy.mkdir(user, '/test')
})
})

beforeEach(() => {
cy.login(user)
cy.visit('/apps/files')
})

it('download permission is retained', () => {
getRowForFile('test').should('be.visible')
triggerActionForFile('test', 'details')

cy.findByRole('combobox', { name: /Enter external recipients/i })
.type('[email protected]')

cy.get('.option[sharetype="4"][user="[email protected]"]')
.parent('li')
.click()
cy.findByRole('button', { name: /advanced settings/i })
.should('be.visible')
.click()

cy.intercept('PUT', '**/ocs/v2.php/apps/files_sharing/api/v1/shares/*').as('update')

cy.findByRole('checkbox', { name: /hide download/i })
.should('exist')
.and('not.be.checked')
.check({ force: true })
cy.findByRole('button', { name: /save share/i })
.click()

cy.wait('@update')

openLinkShareDetails(1)
cy.findByRole('button', { name: /advanced settings/i })
.click()
cy.findByRole('checkbox', { name: /hide download/i })
.should('exist')
.and('be.checked')
})
})
})
15 changes: 12 additions & 3 deletions cypress/e2e/files_sharing/public-share/setup-public-share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,24 @@ export function createShare(context: ShareContext, shareName: string, options: S
}

/**
* Adjust share permissions to be editable
* open link share details for specific index
*
* @param index
*/
function adjustSharePermission(): void {
export function openLinkShareDetails(index: number) {
cy.findByRole('list', { name: 'Link shares' })
.findAllByRole('listitem')
.first()
.eq(index)
.findByRole('button', { name: /Actions/i })
.click()
cy.findByRole('menuitem', { name: /Customize link/i }).click()
}

/**
* Adjust share permissions to be editable
*/
function adjustSharePermission(): void {
openLinkShareDetails(0)

cy.get('[data-cy-files-sharing-share-permissions-bundle]').should('be.visible')
cy.get('[data-cy-files-sharing-share-permissions-bundle="upload-edit"]').click()
Expand Down
4 changes: 2 additions & 2 deletions dist/3456-3456.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/3456-3456.js.map

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions dist/3738-3738.js

Large diffs are not rendered by default.

File renamed without changes.
1 change: 1 addition & 0 deletions dist/3738-3738.js.map

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions dist/3738-3738.js.map.license
2 changes: 0 additions & 2 deletions dist/8309-8309.js

This file was deleted.

1 change: 0 additions & 1 deletion dist/8309-8309.js.map

This file was deleted.

1 change: 0 additions & 1 deletion dist/8309-8309.js.map.license

This file was deleted.

4 changes: 2 additions & 2 deletions dist/files_sharing-files_sharing_tab.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files_sharing-files_sharing_tab.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/files_sharing-init.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/files_sharing-init.js.map

Large diffs are not rendered by default.

Loading