CRM-17267 Fix change event selections impossible for sold out options

[JKingsnorth]

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

[JKingsnorth]

Just need to double check this use of is_full:
https://github.com/JKingsnorth/civicrm-core/blob/f8f517faebfcfac374d2908e867f572cba74c073/CRM/Event/Form/Registration/Register.php#L228

[JKingsnorth]

Yes, this is ready for QA

[JKingsnorth]

Jenkins, would you be so kind as to retest this please?

[eileenmcnaughton]

@JKingsnorth I tried to replicate this & failed - the screenshot is without - you can see I have the option of deselecting

I DID hit #11934

[JKingsnorth]

Clarified the steps to recreate on JIRA. Try it with a 'checkboxes' and a 'required' field.

[eileenmcnaughton]

Ok I can see it now

BEFORE

AFTER

What bothers me about this is that
a) we are allowing back office user to select a sold out option - I can see a case for and against this but
b) we not notifiying them in any way the difference between them submitting the form with the sold out option STILL selected (ie. sold out total NOT exceeded) and the sold out option NEWLY selected (ie if they submit the form the sold out total WILL be exceeded)

I feel like we need to mark this pretty clearly in the UI if we are going to make this change. Perhaps a confirm popup on submit (although my recollection is we don't have a good easy way to do this) if sold out is exceeded or js to add some red text beside the option that will exceed saying 'if you submit this the event will be oversubscribed'

ping @agh1 @Stoob @jusfreeman @colemanw if anyone has thoughts

[JKingsnorth]

Hmn I thought I'd configured this to only 'unfreeze' the sold out option if it was 'ticked' - ie: to allow people to deselect it, but not select it for new people.

Apparently that's not what's happening. But I think that would be a good solution that would prevent the event becoming oversubscribed?

Edit: It's been a while since I looked at this, so I'd need to get my head back into it again.

[agh1]

@JKingsnorth that makes sense to have the option only be available if it's already selected.

If that's not feasible, or if you prefer, one idea might be to have a checkbox called "Show sold-out options". That might act a little like the glass you need to break before pulling the fire alarm.

[monishdeb]

@JKingsnorth lets change it 1 line:

$overrideIsFull = (in_array($className, ['CRM_Event_Form_Participant', 'CRM_Event_Form_ParticipantFeeSelection']));

[eileenmcnaughton]

@monishdeb you mean change that one line in this PR?

[eileenmcnaughton]

oh I see - quicker way to say it

[monishdeb]

Yes, but this is no longer required if we move this check below at L739 :)

[eileenmcnaughton]

@monishdeb I think if ($form->isBackOffice) would work :-)

[monishdeb]

Umm nope as CRM_Event_Form_ParticipantFeeSelection doesn't inherit that property. Unless we should add that property to CRM_Core_Form and let the child form decide wether they like to be in front or as a silent worker

[eileenmcnaughton]

We can just add it to CRM_Event_Form_ParticipantFeeSelection I guess - if it IS always backoffice?

[monishdeb]

Oh we can further reduce it too:
$option['is_full'] = (in_array($className, ['CRM_Event_Form_Participant', 'CRM_Event_Form_ParticipantFeeSelection'])) ? FALSE : $isFull;

[eileenmcnaughton]

@monishdeb so the issue here is that it unlocks the full options when not selected too I think - but if we could resolve a new PR & get it merged....

[eileenmcnaughton]

@monishdeb I'm inclined to close this in favour of a much simpler PR

     $defaultPricefieldIds = array();
     if (!empty($form->_values['line_items'])) {
       foreach ($form->_values['line_items'] as $lineItem) {
-        $defaultPricefieldIds[] = $lineItem['price_field_value_id'];
+        if ($lineItem['qty'] > 0) {
+          $defaultPricefieldIds[] = $lineItem['price_field_value_id'];
+        }
       }
     }
     if (!$priceSetId ||
@@ -724,7 +726,7 @@ class CRM_Event_Form_Registration_Register extends CRM_Event_Form_Registration {
         ) {
           $isFull = TRUE;
           $optionFullIds[$optId] = $optId;
-          if ($field['html_type'] != 'Select') {
+          if ($field['html_type'] != 'Select' && $field['html_type'] !== 'CheckBox') {
             if (in_array($optId, $defaultPricefieldIds)) {
               $optionFullTotalAmount += CRM_Utils_Array::value('amount', $option);
             }

What do you think?

[monishdeb]

@eileenmcnaughton that patch makes sense to me. Can you create a new PR with before/after screenshot. Will then close it in favor of that PR.

[JKingsnorth]

I'm happy to review and test the new PR, link it from here when it's up and I'll take a look.

[eileenmcnaughton]

@JKingsnorth @monishdeb done #13096

I'll close this in favour of that