Skip to content

Commit

Permalink
fix(shareApiController): avoid duplicated expiryDate operations
Browse files Browse the repository at this point in the history
`expireDate` can be set once and used anywhere needed, the current implementation,

duplicates this behavior which leads to `parseDate` receiving an a date object it

parsed and returend earlier in the createShare method.

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
  • Loading branch information
Fenn-CS committed Apr 18, 2024
1 parent abd24c8 commit 4292ff6
Show file tree
Hide file tree
Showing 5 changed files with 851 additions and 29 deletions.
40 changes: 11 additions & 29 deletions apps/files_sharing/lib/Controller/ShareAPIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,10 @@ public function __construct(
IUserManager $userManager,
IRootFolder $rootFolder,
IURLGenerator $urlGenerator,
string $userId = null,
IL10N $l10n,
IConfig $config,
IAppManager $appManager,
IServerContainer $serverContainer,
ContainerInterface $serverContainer,
IUserStatusManager $userStatusManager,
IPreview $previewManager,
private IDateTimeZone $dateTimeZone,
Expand Down Expand Up @@ -647,6 +646,16 @@ public function createShare(
$share = $this->setShareAttributes($share, $attributes);
}

//Expire date
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));

Check failure

Code scanning / Psalm

UndefinedThisPropertyFetch Error

Instance property OCA\Files_Sharing\Controller\ShareAPIController::$l is not defined
}
}

$share->setSharedBy($this->currentUser);
$this->checkInheritedAttributes($share);

Expand Down Expand Up @@ -733,15 +742,6 @@ public function createShare(

$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}

$share->setSharedWithDisplayName($this->getCachedFederatedDisplayName($shareWith, false));
} elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) {
Expand All @@ -754,14 +754,6 @@ public function createShare(

$share->setSharedWith($shareWith);
$share->setPermissions($permissions);
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}
} elseif ($shareType === IShare::TYPE_CIRCLE) {
if (!\OC::$server->getAppManager()->isEnabledForUser('circles') || !class_exists('\OCA\Circles\ShareByCircleProvider')) {
throw new OCSNotFoundException($this->l->t('You cannot share to a Circle if the app is not enabled'));
Expand Down Expand Up @@ -797,16 +789,6 @@ public function createShare(
throw new OCSBadRequestException($this->l->t('Unknown share type'));
}

//Expire date
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
$share->setExpirationDate($expireDate);
} catch (\Exception $e) {
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
}
}

$share->setShareType($shareType);

if ($note !== '') {
Expand Down
70 changes: 70 additions & 0 deletions apps/files_trashbin/tests/js/appSpec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
* @copyright 2014 Vincent Petry <pvince81@owncloud.com>
*
* @author Vincent Petry <vincent@nextcloud.com>
*
* @license AGPL-3.0-or-later
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

describe('OCA.Trashbin.App tests', function() {
var App = OCA.Trashbin.App;

beforeEach(function() {
$('#testArea').append(
'<div id="app-navigation">' +
'<ul><li data-id="files"><a>Files</a></li>' +
'<li data-id="trashbin"><a>Trashbin</a></li>' +
'</div>' +
'<div id="app-content">' +
'<div id="app-content-files" class="hidden">' +
'</div>' +
'<div id="app-content-trashbin" class="hidden">' +
'</div>' +
'</div>' +
'</div>'
);
App.initialize($('#app-content-trashbin'));
});
afterEach(function() {
App._initialized = false;
App.fileList = null;
});

describe('initialization', function() {
it('creates a custom filelist instance', function() {
App.initialize();
expect(App.fileList).toBeDefined();
expect(App.fileList.$el.is('#app-content-trashbin')).toEqual(true);
});

it('registers custom file actions', function() {
var fileActions;
App.initialize();

fileActions = App.fileList.fileActions;

expect(fileActions.actions.all).toBeDefined();
expect(fileActions.actions.all.Restore).toBeDefined();
expect(fileActions.actions.all.Delete).toBeDefined();

expect(fileActions.actions.all.Rename).not.toBeDefined();
expect(fileActions.actions.all.Download).not.toBeDefined();

expect(fileActions.defaults.dir).toEqual('Open');
});
});
});
Loading

0 comments on commit 4292ff6

Please sign in to comment.