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

Discard expiration date from result for non-link shares #19122

Merged
merged 1 commit into from
Sep 21, 2015

Conversation

PVince81
Copy link
Contributor

Fixes #11396.

Note that this is only a quickfix that is backportable.

A clean fix should be done separately that prevents setting expirationDate for non-link shares in the first place and also repair the database to discard the entries that have it already. But as this is a bigger change, I chose to do a small fix first to make it backportable.

Please review @schiesbn @nickvergessen @rullzer

@PVince81 PVince81 added this to the 8.2-current milestone Sep 17, 2015
@PVince81
Copy link
Contributor Author

Confirmed working by @popov-d #11396 (comment)

@PVince81
Copy link
Contributor Author

We should backport this, please confirm @karlitschek.
Please read the original comment that explains the reasoning.

@karlitschek
Copy link
Contributor

makes sense. please backport 👍

@rullzer
Copy link
Contributor

rullzer commented Sep 17, 2015

I think @butonic won't be happy... see #15459

@rullzer
Copy link
Contributor

rullzer commented Sep 17, 2015

Also, I see that it makes sense to remove it now as there is no way to use it properly.
We might want to bring it back for Sharing 2.0 (but that is of no concern now).

Code looks good (and simple)... so 👍 for that.

But I'm not sure this is the right approach... on the other hand fixing it properly in share.php is probably not going to happen :P

@butonic
Copy link
Member

butonic commented Sep 18, 2015

well ... great ... being able to expire user and group shares was possible in OC5. I thought we considered it a regression that it got removed. We have Users relying on that feature, so I brought it back at least via the ocs api because I expected this to get a proper ui in the new sidebar ... oh well we will just have to use #15459 as a patch and hope user and group share expires will be properly added in the next iteration.

@PVince81
Copy link
Contributor Author

@rullzer @butonic yes I know. But the way it acts now with the messy code is buggy.
Best is to reconsider a cleaner way of introducing this possibility with the new sharing.

@PVince81
Copy link
Contributor Author

@butonic the current OCS API doesn't allow to set expiration date, did you use a patched one ?

@butonic
Copy link
Member

butonic commented Sep 18, 2015

@PVince81 see #15459

@PVince81
Copy link
Contributor Author

Hmmm right, then we might need an alternative fix, maybe we then need to fix this line https://github.com/owncloud/core/blob/master/lib/private/share/share.php#L2214 to make sure the expiration date is a DateTime object and not a String.

@PVince81
Copy link
Contributor Author

@butonic are you ok with still moving forward with this PR ?

I suggest to look into a cleaner solution for user share expiration as part of the share code revamp in 9.0, see #15459 (comment)

@butonic
Copy link
Member

butonic commented Sep 18, 2015

@PVince81 happy to wait for 9.0 if that helps adds it properly.

@nickvergessen
Copy link
Contributor

👍

@PVince81
Copy link
Contributor Author

stable8.1 backport: #19216

DeepDiver1975 added a commit that referenced this pull request Sep 21, 2015
…nkshares

Discard expiration date from result for non-link shares
@DeepDiver1975 DeepDiver1975 merged commit 6006a03 into master Sep 21, 2015
@DeepDiver1975 DeepDiver1975 deleted the discardexpirationdatefornonlinkshares branch September 21, 2015 09:47
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't reshare link with default expiration date
7 participants