Remove some phone type restrictions around Send SMS

[mlutfy]

Overview

The "Send SMS" feature requires that the contact have a phone of type "mobile". The year is 2024 and most people have phones that support SMS (my VoIP provider does anyway).

I sometimes encounter situations where people change those phone types, without knowing that there are hardcoded meanings around some of them. Or they are confused about why a phone would not receive SMS ☎️

Before

Admin can SMS only a contact with a phone of type "mobile".

After

Admin can SMS any number.

Comments

I tried keeping the feature that prioritizes a mobile number, if there is one. I only removed the restrictions on the "Send SMS" activity.

Mass-sms is maybe a bit more tricky, since if you know about the behaviour, maybe you expect it. I was thinking of maybe adding a help-text on that interface. I was curious how people felt about this change first.

This doc would need some tweaks: https://docs.civicrm.org/user/en/latest/sms-text-messaging/everyday-tasks/

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[mlutfy]

cc folks who seem to use SMS: @michaelmcandrew @shaneonabike @jmcclelland @jaapjansma @kainuk (smsapi folks)

[colemanw]

@mlutfy there are 3 relevant test fails. Probably the tests just need to be updated to account for the less-restrictive rules.

[mlutfy]

jenkins, test this please

[mlutfy]

(weird, I thought I had fixed them, they pass locally) my bad, checking

[colemanw]

retest this please

[mlutfy]

I did a bit more cleanup, since that allPhones function was really bugging me, although I didn't feel comfortable removing the last call to it, since the code requires more cleanup.

There is one thing that might be a deal breaker: in CRM/Contact/Form/Task/SMSTrait.php, there is an api4 call to get the list of phones. I will comment inline separately.

[mlutfy]

I'm hesitant here. If the contact has a Non-Primary Mobile Number, it will not be used, the Primary will always be used.

Either the new getContactMobileOrPrimary function needs to be rewritten with more complex SQL that says "pick mobile or primary or any number", which wouldn't be nice, or we loop through the contact IDs, assuming it is up to 50-100, and it's not a huge performance problem.

(I know this PR is getting weirder and weirder, but I think this is a pretty big issue with Send SMS usability)