-
Notifications
You must be signed in to change notification settings - Fork 12
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
"last uid" is not respected when range and list of user IDs is provided. #10
base: 7.x-1.x
Are you sure you want to change the base?
Conversation
message_subscribe_send_message('node', $this->node, $message, array(), $subscribe_options); | ||
|
||
// Run cron. | ||
$this->cronRun(); |
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.
Thanks for the PR. can we avoid running cron, and invoke the needed function ourself?
I have queued to patches to test bot since Travis build fails. @amitaibu I'll update the request if you have any suggestions on how to check for sent emails without using the capture functionality built-in Simpletest. |
Tests seem to be failing because Travis needs to be updated - drush should be installed via @ordavidil can you please review. |
@capuleto |
$uids = $subscribe_options['range'] ? array_slice($subscribe_options['uids'], 0, $subscribe_options['range'], TRUE) : $subscribe_options['uids']; | ||
// If 'last uid' is provided, we need to start from next 'uid' key in array. | ||
if (!empty($subscribe_options['last uid'])) { | ||
$index = array_search($subscribe_options['last uid'], array_keys($subscribe_options['uids'])) + 1; |
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.
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.
Can we live with that assumption ?
You'll have to check, and I don't remember :)
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.
Message subscribe does not use "last uid" at all when the list of user IDs is provided and range is enabled in subscribe options.
https://www.drupal.org/node/2256899