Skip to content

Process eventsubscriber#37

Closed
dpi wants to merge 23 commits into8.x-1.xfrom
process-eventsubscriber
Closed

Process eventsubscriber#37
dpi wants to merge 23 commits into8.x-1.xfrom
process-eventsubscriber

Conversation

@dpi
Copy link
Copy Markdown
Owner

@dpi dpi commented May 12, 2016

@dpi dpi force-pushed the process-eventsubscriber branch 2 times, most recently from 8a2d7a8 to 502e7ad Compare May 12, 2016 18:27
@dpi dpi force-pushed the process-eventsubscriber branch from 502e7ad to b80538a Compare May 13, 2016 13:58
@dpi dpi mentioned this pull request May 13, 2016
dpi added 6 commits May 14, 2016 23:39
Removed missing newline in services yml
Fixed assumptions that queue method is a reference, changed to use new queue return value.
Fixed not unsetting correct value in ensureGateways
Fixed redefining recipients all variable in ensureGateways
Fixed ensureGateways assuming messages are entities, will now correctly clone plain messages.
Fixed incorrect variable referenced in defaultsmsprovider::send
Added more tests for queue return value.
Added recipients to some test messages since that is now required.
Tests no messages created if no recipients.
* @return \Drupal\sms\Entity\SmsGatewayInterface[]
*/
public function getGatewaysSorted() {
uasort($this->gateways, function($a, $b) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We shouldn't actually sort $this->gateways since the method name assumes that the original should be left unsorted.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Oh right its a reference... Can do

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

How are you finding dpi/sms_rule_based#1 ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looked at sms_rule_based#1. That looks good (once I can confirm I'm okay with the event approach).

@dpi dpi force-pushed the process-eventsubscriber branch from 98da6c0 to 76bc9ba Compare May 17, 2016 14:42
});

$gateways = [];
foreach ($this->gateways as $tuple) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oops!!! Forgot this part...:)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

fixed

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.

2 participants