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

Fix sharing expiration date handling and saving #35207

Merged
merged 1 commit into from
Nov 16, 2022
Merged

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 16, 2022

Regression from #33915

Fix #34926

Explanations

  • share.expireDate represent the Database expiration data. Which expects a YYYY-MM-DD format
  • The native DatePicker require a javascript Date as value
  • We need to make sure the data we sent is not UTC-aware.
    new Date('2020-01-01 00:00:00').toISOString()
    > '2019-12-31T23:00:00.000Z'
  • The backend forces a time on the expiration field (for clients I'm assuming), so we also need to make sure to ignore this when parsing the data
    $result['expiration'] = $expiration->format('Y-m-d 00:00:00');
    const regex = /([0-9]{4}-[0-9]{2}-[0-9]{2})/i
    return new Date(date.match(regex)?.pop())

@skjnldsv skjnldsv added this to the Nextcloud 26 milestone Nov 16, 2022
@skjnldsv skjnldsv self-assigned this Nov 16, 2022
@skjnldsv skjnldsv requested review from artonge and szaimen and removed request for a team November 16, 2022 11:21
@skjnldsv skjnldsv changed the title Fix date handling and saving Fix sharing expiration date handling and saving Nov 16, 2022
@skjnldsv
Copy link
Member Author

/backport to stable25

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Nitpicks but fine otherwise :)

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Works in my testing. However here we need to add date +1 since otherwise the default will not work as the current date is already reached.

defaultExpirationDate = new Date()

@szaimen

This comment was marked as resolved.

@skjnldsv

This comment was marked as resolved.

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Member Author

Pushed and ready to go!

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Didnt test again but I trust you that it works now :)

Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

Tested, works 👍

@PVince81 PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 16, 2022
@skjnldsv
Copy link
Member Author

--- Failed scenarios:
512 |  
513 | /drone/src/tests/acceptance/features/app-files-sharing.feature:402

Doesn't seems related, also failed on previous merges to master
https://drone.nextcloud.com/nextcloud/server/25892/58/4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: sharing high regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Expiration date field doesn't show previously set value
4 participants