Author Topic: CiviCRM Member renewal by different person (Read 1444 times)

Eileen · November 20, 2012, 07:00:57 pm

We have a requirement for someone other than the member to be able to renew their membership. At the moment this is blocked by the following code in ContributionBase.

Code: [Select]

        if ($membership->find(TRUE)) {
          $this->_defaultMemTypeId = $membership->membership_type_id;
          if ($membership->contact_id != $this->_userID) {
            require_once 'CRM/Contact/BAO/Relationship.php';
            $employers = CRM_Contact_BAO_Relationship::getPermissionedEmployer($this->_userID);
            if (array_key_exists($membership->contact_id, $employers)) {
              $this->_membershipContactID = $membership->contact_id;
              $this->assign('membershipContactID', $this->_membershipContactID);
              $this->assign('membershipContactName', $employers[$this->_membershipContactID]['name']);
            }
            else {
              CRM_Core_Session::setStatus(ts("Oops. The membership you're trying to renew appears to be invalid. Contact your site administrator if you need assistance. If you continue, you will be issued a new membership."));
            }
          }

It seems like there is a valid use case for anyone to renew the membership of someone they have permission over - ie. check all permissioned contacts not just permissionedEmployers. Adding a checksum doesn't seem to work - but neither does it seem correct since I have on good authority (petednz) that once you use a checksum you have a session as that person - which gives you their drupal priviledges as well as civi.

The next challenge here is that once you have been 'allowed' through you are still loading your own details rather than theirs into the profile form. We have been managing this by saving those profile fields to the person making the contribution and the gift membership fields to the subscriber. (The honor_contact_id and the membership_payment link the two - this pattern is in core for back-office)

e.g https://www.themonthly.com.au/civicrm/contribute/transact?reset=1&id=1

I guess having got past that permission check we could probably manage to load those defaults through the module (& possibly hide some for renewals).

But that leaves the question of how that would appear in core - since there is a somewhat generic possibility of someone renewing another person with the contribution being corrected associate with either of the contacts (ie. different sites might see which contact the contribution belongs to differently. It seems that actually the profile fields should relate to the contact doing the renewing (& probably a minimal form would make sense in most cases here.)

Eileen · November 23, 2012, 03:41:45 pm

11:44:31 AM) dgg: eileen: i stared at that forum post for a while and don't even understand the beginning context in terms of how the contribution form would identify that the logged in person is 'renewing' (since they themselves don't have a membership)
(11:45:10 AM) eileen: dgg - membership id in the url
(11:45:33 AM) eileen: (which is what happens for permissioned employers - but the function that checks for them is very specific)
(11:45:54 AM) dgg: and the url comes to them via email
(11:45:55 AM) dgg: ?
(11:46:04 AM) eileen: dgg yes
(11:46:35 AM) eileen: dgg - or the dashboard
(11:46:55 AM) dgg: and what's the downside(s) of expanding that check to 'any permissioned relationship' ?
(11:47:57 AM) eileen: dgg - I'm not sure - obviously there will probably be further complexities to be worked through
(11:48:47 AM) eileen: I guess the end site can control the fields that they can change by choosing which profiles to put on the page
(11:50:23 AM) eileen: Oh - the only thing is, in our context we are saving the main fields to the person accessing the form & the 'honor-of' fields to the recipient - which reflects the back-office entry
(11:51:08 AM) eileen: we have that happening through a module on post process - which is not too bad - whereas the blocker is in a function we can't easily avoid
(11:51:26 AM) dgg: eileen: this change would relate to how it works for the "employee renewing employer membership" flow - what happens with the profiles in core install in that flow?
(11:52:10 AM) eileen: The org uses an 'on behalf of' profile
(11:52:27 AM) eileen: so the main fields go to the person filling in the form - so that's not dissimilar
(11:54:44 AM) dgg: ok so if the permissionedEmployer check was opened up, the "On behalf profile" used for that page would need to work for the right type of contact.
(11:55:35 AM) dgg: i have no idea if you currently use an individual profile for "On Behalf" if that works or not.
(11:55:51 AM) eileen: dgg - yes, we were doing that (only in a module at the moment) & converting the honor contact fields to be a profile
(11:56:35 AM) eileen: because that is the underlying db structure we went with for back office on behalf of (because the built in on-behalf-of really implies an inherited membership)
(11:56:38 AM) dgg: eileen: but that's different from verifying if the core On Behalf functionality can work with an individual profile
(11:57:31 AM) eileen: dgg - yes, but what I mean - is that the actual core on behalf of is tied to some concepts specific to inherited membership
(11:58:16 AM) dgg: ok so are u proposing changing that as well?
(11:58:43 AM) eileen: dgg - for individual purchases I suspect it may be the easier path
(11:59:08 AM) eileen: not for organisations - because there is so much built into it I believe
(11:59:51 AM) dgg: well i think the code would also need to somehow still limit the permiisioned relationships it retrieves - since the profile type included in the contribution page needs to match the contact type of the 'related contact'
(12:00:05 PM) eileen: but, that brings us back to how you designate that the 1) this is the profile to use for honor contacts and 2) who gets the membership
(12:00:52 PM) dgg: eileen: which leads me to "doing this in core implies major refactoring of that form" - and not good candidate for 4.3
(12:01:13 PM) eileen: dgg - so I'm left with having to make it an extension
(12:01:57 PM) eileen: but, the problem I have is I don't know how to do it as an extension because I can't get past that piece of code unless I resort to overriding that php file in the extension
(12:02:13 PM) dgg: eileen: maybe totten will have an idea on that?
(12:03:00 PM) totten: dgg: need to read above
(12:03:15 PM) dgg: totten: thanks
(12:05:21 PM) eileen: customer is on 4.1 still so I guess I could just hack it & tell them that when they upgrade they will need to deal with it
(12:06:55 PM) eileen: (customer is pretty good at contributing to core - but obviously if you can't put something in core when they need the fix then you generally have to delay it until next time they need something)
(12:06:59 PM) totten: eileen: what's the file/code that needs to be overriden?
(12:07:16 PM) eileen: ContributionBase::PreProcess
(12:08:17 PM) totten: eileen: that function is 300 lines long!
(12:08:34 PM) colemanw: not cute
(12:08:52 PM) totten: what part(s) of the function?
(12:09:14 PM) eileen: totten - http://forum.civicrm.org/index.php/topic,26875.0.html
(12:09:37 PM) eileen: colemanw - not cute!
(12:09:45 PM) totten: eileen: thanks
(12:10:14 PM) colemanw: eileen: have you ever thought about how we might get sql triggers going on a test db?
(12:10:47 PM) colemanw: I just started writing a test for my phone trigger and realized that it wouldn't work so well on a db with no triggers!
(12:12:25 PM) eileen: colemanw - do we tell people they HAVE to have SUPER permission - hershel will be pleased if we up the minimum hosting requirements :-)
(12:13:00 PM) colemanw: eileen: right, but our unit test constructor fn doesn't actually build the triggers
(12:13:51 PM) eileen: colemanw - sorry - are you using triggers for 'is_primary' or is this you phone data thing
(12:14:04 PM) colemanw: phone_numeric trigger
(12:14:09 PM) eileen: & is it an optional thing?
(12:14:41 PM) colemanw: eileen: no
(12:15:18 PM) eileen: OK - so everyone will need mysql SUPER permission to run CiviCRM now?
(12:15:45 PM) eileen: (you should ask Hershel for some money towards that - improves his business :-)
(12:16:18 PM) colemanw: eileen: yes, I think so... we have quite a lot of triggers in 4.3, I don't think they're optional
(12:16:22 PM) eileen: colemanw - if it's compulsory then it should be part of the main UnitTest setup routine as it's part of createing a database
(12:17:01 PM) colemanw: I'll look into it
(12:17:39 PM) colemanw: eileen, totten: to your knowledge is there an api for triggerRebuild?
(12:18:08 PM) totten: colemanw: system.flush
(12:18:22 PM) eileen: colemanw there is a param for it - check getfields (hopefully)
(12:19:56 PM) totten: colemanw: but you generally don't want to do trigger rebuild for every unit-test -- just like you don't drop/create tables for every unit-test
(12:20:36 PM) eileen: totten - it can be part of populate once stuff
(12:20:51 PM) totten: you'd probably only do it if the triggers are really dynamic (diff behavior at diff times) -- and if you're trying to test that the trigger gets generated correctly in diff conditions
(12:21:01 PM) totten: eileen: yeah, prob a good balance
(12:21:32 PM) totten: eileen: re forum posting...
(12:21:55 PM) totten: it seems like one could naively take that fragment of code and hook-ify it
(12:22:10 PM) eileen: totten - as an aside - does this look right? http://pastebin.com/JWfUcj4Y
(12:22:42 PM) eileen: totten & you think I should consider that?
(12:22:42 PM) totten: re: forum with hook -- it's not great from a maintainability perspective, but it keeps the change small
(12:24:27 PM) totten: eileen: well, i don't know that part of the code well, but it seem slike the alternatives are to (a) refactor that 300-line behemoth (and its coconspirators) until you get something grokkable -- and then, once you have something grokkable, figure out how ot make the change
(12:25:10 PM) eileen: yes, I guess so - I guess the thing is the customer needs something for xmas gift renewals. They are on 4.1 & so perhaps whatever I do I'm just going to be hacking their core in some way & delaying dealing with it until they have money at a time when it's appropriate to build something into trunk
(12:25:26 PM) eileen: AND I have time
(12:25:31 PM) totten: or (b) work on adding the functionality to one of the alternative form-customization layers -- something that's intended to work with multiple entities (like webform_civicrm or the future profile revisions)
(12:26:23 PM) eileen: yes, and to be honest - I was going to do the refactoring of that form about 2 months ago - but thought I shouldn't because the profile stuff would be so re-worked in 4.3 anyway
(12:26:31 PM) totten: eileen: yeah, given your timeline, i can see how hacking a 4.1 fork or doing the hook would be the only real options
(12:28:29 PM) eileen: I guess with the hook I could go back a step & say "if mid is in the URL then call this hook & if it returns true then skip all the other validation"
(12:28:38 PM) eileen: which is a bit more generic
(12:29:30 PM) eileen: it would be potentially something like 'allowFormLoad('Contribution', $params)
(12:30:21 PM) totten: eileen: _civicrm_api3_system_flush_spec: yeah, looks about right. might want to say that the values are booleans.
(12:31:21 PM) eileen: then I'll have to look up what the type is for booleans
(12:32:04 PM) eileen: too lazy - I did it in the title
(12:32:07 PM) jamessha_ left the room (quit: Remote host closed the connection).
(12:32:26 PM) civi-bot: r43820 by eileen: add code documentation -- https://fisheye2.atlassian.com/changelog/CiviCRM?cs=43820
(12:33:21 PM) totten: eileen: re: membership forms: yeah, if you think that the hook isn't much more difficult than forking, then i'd probably do the hook
(12:33:35 PM) colemanw: ok I just added the system flush api call to the populateOnce stuff. I'm honestly not sure why it worked (because I assumed civicrm was not initialized at that point) but it does.
(12:34:28 PM) totten: but in terms of setting expectations -- the hook approach is probably only going to make the code more supportable for 4.2 or maybe 4.3 -- but long run probably won't help too much (since, as you say,, the section needs refactoring/bigger changes)
(12:34:59 PM) eileen: totten - yes, as long as it makes some sort of sense - & to be honest there are a few places in that form where loading gets blocked so it might be useful & even slightly generic -ie allowFormLoad might make sense in other places
(12:36:18 PM) totten: eileen: aside: if you find that there are several spots that need hooking in that file, then i'd probably use drupal's hook_foo($op) pattern rather than creating several hooks
(12:36:44 PM) totten: eileen: ok
(12:37:06 PM) eileen: totten - I was thinking about calling it once - e.g. $formAllowed = hook::AllowFormBuild & then using that var later in the file

CiviCRM Community Forums (archive)

News:

Author Topic: CiviCRM Member renewal by different person (Read 1444 times)

Eileen

CiviCRM Member renewal by different person

Eileen

Re: CiviCRM Member renewal by different person