Skip to content

Commit 4292ff6

Browse files
committed
fix(shareApiController): avoid duplicated expiryDate operations
`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>
1 parent abd24c8 commit 4292ff6

5 files changed

Lines changed: 851 additions & 29 deletions

File tree

apps/files_sharing/lib/Controller/ShareAPIController.php

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,10 @@ public function __construct(
110110
IUserManager $userManager,
111111
IRootFolder $rootFolder,
112112
IURLGenerator $urlGenerator,
113-
string $userId = null,
114113
IL10N $l10n,
115114
IConfig $config,
116115
IAppManager $appManager,
117-
IServerContainer $serverContainer,
116+
ContainerInterface $serverContainer,
118117
IUserStatusManager $userStatusManager,
119118
IPreview $previewManager,
120119
private IDateTimeZone $dateTimeZone,
@@ -647,6 +646,16 @@ public function createShare(
647646
$share = $this->setShareAttributes($share, $attributes);
648647
}
649648

649+
//Expire date
650+
if ($expireDate !== '') {
651+
try {
652+
$expireDate = $this->parseDate($expireDate);
653+
$share->setExpirationDate($expireDate);
654+
} catch (\Exception $e) {
655+
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
656+
}
657+
}
658+
650659
$share->setSharedBy($this->currentUser);
651660
$this->checkInheritedAttributes($share);
652661

@@ -733,15 +742,6 @@ public function createShare(
733742

734743
$share->setSharedWith($shareWith);
735744
$share->setPermissions($permissions);
736-
if ($expireDate !== '') {
737-
try {
738-
$expireDate = $this->parseDate($expireDate);
739-
$share->setExpirationDate($expireDate);
740-
} catch (\Exception $e) {
741-
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
742-
}
743-
}
744-
745745
$share->setSharedWithDisplayName($this->getCachedFederatedDisplayName($shareWith, false));
746746
} elseif ($shareType === IShare::TYPE_REMOTE_GROUP) {
747747
if (!$this->shareManager->outgoingServer2ServerGroupSharesAllowed()) {
@@ -754,14 +754,6 @@ public function createShare(
754754

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

800-
//Expire date
801-
if ($expireDate !== '') {
802-
try {
803-
$expireDate = $this->parseDate($expireDate);
804-
$share->setExpirationDate($expireDate);
805-
} catch (\Exception $e) {
806-
throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
807-
}
808-
}
809-
810792
$share->setShareType($shareType);
811793

812794
if ($note !== '') {
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/**
2+
* @copyright 2014 Vincent Petry <pvince81@owncloud.com>
3+
*
4+
* @author Vincent Petry <vincent@nextcloud.com>
5+
*
6+
* @license AGPL-3.0-or-later
7+
*
8+
* This program is free software: you can redistribute it and/or modify
9+
* it under the terms of the GNU Affero General Public License as
10+
* published by the Free Software Foundation, either version 3 of the
11+
* License, or (at your option) any later version.
12+
*
13+
* This program is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+
* GNU Affero General Public License for more details.
17+
*
18+
* You should have received a copy of the GNU Affero General Public License
19+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
20+
*
21+
*/
22+
23+
describe('OCA.Trashbin.App tests', function() {
24+
var App = OCA.Trashbin.App;
25+
26+
beforeEach(function() {
27+
$('#testArea').append(
28+
'<div id="app-navigation">' +
29+
'<ul><li data-id="files"><a>Files</a></li>' +
30+
'<li data-id="trashbin"><a>Trashbin</a></li>' +
31+
'</div>' +
32+
'<div id="app-content">' +
33+
'<div id="app-content-files" class="hidden">' +
34+
'</div>' +
35+
'<div id="app-content-trashbin" class="hidden">' +
36+
'</div>' +
37+
'</div>' +
38+
'</div>'
39+
);
40+
App.initialize($('#app-content-trashbin'));
41+
});
42+
afterEach(function() {
43+
App._initialized = false;
44+
App.fileList = null;
45+
});
46+
47+
describe('initialization', function() {
48+
it('creates a custom filelist instance', function() {
49+
App.initialize();
50+
expect(App.fileList).toBeDefined();
51+
expect(App.fileList.$el.is('#app-content-trashbin')).toEqual(true);
52+
});
53+
54+
it('registers custom file actions', function() {
55+
var fileActions;
56+
App.initialize();
57+
58+
fileActions = App.fileList.fileActions;
59+
60+
expect(fileActions.actions.all).toBeDefined();
61+
expect(fileActions.actions.all.Restore).toBeDefined();
62+
expect(fileActions.actions.all.Delete).toBeDefined();
63+
64+
expect(fileActions.actions.all.Rename).not.toBeDefined();
65+
expect(fileActions.actions.all.Download).not.toBeDefined();
66+
67+
expect(fileActions.defaults.dir).toEqual('Open');
68+
});
69+
});
70+
});

0 commit comments

Comments
 (0)