Skip to content

Inherited Shares - #15952

Merged
rullzer merged 4 commits into
masterfrom
feature/noid/inherited-shares
Dec 3, 2019
Merged

Inherited Shares#15952
rullzer merged 4 commits into
masterfrom
feature/noid/inherited-shares

Conversation

@ArtificialOwl

@ArtificialOwlArtificialOwl commented Jun 13, 2019•
edited
Loading

Copy link
Copy Markdown
Member

add endpoint to retrieve all inherited shares

WIP:

  • code
  • test compliance
  • new tests ?

kesselb
kesselb reviewed Jun 14, 2019
View reviewed changes
Comment threadapps/files_sharing/lib/Controller/ShareAPIController.php Outdated
kesselb
kesselb reviewed Jun 14, 2019
View reviewed changes
Comment threadapps/files_sharing/lib/Controller/ShareAPIController.php Outdated
@ArtificialOwl

ArtificialOwl commented Jun 14, 2019

Copy link
Copy Markdown
MemberAuthor

The name will be getInheritedShares()

@kesselb

kesselb commented Jun 14, 2019

Copy link
Copy Markdown
Contributor

The name will be getInheritedShares()

Very nice 👏

@ArtificialOwl

ArtificialOwl commented Jun 15, 2019

Copy link
Copy Markdown
MemberAuthor

Edit

  • getSharedWithMe now returns an array.
  • getSharesInDir now returns an array
  • getShares is now splited in getFormattedShares(): string[] and getSharesFromNode(): Share[] to get the usual result. The method still returns DataResponse.

New

  • getFormattedShares returns an array. It gets the shares using getSharesFromNode, parse the result and returns the list of shares in a formatted array (based on resharing rights). (the code comes from old getShares)
  • getSharesFromNode get IShare[] from all providers. It replaces the old code from getShares in a foreach with the list of providers.
  • hasSharingRights/confirmSharingRights check if [current] user have resharing rights on the Node.
  • getInheritedShares (main purpose of the PR): check if current user have resharing rights. if so, will get all shares inherited from parent folder, using the owner path to node

@ArtificialOwl
ArtificialOwlforce-pushed the feature/noid/inherited-shares branch from e62ecac to 53684d8CompareJune 15, 2019 10:07
@ArtificialOwlArtificialOwl added the 2. developing Work in progress label Jun 15, 2019
@ArtificialOwlArtificialOwl added this to the Nextcloud 17 milestone Jun 15, 2019
@ArtificialOwlArtificialOwl added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 17, 2019
@ArtificialOwl
ArtificialOwlforce-pushed the feature/noid/inherited-shares branch from efb7568 to e1a34bbCompareJune 17, 2019 11:58
juliusknorr
juliusknorr reviewed Jun 19, 2019
View reviewed changes
Comment threadapps/files_sharing/lib/Exceptions/SharingRightsException.php Outdated
@ArtificialOwl
ArtificialOwlforce-pushed the feature/noid/inherited-shares branch from 4b7629e to a95eb09CompareJune 25, 2019 20:35
@MorrisJobkeMorrisJobke mentioned this pull request Jul 3, 2019
Closed
@MorrisJobkeMorrisJobke mentioned this pull request Jul 15, 2019
Merged
28 tasks
@MorrisJobkeMorrisJobke changed the title [nc17] Inherited SharesInherited SharesJul 15, 2019
@MorrisJobke

This comment has been minimized.

Sign in to view
@wiswedel

wiswedel commented Sep 25, 2019

Copy link
Copy Markdown
Contributor

waiting for @skjnldsv to confirm functionality after implementing the front-end (https://github.com/nextcloud-gmbh/professional_services/issues/139#issuecomment-534655263)

@skjnldsv

skjnldsv commented Oct 10, 2019

Copy link
Copy Markdown
Member

@daita please rebase :)

@ArtificialOwl
ArtificialOwlforce-pushed the feature/noid/inherited-shares branch from a95eb09 to 9c468a9CompareOctober 10, 2019 12:51
@ArtificialOwl

ArtificialOwl commented Oct 10, 2019

Copy link
Copy Markdown
MemberAuthor

@skjnldsv should be good now

@skjnldsv

skjnldsv commented Oct 10, 2019

Copy link
Copy Markdown
Member

@skjnldsv should be good now

You're awesome :)

@skjnldsv
skjnldsvforce-pushed the feature/noid/inherited-shares branch from 1977ad6 to 19f3360CompareOctober 11, 2019 08:01
skjnldsv
skjnldsv reviewed Oct 11, 2019
View reviewed changes
Comment threadapps/files_sharing/lib/Controller/ShareAPIController.php Outdated
@ArtificialOwl

