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.

Sign up for GitHub

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

Jump to bottom

Add API to undelete delete group shares #9909

Merged
merged 9 commits into from
Jul 5, 2018
Merged

Add API to undelete delete group shares #9909

merged 9 commits into from
Jul 5, 2018

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Jun 19, 2018

When a group share is deleted we keep track of this in the DB.
Right now it is only possible for a recipient to get back the share by
asking the sharer to delete it and to share it again. This doesn't
scale.

This endpoint makes it possible to get back the share.

Signed-off-by: Roeland Jago Douma roeland@famdouma.nl

@rullzer rullzer added this to the Nextcloud 14 milestone Jun 19, 2018
@codecov
Copy link

codecov bot commented Jun 19, 2018 •
edited
Loading

Codecov Report

Merging #9909 into master will decrease coverage by 0.13%.
The diff coverage is 5.51%.

@@             Coverage Diff              @@
##             master    #9909      +/-   ##
============================================
- Coverage     51.42%   51.29%   -0.14%     
+ Complexity    26413    26031     -382     
============================================
  Files          1690     1596      -94     
  Lines         97716    89005    -8711     
  Branches       1292        0    -1292     
============================================
- Hits          50248    45652    -4596     
+ Misses        47468    43353    -4115
Impacted Files Coverage Δ Complexity Δ
...iles_sharing/lib/Controller/ShareAPIController.php 68.03% <ø> (ø) 160 <0> (ø) ⬇️
...iles_sharing/composer/composer/autoload_static.php 0% <ø> (ø) 1 <0> (ø) ⬇️
apps/files_sharing/appinfo/routes.php 0% <0%> (ø) 0 <0> (ø) ⬇️
...aring/lib/Controller/DeletedShareAPIController.php 0% <0%> (ø) 14 <14> (?)
lib/private/Share20/Manager.php 84.84% <0%> (-1.8%) 240 <4> (+4)
...es_sharing/composer/composer/autoload_classmap.php 0% <0%> (ø) 0 <0> (ø) ⬇️
...ederatedfilesharing/lib/FederatedShareProvider.php 66.97% <0%> (-1.44%) 85 <1> (+3)
apps/sharebymail/lib/ShareByMailProvider.php 58.57% <0%> (-0.23%) 86 <1> (+1)
apps/files_sharing/appinfo/app.php 92.3% <100%> (+2.3%) 0 <0> (ø) ⬇️
lib/private/Share20/DefaultShareProvider.php 89.01% <4.54%> (-2.82%) 115 <1> (+1)
... and 161 more

@skjnldsv skjnldsv force-pushed the feature/2192/allow_group_share_undeletion branch from 65b5bd4 to 0861a86 Compare June 20, 2018 17:15
@skjnldsv skjnldsv mentioned this pull request Jun 22, 2018
Closed
2 tasks
@skjnldsv skjnldsv force-pushed the feature/2192/allow_group_share_undeletion branch from b051bee to 13cca8f Compare June 26, 2018 06:21
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 26, 2018
MorrisJobke
MorrisJobke reviewed Jun 26, 2018
View reviewed changes
lib/public/Share/IManager.php Outdated
* @param int $limit The maximum number of shares returned, -1 for all
* @param int $offset
* @return IShare[]
* @since 9.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Let me fix this. ;)

MorrisJobke
MorrisJobke approved these changes Jun 26, 2018
View reviewed changes
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 code looks good 👍

@MorrisJobke
Copy link
Member

@rullzer Could you fix the CI - then this is good to go in.

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 26, 2018
@rullzer
Copy link
Member Author

rullzer commented Jun 26, 2018

I can have a look.
@skjnldsv what did you break ;)

@rullzer rullzer force-pushed the feature/2192/allow_group_share_undeletion branch from ca35f1b to 6bdffa9 Compare June 26, 2018 07:33
@skjnldsv
Copy link
Member

@rullzer 🙈

nickvergessen
nickvergessen reviewed Jun 26, 2018
View reviewed changes
apps/files_sharing/lib/Controller/DeletedShareAPIController.php Outdated
@@ -0,0 +1,144 @@
<?php

namespace OCA\Files_Sharing\Controller;
Copy link
Member

Choose a reason for hiding this comment

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

license missing

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Jun 26, 2018
@rullzer
Copy link
Member Author

rullzer commented Jun 26, 2018

@skjnldsv could you have a look at the failing js unit tests?

@rullzer rullzer force-pushed the feature/2192/allow_group_share_undeletion branch from 6bdffa9 to 874162e Compare June 27, 2018 18:33
@skjnldsv skjnldsv added the 3. to review Waiting for reviews label Jun 28, 2018
@skjnldsv skjnldsv removed the 2. developing Work in progress label Jun 28, 2018
@rullzer
Copy link
Member Author

