[REF] simplify CRM_Activity_BAO_Activity function by using early returns

[eileenmcnaughton]

Overview

Minor code simplification - this is simpler than it looks in github - it removes 2 if blocks that can be avoided by an early return - and makes the function more legible

Before

function less legible

After

function more legible

Technical Details

This function returns a bool. In the process it sets the variable, however, once set to false in two places
it can never be set to true, so I am simply replacing a big if block with an early return to simplify the code.

This is a further block that can go but it is a slightly different patter & would remove 2 levels of 'if'
so it feels like an easier review if I leave that out of scope

Comments

@colemanw @seamuslee001 any chance one of you could take a look. It's actually pretty minor & covered by a handful of tests like testGetActivityByAclCannotViewAllContacts added to api_v3_ACLPermissionTest in preparation for cleanup

[seamuslee001]

(CiviCRM Review Template WORD-1.2)

• General standards
  • (r-explain) Pass
  • (r-user) Pass
  • (r-doc) Pass
  • (r-run) Pass this is well covered by unit tests already so as long as Jenkins approves we should be fine
• Developer standards
  • (r-tech) Pass
  • (r-code) Pass reviewed the code it simplifies it down a bit and just makes it a bit more straight forward
  • (r-maint) Pass
  • (r-test) Pass

merging

[eileenmcnaughton]

thanks @seamuslee001

[magnolia61]

Would this solve the "Deprecated function: Non-static method CRM_Activity_BAO_Activity::checkEditInboundEmailsPermissions() should not be called statically in CRM_Activity_Page_Tab->preProcess()" I am getting, or am I utterly confused?

[eileenmcnaughton]

@magnolia61 no - it's unrelated - that function should probably be made static