ArtificialOwl commented Nov 28, 2019

Copy link
Copy Markdown
MemberAuthor
* Main row text should be either only "Others with access", or "4 others with access" with the number prepended

Keeping in mind that on first load of the panel, we have no idea how many inherited shared of even if there is any. However, the text can be changed after the request on the back-end for inherited shares to include a count/number

@rullzer

rullzer commented Nov 28, 2019

Copy link
Copy Markdown
Member

Lets be sure to get this in so we can polish later.

@rullzer
rullzerforce-pushed the feature/noid/inherited-shares branch 2 times, most recently from bc44d32 to 06256edCompareNovember 28, 2019 19:36
@rullzer

rullzer commented Nov 28, 2019

Copy link
Copy Markdown
Member

/compile amend /

@skjnldsv

skjnldsv commented Dec 2, 2019•
edited
Loading

Copy link
Copy Markdown
Member

auto-approval

@rullzer

rullzer commented Dec 2, 2019

Copy link
Copy Markdown
Member

@daita you broke the unit tests...

@jancborchardt

jancborchardt commented Dec 2, 2019

Copy link
Copy Markdown
Member

Lets be sure to get this in so we can polish later.

@skjnldsv are you doing a follow-up pull request to address the design feedback in #15952 (comment) ? :)

@skjnldsv

skjnldsv commented Dec 2, 2019

Copy link
Copy Markdown
Member

@skjnldsv are you doing a follow-up pull request to address the design feedback in #15952 (comment) ? :)

Yes :)

@ArtificialOwl

ArtificialOwl commented Dec 2, 2019

Copy link
Copy Markdown
MemberAuthor

@rullzer might need to restart the tests, it is working locally, and the error on CI is Row for file welcome.txt in file list could not be found after 100 seconds (NoSuchElementException)

@ArtificialOwl
ArtificialOwlforce-pushed the feature/noid/inherited-shares branch 2 times, most recently from 9bed2ea to 74e7ae6CompareDecember 2, 2019 18:20
@rullzer
rullzerforce-pushed the feature/noid/inherited-shares branch from 74e7ae6 to 06256edCompareDecember 2, 2019 18:24
@ArtificialOwl

ArtificialOwl commented Dec 2, 2019

Copy link
Copy Markdown
MemberAuthor

So, unit tests were fixed in 7fa203f

  • shareProviderExists() in the mock will returns true,
  • getFormattedShares only returns unique Shares (by Id).
  • meaning that one test that was expecting 3 duplicate shares have been replaced to expect only one share

ArtificialOwland others added 4 commits December 3, 2019 08:10
@ArtificialOwl@rullzer
Inherited Shares
5794f14
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
working on users with resharing rights
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
-getLogger()
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
cleaning
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
fix type
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
Update SharingRightsException.php
@ArtificialOwl@rullzer
integration tests
ccf7d87
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
add tests on non-owner pov
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
duplicate
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
small fixes
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
removed tags
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@skjnldsv@rullzer
Inherited share UI
3a86b99
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@ArtificialOwl@rullzer
fixing tests + avoid duplicate in getFormattedShares()
f2c7ca4
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
@rullzer
rullzerforce-pushed the feature/noid/inherited-shares branch from 7fa203f to f2c7ca4CompareDecember 3, 2019 07:13
@skjnldsvskjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Dec 3, 2019
@rullzer
rullzer merged commit 109aee5 into masterDec 3, 2019
@rullzer
rullzer deleted the feature/noid/inherited-shares branch December 3, 2019 09:13
@skjnldsv

skjnldsv commented Dec 3, 2019

Copy link
Copy Markdown
Member

🎉

@rullzer

rullzer commented Dec 3, 2019

Copy link
Copy Markdown
Member

@skjnldsv can you create an issue/followup with the comments from @jancborchardt ?

@skjnldsv

skjnldsv commented Dec 3, 2019

Copy link
Copy Markdown
Member

yes :)

@skjnldsvskjnldsv mentioned this pull request Dec 3, 2019
Closed
5 tasks
@skjnldsvskjnldsv mentioned this pull request May 5, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kesselbkesselbkesselb left review comments
@rullzerrullzerrullzer approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishfeature: sharinghigh

Projects

None yet

Milestone

Nextcloud 18.0.0

Development

Successfully merging this pull request may close these issues.

9 participants

@ArtificialOwl@kesselb@MorrisJobke@wiswedel@skjnldsv@rullzer@jancborchardt@ChristophWurst@juliusknorr