Skip to content

handle mail send error gracefully - #12636

Merged
rullzer merged 4 commits into
masterfrom
fix-12597
Jan 30, 2019
Merged

handle mail send error gracefully#12636
rullzer merged 4 commits into
masterfrom
fix-12597

Conversation

@schiessle

@schiessleschiessle commented Nov 23, 2018•
edited
Loading

Copy link
Copy Markdown
Member

log the error in case a notification mail of a new share couldn't
be send to the recipient and finish the share operation successfully

I would suggest to backport it at least to stable15 and stable14

fix#12597

@schiessleschiessle added this to the Nextcloud 15 milestone Nov 23, 2018
@rullzer

rullzer commented Nov 26, 2018

Copy link
Copy Markdown
Member

/backport to stable15

@rullzer

rullzer commented Nov 26, 2018

Copy link
Copy Markdown
Member

/backport to stable14

MorrisJobke
MorrisJobke reviewed Nov 26, 2018
View reviewed changes
Comment threadlib/private/Share20/Manager.php Outdated
@MorrisJobkeMorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Nov 28, 2018
@ChristophWurstChristophWurst added 3. to review Waiting for reviews and removed 4. to release Ready to be released and/or waiting for tests to finish labels Nov 29, 2018
ChristophWurst
ChristophWurst requested changes Nov 29, 2018
View reviewed changes

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Typo 😉

Is the returned bool used?

Comment threadlib/private/Share20/Manager.php Outdated
ChristophWurst
ChristophWurst requested changes Nov 29, 2018
View reviewed changes

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Result is never checked.

Ref

server/lib/private/Share20/Manager.php

Lines 687 to 694 in 0f8de6e

$this->sendMailNotification(
$l,
$share->getNode()->getName(),
$this->urlGenerator->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $share->getNode()->getId()]),
$share->getSharedBy(),
$emailAddress,
$share->getExpirationDate()
);

@ChristophWurst

This comment has been minimized.

Sign in to view
@ChristophWurst

This comment has been minimized.

Sign in to view
blizzz
blizzz reviewed Nov 30, 2018
View reviewed changes
Comment threadlib/private/Share20/Manager.php Outdated
@MorrisJobkeMorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 30, 2018
@skjnldsvskjnldsv mentioned this pull request Dec 4, 2018
Closed
@schiessleschiessle added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 4, 2018
@schiessle

schiessle commented Dec 4, 2018

Copy link
Copy Markdown
MemberAuthor

typo fixed

MorrisJobke
MorrisJobke reviewed Dec 5, 2018
View reviewed changes
Comment threadlib/private/Share20/Manager.php Outdated
@MorrisJobkeMorrisJobke mentioned this pull request Jan 4, 2019
Merged
3 tasks
@MorrisJobkeMorrisJobke mentioned this pull request Jan 30, 2019
Merged
@schiessle@ChristophWurst
handle mail send error gracefully
a943580
log the error in case a notification mail of a new share couldn't
be send to the recipient and finish the share operation successfully
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle@ChristophWurst
log full exception
ab01402
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
@schiessle@ChristophWurst
fix typo
8ed004d
Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
ChristophWurst
ChristophWurst approved these changes Jan 30, 2019
View reviewed changes

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm fine with @schiessle's changes as this method is solely used in this class 👍

@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 30, 2019
kesselb
kesselb reviewed Jan 30, 2019
View reviewed changes
Comment threadlib/private/Share20/Manager.php Outdated
@ChristophWurst
Fix typos and unused return values
7e3e3be
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
MorrisJobke
MorrisJobke approved these changes Jan 30, 2019
View reviewed changes

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good 👍

@MorrisJobkeMorrisJobke mentioned this pull request Jan 30, 2019
Merged
@rullzer
rullzer merged commit 85d6d7b into masterJan 30, 2019
@rullzer
rullzer deleted the fix-12597 branch January 30, 2019 19:05
@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Jan 30, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Jan 30, 2019

Copy link
Copy Markdown

backport to stable15 in #13930

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Jan 30, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Jan 30, 2019

Copy link
Copy Markdown

backport to stable14 in #13931

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChristophWurstChristophWurstChristophWurst approved these changes
@blizzzblizzzblizzz left review comments
@kesselbkesselbkesselb left review comments
@rullzerrullzerrullzer approved these changes
@MorrisJobkeMorrisJobkeMorrisJobke approved these changes

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: sharing

Projects

None yet

Milestone

Nextcloud 16

Development

Successfully merging this pull request may close these issues.

Error when sharing to user with email

6 participants

@schiessle@rullzer@ChristophWurst@MorrisJobke@blizzz@kesselb