Skip to content

Only set the default expiration date on share creation#22192

Merged
DeepDiver1975 merged 3 commits intomasterfrom
fix_19685
Feb 9, 2016
Merged

Only set the default expiration date on share creation#22192
DeepDiver1975 merged 3 commits intomasterfrom
fix_19685

Conversation

@rullzer
Copy link
Copy Markdown
Contributor

@rullzer rullzer commented Feb 8, 2016

Fixes #19685

The default expiration date should only be set when we create a new
share. So if a share is created and the expiration date is unset. And
after that the password is updated the expiration date should remain
unset.

CC: @MorrisJobke @PVince81 @nickvergessen @schiesbn

@MorrisJobke
Copy link
Copy Markdown
Contributor

Tested and works 👍

@MorrisJobke
Copy link
Copy Markdown
Contributor

Code also looks good :)

Comment thread lib/private/share20/share.php Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems like a violation of the PHPDoc as that seems to document only string?

2016-02-08_11-40-59

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

mmmm suggestions for an exception to use here then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok fixed... just added | null in the phpdoc...

Comment thread lib/public/share/ishare.php Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It always feels bad and it caused a lot of trouble in the past. Can't we handle this via exception?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure... which one do you suggest...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something like AttributeNotAvailableException ? cc @DeepDiver1975

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

mmm we already have such a zoo of exceptions. i'd rather not have yet another custom one here... The exception name is fine from my POV but for something such generic it shouldnot be in specifc to the share...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe UnexceptedValueException?

Sounds good.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@MorrisJobke
Copy link
Copy Markdown
Contributor

Tested and works 👍

@MorrisJobke
Copy link
Copy Markdown
Contributor

@rullzer Please check out the failing integration tests ;)

@rullzer
Copy link
Copy Markdown
Contributor Author

rullzer commented Feb 8, 2016

Should be fixed now...

Comment thread lib/public/share/ishare.php Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if either ? This english not is :yoda:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have no idea what I wanted to write there...

Fixes #19685

The default expiration date should only be set when we create a new
share. So if a share is created and the expiration date is unset. And
after that the password is updated the expiration date should remain
unset.
@MorrisJobke
Copy link
Copy Markdown
Contributor

Tested and still works 👍

@PVince81
Copy link
Copy Markdown
Contributor

PVince81 commented Feb 9, 2016

👍

DeepDiver1975 added a commit that referenced this pull request Feb 9, 2016
Only set the default expiration date on share creation
@DeepDiver1975 DeepDiver1975 merged commit 29f6f45 into master Feb 9, 2016
@DeepDiver1975 DeepDiver1975 deleted the fix_19685 branch February 9, 2016 22:38
@lock
Copy link
Copy Markdown

lock Bot commented Aug 7, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Aug 7, 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.

Changing sharing password activates default sharing expiration date

6 participants