Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PSHEASSN-517: Handle fees block in Event Info page #11

Open
wants to merge 5 commits into
base: PSHEASSN-463-member-and-non-member-pricing
Choose a base branch
from

Conversation

ahed-compucorp
Copy link
Contributor

@ahed-compucorp ahed-compucorp commented Apr 15, 2021

Overview

This PR is to alter the fee block in the Event Info page to show the right price field values depending on the current user's membership.

Created price set with two price field and each one has two options then configured members only event.
Screenshot from 2021-04-16 17-41-16
Screenshot from 2021-04-16 17-41-59

Before - Event Info

Event Info page will show all fee items.
Screenshot from 2021-04-16 17-37-47

After - Event Info

Event Info page will show fee items depending on the user's membership - below you will see non member price field values.
Screenshot from 2021-04-16 17-40-38

Before - Event Registration

There is no checkbox options.
Screenshot from 2021-04-16 17-39-24

After - Event Registration

Checkbox options are visible.
Screenshot from 2021-04-16 17-40-17

Technical Details

I used the hook_civicrm_pageRun hook to alter the Event Info page, but the values are already proccessed and sent to Smarty template.

Below you can see $_template->_tpl_vars['feeBlock'] value:

array(4) {
  ["value"]=>
  array(6) {
    [1]=>
    string(0) ""
    [2]=>
    string(12) "50.000000000"
    [3]=>
    string(12) "20.000000000"
    [4]=>
    string(0) ""
    [5]=>
    string(12) "50.000000000"
    [6]=>
    string(12) "20.000000000"
  }
  ["label"]=>
  array(6) {
    [1]=>
    string(26) "Event Fee - CheckBox Group"
    [2]=>
    string(23) "Non-member fee CheckBox"
    [3]=>
    string(19) "Member fee CheckBox"
    [4]=>
    string(23) "Event Fee - Radio Group"
    [5]=>
    string(20) "Non-member fee Radio"
    [6]=>
    string(16) "Member fee Radio"
  }
  ["lClass"]=>
  array(6) {
    [1]=>
    string(28) "price_set_option_group-label"
    [2]=>
    string(22) "price_set_option-label"
    [3]=>
    string(22) "price_set_option-label"
    [4]=>
    string(28) "price_set_option_group-label"
    [5]=>
    string(22) "price_set_option-label"
    [6]=>
    string(22) "price_set_option-label"
  }
  ["isDisplayAmount"]=>
  array(6) {
    [1]=>
    string(1) "1"
    [2]=>
    string(1) "1"
    [3]=>
    string(1) "1"
    [4]=>
    string(1) "1"
    [5]=>
    string(1) "1"
    [6]=>
    string(1) "1"
  }
}

As you can see the fee items include both price fields and price field values and it is bad idea to guess the ID from the label. Instead, the method getFeeItems will return fee items with their ID then altered $_template->_tpl_vars['feeBlock'] to hide price field values depending on user's membership.

Comments

This PR contains these small fixes/updates :

  • Fixed undefined property in BuildForm hook
  • Fixed a typo in the empty condition.
  • Fixed the missing checkbox options issue.
  • Updated multi-select label

@ahed-compucorp ahed-compucorp force-pushed the PSHEASSN-517-fix-bugs branch 6 times, most recently from 7fc8a4d to 8da8e62 Compare April 15, 2021 15:58
Copy link
Member

@omarabuhussein omarabuhussein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't understand the "technical details" of this ticket that are related to this commit 8da8e62 and not sure what are you trying to solve here, can you maybe try to word it differently, can you also try to write what is the issue from the user perspective in the "before" and "after" sections, because the screenshots alone does not tell much.

@@ -62,7 +62,14 @@ protected function buildForm($formName, &$form) {
continue 2;
}

$priceFieldValueID = $element->getAttribute('value');
if ($isCheckboxElement) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would alter this to :


  if ($isCheckboxElement) {
   // for civicrm versions >= 5.35.0
    $priceFieldValueID = $element->getAttribute('id');
  }
  else {
    // for civicrm versions < 5.35.0
    $priceFieldValueID = $element->getAttribute('value');
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I've updated my PR,
This is not CiviCRM 5.35.1 issue, I tried it on 5.28 and it seems that $element->getAttribute('value') for checkbox will always return "1"
Hence we need to use the id attribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants