Skip to content

Conversation

@adduxa
Copy link
Contributor

@adduxa adduxa commented Feb 5, 2019

Problem

Disabling expiration date for the link share leads to creation of a new link share.

Reason

Since 1bd6d39 setExpirationDate() accepts not only expireDate, but also shareId:

setExpirationDate: function(expireDate, shareId) {

It was applied at
self.setExpirationDate(expireDate, shareId);
but not at
this.setExpirationDate('');

So it ends up with creating a new link share, and not editing the existing one instead.
call = this.addShare(attributes, options);

Solution

Pass correct shareId to setExpirationDate()

Pass shareId to setExpirationDate()
@kesselb kesselb requested review from ChristophWurst and skjnldsv and removed request for ChristophWurst February 5, 2019 20:42
@kesselb kesselb added bug 3. to review Waiting for reviews labels Feb 5, 2019
@kesselb kesselb added this to the Nextcloud 16 milestone Feb 5, 2019
Signed-off-by: Morris Jobke <[email protected]>
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works. 👍 I also checked in the compiled assets because that is what we are doing here. It's not obvious to new contributors, so don't worry. We are currently about to improve our on boarding here 😃

@MorrisJobke
Copy link
Member

We also use "signed-off-by" messages inside the commit body to indicate that the developer certificate of origin is respected. You can read more about this in https://github.com/nextcloud/server/blob/master/.github/CONTRIBUTING.md#sign-your-work and what this means and how to apply it. Don't be scare it sounds more than it actually is.

And welcome aboard 🚀

@MorrisJobke
Copy link
Member

/backport to stable15

@adduxa
Copy link
Contributor Author

adduxa commented Feb 8, 2019

@MorrisJobke thanks for the clarification

Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

Nice explanation, thanks :-)

@danxuliu
Copy link
Member

@MorrisJobke Wait for the "Signed-off-by"? Squash both commits? Merge as is?

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 13, 2019
@MorrisJobke MorrisJobke merged commit 34630fb into nextcloud:master Feb 13, 2019
@welcome
Copy link

welcome bot commented Feb 13, 2019

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
Most developers hang out on IRC. So join #nextcloud-dev on Freenode for a chat!

@backportbot-nextcloud
Copy link

The backport to stable15 failed. Please do this backport manually.

@MorrisJobke
Copy link
Member

Backport is in #14212

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants