[REF] Extract transferParticipantRegistration function #16976
Conversation
This is the extraction portion of civicrm#16956 On testing that I hit an error but getting the extraction merged first should make it easierr to review future iterations as it will reduce noise in the code. Note I made the function non-static since it is still on the form & passed in event ID since we have it but it's otherwise pretty much the same as the extraction part of that PR
|
(Standard links)
|
|
test this please |
| $params = [1 => [$fromParticipantID, 'Integer']]; | ||
| $dao = CRM_Core_DAO::executeQuery($query, $params); | ||
| //copy line items to new participant | ||
| $line_items = CRM_Price_BAO_LineItem::getLineItems($this->_from_participant_id); |
There was a problem hiding this comment.
weird to see the test included in this PR is passing without failure as I wrote it to assert the incorrect line items attached to the previous contribution id. On checking, it seems the test passes because the above variable $this->_from_participant_id arrives as empty while it is executed.
Shouldn't it be set to the function parameter $fromParticipantID so that it is also verified in testing process? Or else as this function isn't static anymore, I think we might need to update the test to have a value for $form-> _from_participant_id before calling this function. We might also need to remove the unnecessary params included in this function.
There was a problem hiding this comment.
@jitendrapurohit the test can be fixed to fail when we add the fix to fix :-)
|
@jitendrapurohit I will merge this as a reviewer commit if you are OK with it - per comments above we can break the tests in the next round :-) |
|
yes, sure. since this isn't breaking anything from UI side, I'm happy to cover the test part in another PR. Pls merge so I can update mine :) |
|
Done! - Make sure you check yours on non-text fields cos they failed for me |
Overview
This is the extraction portion of #16956
On testing that I hit an error but getting the extraction merged first should make it easier
to review future iterations as it will reduce noise in the code.
Before
One long function
After
transferParticipantRegistration extracted
Technical Details
Note I made the function non-static since it is still on the form & passed in
event ID since we have it but it's otherwise pretty much the same as the
extraction part of that PR
Comments