Get access list share by email recipients #32631

ASerbinski · 2022-05-27T14:50:34Z

Close #32629

Summary:

Return the recipients for an email share.

We already have mail, but it's unused / broken.

The backstory:

In #2834 mail => bool was documented but not implemented.

Commit to change mail to public: cab4111

apps/sharebymail/lib/ShareByMailProvider.php

CarlSchwan · 2022-05-27T15:31:37Z

lib/public/Share/IManager.php

 *
 * The access list to '/folder1/folder2/fileA' **without** $currentAccess is:
 * [
 * users => ['user1', 'user2', 'user4'],
 * remote => bool,
 * public => bool
- * mail => bool
+ * mail => ['email1@maildomain1', 'email2@maildomain2']


this is an api break :( not sure if we can change this, but if it never worked it's probably fine

Yes, it certainly is an API break. I went through the history and it never actually used the correct array key.

lib/private/Share20/Manager.php

kesselb · 2023-01-23T14:31:42Z

Hi @ASerbinski

Thanks for your pull request 👍

Sorry for not getting back to you sooner.
Would you mind rebasing your branch to pull the latest changes?

kesselb · 2023-01-23T14:38:36Z

Would you mind splitting the bug fix and enhancement into own pull requests?

As Carl mentioned, the enhancement comes with an API break.
That's harder to review and accept than a bug report.

ASerbinski · 2023-01-23T15:44:20Z

Hi @kesselb ;

As you've requested, I've rebased on master and split the bug fix into #36305

This PR now depends on that bug fix.

blizzz · 2024-03-20T22:22:57Z

Failing test seem related, for example:

There was 1 failure:

1) OCA\ShareByMail\Tests\ShareByMailProviderTest::testGetAccessList
Failed asserting that true is false.

/home/runner/actions-runner/_work/server/server/apps/sharebymail/tests/ShareByMailProviderTest.php:1093

AndyScherzinger · 2024-03-20T22:37:45Z

@ASerbinski see comment from blizzz #32631 (comment)

ASerbinski · 2024-03-21T20:26:31Z

@AndyScherzinger @blizzz ;

Added a commit based on php documentation; https://www.php.net/manual/en/pdostatement.rowcount.php

"For statements that produce result sets, such as SELECT, the behavior is undefined"

Passing applicable unit test;

$ ./autotest.sh sqlite apps/sharebymail/tests/ShareByMailProviderTest.php
...
Time: 00:01.135, Memory: 36.00 MB

OK (37 tests, 186 assertions)

Other failures are unrelated and seem to refer to some kind of network failure.

Previously was returning only boolean true if the Node was shared by email, or false if not. Now provides an array containing the list of email share recipients. Signed-off-by: Adam Serbinski <adam@serbinski.com>

This allows the share URI to be regenerated. Signed-off-by: Adam Serbinski <adam@serbinski.com>

Signed-off-by: Adam Serbinski <adam@serbinski.com>

…'mail' To preserve prior behaviour where 'public' was set 'true' if there are any mail recipients. The 'mail' key will be an array of email recipients. Signed-off-by: Adam Serbinski <adam@serbinski.com>

PDOStatement::rowCount behavior is undefined for SELECT statements for some database types, therefore manually set the value for 'public' based on actual results fetched. Signed-off-by: Adam Serbinski <adam@serbinski.com>

ASerbinski · 2024-03-24T12:39:49Z

@AndyScherzinger is it good now?

AndyScherzinger · 2024-03-24T14:59:16Z

I am AFK but asked @sorbaugh to lend a hand in getting it merged

welcome · 2024-03-25T11:40:13Z

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

AndyScherzinger · 2024-03-25T11:44:36Z

@ASerbinski merged 👍 Fancy writing a piece for the documentation repo to make the command easy to discover for others? 🙏

Thanks for the work on implementing this feature and for yet another contribution 🎉💯🚀

ASerbinski · 2024-03-25T11:57:19Z

Thank you very much!
I'll give the documentation a shot.

ASerbinski linked an issue May 27, 2022 that may be closed by this pull request

API: require means of querying list of email share recipients #32629

Closed

ASerbinski requested review from kesselb, PVince81, tcitworld and artonge May 27, 2022 14:52

ASerbinski linked an issue May 27, 2022 that may be closed by this pull request

[Bug]: getAccessList in ShareByMailProvider uses incorrect array key #32630

Closed

8 tasks

CarlSchwan reviewed May 27, 2022

View reviewed changes

ASerbinski requested a review from CarlSchwan May 27, 2022 16:25

tcitworld removed their request for review June 5, 2022 15:15

ASerbinski requested a review from AndyScherzinger June 6, 2022 17:47

AndyScherzinger removed their request for review June 6, 2022 18:34

szaimen added enhancement 3. to review Waiting for reviews labels Jun 9, 2022

szaimen added this to the Nextcloud 25 milestone Jun 9, 2022

This was referenced Aug 12, 2022

25.0.0 beta 1 #33505

Merged

25.0.0 beta 2 #33592

Merged

This was referenced Aug 24, 2022

25.0.0 beta 3 #33674

Merged

25.0.0 beta 4 #33762

Merged

This was referenced Sep 6, 2022

25.0.0 beta 5 #33913

Merged

25.0.0 beta 6 #34002

Merged

skjnldsv mentioned this pull request Sep 15, 2022

25.0.0 beta 7 #34095

Merged

This was referenced Sep 20, 2022

25.0.0 beta 8 #34164

Closed

25.0.0 RC 1 #34185

Merged

blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022

ASerbinski force-pushed the feature/32629/getAccessList-share-by-email-recipients branch 2 times, most recently from a9c0f18 to 3191654 Compare January 23, 2023 15:36

AndyScherzinger force-pushed the feature/32629/getAccessList-share-by-email-recipients branch from e6f987d to 53bfa7c Compare February 27, 2024 13:07

Altahrim mentioned this pull request Mar 12, 2024

29.0.0 beta 2 #44152

Merged

ChristophWurst added the pending documentation This pull request needs an associated documentation update label Mar 13, 2024

This was referenced Mar 14, 2024

29.0.0 beta 3 #44186

Merged

29.0.0 beta 4 #44275

Merged

29.0.0 beta 5 #44361

Merged

ASerbinski force-pushed the feature/32629/getAccessList-share-by-email-recipients branch from 53bfa7c to dfe8e45 Compare March 20, 2024 15:31

AndyScherzinger requested review from sorbaugh and removed request for PVince81 and CarlSchwan March 20, 2024 19:44

ASerbinski force-pushed the feature/32629/getAccessList-share-by-email-recipients branch from 37d9142 to 1c92874 Compare March 21, 2024 20:01

ASerbinski added 5 commits March 21, 2024 18:25

ShareByMailProvider: getAccessList: return full list of recipients

26ae5e6

Previously was returning only boolean true if the Node was shared by email, or false if not. Now provides an array containing the list of email share recipients. Signed-off-by: Adam Serbinski <adam@serbinski.com>

ShareByMailProvider: getAccessList: add token to returned array

9c528d4

This allows the share URI to be regenerated. Signed-off-by: Adam Serbinski <adam@serbinski.com>

ShareByMailProvider: getAccessList: recommended style changes

34ca6bd

Signed-off-by: Adam Serbinski <adam@serbinski.com>

ShareByMailProvider: getAccessList: set both array keys 'public' and …

01e3234

…'mail' To preserve prior behaviour where 'public' was set 'true' if there are any mail recipients. The 'mail' key will be an array of email recipients. Signed-off-by: Adam Serbinski <adam@serbinski.com>

ASerbinski force-pushed the feature/32629/getAccessList-share-by-email-recipients branch from 1c92874 to f45eb75 Compare March 21, 2024 22:25

artonge approved these changes Mar 25, 2024

View reviewed changes

Altahrim mentioned this pull request Mar 25, 2024

29.0.0 beta 6 #44449

Merged

AndyScherzinger approved these changes Mar 25, 2024

View reviewed changes

AndyScherzinger merged commit 46906b7 into master Mar 25, 2024
166 of 167 checks passed

AndyScherzinger deleted the feature/32629/getAccessList-share-by-email-recipients branch March 25, 2024 11:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get access list share by email recipients #32631

Get access list share by email recipients #32631

ASerbinski commented May 27, 2022 •

edited by kesselb

Loading

CarlSchwan May 27, 2022

ASerbinski May 27, 2022

kesselb commented Jan 23, 2023

kesselb commented Jan 23, 2023

ASerbinski commented Jan 23, 2023

blizzz commented Mar 20, 2024

AndyScherzinger commented Mar 20, 2024

ASerbinski commented Mar 21, 2024

ASerbinski commented Mar 24, 2024

AndyScherzinger commented Mar 24, 2024

welcome bot commented Mar 25, 2024

AndyScherzinger commented Mar 25, 2024

ASerbinski commented Mar 25, 2024

Get access list share by email recipients #32631

Get access list share by email recipients #32631

Conversation

ASerbinski commented May 27, 2022 • edited by kesselb Loading

Summary:

CarlSchwan May 27, 2022

Choose a reason for hiding this comment

ASerbinski May 27, 2022

Choose a reason for hiding this comment

kesselb commented Jan 23, 2023

kesselb commented Jan 23, 2023

ASerbinski commented Jan 23, 2023

blizzz commented Mar 20, 2024

AndyScherzinger commented Mar 20, 2024

ASerbinski commented Mar 21, 2024

ASerbinski commented Mar 24, 2024

AndyScherzinger commented Mar 24, 2024

welcome bot commented Mar 25, 2024

AndyScherzinger commented Mar 25, 2024

ASerbinski commented Mar 25, 2024

ASerbinski commented May 27, 2022 •

edited by kesselb

Loading