returns reshares in API #14605

ArtificialOwl · 2019-03-09T10:05:15Z

(#14112) Returns reshares to the API

tobiasKaminsky

Works for me 👍

MorrisJobke · 2019-03-20T09:20:10Z

@daita This breaks the sharing tests.

tobiasKaminsky · 2019-03-20T12:39:33Z

@daita for me it seems as if "path" is wrong:

have a file /foo with
- shared with you by user0
- shared to user1
- shared to user3
be in "/"
query /nc/ocs/v2.php/apps/files_sharing/api/v1/shares?path=%2F&reshares=true&subfiles=true
get

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>200</statuscode>
  <message>OK</message>
 </meta>
 <data>
  <element>
   <id>401</id>
   <share_type>0</share_type>
   <uid_owner>user0</uid_owner>
   <displayname_owner>user0</displayname_owner>
   <permissions>15</permissions>
   <stime>1549621868</stime>
   <parent/>
   <expiration/>
   <token/>
   <uid_file_owner>user0</uid_file_owner>
   <note></note>
   <label/>
   <displayname_file_owner>user0</displayname_file_owner>
   <path>/</path>
   <item_type>folder</item_type>
   <mimetype>httpd/unix-directory</mimetype>
   <storage_id>home::user2</storage_id>
   <storage>28</storage>
   <item_source>75844</item_source>
   <file_source>75844</file_source>
   <file_parent>75843</file_parent>
   <file_target>/foo</file_target>
   <share_with>user1</share_with>
   <share_with_displayname>user1</share_with_displayname>
   <mail_send>0</mail_send>
   <hide_download>0</hide_download>
  </element>
  <element>
   <id>402</id>
   <share_type>0</share_type>
   <uid_owner>user0</uid_owner>
   <displayname_owner>user0</displayname_owner>
   <permissions>31</permissions>
   <stime>1549621887</stime>
   <parent/>
   <expiration/>
   <token/>
   <uid_file_owner>user0</uid_file_owner>
   <note></note>
   <label/>
   <displayname_file_owner>user0</displayname_file_owner>
   <path>/</path>
   <item_type>folder</item_type>
   <mimetype>httpd/unix-directory</mimetype>
   <storage_id>home::user2</storage_id>
   <storage>28</storage>
   <item_source>75844</item_source>
   <file_source>75844</file_source>
   <file_parent>75843</file_parent>
   <file_target>/foo</file_target>
   <share_with>user2</share_with>
   <share_with_displayname>user2</share_with_displayname>
   <mail_send>0</mail_send>
   <hide_download>0</hide_download>
  </element>
  <element>
   <id>403</id>
   <share_type>0</share_type>
   <uid_owner>user0</uid_owner>
   <displayname_owner>user0</displayname_owner>
   <permissions>31</permissions>
   <stime>1549622481</stime>
   <parent/>
   <expiration/>
   <token/>
   <uid_file_owner>user0</uid_file_owner>
   <note></note>
   <label/>
   <displayname_file_owner>user0</displayname_file_owner>
   <path>/</path>
   <item_type>folder</item_type>
   <mimetype>httpd/unix-directory</mimetype>
   <storage_id>home::user2</storage_id>
   <storage>28</storage>
   <item_source>75844</item_source>
   <file_source>75844</file_source>
   <file_parent>75843</file_parent>
   <file_target>/foo</file_target>
   <share_with>user3</share_with>
   <share_with_displayname>user3</share_with_displayname>
   <mail_send>0</mail_send>
   <hide_download>0</hide_download>
  </element>
 </data>
</ocs>

With master (on a file I shared with user admin):

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>200</statuscode>
  <message>OK</message>
 </meta>
 <data>
  <element>
   <id>423</id>
   <share_type>0</share_type>
   <uid_owner>user2</uid_owner>
   <displayname_owner>user2</displayname_owner>
   <permissions>31</permissions>
   <stime>1553085500</stime>
   <parent/>
   <expiration/>
   <token/>
   <uid_file_owner>user2</uid_file_owner>
   <note></note>
   <label/>
   <displayname_file_owner>user2</displayname_file_owner>
   <path>/share</path>
   <item_type>folder</item_type>
   <mimetype>httpd/unix-directory</mimetype>
   <storage_id>home::user2</storage_id>
   <storage>28</storage>
   <item_source>77824</item_source>
   <file_source>77824</file_source>
   <file_parent>75844</file_parent>
   <file_target>/share</file_target>
   <share_with>admin</share_with>
   <share_with_displayname>admin</share_with_displayname>
   <mail_send>0</mail_send>
   <hide_download>0</hide_download>
  </element>
 </data>
</ocs>

tobiasKaminsky · 2019-03-20T13:02:03Z

Another question: should this really return the same user?
I can filter this on client side, but is not useful at all, or?

ArtificialOwl · 2019-03-21T10:36:26Z

@MorrisJobke @ChristophWurst any reason that most of the errors are from the TwoFactorBackupCodes app ?

MorrisJobke · 2019-03-21T12:28:37Z

@MorrisJobke @ChristophWurst any reason that most of the errors are from the TwoFactorBackupCodes app ?

Ignore the risky tests, those are just warnings that there was no check (due to the environment) - the problem here are the failures.

ArtificialOwl · 2019-03-21T15:39:26Z

@MorrisJobke How can I just run one scenario from the tests ?

MorrisJobke · 2019-03-21T15:57:40Z

@MorrisJobke How can I just run one scenario from the tests ?

That should do the trick:

cd tests acceptance
./run.sh features/app-files-sharing-link.feature

ArtificialOwl · 2019-03-30T21:53:33Z

@MorrisJobke could not point the source of the issue but my guess is that a rebase (done) should do the trick

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

ArtificialOwl · 2019-03-31T11:42:02Z

@faily-bot still returns error, but nothing on my own tests, rebasing.

maxence@stealth:~/PhpstormProjects/server$ ./autotest.sh sqlite apps/files_sharing/tests/ApiTest.php 
Using PHP executable /usr/bin/php
Parsing all files in lib/public for the presence of @since or @deprecated on each method...

Using database oc_autotest
Setup environment for sqlite testing on local storage ...
Installing ....
Nextcloud was successfully installed
Testing with sqlite ...
/usr/bin/phpunit --configuration phpunit-autotest.xml --coverage-clover autotest-clover-sqlite.xml --coverage-html coverage-html-sqlite --log-junit autotest-results-sqlite.xml ../apps/files_sharing/tests/ApiTest.php 
PHPUnit 6.5.5 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.2.15-0ubuntu0.18.04.2 with Xdebug 2.6.0
Configuration: /home/maxence/PhpstormProjects/server/tests/phpunit-autotest.xml

...................................                               35 / 35 (100%)

Time: 29.39 seconds, Memory: 56.00MB

OK (35 tests, 76 assertions)

ArtificialOwl · 2019-03-31T13:00:40Z

@MorrisJobke : looks green to me, some hiccup on drone

tobiasKaminsky · 2019-04-01T07:03:26Z

@daita have you fixed
#14605 (comment) and #14605 (comment)?

ArtificialOwl · 2019-04-01T07:27:39Z

The first one is fixed and was the reason of the tests issue.
I don't understand the second one

MorrisJobke · 2019-04-01T09:11:00Z

lib/private/Share20/DefaultShareProvider.php

@@ -609,7 +609,7 @@ public function getSharesInFolder($userId, Folder $node, $reshares) {
 	 */
 	public function getSharesBy($userId, $shareType, $node, $reshares, $limit, $offset) {
 		$qb = $this->dbConn->getQueryBuilder();
-		$qb->select('*')
+		$qb->selectDistinct('*')


cc @rullzer

note that I could swap back to select() as I filters duplicate results

So this can go then? Or should we leave it ?

tobiasKaminsky · 2019-04-01T10:23:19Z

The first one is fixed and was the reason of the tests issue.

👍 working

I don't understand the second one

share a file /A from user admin to user tobi
query http://localhost/nc/ocs/v1.php/apps/files_sharing/api/v1/shares?format=json&reshares=true&subfiles=true&path=%2F
see

 {
        "id": "364",
        "share_type": 0,
        "uid_owner": "admin",
        "displayname_owner": "admin",
        "permissions": 31,
        "stime": 1540475407,
        "parent": null,
        "expiration": null,
        "token": null,
        "uid_file_owner": "admin",
        "note": "",
        "label": null,
        "displayname_file_owner": "admin",
        "path": "\/__sharedByAdmin",
        "item_type": "folder",
        "mimetype": "httpd\/unix-directory",
        "storage_id": "shared::\/__sharedByAdmin",
        "storage": 1,
        "item_source": 72003,
        "file_source": 72003,
        "file_parent": 103,
        "file_target": "\/__sharedByAdmin",
        "share_with": "tobi",
        "share_with_displayname": "Kaminsky Tobi",
        "mail_send": 0,
        "hide_download": 0
      },

This means I cannot simply display all received shares for a given folder, but first have to check if "share_with" is not the username.
As this behaviour changed, I expect that other apps on mobile/web/dav will also need to be changed…

ArtificialOwl · 2019-04-01T11:14:30Z

@tobiasKaminsky I think it is the main idea behind the edit: displaying all shares if you have reshares rights ? or am I missing something ?

tobiasKaminsky · 2019-04-01T11:55:40Z

@tobiasKaminsky I think it is the main idea behind the edit: displaying all shares if you have reshares rights ? or am I missing something ?

But seeing myself as a share is kind of useless, or?

This is how it currently looks like:
File is shared by admin with me (Kaminsky Tobi) and I additionally see myself as a sharee.

In my expectation it should be this way:

with reshare permission see all sharees except your own
- see what other users admin shared to
- see your sharees

So, if admin shared this files also to userA, userB and I shared it to friendA, friendB, I would expect that I get this returend via API:

userA (read only as I am not allowed to edit this)
userB (read only as I am not allowed to edit this)
friendA (I can change this)
friendB (I can change this)

but I do not see myself in this list

ArtificialOwl · 2019-04-01T11:57:41Z

Ok, wasn't clear who was the viewer, let me check what can be done

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

ArtificialOwl · 2019-04-01T12:18:34Z

@tobiasKaminsky can you try now ?

(Also, in my opinion, it should not be filtered by the API but by the client itself)

tobiasKaminsky · 2019-04-02T09:06:28Z

@tobiasKaminsky can you try now ?

(Also, in my opinion, it should not be filtered by the API but by the client itself)

works for me now as expected 🎉

MorrisJobke · 2019-04-02T15:11:59Z

@rullzer Mind to review?

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

rullzer

Let do this then. 🐘 saw @tobiasKaminsky test it and code makes sese

See nextcloud/server#14605 for more information Signed-off-by: Joas Schilling <coding@schilljs.com>

ArtificialOwl requested review from tobiasKaminsky, nickvergessen and rullzer March 9, 2019 10:06

ArtificialOwl added bug 3. to review Waiting for reviews labels Mar 9, 2019

ArtificialOwl added this to the Nextcloud 16 milestone Mar 9, 2019

tobiasKaminsky approved these changes Mar 11, 2019

View reviewed changes

MorrisJobke mentioned this pull request Mar 20, 2019

16.0.0 Beta 1 #14573

Merged

9 tasks

MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 20, 2019

rullzer mentioned this pull request Mar 26, 2019

16.0.0 Beta 2 #14872

Merged

9 tasks

ArtificialOwl force-pushed the bugfix/14112/display-reshares branch from a8c14c3 to 2923887 Compare March 30, 2019 21:51

ArtificialOwl added 3 commits March 31, 2019 10:40

returns reshares in API

44c1feb

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

issue during last push

efbadf0

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

fixing share format

0166990

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

ArtificialOwl force-pushed the bugfix/14112/display-reshares branch from d9df077 to 0166990 Compare March 31, 2019 11:40

MorrisJobke reviewed Apr 1, 2019

View reviewed changes

do not returns shared_with === currentUser

749fdab

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

nextcloud deleted a comment from faily-bot bot Apr 2, 2019

MorrisJobke added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 2, 2019

revert to select()

586a3a1

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

rullzer approved these changes Apr 2, 2019

View reviewed changes

rullzer merged commit b365840 into master Apr 2, 2019

rullzer deleted the bugfix/14112/display-reshares branch April 2, 2019 20:59

rullzer mentioned this pull request Apr 2, 2019

Share api does not show all reshares #14112

Closed

MorrisJobke mentioned this pull request Apr 3, 2019

16.0.0 Beta 3 #14949

Merged

2 tasks

nickvergessen added a commit to nextcloud/spreed that referenced this pull request Apr 9, 2019

Resharing behaviour has been adjusted for 16

5c23841

See nextcloud/server#14605 for more information Signed-off-by: Joas Schilling <coding@schilljs.com>

nickvergessen mentioned this pull request Apr 9, 2019

Resharing behaviour has been adjusted for 16 nextcloud/spreed#1694

Merged

skjnldsv mentioned this pull request Aug 19, 2019

Cleanup and do not list current user shares in getShares too #16789

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

returns reshares in API #14605

returns reshares in API #14605

ArtificialOwl commented Mar 9, 2019 •

edited

Loading

tobiasKaminsky left a comment

MorrisJobke commented Mar 20, 2019

tobiasKaminsky commented Mar 20, 2019

tobiasKaminsky commented Mar 20, 2019

ArtificialOwl commented Mar 21, 2019

MorrisJobke commented Mar 21, 2019

ArtificialOwl commented Mar 21, 2019

MorrisJobke commented Mar 21, 2019

ArtificialOwl commented Mar 30, 2019

ArtificialOwl commented Mar 31, 2019

ArtificialOwl commented Mar 31, 2019 •

edited

Loading

tobiasKaminsky commented Apr 1, 2019

ArtificialOwl commented Apr 1, 2019

MorrisJobke Apr 1, 2019

ArtificialOwl Apr 1, 2019

MorrisJobke Apr 2, 2019

tobiasKaminsky commented Apr 1, 2019

ArtificialOwl commented Apr 1, 2019

tobiasKaminsky commented Apr 1, 2019

ArtificialOwl commented Apr 1, 2019

ArtificialOwl commented Apr 1, 2019

tobiasKaminsky commented Apr 2, 2019

MorrisJobke commented Apr 2, 2019

rullzer left a comment

returns reshares in API #14605

returns reshares in API #14605

Conversation

ArtificialOwl commented Mar 9, 2019 • edited Loading

tobiasKaminsky left a comment

Choose a reason for hiding this comment

MorrisJobke commented Mar 20, 2019

tobiasKaminsky commented Mar 20, 2019

tobiasKaminsky commented Mar 20, 2019

ArtificialOwl commented Mar 21, 2019

MorrisJobke commented Mar 21, 2019

ArtificialOwl commented Mar 21, 2019

MorrisJobke commented Mar 21, 2019

ArtificialOwl commented Mar 30, 2019

ArtificialOwl commented Mar 31, 2019

ArtificialOwl commented Mar 31, 2019 • edited Loading

tobiasKaminsky commented Apr 1, 2019

ArtificialOwl commented Apr 1, 2019

MorrisJobke Apr 1, 2019

Choose a reason for hiding this comment

ArtificialOwl Apr 1, 2019

Choose a reason for hiding this comment

MorrisJobke Apr 2, 2019

Choose a reason for hiding this comment

tobiasKaminsky commented Apr 1, 2019

ArtificialOwl commented Apr 1, 2019

tobiasKaminsky commented Apr 1, 2019

ArtificialOwl commented Apr 1, 2019

ArtificialOwl commented Apr 1, 2019

tobiasKaminsky commented Apr 2, 2019

MorrisJobke commented Apr 2, 2019

rullzer left a comment

Choose a reason for hiding this comment

ArtificialOwl commented Mar 9, 2019 •

edited

Loading

ArtificialOwl commented Mar 31, 2019 •

edited

Loading