-
-
Notifications
You must be signed in to change notification settings - Fork 943
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
Group join approved notification #1620
Conversation
…et/11745 * 'ticket/11744' of github.com:EXreaction/phpbb3: [ticket/11744] Helper function to assert notifications in base test
@@ -414,6 +414,8 @@ | |||
2 => '<strong>%d</strong> Notifications', | |||
), | |||
'NOTIFICATION_BOOKMARK' => '%1$s replied to the topic "%2$s" you have bookmarked.', | |||
'NOTIFICATION_GROUP_REQUEST' => '%1$s is requesting to join the group %2$s.', | |||
'NOTIFICATION_GROUP_REQUEST_APPROVED' => 'Your request to join the "%1$s" group on has been approved.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "on" sounds wrong to me?
And maybe order the same way as in the previous sentence.
Your request to join the group %1$s has been approved.
$row = $this->db->sql_fetchrow($result); | ||
$this->db->sql_freeresult($result); | ||
|
||
return (!empty($row)) ? true : false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpicky, but that could be shortened to just:
return !empty($row);
Group join approved notification
Why was this merged, even the the email notification is removed which i pointed out and obviously would have required some discussion on whether this is the way to go? |
Contains #1617
http://tracker.phpbb.com/browse/PHPBB3-11745