-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Builds on #27049 to extract getPriceSetID() #27050
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -124,7 +124,6 @@ public function setDefaultValues() { | |
$params = ['id' => $this->_participantId]; | ||
|
||
CRM_Event_BAO_Participant::getValues($params, $defaults, $ids); | ||
$priceSetId = CRM_Price_BAO_PriceSet::getFor('civicrm_event', $this->_eventId); | ||
|
||
$priceSetValues = CRM_Event_Form_EventFees::setDefaultPriceSet($this->_participantId, $this->_eventId, FALSE); | ||
$priceFieldId = (array_keys($this->_values['fee'])); | ||
|
@@ -172,7 +171,7 @@ public function buildQuickForm() { | |
//retrieve custom information | ||
$this->_values = []; | ||
|
||
CRM_Event_Form_Registration::initEventFee($this, $event['id'], $this->_action !== CRM_Core_Action::UPDATE, $this->getDiscountID()); | ||
CRM_Event_Form_Registration::initEventFee($this, $event['id'], $this->_action !== CRM_Core_Action::UPDATE, $this->getPriceSetID()); | ||
CRM_Event_Form_Registration_Register::buildAmount($this, TRUE); | ||
|
||
if (!CRM_Utils_System::isNull($this->_values['line_items'] ?? NULL)) { | ||
|
@@ -426,4 +425,30 @@ public function getEventID(): int { | |
return $this->_eventId; | ||
} | ||
|
||
/** | ||
* Get the price set ID for the event. | ||
* | ||
* @return int|null | ||
* | ||
* @api This function will not change in a minor release and is supported for | ||
* use outside of core. This annotation / external support for properties | ||
* is only given where there is specific test cover. | ||
* | ||
* @noinspection PhpUnhandledExceptionInspection | ||
* @noinspection PhpDocMissingThrowsInspection | ||
*/ | ||
public function getPriceSetID(): ?int { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably use this at 127 as well for consistency. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, $priceSetId is unused, we can just remove 127. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done! |
||
if ($this->getDiscountID()) { | ||
$priceSetID = CRM_Core_DAO::getFieldValue('CRM_Core_BAO_Discount', $this->getDiscountID(), 'price_set_id'); | ||
} | ||
else { | ||
$priceSetID = CRM_Price_BAO_PriceSet::getFor('civicrm_event', $this->getEventID()); | ||
} | ||
// Currently some extensions, eg. civi-discount, might look for this. Once we can be | ||
// sure all financial forms have the api-supported function `getPriceSetID` we can | ||
// add some noise to attempts to get it & move people over. | ||
$this->set('priceSetId', $priceSetID); | ||
return $priceSetID; | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it makes any difference as things stand now, but shouldn't it be $form->getPriceSetID for consistency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm - I've been trying to go the other way for consistency - ditch the use of form & even the passing of it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. I guess it doesn't matter here as long as this doesn't get called from anywhere else, just looks a bit odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is affected or not https://github.com/eileenmcnaughton/civicrm-core/blob/master/tests/phpunit/CRM/Event/Form/ParticipantTest.php#L360
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seamuslee001 yeah that needs cleaning up - although as long as this PR passes tests it is out of scope for it