CRM-21576 Add a 'send SMS' permission

[michaelmcandrew]

This PR adds a permission to Send SMS. More specifically, it:

• Adds a permission to CRM_Core_Permissions::getCorePermissions
• Ensures that all navigation menu entries ('Find SMS' and 'New SMS') respect the new permission
• Ensures that all paths (civicrm/sms/send, civicrm/activity/sms/add, civicrm/mailing) respect the new permission
• Only show 'Outbound SMS' action from the action menu on the contact screen to users with the send SMS permission
• Only show 'SMS - schedule/send' from the advanced search actions to users with the send SMS permission

All CRM/SMS tests pass (there are only 6 of them!). I am awaiting results from the full (tests/phpunit --group headless) test suite but thought I would post this in the meantime.

This is a WIP. A couple of things left to do:

• add an upgrade message (I couldn't see any instructions on how to do that here: https://docs.civicrm.org/dev/en/latest/framework/upgrade/)
• write a test or two (anyone have any pointers on permissions tests?)
• some feedback from the client on whether this is working as expected

Feedback welcome, especially thoughts on stuff I might have overlooked.

---

• CRM-21576: Implement Send SMS permission

[michaelmcandrew]

On other thing to note. Along the way I fixed a bug with Sending SMS #11589 and I have merged that commit into this branch. Not sure if that is acceptable protocol or not...

[michaelmcandrew]

@JKingsnorth - thought you might be interested in this.

[seamuslee001]

@michaelmcandrew for the upgrade message you have a choice of if you want it to show before the upgrade runs or after the upgrade runs. if you want it to be before the upgrade add it here https://github.com/civicrm/civicrm-core/blob/master/CRM/Upgrade/Incremental/php/FourSeven.php#L43 if you want it after add it here https://github.com/civicrm/civicrm-core/blob/master/CRM/Upgrade/Incremental/php/FourSeven.php#L95

As per the docs here https://docs.civicrm.org/dev/en/latest/framework/upgrade/#incremental-php-files there is the preUpgrade and PostUpgrade message functions

[JKingsnorth]

Thanks for the heads up.

[michaelmcandrew]

Hello again. A couple more updates to this PR:

• Feedback from the client is good. It is working as expected and nothing has broken
• I have added an upgrade message for 4.7.32 - is that the version that we can expect to see this released in (bar any further changes/delays) @seamuslee001 - thanks for your pointers. Does it look right to you?
• I added a further permissions check to CRM_Activity_BAO_Activity::sendSMS
• I wrote a test to check the permissions check in CRM_Activity_BAO_Activity::sendSMS

@mattwire - any chance you could have a look?

[michaelmcandrew]

Jenkins, test this please

[michaelmcandrew]

jenkins, test this please

[eileenmcnaughton]

@michaelmcandrew I think some git foo will fix jenkins - you have a merge commit in there so try rebasing...

[GitScan\Exception\ProcessErrorException]
Process failed:
[[ COMMAND: git apply --check --ignore-whitespace ]]
[[ CWD: /home/jenkins/buildkit/build/core-11590-7by38/sites/all/modules/civicrm ]]
[[ EXIT CODE: 1 ]]
[[ STDOUT ]]
[[ STDERR ]]
error: patch failed: xml/templates/civicrm_navigation.tpl:817
error: xml/templates/civicrm_navigation.tpl: patch does not apply
error: patch failed: CRM/Mailing/BAO/Mailing.php:224
error: CRM/Mailing/BAO/Mailing.php: patch does not apply
error: patch failed: CRM/Contact/Task.php:190
error: CRM/Contact/Task.php: patch does not apply
error: patch failed: CRM/Contact/Task.php:365
error: CRM/Contact/Task.php: patch does not apply
error: patch failed: CRM/Mailing/BAO/Mailing.php:224
error: CRM/Mailing/BAO/Mailing.php: patch does not apply
error: patch failed: CRM/Contact/Task.php:309
error: CRM/Contact/Task.php: patch does not apply

[colemanw]

I think this is a good change and agree it ought to be in core.
@civicrm-builder retest this please.

[colemanw]

Oh I just saw Eileen's comment.
@michaelmcandrew could you rebase out the merge commits and squash the rest?

[michaelmcandrew]

@eileenmcnaughton and @colemanw - thanks for the feedback. Here is a new commit. Fairly sure it captures the necessary and removes the crap. Lets see what Jenkins has to say.

[colemanw]

I should disclaim my "ought to be in core" comment by saying that IMO CiviSMS should not have ever been put into core in the first place. But since it's there and we're stuck with it for the time being, this permission belongs in core too.

[michaelmcandrew]

@eileenmcnaughton and @colemanw - looks like all tests have passed, finally :)

[michaelmcandrew]

thanks @colemanw