"Recipient phone number is invalid or recipient does not want to receive SMS" error and Contacts with DoNotSms preference failing to filter out during Mass Sms in 5.x

"Recipient phone number is invalid or recipient does not want to receive SMS" error occurs during individual SMS. https://civicrm.stackexchange.com/questions/25478/clickatell-sms-recipient-phone-number-is-invalid-or-recipient-does-not-want-to. Contacts marked as 'donotsms' is not filtered out during mass SMS.

added needs:concept approval sig:bug triaged type:proposal labels

@michaelmcandrew ping

https://github.com/civicrm/civicrm-core/pull/12537

I think that CiviCRM basically expects do_not_sms to mean .... 'Do not SMS' - I think you are wanting some way to opt people out of bulk sms while still being able to SMS them? I think for email we use is_bulk_mail - although I'm not 100% sure it's reliable

@eileen Few contacts may not be interested to receive SMS. When SMS is sent to groups using mass sms option, in v5.x the contacts with preference 'Do not SMS' also receives SMS. In v4.7.8, it works fine and 'Do not SMS' preferred contacts are filtered out.

I also had a report that this broke in 5.3.x. The client was last on 4.7.30.

I tried digging for a commit that would have caused a regression, but did not find it.

The code (edit: from PR 12254) seems OK to me:

• The previous code did not make much sense.
• If I set "do not sms" on a contact record, the "Send an SMS" action is still not shown (which is as expected).
• If I set "do not sms", the contact is not shown in the recipients for a mass SMS (I also checked that if "do not sms" is not checked, the contact is in the list of recipients).

I think it's worth noting the sms code is pretty bad & a test should be mandatory for any fix.

We just received two reports of both problems as well - and they seem to be distinct problems.

Problem one: You cannot send an SMS activity to a contact via the one-off SMS outbound activity type. Even if the contact does not designate "Do not SMS" you still get the message that the contact is not configured to receive SMS messages.

This seems to be a regression caused by a commit designed to cleanup the messy SMS code: https://github.com/civicrm/civicrm-core/pull/10946

Problem two: when generating mass SMS messages, we exclude contacts that have "opt_out" designated but we don't exclude contacts that have 'do_not_sms' designated. Since opt_out, in the UI, seems to clearly be about email messages, it seems this should simply be replaced with do_not_sms.

This change seems to have been introduced via: https://github.com/civicrm/civicrm-core/pull/11558 (it seems the SMS smart group code has never properly worked??).

I haven't had a chance to fix/create proper tests, but added a pull request to fix these problems here: https://github.com/civicrm/civicrm-core/pull/12638

I would welcome anyone who wants to add a test and/or take over the PR to make it better!

@jamie See also: https://github.com/civicrm/civicrm-core/pull/12554

Despite the syntax error, it makes sense imho to just remove the variable.

Can we leave is_opt_out in there? Otherwise it seems like it would be a change in behaviour?

@seamuslee did an alternative

https://github.com/civicrm/civicrm-core/pull/12634

Worth noting that prior to https://github.com/civicrm/civicrm-core/pull/10946 there were NO unit tests on this part of the code so breakage was highly likely. Nice to see that in https://github.com/civicrm/civicrm-core/pull/12634 we can extend those unit tests to lock in the expected functionality.

mentioned in issue eileen/Product-maintenance#16

mentioned in commit a9b7ee41

mentioned in commit 5fce4a23

mentioned in commit 11aed945

mentioned in commit 3fdf7543

https://github.com/civicrm/civicrm-core/pull/12634 was merged so am going to close this issue

closed

changed milestone to %5.5.0