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

[REF] return determination of whether to show expired fields to the calling function #15934

Merged
merged 1 commit into from
Nov 22, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Minor code cleanup towards larger effort of making amount handling accurately tested

Before

initEventFee determines whether to include expired fields based on a determination of which class is calling it

After

$includeExpiredFields is an input parameter

Technical Details

As part of my efforts to ensure we are consistency creating valid transactions I'm trying to sort out the way we calculate
amounts to be re-usable from tests but I feel stymied at every turn by spaghetti code.

This unravels a small piece

Comments

…alling function

As part of my efforts to ensure we are consistency creating valid transactions I'm trying to sort out the way we calculate
amounts to be re-usable from tests but I feel stymied at every turn by spaghetti code.

This unravels a small piece
@civibot
Copy link

civibot bot commented Nov 22, 2019

(Standard links)

@civibot civibot bot added the master label Nov 22, 2019
@@ -152,7 +157,8 @@ public function buildQuickForm() {

//retrieve custom information
$this->_values = [];
CRM_Event_Form_Registration::initEventFee($this, $event['id']);

CRM_Event_Form_Registration::initEventFee($this, $event['id'], $this->_action !== CRM_Core_Action::UPDATE);
CRM_Event_Form_Registration_Register::buildAmount($this, TRUE);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ie FALSE if $form->_action == CRM_Core_Action::UPDATE

@seamuslee001
Copy link
Contributor

Makes sense to me looks logically clear merging

@seamuslee001 seamuslee001 merged commit 47da5c2 into civicrm:master Nov 22, 2019
@seamuslee001 seamuslee001 deleted the part_sane branch November 22, 2019 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants