Skip to content

[FIX] DM email notifications always being sent regardless of account setting#8917

Merged
rodrigok merged 2 commits intoRocketChat:developfrom
ashward:fix-email-dm-notification
Dec 4, 2017
Merged

[FIX] DM email notifications always being sent regardless of account setting#8917
rodrigok merged 2 commits intoRocketChat:developfrom
ashward:fix-email-dm-notification

Conversation

@ashward
Copy link
Copy Markdown
Contributor

@ashward ashward commented Nov 22, 2017

@RocketChat/core

Closes #8619

PR #8810 fixed an issue with DM email notifications always being sent regardless of account setting. This had an issue though if the subscription had never been set (alluded to in a comment #8810 (comment)

This PR should fix that lingering issue.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Nov 22, 2017

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

hey, thanks for you contribution =D

looks like this introduces a new bug. I did the following steps:

  • registered a new user
  • as an old user sent a direct message to the new one
  • no email was sent

but the default configuration is to sent an email on every mention/dm, so it should be sending emails in this test.

@ashward
Copy link
Copy Markdown
Contributor Author

ashward commented Nov 23, 2017

Good spot. Looks like the logic on the existing code for userEmailPreferenceIsDisabled is defaulting to true rather than false if the setting isn't set. I'll fix and update the PR.

The existing code logic was that if the user preference wasn't set then emails were defaulting to disabled. This should default to enabled.
@ashward
Copy link
Copy Markdown
Contributor Author

ashward commented Nov 23, 2017

I've tweaked that existing logic and it looks to be working as expected now.

@sampaiodiego
Copy link
Copy Markdown
Member

Looks good now =) thx again

@rodrigok rodrigok added this to the 0.60.0 milestone Dec 4, 2017
@rodrigok rodrigok changed the title [FIX] Small tweak to PR #8810 to take into account null subscription [FIX] DM email notifications always being sent regardless of account setting Dec 4, 2017
@rodrigok rodrigok merged commit b7741f1 into RocketChat:develop Dec 4, 2017
@ashward ashward deleted the fix-email-dm-notification branch December 5, 2017 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"offline chat notification" setting is not taken into account

5 participants