CRM-21009 Only count line items with a quantity towards max capacity

[JKingsnorth]

See https://issues.civicrm.org/jira/browse/CRM-21009

[eileenmcnaughton]

So, looking at this, that function is only called from one place -

Register::formatFieldsForOptionFull()
$recordedOptionsCount = CRM_Event_BAO_Participant::priceSetOptionsCount($form->_eventId, $skipParticipants);

& then that is used in
$dbTotalCount = CRM_Utils_Array::value($optId, $recordedOptionsCount, 0);

I'm feeling confused as to why the code is so complicated to get the total number of rows (& where the unit tests are!)

[JKingsnorth]

I think it's partly complicated by the fact that it needs to account for the spaces taken by the booking in progress, added to the number of 'counted' spaces taken by bookings in the database. That's why it's been taken out into its own function. I guess the author thought it could be reusable, hence all the parameters. Agreed that unit tests would be good here, to capture the different scenarios.

…

On 11 Aug 2017 23:20, "Eileen McNaughton" ***@***.***> wrote: So, looking at this, that function is only called from one place - Register::formatFieldsForOptionFull() $recordedOptionsCount = CRM_Event_BAO_Participant:: priceSetOptionsCount($form->_eventId, $skipParticipants); & then that is used in $dbTotalCount = CRM_Utils_Array::value($optId, $recordedOptionsCount, 0); I'm feeling confused as to why the code is so complicated to get the total number of rows (& where the unit tests are!) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#10805 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE-JuYKN2WbAtcTFKdjOXPeg680BKtxyks5sXNO0gaJpZM4OrDJ1> .

[eileenmcnaughton]

Right - we should ideally remove these 3 options from the function signature

$considerCounted = TRUE,
$considerWaiting = TRUE,
$considerTestParticipants = FALSE

As they are never passed in, and simplify the function

& presumably if we passed in an array of option ids then we could simply return an integer $count from the function & not create an array that needs to be iterated through to extract one value out of

[JKingsnorth]

OK, sounds fair @eileenmcnaughton . We should also write a test! But can we do those things as a follow-up to this issue? I think it would be good to get this fixed. I'm happy to work on the follow-up, but it doesn't seem as urgent. Let me know if you feel otherwise and I'll try to set aside some time to work on it sooner.

[JKingsnorth]

Jenkins retest this please

[JKingsnorth]

Jenkins retest this please

[eileenmcnaughton]

@monishdeb based on my understanding of how you are dealing with changes in line items this seems relatively high priority now. I'm going to merge this since it was previously blocked on 'it would be nice if you did more' and it seems to have been held up in that stage for rather too long.

[JKingsnorth]

Thanks, I'll create the follow up issues to tackle when I can.