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

[stable27] fix: avoid douple expireDate parsing #44911

Closed
wants to merge 2 commits into from

Conversation

backportbot[bot]
Copy link

@backportbot backportbot bot commented Apr 18, 2024

Backport of #44838

Warning, This backport's changes differ from the original and might be incomplete ⚠️

Todo

  • Review and resolve any conflicts
  • Amend HEAD commit to remove the line stating to skip CI

Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@backportbot backportbot bot added 3. to review Waiting for reviews feature: sharing labels Apr 18, 2024
@backportbot backportbot bot added this to the Nextcloud 27.1.9 milestone Apr 18, 2024
… logger

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
`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>
@Fenn-CS Fenn-CS marked this pull request as ready for review April 18, 2024 09:55
@@ -138,22 +114,11 @@
IUserStatusManager $userStatusManager,
IPreview $previewManager,
private IDateTimeZone $dateTimeZone,
private LoggerInterface $logger,
?string $userId = null

Check failure

Code scanning / Psalm

DuplicateParam Error

Duplicate param $userId in docblock for OCA\Files_Sharing\Controller\ShareAPIController::__construct
$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
Copy link
Contributor

@Fenn-CS Fenn-CS left a comment

Choose a reason for hiding this comment

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

Some update needs need to made as the backport is not totally graceful

@solracsf solracsf added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 18, 2024
@Altahrim Altahrim mentioned this pull request Apr 18, 2024
4 tasks
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

//Expire date
if ($expireDate !== '') {
try {
$expireDate = $this->parseDate($expireDate);
Copy link
Member

Choose a reason for hiding this comment

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

Needs #44916 otherwise it breaks opencloudmesh, deck and talk

@Fenn-CS
Copy link
Contributor

Fenn-CS commented Apr 20, 2024

Would backport #44912 [Same original PR but from stable28]

@Fenn-CS Fenn-CS closed this Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants