CRM-20238 - Create hook for inbound SMS #10347
Conversation
seamuslee001
commented
May 12, 2017
•
seamuslee001
commented
- CRM-20238: Hook for inbound SMS messages
|
@totten @ineffyble Tim does this look closer to what you were thinking of? Effy do you understand my changes that i've made? |
seamuslee001
force-pushed
the
CRM-20238
branch
3 times, most recently
from
181c602 to
500e366
Compare
|
Looks good to me ✅ |
CRM/SMS/Provider.php
Outdated
| $actStatusIDs = array_flip(CRM_Core_OptionGroup::values('activity_status')); | ||
| $activityTypeID = CRM_Core_OptionGroup::getValue('activity_type', 'Inbound SMS', 'name'); |
There was a problem hiding this comment.
@eileenmcnaughton stumbled upon another one of these (there were no tests before so Jenkins wasn't getting annoyed)
There was a problem hiding this comment.
ah yes - that was part of the cunning plan
seamuslee001
force-pushed
the
CRM-20238
branch
2 times, most recently
from
06eceaf to
da74a48
Compare
|
Jenkins re test this please |
seamuslee001
force-pushed
the
CRM-20238
branch
2 times, most recently
from
d390629 to
8119455
Compare
…nd Paramatise a little Further use of new Message object
|
Yup, the hook-signature/class-structure looks better. :) |
There was a problem hiding this comment.
This looks good! The existing code is smelly - in particular, I suspect formatPhone() should be removed in favor of a solution that uses phone_numeric - but this PR adds tests and is an overall improvement. I particularly like the use of the object to pass to the new hook to avoid locking in a bad function signature on the hook.
