multiEnum Validation in OroCRM 6.1

[jimohalloran]

Summary
Using the count validation on a multiEnum field no longer works in Oro 6.1. Trying to investigate why, led to the #1123 issue. This seems somewhat related, in that vales seem to be assigned to the multiEnum field, but then the get call returns the original value. However, I think this is a separate issue since it has some different presentations.

Steps to reproduce
I've packaged everything required to demonstrate this issue into a self contained bundle: https://github.com/jimohalloran/oro-serialized-data-multienum

• Obtain the bundle linked above, and place it into the src/ folder of an OroCRM 6.1 instance.
• Run migrations (oro:migration:load) and load fixtures (oro:migration:data:load)
• Log into the admin UI, and access the CRUD interface for the Demo entity (theres a new "Demo" option at the tiop of the "System" menu.
• Create a new "Demo" entity. Attempt to save the entity with no letters selected. Note that there is validation in place to prevent this.
• Save your new entity with one or more letters selected.
• Now, from the datagrid edit the entity you just created and untick all of the letter checkboxes.
• Save the entity again.

This can also be reproduced in unit tests by:

• Obtain and install the bundle as above, run migrations and fixtures.
• Enable the Oro 6.1 unit test by renaming src/Acme/Bundle/SerializeBundle/Tests/Unit/DemoEntity61Test.disabled to src/Acme/Bundle/SerializeBundle/Tests/Unit/DemoEntity61Test.php
• Run the unit tests, note that the testRemoveMultiEnum test fails. If other tests fail that is issue Access/Storage for multiEnums in Oro 6.1 #1123 which may not be the same issue as this one.

Actual Result
When editing existing entities the validation for a minimum count of letters is not enforced.

Test number 5 should not fail.

Expected Result
The validation.yml file defines validation for the letters field as follows:

    letters:
      - Valid: ~
      - Count:
          min: 1

The system should be enforcing a minimum count on both new and existing entities, however the validation only seems to work on new entities.

In the unit test, removing an item should remove it from the array and decrease the count when you call the getter to retrieve the array.

Details about your environment

• OroPlatform version: 6.1.0
• PHP version: 8.4.6
• Database (MySQL, PostgreSQL) version: PostgreSQL 15.4
• [Optional] Server operating system: Ubuntu 24.04

Additional information
I like to set a breakpoint on line 41 of \Symfony\Component\Validator\Constraints\CountValidator so that I can see the value that is being validated and the resulting count. For the new entity $value is the selected enums (as expected). However when editing an existing entity it looks as though the system assigns the correct data to the entity but the old value is what is validated.

If you install this bundle in OroCRM 6.0 the validation on both new and existing entities works as expected. In OrmCRM 6.1 only new entities are validated correctly. The difference with this issue vs #1123 is that it doesn't matter whether an upgrade was performed or not, the validation is always buggy in 6.1. So I've raised this separately because #1123 is an upgrade only issue, whereas this is always present in on 6.1, so although the symptoms are similar, it might be a different issue.

I know the form infrastructure and validation is all Symfony, however I don't think it's an issue with Symfony code. The issue is oro version specific and works fine with new entities. I suspect there's an issue with the way ExtendEntityTrait is handling the data, not with the actual form or validation thereof.

This is all basic form and validation stuff, so the bundle should work on Oro Platform or OroCommerce. But I've been working in an OfoCRM instance, so I know the navigation, etc provided by the bundle works correctly with OroCRM.

[jimohalloran]

Two test cases I've run through the UI so far. Start both by creating an entity with Alpha selected, and set a breakpoint in CountValidator.

1. Edit the entity, un-check Alpha and save. Saving the entity with no letters selected should result in $value being empty when the breakpoint is hit. Instead $value is a 1 element array containing the EnumOption for alpha. Continuing from the breakpoint we expect validation to fail (zero count should add a violation), but instead the entity is saved.
2. Starting from an entity with Alpha selected again. Edit the entity, uncheck Alpha and check Bravo and Charlie, then Save. When the breakpoint is hit we would expect a two element Array (Bravo and Charlie), but instead we have three (Alpha, Bravo and Charlie).

In both cases it seems like the removeLetter call is not updating the same storage as the get call returns. Somewhere during the request the serialized_data is copied to serialized_normalized, and removeLetter updates the serialized_data, but serialized_normalized is untouched and that's what the get call is returning to the validator.

The issue only appears when editing an entity because there is no data to remove on a new entity. Similarly if you edit the entity without unchecking any of the existing letters I'd expect (but haven't tested) the correct validator to get the correct data.

I'd like to demonstrate this via a unit test, but I don't have one ready just yet.

[jimohalloran]

Ok, the unit test was much simpler than I expected...

    public function testRemoveMultiEnum()
    {
        $demo = new Demo();
        $demo->addLetter($this->a);

        $this->assertCount(1, $demo->getLetters());

        $demo->removeLetter($this->a);
        $demo->addLetter($this->b);
        $demo->addLetter($this->c);

        $this->assertCount(2, $demo->getLetters());
    }

The second assertion fails (expected count 2, actual count 3). I've added this test to my demo bundle.

[jimohalloran]

See oroinc/OroEntitySerializedFieldsBundle#5 for a proposed fix. It looks like there's some simple issues with the removeMultiEnumOption method of \Oro\Bundle\EntitySerializedFieldsBundle\EntityExtend\SerializedEntityFieldExtension. I assume the two code blocks that update $serialized and $normalized (between lines 182 and 195) should be identical. However there were two small differences that prevented the $normalize update from working. Merging this PR resolves this and allows the test above to pass.

[timofiyprisyazhnyuk]

Yes, you are right, I already have a similar solution to fix it, which I indicated in #1123. I will also attach a patch with the solution here.

fix_serialized_normalized_enum.zip

[jimohalloran]

I backed out my temporary fixes and installed the patches using the composer-patches plugin yesterday, then migrated a fresh database. The issues I reported have been resolved. Based on my review of the patches, the solution looks solid. Looking forward to seeing these fixes in a future release!