oroinc / orocommerce

Main OroCommerce package with core functionality.
http:///www.orocommerce.com/
Other
193 stars 84 forks source link

Unable to make default data to null for QuantityType #146

Open chrisaligent opened 2 years ago

chrisaligent commented 2 years ago

I have a special case where my custom form needed to use an Oro QuantityType form type, however I needed it to be nullable (as it was an optional field).

https://github.com/oroinc/orocommerce/blob/master/src/Oro/Bundle/ProductBundle/Form/Type/QuantityType.php#L61

I examined the class and saw that it calls $event->setData(null); if the default_data option is null, however it doesn't work because the !is_numeric($defaultData) check a few lines above prevents this (null is not numeric). This caused my Quantity field to always default to zero if the user clears the value before persisting.

Potential remediation

I believe one of the three following changes is necessary:

  1. Remove the is_numeric() check entirely
  2. Expand it to allow null values (ie if (!is_numeric($defaultData) && !is_null($defaultData)))
  3. Remove the $event->setData(null); statement (and surrounding if statement) as it's entirely unreachable code
    public function setDefaultData(FormEvent $event)
    {
        $options = $event->getForm()->getConfig()->getOptions();

        $defaultData = $options['default_data'];
        if (!is_numeric($defaultData)) {
            return;
        }

        $data = $event->getData();
        if (!$data) {
            if ($defaultData === null) {
                $event->setData(null);
            } else {
                //number type expected string value in submitted data
                $event->setData((string)$defaultData);
            }
        }
    }

My IDE highlighted the same issue: 2022-03-15_15-52

Workaround

I have switched my Quantity field to use the NumberType with a Decimal constraint, this seems to be working for now. This might not be a suitable workaround if Product Unit Precision is required for this field, but that wasn't something I needed here.