Skip to content

Implement expiration date for federated shares - #25320

Merged
juliusknorr merged 4 commits into
masterfrom
bugfix/noid/save-fed-share-expiration
Apr 15, 2021
Merged

Implement expiration date for federated shares#25320
juliusknorr merged 4 commits into
masterfrom
bugfix/noid/save-fed-share-expiration

Conversation

@PVince81

@PVince81PVince81 commented Jan 25, 2021•
edited
Loading

Copy link
Copy Markdown
Member

Fixes issue that the UI checkbox for expiration date was not being saved for federated share.

Todos

  • Bring back expiration date field (revert Hide expiration date field for remote shares #25933)
  • Setting expiration date is saved in DB
  • Clearing expiration date is saved in DB
  • Refresh the page and open the share panel: expiration date is sameas DB
  • The share actually expires (tested by manually setting a past date in DB)
  • When expiring, should notify the remote servers of the disappearance => not needed, they find out during next access
  • Add to NextCloud properly saves expiration date from link (can't test due to another bug)
  • Default expiration date applied to remote share
  • Remote group thing ??
  • New capability for clients to recognize this support: had to add a new one, there was a bogus old one there...
  • Extend unit tests
  • Add separate setting for expiration date of federated shares instead of reusing the one from internal shares ?

Notes

I do wonder if that checkbox was added by mistake in the UI through recent refactorings.
The more I dig, the more it looks like this feature was never implemented.
We removed it here #25933 and will add it back in this PR.

@rullzer

@PVince81PVince81 added this to the Nextcloud 21 milestone Jan 25, 2021
@PVince81PVince81 self-assigned this Jan 25, 2021
@PVince81
PVince81force-pushed the bugfix/noid/save-fed-share-expiration branch from 59c1599 to 72e2c0aCompareJanuary 25, 2021 14:46
This was referenced Jan 27, 2021
Merged
Merged
@PVince81PVince81 mentioned this pull request Mar 4, 2021
Merged
@PVince81
PVince81force-pushed the bugfix/noid/save-fed-share-expiration branch 2 times, most recently from 4ffd5e5 to aa75e33CompareMarch 16, 2021 10:09
@PVince81

PVince81 commented Mar 16, 2021

Copy link
Copy Markdown
MemberAuthor

been lucky so far, the missing bit was reading expiration date from the DB.
after that, expiration already worked automagically

@PVince81

PVince81 commented Mar 16, 2021

Copy link
Copy Markdown
MemberAuthor

confusing report...
image

I did run cs:fix locally on related folders but nothing got fixed. I'll try a full run then.

@PVince81PVince81 mentioned this pull request Mar 16, 2021
Merged
@PVince81

PVince81 commented Mar 16, 2021

Copy link
Copy Markdown
MemberAuthor

attempt at fixing CS on master, as a result of running composer cs:fix: #26143

@PVince81
PVince81force-pushed the bugfix/noid/save-fed-share-expiration branch from 16691c6 to cac7062CompareMarch 16, 2021 10:47
@PVince81

PVince81 commented Mar 16, 2021

Copy link
Copy Markdown
MemberAuthor

I've also tested remote group share and made adjustment, it works and expires properly too

@PVince81
PVince81force-pushed the bugfix/noid/save-fed-share-expiration branch from cac7062 to d872821CompareMarch 16, 2021 13:37
@PVince81PVince81 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 16, 2021
@PVince81
PVince81 marked this pull request as ready for review March 16, 2021 13:39
@PVince81

PVince81 commented Mar 16, 2021

Copy link
Copy Markdown
MemberAuthor

Implementation is done, here are topics to discuss before I move forward with unit tests:

  1. Do we want a separate setting for the default/enforced expiration of shares ?

  2. Should we delete the bogus "federation.expire_data" capability ? It was there before, stating that expiration date works for federation when it doesn't.

@PVince81

PVince81 commented Mar 16, 2021•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

@PVince81
PVince81force-pushed the bugfix/noid/save-fed-share-expiration branch from 53ea4a8 to 553afe1CompareMarch 26, 2021 12:07
@PVince81

PVince81 commented Mar 26, 2021

Copy link
Copy Markdown
MemberAuthor

rebased for CI

@skjnldsv

skjnldsv commented Mar 26, 2021

Copy link
Copy Markdown
Member

failure unrelated

@PVince81

PVince81 commented Apr 12, 2021

Copy link
Copy Markdown
MemberAuthor

rebased and recompiled

@PVince81
PVince81force-pushed the bugfix/noid/save-fed-share-expiration branch from 553afe1 to eb5aefeCompareApril 12, 2021 08:10
@PVince81PVince81 mentioned this pull request Apr 12, 2021
Closed
@PVince81

PVince81 commented Apr 12, 2021

Copy link
Copy Markdown
MemberAuthor

the random failure was supposed to be fixed but is still happening: #26314

rullzer
rullzer approved these changes Apr 12, 2021
View reviewed changes

@rullzerrullzer left a comment

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.

Didn't fully test but looks good

@PVince81

PVince81 commented Apr 14, 2021

Copy link
Copy Markdown
MemberAuthor

/compile amend /

@PVince81PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 14, 2021
@PVince81
Implement expiration date for federated shares
8680baf
Add expiration date field in UI.
Save expiration date when creating or updating federated share.
Read expiration date from DB in federated share provider.
Applies to both federated user and group shares.
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81
Fix expire date capability for federated shares
b0aaafe
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81
Unit tests for federated share expiration date
2650da7
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81
Separate settings for remote share expiration
af61486
Added separate settings for default and enforced expiration date for
remote shares.
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81
PVince81force-pushed the bugfix/noid/save-fed-share-expiration branch from eb5aefe to af61486CompareApril 15, 2021 08:06
@PVince81

PVince81 commented Apr 15, 2021

Copy link
Copy Markdown
MemberAuthor

manual rebase + recompile

@juliusknorr
juliusknorr merged commit 1c35b38 into masterApr 15, 2021
@juliusknorr
juliusknorr deleted the bugfix/noid/save-fed-share-expiration branch April 15, 2021 13:06
LukasReschke added a commit to nextcloud/documentation that referenced this pull request May 11, 2021
@LukasReschke
Document lack of expiration propagation
b95d2e6
Ref nextcloud/server#25320 for Nextcloud 22
@LukasReschkeLukasReschke mentioned this pull request May 11, 2021
Merged
backportbot-nextcloudBot pushed a commit to nextcloud/documentation that referenced this pull request May 11, 2021
@LukasReschke@backportbot
Document lack of expiration propagation
99bd4c9
Ref nextcloud/server#25320 for Nextcloud 22
backportbot-nextcloudBot pushed a commit to nextcloud/documentation that referenced this pull request May 11, 2021
@LukasReschke@backportbot
Document lack of expiration propagation
d4c2dfc
Ref nextcloud/server#25320 for Nextcloud 22
backportbot-nextcloudBot pushed a commit to nextcloud/documentation that referenced this pull request May 11, 2021
@LukasReschke@backportbot
Document lack of expiration propagation
8ec8bfe
Ref nextcloud/server#25320 for Nextcloud 22
This was referenced Sep 30, 2021
Closed
Closed
Closed
@tobiasKaminskytobiasKaminsky mentioned this pull request Oct 28, 2021
Closed
3 tasks
@tfliddtflidd mentioned this pull request Jul 10, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzerrullzerrullzer approved these changes
@LukasReschkeLukasReschkeLukasReschke approved these changes
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@icewind1991icewind1991Awaiting requested review from icewind1991
@nickvergessennickvergessenAwaiting requested review from nickvergessen

Assignees

@PVince81PVince81

Labels

4. to releaseReady to be released and/or waiting for tests to finishenhancementfeature: federation

Projects

None yet

Milestone

Nextcloud 22

Development

Successfully merging this pull request may close these issues.

6 participants

@PVince81@skjnldsv@rullzer@LukasReschke@icewind1991@juliusknorr