rullzer commented Jun 28, 2018

@skjnldsv still failing jsunit tests :S

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jul 4, 2018
@skjnldsv
Copy link
Member

skjnldsv commented Jul 4, 2018

@rullzer sorry, missed this
will fix asap

@skjnldsv skjnldsv force-pushed the feature/2192/allow_group_share_undeletion branch from 874162e to 3a9616c Compare July 4, 2018 17:35
@MorrisJobke
Copy link
Member

MorrisJobke commented Jul 5, 2018 •
edited by skjnldsv
Loading

  • JS unit tests do still fail:
PhantomJS 2.1.1 (Linux 0.0.0) OCA.Sharing.Util tests Share action clicking share action opens sidebar and share tab FAILED
	Expected false to equal true.
	apps/files_sharing/tests/js/shareSpec.js:251:50
	TypeError: null is not an object (evaluating 'showDetailsViewStub.getCall(0).args') in apps/files_sharing/tests/js/shareSpec.js (line 252)
	apps/files_sharing/tests/js/shareSpec.js:252:41
PhantomJS 2.1.1 (Linux 0.0.0) OCA.Sharing.Util tests Share action adds share icon after sharing a non-shared file FAILED
	TypeError: undefined is not an object (evaluating 'shareTab._dialog.model') in apps/files_sharing/tests/js/shareSpec.js (line 276)
	apps/files_sharing/tests/js/shareSpec.js:276:20
PhantomJS 2.1.1 (Linux 0.0.0) OCA.Sharing.Util tests Share action updates share icon after updating shares of a file FAILED
	TypeError: undefined is not an object (evaluating 'shareTab._dialog.model') in apps/files_sharing/tests/js/shareSpec.js (line 308)
	apps/files_sharing/tests/js/shareSpec.js:308:20
PhantomJS 2.1.1 (Linux 0.0.0) OCA.Sharing.Util tests Share action removes share icon after removing all shares from a file FAILED
	TypeError: undefined is not an object (evaluating 'shareTab._dialog.model') in apps/files_sharing/tests/js/shareSpec.js (line 340)
	apps/files_sharing/tests/js/shareSpec.js:340:20
PhantomJS 2.1.1 (Linux 0.0.0) OCA.Sharing.Util tests Share action keep share text after updating reshare FAILED
	TypeError: undefined is not an object (evaluating 'shareTab._dialog.model') in apps/files_sharing/tests/js/shareSpec.js (line 367)
	apps/files_sharing/tests/js/shareSpec.js:367:20
PhantomJS 2.1.1 (Linux 0.0.0) OCA.Sharing.Util tests Share action keep share text after unsharing reshare FAILED
	TypeError: undefined is not an object (evaluating 'shareTab._dialog.model') in apps/files_sharing/tests/js/shareSpec.js (line 398)
	apps/files_sharing/tests/js/shareSpec.js:398:20

rullzer and others added 9 commits July 5, 2018 12:43
@rullzer @skjnldsv
Add API to undelete delete group shares
d805959 
When a group share is deleted we keep track of this in the DB.
Right now it is only possible for a recipient to get back the share by
asking the sharer to delete it and to share it again. This doesn't
scale.

This endpoint makes it possible to get back the share.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@skjnldsv
getDeletedSharedWith method
6a05522 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Js magic for deleted shares
3b835d8 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Disable sidebar on deleted share
d86c1e9 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv
Always show menu
33f85f6 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@MorrisJobke @skjnldsv
Fix PHPDoc
ced8789 
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@rullzer @skjnldsv
Fix tests
a1a8456 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer @skjnldsv
Fix exception
b49bc11 
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@skjnldsv
Fixed jsunit
f39dfc7 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the feature/2192/allow_group_share_undeletion branch from 3a9616c to f39dfc7 Compare July 5, 2018 10:44
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Jul 5, 2018
@skjnldsv
Copy link
Member

skjnldsv commented Jul 5, 2018

Failure unrelated:
sh: 1: kill: No such process

@MorrisJobke MorrisJobke merged commit cbfcfb2 into master Jul 5, 2018
@skjnldsv skjnldsv deleted the feature/2192/allow_group_share_undeletion branch July 5, 2018 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickvergessen nickvergessen nickvergessen left review comments

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@schiessle schiessle Awaiting requested review from schiessle

@blizzz blizzz Awaiting requested review from blizzz

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement feature: sharing
Projects
None yet
Milestone
Nextcloud 14
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@rullzer @MorrisJobke @skjnldsv @nickvergessen