Skip to content

Commit

Permalink
fix(files_sharing): adjust IAttributes API and files_versions
Browse files Browse the repository at this point in the history
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
  • Loading branch information
skjnldsv committed Jul 12, 2024
1 parent d603148 commit b55bc69
Show file tree
Hide file tree
Showing 15 changed files with 40 additions and 43 deletions.
2 changes: 1 addition & 1 deletion apps/files/js/fileinfomodel.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@
for (const i in this.attributes.shareAttributes) {
const attr = this.attributes.shareAttributes[i]
if (attr.scope === 'permissions' && attr.key === 'download') {
return attr.enabled
return attr.value === true
}
}

Expand Down
2 changes: 1 addition & 1 deletion apps/files/src/actions/downloadAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const isDownloadable = function(node: Node) {
if (node.attributes['mount-type'] === 'shared') {
const shareAttributes = JSON.parse(node.attributes['share-attributes'] ?? 'null')
const downloadAttribute = shareAttributes?.find?.((attribute: { scope: string; key: string }) => attribute.scope === 'permissions' && attribute.key === 'download')
if (downloadAttribute !== undefined && downloadAttribute.enabled === false) {
if (downloadAttribute !== undefined && downloadAttribute.value === false) {
return false
}
}
Expand Down
4 changes: 2 additions & 2 deletions apps/files/src/actions/moveOrCopyActionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const getQueue = () => {
}

type ShareAttribute = {
enabled: boolean
value: any
key: string
scope: string
}
Expand All @@ -48,7 +48,7 @@ export const canMove = (nodes: Node[]) => {
export const canDownload = (nodes: Node[]) => {
return nodes.every(node => {
const shareAttributes = JSON.parse(node.attributes?.['share-attributes'] ?? '[]') as Array<ShareAttribute>
return !shareAttributes.some(attribute => attribute.scope === 'permissions' && attribute.enabled === false && attribute.key === 'download')
return !shareAttributes.some(attribute => attribute.scope === 'permissions' && attribute.value === false && attribute.key === 'download')

})
}
Expand Down
8 changes: 4 additions & 4 deletions apps/files/src/views/FilesList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -396,14 +396,14 @@ export default defineComponent({
return { ...this.$route, query: { dir } }
},
shareAttributes(): number[] | undefined {
shareTypesAttributes(): number[] | undefined {
if (!this.currentFolder?.attributes?.['share-types']) {
return undefined
}
return Object.values(this.currentFolder?.attributes?.['share-types'] || {}).flat() as number[]
},
shareButtonLabel() {
if (!this.shareAttributes) {
if (!this.shareTypesAttributes) {
return t('files', 'Share')
}
Expand All @@ -413,12 +413,12 @@ export default defineComponent({
return t('files', 'Shared')
},
shareButtonType(): Type | null {
if (!this.shareAttributes) {
if (!this.shareTypesAttributes) {
return null
}
// If all types are links, show the link icon
if (this.shareAttributes.some(type => type === Type.SHARE_TYPE_LINK)) {
if (this.shareTypesAttributes.some(type => type === Type.SHARE_TYPE_LINK)) {
return Type.SHARE_TYPE_LINK
}
Expand Down
27 changes: 10 additions & 17 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@ public function getInheritedShares(string $path): DataResponse {
}

if (!($node->getPermissions() & Constants::PERMISSION_SHARE)) {
throw new SharingRightsException('no sharing rights on this item');
throw new SharingRightsException($this->l->t('no sharing rights on this item'));
}

// The current top parent we have access to
Expand Down Expand Up @@ -1171,7 +1171,7 @@ public function updateShare(
}

if (!$this->canEditShare($share)) {
throw new OCSForbiddenException('You are not allowed to edit incoming shares');
throw new OCSForbiddenException($this->l->t('You are not allowed to edit incoming shares'));
}

if (
Expand Down Expand Up @@ -1218,7 +1218,7 @@ public function updateShare(
*/

if ($share->getSharedBy() !== $this->currentUser) {
throw new OCSForbiddenException('You are not allowed to edit link shares that you don\'t own');
throw new OCSForbiddenException($this->l->t('You are not allowed to edit link shares that you don\'t own'));
}

// Update hide download state
Expand Down Expand Up @@ -1640,7 +1640,7 @@ private function parseDate(string $expireDate): \DateTime {
// Make sure it expires at midnight in owner timezone
$date->setTime(0, 0, 0);
} catch (\Exception $e) {
throw new \Exception('Invalid date. Format must be YYYY-MM-DD');
throw new \Exception($this->l->t('Invalid date. Format must be YYYY-MM-DD'));
}

return $date;
Expand Down Expand Up @@ -1845,7 +1845,7 @@ private function getSharesFromNode(string $viewer, $node, bool $reShares): array
*/
private function confirmSharingRights(Node $node): void {
if (!$this->hasResharingRights($this->currentUser, $node)) {
throw new SharingRightsException('no sharing rights on this item');
throw new SharingRightsException($this->l->t('No sharing rights on this item'));
}
}

Expand Down Expand Up @@ -2008,21 +2008,14 @@ private function setShareAttributes(IShare $share, ?string $attributesString) {
$formattedShareAttributes = \json_decode($attributesString, true);
if (is_array($formattedShareAttributes)) {
foreach ($formattedShareAttributes as $formattedAttr) {
// Legacy handling of the 'enabled' attribute
if (array_key_exists('enabled', $formattedAttr)) {
$formattedAttr['value'] = is_string($formattedAttr['enabled'])
? (bool) \json_decode($formattedAttr['enabled'])
: $formattedAttr['enabled'];
}

$newShareAttributes->setAttribute(
$formattedAttr['scope'],
$formattedAttr['key'],
$formattedAttr['value'],
);
}
} else {
throw new OCSBadRequestException('Invalid share attributes provided: \"' . $attributesString . '\"');
throw new OCSBadRequestException($this->l->t('Invalid share attributes provided: "%s"', [$attributesString]));
}
}
$share->setAttributes($newShareAttributes);
Expand All @@ -2044,10 +2037,10 @@ private function checkInheritedAttributes(IShare $share): void {
if ($storage instanceof Wrapper) {
$storage = $storage->getInstanceOfStorage(SharedStorage::class);
if ($storage === null) {
throw new \RuntimeException('Should not happen, instanceOfStorage but getInstanceOfStorage return null');
throw new \RuntimeException($this->l->t('Should not happen, instanceOfStorage but getInstanceOfStorage return null'));
}
} else {
throw new \RuntimeException('Should not happen, instanceOfStorage but not a wrapper');
throw new \RuntimeException($this->l->t('Should not happen, instanceOfStorage but not a wrapper'));
}
/** @var \OCA\Files_Sharing\SharedStorage $storage */
$inheritedAttributes = $storage->getShare()->getAttributes();
Expand Down Expand Up @@ -2085,15 +2078,15 @@ public function sendShareEmail(string $id, $password = ''): DataResponse {
}

if (!$this->canEditShare($share)) {
throw new OCSForbiddenException('You are not allowed to send mail notifications');
throw new OCSForbiddenException($this->l->t('You are not allowed to send mail notifications'));
}

// For mail and link shares, the user must be
// the owner of the share, not only the file owner.
if ($share->getShareType() === IShare::TYPE_EMAIL
|| $share->getShareType() === IShare::TYPE_LINK) {
if ($share->getSharedBy() !== $this->currentUser) {
throw new OCSForbiddenException('You are not allowed to send mail notifications');
throw new OCSForbiddenException($this->l->t('You are not allowed to send mail notifications'));
}
}

Expand Down
2 changes: 1 addition & 1 deletion apps/files_sharing/tests/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ public function testUpdateShare() {
$ocs = $this->createOCS(self::TEST_FILES_SHARING_API_USER1);
$ocs->updateShare(
$share1->getId(), 1, null, null, null, null, null, null, null,
'[{"scope": "app1", "key": "attr1", "enabled": true}]'
'[{"scope": "app1", "key": "attr1", "value": true}]'
);
$ocs->cleanup();

Expand Down
2 changes: 1 addition & 1 deletion apps/files_versions/src/components/Version.vue
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export default defineComponent({
const downloadAttribute = this.fileInfo.shareAttributes
.find((attribute) => attribute.scope === 'permissions' && attribute.key === 'download') || {}
// If the download attribute is set to false, the file is not downloadable
if (downloadAttribute?.enabled === false) {
if (downloadAttribute?.value === false) {
return false
}
}
Expand Down
2 changes: 1 addition & 1 deletion build/integration/features/bootstrap/Sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public function createShare($user,
}

if ($viewOnly === true) {
$body['attributes'] = json_encode([['scope' => 'permissions', 'key' => 'download', 'enabled' => false]]);
$body['attributes'] = json_encode([['scope' => 'permissions', 'key' => 'download', 'value' => false]]);
}

$options['form_params'] = $body;
Expand Down
2 changes: 1 addition & 1 deletion core/src/files/fileinfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@
for (const i in this.shareAttributes) {
const attr = this.shareAttributes[i]
if (attr.scope === 'permissions' && attr.key === 'download') {
return attr.enabled
return attr.value === true
}
}

Expand Down
4 changes: 4 additions & 0 deletions cypress/e2e/files_sharing/filesSharingUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export function createShare(fileName: string, username: string, shareSettings: P
export function updateShare(fileName: string, index: number, shareSettings: Partial<ShareSetting> = {}) {
openSharingPanel(fileName)

cy.intercept({ times: 1, method: 'PUT', url: '**/apps/files_sharing/api/v1/shares/*' }).as('updateShare')

cy.get('#app-sidebar-vue').within(() => {
cy.get('[data-cy-files-sharing-share-actions]').eq(index).click()
cy.get('[data-cy-files-sharing-share-permissions-bundle="custom"]').click()
Expand Down Expand Up @@ -82,6 +84,8 @@ export function updateShare(fileName: string, index: number, shareSettings: Part
}

cy.get('[data-cy-files-sharing-share-editor-action="save"]').click({ scrollBehavior: 'nearest' })

cy.wait('@updateShare')
})
}

Expand Down
1 change: 0 additions & 1 deletion cypress/e2e/files_versions/filesVersionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/
/* eslint-disable jsdoc/require-jsdoc */
import type { User } from '@nextcloud/cypress'
import path from 'path'
import { createShare, type ShareSetting } from '../files_sharing/filesSharingUtils'

export const uploadThreeVersions = (user: User, fileName: string) => {
Expand Down
4 changes: 2 additions & 2 deletions dist/files_versions-files_versions.js

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions lib/private/Share20/ShareAttributes.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public function __construct() {
/**
* @inheritdoc
*/
public function setAttribute($scope, $key, $value) {
public function setAttribute(string $scope, string $key, mixed $value): IAttributes {
if (!\array_key_exists($scope, $this->attributes)) {
$this->attributes[$scope] = [];
}
Expand All @@ -31,7 +31,7 @@ public function setAttribute($scope, $key, $value) {
/**
* @inheritdoc
*/
public function getAttribute($scope, $key) {
public function getAttribute(string $scope, string $key): mixed {
if (\array_key_exists($scope, $this->attributes) &&
\array_key_exists($key, $this->attributes[$scope])) {
return $this->attributes[$scope][$key];
Expand All @@ -42,14 +42,14 @@ public function getAttribute($scope, $key) {
/**
* @inheritdoc
*/
public function toArray() {
public function toArray(): array {
$result = [];
foreach ($this->attributes as $scope => $keys) {
foreach ($keys as $key => $value) {
$result[] = [
"scope" => $scope,
"key" => $key,
"value" => $value
"value" => $value,
];
}
}
Expand Down
13 changes: 7 additions & 6 deletions lib/public/Share/IAttributes.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,40 +13,41 @@
*/
interface IAttributes {
/**
* Sets an attribute enabled/disabled. If the key did not exist before it will be created.
* Sets an attribute. If the key did not exist before it will be created.
*
* @param string $scope scope
* @param string $key key
* @param bool|string|array|null $value value
* @return IAttributes The modified object
* @since 25.0.0
*/
public function setAttribute($scope, $key, $value);
public function setAttribute(string $scope, string $key, mixed $value): IAttributes;

/**
* Returns if attribute is enabled/disabled for given scope id and key.
* Returns the attribute for given scope id and key.
* If attribute does not exist, returns null
*
* @param string $scope scope
* @param string $key key
* @return bool|string|array|null
* @since 25.0.0
*/
public function getAttribute($scope, $key);
public function getAttribute(string $scope, string $key): mixed;

/**
* Formats the IAttributes object to array with the following format:
* [
* 0 => [
* "scope" => <string>,
* "key" => <string>,
* "enabled" => <bool>
* "value" => <bool|string|array|null>,
* ],
* ...
* ]
*
* @return array formatted IAttributes
* @since 25.0.0
* @since 30.0.0, `enabled` was renamed to `value`
*/
public function toArray();
public function toArray(): array;
}

0 comments on commit b55bc69

Please sign in to comment.