Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Notification not working for group mentions and not respecting ignored users #11024

Merged
merged 5 commits into from
Jun 10, 2018

Conversation

sampaiodiego
Copy link
Member

Closes #11018

@sampaiodiego sampaiodiego added this to the 0.66.0 milestone Jun 6, 2018
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11024 June 6, 2018 19:18 Inactive
@@ -199,11 +199,15 @@ function sendAllNotifications(message, room) {
[notificationField]: 'mentions',
'u._id': { $in: mentionIdsWithoutGroups }
});
} else if ((hasMentionToAll || hasMentionToHere) && !disableAllMessageNotifications) {
Copy link
Member

@ggazzo ggazzo Jun 8, 2018

Choose a reason for hiding this comment

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

I like to make small tests before

} else if (!disableAllMessageNotifications && (hasMentionToAll || hasMentionToHere))

what do you think?

}

const serverField = kind === 'email' ? 'emailNotificationMode' : `${ kind }Notifications`;
const serverPreference = RocketChat.settings.get(`Accounts_Default_User_Preferences_${ serverField }`);
if ((room.t === 'd' && serverPreference === 'mentions') || (serverPreference === 'all' && !disableAllMessageNotifications)) {
if ((room.t === 'd' && serverPreference === 'mentions') || ((serverPreference === 'all' || hasMentionToAll || hasMentionToHere) && !disableAllMessageNotifications)) {
Copy link
Member

@ggazzo ggazzo Jun 8, 2018

Choose a reason for hiding this comment

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

we really need room.t === 'd' && serverPreference === 'mentions' or just room.t === 'd' here? Because I think as we have serverPreference === 'all' that I think take care of the problem :)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes we do.. because serverPreference can be nothing

@sampaiodiego sampaiodiego temporarily deployed to rocket-chat-pr-11024 June 8, 2018 19:47 Inactive
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-11024 June 8, 2018 19:55 Inactive
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-11024 June 10, 2018 14:30 Inactive
@sampaiodiego sampaiodiego merged commit 36689b1 into develop Jun 10, 2018
@sampaiodiego sampaiodiego deleted the fix-notification-all-and-ignores branch June 10, 2018 19:07
trongthanh added a commit to goalifyplus/Goalify.Chat that referenced this pull request Jun 11, 2018
* develop: (215 commits)
  [FIX] Rooms list sorting by activity multiple re-renders and case sensitive sorting alphabetically (RocketChat#9959)
  [FIX] Notification not working for group mentions and not respecting ignored users (RocketChat#11024)
  fixed searchbar icon and text overlap (RocketChat#10294)
  [FIX] Link previews not being removed from messages after removed on editing (RocketChat#11063)
  avoid send presence without loggin
  Fix: Exception in metrics generation
  Update release template
  Update issue templates
  Update pt-BR.i18n.json
  LingoHub Update 🚀
  update import capnp
  update build for Sandstorm, add dependence capnp.
  LingoHub Update 🚀
  Fixing HTML on translation files
  Fixing HTML on translation files
  LingoHub Update 🚀
  Fixing HTML on translation files
  Fixing HTML on translation files
  Fixing HTML on translation files
  LingoHub Update 🚀
  ...

# Conflicts:
#	.meteor/packages
#	README.md
#	client/startup/startup.js
#	package-lock.json
#	packages/rocketchat-error-handler/server/lib/RocketChat.ErrorHandler.js
#	packages/rocketchat-i18n/i18n/en.i18n.json
#	packages/rocketchat-i18n/i18n/vi-VN.i18n.json
#	packages/rocketchat-livechat/config.js
#	server/startup/initialData.js
trongthanh added a commit to goalifyplus/Goalify.Chat that referenced this pull request Jun 11, 2018
* goalify: (217 commits)
  Add more scripts to reduce mannual interaction during server init
  [FIX] Rooms list sorting by activity multiple re-renders and case sensitive sorting alphabetically (RocketChat#9959)
  [FIX] Notification not working for group mentions and not respecting ignored users (RocketChat#11024)
  fixed searchbar icon and text overlap (RocketChat#10294)
  [FIX] Link previews not being removed from messages after removed on editing (RocketChat#11063)
  avoid send presence without loggin
  Fix: Exception in metrics generation
  Update release template
  Update issue templates
  Update pt-BR.i18n.json
  LingoHub Update 🚀
  update import capnp
  update build for Sandstorm, add dependence capnp.
  LingoHub Update 🚀
  Fixing HTML on translation files
  Fixing HTML on translation files
  LingoHub Update 🚀
  Fixing HTML on translation files
  Fixing HTML on translation files
  Fixing HTML on translation files
  ...
@rodrigok rodrigok mentioned this pull request Jun 28, 2018
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.

Desktop Notifications not appearing with "@all" or "@here"
4 participants