CRM-13827 Contribution form doesn't assign is_quick_config variable to smarty template

    Details

      Description

      CRM/Contribute/Form/Contribution/MembershipBlock.tpl opens by checking the value of $is_quick_config, but this is never passed to the template by CRM/Contribute/Form/Contribution/Main.php.

        Attachments

          Activity

          [CRM-13827] Contribution form doesn't assign is_quick_config variable to smarty template
          Frank J. Gómez added a comment -

          Looks like the real problem is that the smarty variable name was changed from is_quick_config to quickConfig. Another patch is forthcoming.

          Frank J. Gómez added a comment -

          PR updated.

          Donald A. Lobo added a comment -


          dgg: might want to check if we use is_quick_config in certain workflows and quickConfig in others

          David Greenberg added a comment -

          The referenced PR is 'empty' on GitHub:
          https://github.com/civicrm/civicrm-core/pull/2083

          ... and the current code in 4.4 still has this:

          {if !empty($useForMember) AND !$is_quick_config}

          Is there another PR ? I don't see it.

          Separately, there's one instance of a different form of the quick config boolean here:

          ./Event/Page/EventInfo.tpl:194:

          {if $isQuickConfig && $lClass EQ "price_set_option_group-label"}

          Can you check and see if there's a similar problem in that code?

          Frank J. Gómez added a comment -

          Dave, it looks like Coleman already merged my PR. Perhaps that's why it shows empty to you? Weird.

          I checked the EventInfo.tpl and the variable is being passed correctly here: https://github.com/civicrm/civicrm-core/blob/232624b1bfe3beee7476775b278b9509e1607487/CRM/Event/Page/EventInfo.php#L153. I'd say we're good here.

          Frank J. Gómez added a comment -

          I guess I looked at that too quickly. I see that Coleman merged a PR with zero commits, additions, subtractions, etc. I'm not sure what happened there. Here's a new PR: https://github.com/civicrm/civicrm-core/pull/2108.

          David Greenberg added a comment - - edited

          Frank - please clarify what workflow is broken in the current codebase due to the misnamed variable. I can't QA the fix w/o understanding the initial problem behavior AND also w/o that background I can't understand why we need new code in Main.php preProcess to handle radio buttons (https://github.com/civicrm/civicrm-core/pull/2087/files)

          Frank J. Gómez added a comment -

          I commented on the workflow in the other issue here: http://issues.civicrm.org/jira/browse/CRM-13831?focusedCommentId=54481&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-54481.

          Here's an another thought: we could simply decide that everything is working as it should be. If we do so, we should delete everything between lines 160 and 238 on https://github.com/civicrm/civicrm-core/blob/232624b1bfe3beee7476775b278b9509e1607487/templates/CRM/Contribute/Form/Contribution/MembershipBlock.tpl#L160, since this is an impossible condition ($is_quick_config can never be true if it isn't set). We can also delete all instances of !$is_quick_config from conditional statements, as these currently always evaluate to false. This makes me a little uneasy, but if this conditional block has been inaccessible for a while (I'm not sure how long) and no one has missed it, then maybe we don't really need it and we should consider it an opportunity to get rid of some cruft.

          David Greenberg added a comment -

          When using Quick Config membership signup (e.g. sample contribution page id=2), the patch causes text in the 'Membership Levels' section on top of the Confirm and Thank-you pages to disappear (this shows the selected membership level before the patch is applied). See screenshots with and w/o the patch.

          I'm also getting a fatal submitting the Confirmation page when using a membership price set to renew an existing membership. The referenced lines show as recent changes by you in git blame:

          Recoverable fatal error: Argument 1 passed to CRM_Member_BAO_Membership::updateRecurMembership() must be an instance of CRM_Member_BAO_Membership, instance of CRM_Member_DAO_Membership given, called in /Users/dgg/git/crm_v4.5/CRM/Member/BAO/Membership.php on line 1367 and defined in CRM_Member_BAO_Membership::updateRecurMembership() (line 1490 of /Users/dgg/git/crm_v4.5/CRM/Member/BAO/Membership.php).

          Frank J. Gómez added a comment -

          Thanks for your careful Q/A of this, Dave. Running through your Q/A steps with

          {debug}

          added to templates/CRM/Contribute/Form/Contribution/MembershipBlock.tpl, I see that $is_quick_config is never defined when presenting the form to the user, as I initially reported.

          However, the follow up screens (Confirm, etc.) do define the variable. I didn't realize the TPL was being used in these additional contexts. My proposed patch does indeed cause breakage later in the workflow.

          I'm closing this ticket with a resolution of "Won't Fix" because I'm not sure there's anything to do here and because "Knucklehead" isn't a resolution option. I'll go ahead and withdraw my PR as well.

            People

            • Assignee:
              Frank J. Gómez
              Reporter:
              Frank J. Gómez

              Dates

              • Created:
                Updated:
                Resolved: