Improvement: make the Petition Contacts group non-public

[ken]

Currently the Petition logic adds signers of petitions to a group whose visibility is 'Public Pages' (the default name of this group is 'Petition Contacts'). The problem with this is that, being Public, it is possible for non-signers to subscribe or unsubscribe from the list. It seems unwise to expose business internals to the public in this way.

I've supplied a patch for v4.3 (below) that makes two changes, and which suggests a third change for consideration.

• Allow the option to limit the visibility of the group 'Petition Contacts' to 'User and User Admin Only'. Currently, making the group non-Public causes an error. I doubt anyone will object to this improvement.
• Going one step further, I suggest making the group 'User and User Admin Only' if CiviCRM has to create it on the fly. This is consistent with the behaviour when creating a new group via the UI, which defaults to 'User and User Admin Only'.
• The third change to consider is to modify the third argument to CRM_Mailing_Event_BAO_Subscribe::subscribe(). Currently the $context argument allows the check "this is a public group" to be skipped when the value of $context is set to 'profile'. It seems someone else needed to skip this check when Profiles are being used. Perhaps a better scheme is a boolean argument $groupMustBePublic which defaults to TRUE. To me, that's easier to comprehend than 'profile'.

The patch for 4.3 is ...

--- CRM/Campaign/BAO/Petition.php   2013-02-08 23:31:05.000000000 +1100
+++ CRM/Campaign/BAO/Petition.php   2013-03-08 17:36:01.082391611 +1100
@@ -515,7 +515,7 @@
     $dao->title = $petitionGroupName;
     if (!$dao->find(TRUE)) {
       $dao->is_active = 1;
-      $dao->visibility = 'Public Pages';
+      $dao->visibility = 'User and User Admin Only';
       $dao->save();
     }
     $group_id = $dao->id;
@@ -574,7 +574,8 @@
         // this will allow using a hash key to confirm email address by sending a url link
         $se = CRM_Mailing_Event_BAO_Subscribe::subscribe($group_id,
           $params['email-Primary'],
-          $params['contactId']
+          $params['contactId'],
+          'profile'
         );
 
         //    require_once 'CRM/Core/BAO/Domain.php';

[xavier]

Hey,

Some custom code might be using $context="profile", so we need to keep that, I doubt we could switch to a boolean.

This being said, agree that "profile" isn't super intuitive. What about we keep the "profile" as it is but introduce a new context="AllowRestricted" (or whatever the name you prefer) and document this one?

Could you as well modify the sql init to put the group petition as private?

X+

[nicolas]

Xav - Can you please elaborate on:
> Could you as well modify the sql init to put the group petition as private?

I have hit this issue and am in the process of creating the patch & JIRA ticket since it seems this bug was never addressed.

Ken - thanks for your patch, it fully resolved this issue.

[xavier]

I meant the upgrade/install sql that creates the group as public. As it works now as well with a private group, I don't see why it shouldn't be private by default.

X+

[nicolas]

Xavier - the group is created in the script, so there is no upgrade/install script that creates it.

Issue created and PR issued: http://issues.civicrm.org/jira/browse/CRM-13763

[ken]

Nicolas,

Thanks! I've only just got on to github, and making a PR for this was my next mini-project. Happy to be beaten to the punch! ;-)

Ken