filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
17.73k stars 2.78k forks source link

Repeater with TextInput field will always fail Livewire assertFormSet assertion due to mismatch between expected data array key name and Filament generated key with UUID #10246

Closed sethjamesofficial closed 4 months ago

sethjamesofficial commented 9 months ago

Package

filament/filament

Package Version

v3.0.56

Laravel Version

v10.24.0

Livewire Version

v3.0.5

PHP Version

PHP 8.2.11

Problem description

WALKTHROUGH

While writing unit tests for an application using Fliament I discovered an issue which prevents proper use of Livewire assertions to assert the contents of a form utilising repeaters.

The section of the form I am testing in the Edit view for a Resource concerns a keywords property. The Model's keywords property is a JSON array described as $table->json('keywords')->nullable() in the database migration and $casts = ['keywords' => 'array'] on the Model itself.

Below is the relevant section of the Resource's form() method:

Forms\Components\Section::make('Keywords')
    ->schema([
        Forms\Components\Repeater::make('keywords')
            ->simple(Forms\Components\TextInput::make('name'))
            ->default([''])
            ->nullable(),
    ]),

(NB: the default(['']) is present to fix the issue described here, however the issue I am describing occurs with or without this value present)

Which results in the following view in the Edit page, here with data loaded: image

Developer testing and UAT shows that this portion of the interface works as expected, users are able to add new entries to the keywords array, which is stored correctly in the DB and can be displayed correctly with the View action and updated with the Edit action that I am attempting to test here.

Below is the truncated feature test associated with the Page which lead to the discovery of this issue:

//simple one dimentional array e.g. ['celery', 'celery & celeriac']
$expectedKeywords = $allergen->keywords;

Livewire::test(AllergenResource\Pages\EditAllergen::class, [
    'record' => $allergen->getRouteKey(),
])
    ->assertSuccessful()
    ->assertFormSet([
        //array map used to create an associative array consistent with state of the keywords property in the form
        //[
        //    'name' => 'celery',
        //    'name' => 'celery & celeriac',
        //]
        'keywords' => array_map(fn ($value) => ['name' => $value], $expectedKeywords),
    ])

Running this test results in the following output:

PHPUnit 10.3.5 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.11
Configuration: /opt/project/phpunit.xml

Failed asserting that null matches expected 'celery'.
Expected :celery
Actual   :null

This is where the issue presents itself. Firing up xdebug and inspecting the codepath that leads to this exception led to the following: image Of note in the image above is the key of the form property to be asserted: data.keywords.0.name

I noticed that this is incongruent with how filament handles repeaters. Inspecting the repeater element reveals that each child element of the repeater is in fact identified with a UUID, not a sequential index. Illustrated: image

This was confirmed by inspecting the state of the repeater in debugger and it's component: image (NB: The above is found in the debugger state under $this {Livewire\Features\SupportTesting\Testable} -> lastState {\Livewire\Features\SupportTesting\ComponentState} -> component {[...resource path...]\Pages\EditAllergen} -> data {array})

From following the codepath via the debugger, it follows that the debugger is making an assertion against an expected entry in this array listed under data.keywords.0.name but because of filament's use of UUIDs instead of indexes, this value is not present when the data_get() method is called as part of the assertion, returning instead the null value that is retrned as the "actual" value in the assertion, even though no null value is present in the page.

As a quick test of this, we can set the value of $key at runtime in the debugger. With the original value of data.keywords.0.name we encounter the exception which would lead to the failed assertion: image

While setting the value of $key to match one of the UUIDs present in data array as described above returns the expected Testable result consistent with a passed assertion: image

SUMMARY

Filament's use of UUIDs to identify the child elements of a Repeater in a Form results in an incompatability with the assertion methods provided as part of Livewire's unit testing suite which instead expects a standard indexed array. Confirmed above in the case of a single repeater with a TextInput form field.

Expected behavior

Test should pass

Steps to reproduce

1) Create Resource with form containing Repeater with TextInput 2) Write test to assert contents of Repeater child elements 3) observe that assertion always fails

See reproduction repository for full code examples, see issue description for more detail on source of the issue

Reproduction repository

https://github.com/sethjamesofficial/filament-repeater-feature-testing-issue-repo-11-12-23

Relevant log output

No response

mokhosh commented 6 months ago

@danharrin do you think this will make it to v4? We're basically not testing any of the repeaters now.

danharrin commented 6 months ago

I don't see a reason why it shouldn't be in v3

mokhosh commented 6 months ago

@danharrin that would be awesome, just put it that way because of the low priority tag on the issue.

danharrin commented 6 months ago

It belongs to the v3 milestone though

mokhosh commented 6 months ago

my bad. thanks.

danharrin commented 4 months ago

12759 adds additional documentation and a new ::fake() method for testing without UUIDs