The hookElements are missing on membership forms. I only noticed them when testing on a form on an existing site (the form id was 12 so it didn't match the membership ids). I only missed this before since the two matched by coincidence.

The problem is that civievent_discount_civicrm_buildForm() tries to compare the form id with the membership ids active for the discount code. This works, I guess, for the event ids since there's a correlation of event id with the form id, but not for memberships. We need to compare the result of _get_discounted_membership_ids() and the member ids active on that particular form. If any of the member ids in the form match then show the discount code textbox. I hacked this together, which is around line 142:

  } elseif ($fname == 'CRM_Contribute_Form_Contribution_Main') {
    $ids = _get_discounted_membership_ids();
    $memtypes = explode(',',$form->_membershipBlock['membership_types']);
    // get the member ids, not form id, from the form and compare
    foreach ($memtypes as $k => $v) {
      if (in_array($v, $ids)) {
        $formid = $v;
      }
    }

Comments

dharmatech’s picture

dharmatech commented

Nubeli,

Which version of CiviCRM are you testing against?

nubeli’s picture

nubeli commented

Both my testing and the live are CiviCRM 3.3.5 - it only worked on the test site by coincidence.

dharmatech’s picture

dharmatech commented

So are you saying that membership forms are not implementing beginHookFormElements? Looking at the CiviCRM code does it seem like a bug/oversight? If so, we should submit a patch to their issue tracker.

nubeli’s picture

nubeli commented

It's an error in civievent_discount.module. The code I posted above fixed it for me. I believe you were originally just comparing the event id with the form id and they would match, but the same isn't true for the memberships. At least that's what I'm guessing, I didn't look at what the event forms are outputting.

dharmatech’s picture

dharmatech commented

Thanks for the comment.

I understand your code above fixed it for your situation. What I'm asking is if there's a better, more consistent way, of finding out what membership/event page we're looking at. If Event and Membership pages have two different methods perhaps it's an oversight in CiviCRM, in which case CiviCRM should be fixed and provided as a patch to their code base. In the process of building this module, we submitted several bug fixes to CiviCRM... most of them inconsistencies in how hooks are implemented. One outstanding issue is that membership forms don't call hook_civicrm_validate(). _postProcess() is messy because of the myriad of ways we have to check the type of form and it all seems very fragile to me.

I'll take a look at the issue over the next couple of days and see if there's a better way. If not, I'll check in your changes.

nubeli’s picture

nubeli commented

I don't think it's an oversight. The memberships can show up on any number of contribution forms so trying to just look for the form id is not going to help. You have to find the membership ids each and every time and then compare them to the membership ids stored in the discount code table. Each form can have one or more membership ids associated with it so if any one of them matches then we need to show the discount code field. So it doesn't look like an error in CiviCRM to me.

dharmatech’s picture

dharmatech commented
Assigned: Unassigned » dharmatech
Status: Active » Fixed

Fixed in latest -dev release. You may need to wait 24 hours for the next release.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.