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

[stable28] Respect empty expiryDate value in server #45604

Merged
merged 4 commits into from
Jun 13, 2024

Conversation

backportbot[bot]
Copy link

@backportbot backportbot bot commented May 31, 2024

Backport of #44485 via #45483

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 requested review from Antreesy and Fenn-CS May 31, 2024 12:57
@backportbot backportbot bot added this to the Nextcloud 28.0.7 milestone May 31, 2024
@Fenn-CS Fenn-CS changed the title [stable28] [stable29] Respect empty expiryDate value in server [stable28] Respect empty expiryDate value in server May 31, 2024
@Fenn-CS Fenn-CS force-pushed the backport/45483/stable28 branch 2 times, most recently from 88cd70e to 1975c87 Compare May 31, 2024 13:07
@Fenn-CS Fenn-CS marked this pull request as ready for review May 31, 2024 13:07
@Fenn-CS Fenn-CS enabled auto-merge May 31, 2024 13:08
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
lib/private/Share20/Manager.php Fixed Show fixed Hide fixed
@Fenn-CS Fenn-CS force-pushed the backport/45483/stable28 branch 5 times, most recently from a9c5950 to 78587cf Compare May 31, 2024 14:35
@Fenn-CS
Copy link
Contributor

Fenn-CS commented May 31, 2024

Way too many issues came up with back-porting this due to : #44029 being missing, added it.

@Antreesy Antreesy requested review from icewind1991 and removed request for Antreesy May 31, 2024 14:40
@Antreesy
Copy link
Contributor

Dismissing myself due to lack of PHP-knowledge 🦭
I believe Robin could approve that as author

@Fenn-CS Fenn-CS requested a review from solracsf June 5, 2024 06:10
@blizzz blizzz mentioned this pull request Jun 11, 2024
6 tasks
@blizzz

This comment was marked as outdated.

@blizzz
Copy link
Member

blizzz commented Jun 13, 2024

Resolved the conflict in apps/files_sharing/lib/Controller/ShareAPIController.php with this diff:

@@ -647,12 +648,18 @@ class ShareAPIController extends OCSController {
        }
 
        //Expire date
-       if ($expireDate !== '') {
-           try {
-               $expireDateTime = $this->parseDate($expireDate);
-               $share->setExpirationDate($expireDateTime);
-           } catch (\Exception $e) {
-               throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
+       if ($expireDate !== null) {
+           if ($expireDate !== '') {
+               try {
+                   $expireDateTime = $this->parseDate($expireDate);
+                   $share->setExpirationDate($expireDateTime);
+               } catch (\Exception $e) {
+                   throw new OCSNotFoundException($this->l->t('Invalid date, date format must be YYYY-MM-DD'));
+               }
+           } else {
+               // Client sent empty string for expire date.
+               // Set noExpirationDate to true so overwrite is prevented.
+               $share->setNoExpirationDate(true);
            }
        }

The expire date section was not present previously.

@blizzz
Copy link
Member

blizzz commented Jun 13, 2024

cannot judge the cypress failures.

@artonge artonge added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 13, 2024
@blizzz
Copy link
Member

blizzz commented Jun 13, 2024

AssertionError: Timed out retrying after 4000ms: expected '<div.files-list__drag-drop-notice>' to be 'visible'

Keeps failing ^

icewind1991 and others added 4 commits June 13, 2024 18:34
Signed-off-by: Robin Appelman <robin@icewind.nl>
If `expireDate` is an empty string and not `null` then the server should not set a default.

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
- Verify that explicitly sending empty `expireDate` param to server overwrite default

and sets not expiry date, if non is enforced.

- Update tests to avoid converting empty string to date.

Signed-off-by: fenn-cs <fenn25.fn@gmail.com>
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.

6 participants