use relative path in shared_by_email activity #37555

micbar · 2020-06-18T11:08:05Z

Description

Do not use the absolute Webdav Path for the shared_by_email activity.

Related Issue

Fixes https://github.com/owncloud/enterprise/issues/4045

Motivation and Context

Do not display unecessary information.

How Has This Been Tested?

manually

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

Tasks

Needs Changelog item
Needs manual testing

update-docs · 2020-06-18T11:08:08Z

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

karakayasemi · 2020-06-18T11:43:07Z

lib/private/Share/MailNotifications.php

@@ -272,7 +279,8 @@ public function sendLinkShareMail($sender, $recipients, $link, $personalNote = n
 		$failedRecipients =  $this->sendLinkShareMailFromBody($recipients, $subject, $htmlBody, $textBody, $from, $replyTo);
 		if (empty($failedRecipients)) {
 			$event = $this->activityManager->generateEvent();
-			$path = $shareNode->getPath();
+			$userFolder = $this->rootFolder->getUserFolder($sender->getUID());
+			$path = $userFolder->getRelativePath($shareNode->getPath());


For reshare case, since the node path is outside of the re-sharer folder, $path will be null.

good catch.

what could we do? We are not allowed to have a userid here

karakayasemi · 2020-06-18T11:55:36Z

Units tests also need to be adjusted. @micbar if you prefer, I can take over this one.

karakayasemi · 2020-06-18T14:13:02Z

@micbar The code now gets the actual file node of the sender. In this way, I believe the relative path calculated correctly. I manually tested the code with both re-share and the normal case, everything worked without problem. Also, unit tests adjusted.

codecov · 2020-06-18T15:11:02Z

Codecov Report

Merging #37555 into master will increase coverage by 0.00%.
The diff coverage is 83.33%.

@@            Coverage Diff            @@
##             master   #37555   +/-   ##
=========================================
  Coverage     64.66%   64.66%           
  Complexity    19343    19343           
=========================================
  Files          1279     1279           
  Lines         75600    75604    +4     
  Branches       1333     1333           
=========================================
+ Hits          48885    48889    +4     
  Misses        26323    26323           
  Partials        392      392

Flag	Coverage Δ	Complexity Δ
#javascript	`54.03% <ø> (ø)`	`0.00 <ø> (ø)`
#phpunit	`65.83% <83.33%> (+<0.01%)`	`19343.00 <0.00> (ø)`

Impacted Files	Coverage Δ	Complexity Δ
..._sharing/lib/Controller/NotificationController.php	`42.10% <0.00%> (ø)`	`14.00 <0.00> (ø)`
lib/private/Share/MailNotifications.php	`95.74% <100.00%> (+0.09%)`	`33.00 <0.00> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 187bd0d...0042812. Read the comment docs.

codecov · 2020-06-18T15:12:20Z

Codecov Report

Merging #37555 into master will increase coverage by 0.00%.
The diff coverage is 83.33%.

@@            Coverage Diff            @@
##             master   #37555   +/-   ##
=========================================
  Coverage     64.66%   64.66%           
  Complexity    19343    19343           
=========================================
  Files          1279     1279           
  Lines         75600    75604    +4     
  Branches       1333     1333           
=========================================
+ Hits          48885    48889    +4     
  Misses        26323    26323           
  Partials        392      392

Flag	Coverage Δ	Complexity Δ
#javascript	`54.03% <ø> (ø)`	`0.00 <ø> (ø)`
#phpunit	`65.83% <83.33%> (+<0.01%)`	`19343.00 <0.00> (ø)`

Impacted Files	Coverage Δ	Complexity Δ
..._sharing/lib/Controller/NotificationController.php	`42.10% <0.00%> (ø)`	`14.00 <0.00> (ø)`
lib/private/Share/MailNotifications.php	`95.74% <100.00%> (+0.09%)`	`33.00 <0.00> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0b2901...5de0aab. Read the comment docs.

karakayasemi · 2020-06-18T17:13:40Z

Changelog added.

micbar

Do not merge yet. We need to decide if we release it woth 10.5.0

micbar · 2020-06-18T19:50:35Z

lib/private/Share/MailNotifications.php

-			$path = $shareNode->getPath();
+			// it can be a re-share, get actual share file node
+			$userFolder = $this->rootFolder->getUserFolder($sender->getUID());
+			$shareFileNodes = $userFolder->getById($shareNode->getId(), true);


Why not $share->getNodeId()?

No difference. $shareNode already defined in above, no drawback to using it also. There is similar actual share node detection logic in the share manager. Copied it from there.

core/lib/private/Share20/Manager.php

Lines 358 to 361 in f0b2901

// retrieve received share node $shareFileNode being reshared with $share

$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());

$shareFileNodes = $userFolder->getById($shareNode->getId(), true);

$shareFileNode = $shareFileNodes[0] ?? null;

micbar self-assigned this Jun 18, 2020

micbar added the Type:Bug label Jun 18, 2020

micbar added this to the development milestone Jun 18, 2020

micbar added sev2-high p1-urgent Critical issue, need to consider hotfix with just that issue labels Jun 18, 2020

micbar requested review from karakayasemi and jvillafanez June 18, 2020 11:09

micbar force-pushed the fix/mail-resource-path branch from 1e46b91 to 576a9b7 Compare June 18, 2020 11:18

karakayasemi suggested changes Jun 18, 2020

View reviewed changes

micbar requested a review from karakayasemi June 18, 2020 16:06

karakayasemi approved these changes Jun 18, 2020

View reviewed changes

micbar and others added 2 commits June 18, 2020 10:09

use relative path in shared_by_email activity

5222318

consider reshare case when returning relative path and adjust unit tests

5de0aab

karakayasemi force-pushed the fix/mail-resource-path branch from 0042812 to 5de0aab Compare June 18, 2020 17:10

micbar commented Jun 18, 2020

View reviewed changes

micbar merged commit 9b395f0 into master Jun 22, 2020

delete-merged-branch bot deleted the fix/mail-resource-path branch June 22, 2020 08:37

davitol mentioned this pull request Jun 25, 2020

Tracking test results for ownCloud 10.5.0 #37593

Closed

65 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use relative path in shared_by_email activity #37555

use relative path in shared_by_email activity #37555

micbar commented Jun 18, 2020 •

edited by karakayasemi

Loading

update-docs bot commented Jun 18, 2020

karakayasemi Jun 18, 2020

micbar Jun 18, 2020

micbar Jun 18, 2020

karakayasemi commented Jun 18, 2020

karakayasemi commented Jun 18, 2020 •

edited

Loading

codecov bot commented Jun 18, 2020

codecov bot commented Jun 18, 2020 •

edited

Loading

karakayasemi commented Jun 18, 2020

micbar left a comment

micbar Jun 18, 2020

karakayasemi Jun 18, 2020

	// retrieve received share node $shareFileNode being reshared with $share
	$userFolder = $this->rootFolder->getUserFolder($share->getSharedBy());
	$shareFileNodes = $userFolder->getById($shareNode->getId(), true);
	$shareFileNode = $shareFileNodes[0] ?? null;

use relative path in shared_by_email activity #37555

use relative path in shared_by_email activity #37555

Conversation

micbar commented Jun 18, 2020 • edited by karakayasemi Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Tasks

update-docs bot commented Jun 18, 2020

karakayasemi Jun 18, 2020

Choose a reason for hiding this comment

micbar Jun 18, 2020

Choose a reason for hiding this comment

micbar Jun 18, 2020

Choose a reason for hiding this comment

karakayasemi commented Jun 18, 2020

karakayasemi commented Jun 18, 2020 • edited Loading

codecov bot commented Jun 18, 2020

Codecov Report

codecov bot commented Jun 18, 2020 • edited Loading

Codecov Report

karakayasemi commented Jun 18, 2020

micbar left a comment

Choose a reason for hiding this comment

micbar Jun 18, 2020

Choose a reason for hiding this comment

karakayasemi Jun 18, 2020

Choose a reason for hiding this comment

micbar commented Jun 18, 2020 •

edited by karakayasemi

Loading

karakayasemi commented Jun 18, 2020 •

edited

Loading

codecov bot commented Jun 18, 2020 •

edited

Loading