filament icon indicating copy to clipboard operation
filament copied to clipboard

Exception with not found model (soft deleted) with form and SelectInput with editOptionForm

Open gpibarra opened this issue 10 months ago • 2 comments

Package

filament/forms

Package Version

v3.2.40

Laravel Version

v11.5.0

Livewire Version

v3.4.11

PHP Version

PHP 8.3.6

Problem description

On a form with a SelectInput field with a BelongsTo relationship, using editOptionForm to edit the related record, on an edit page with an existing and related record, fails if the already related model does not exist (at the database level, records are related correctly and the contrains do not fail, but since the related model uses softDelete, the model does not exist in the relationship)

Expected behavior

The SelectInput field shows the id and not the name if the relationship model cannot be retrieved, this does not give an exception, but I don't think it is expected behavior. As expected, I think the field should be blank, and the edit option button should not be displayed.

Steps to reproduce


# composer create-project laravel/laravel .
curl -o filament-issue.zip https://filament-issue.unitedbycode.com/storage/filament-issue.zip
unzip filament-issue.zip
php artisan migrate
php artisan make:migration alter_users_table_column_soft_deletes --table=users
php artisan make:model Post -m

Changes in mgrations database/migrations/2024_04_25_150421_alter_users_table_column_soft_deletes.php

    public function up(): void
    {
        Schema::table('users', function (Blueprint $table) {
-             //
+             $table->softDeletes();
        });
    }
    public function down(): void
    {
        Schema::table('users', function (Blueprint $table) {
-             //
+             $table->dropSoftDeletes();
        });
    }

database/migrations/2024_04_25_150500_create_posts_table.php

    public function up(): void
    {
        Schema::table('users', function (Blueprint $table) {
            $table->id();
+             $table->string('title');
+             $table->text('content');
+             $table->foreignId('author_user_id')->constrained()->onDelete('cascade');
            $table->timestamps();
+             $table->softDeletes();
        });
    }

Changes in models app/Models/User.php


class User extends Authenticatable
{
    use HasFactory, Notifiable;
+     use \Illuminate\Database\Eloquent\SoftDeletes;
+ 
+     public function posts(): \Illuminate\Database\Eloquent\Relations\HasMany
+     {
+         return $this->hasMany(\App\Models\Post::class, 'author_user_id');
+     }

    /**

app/Models/Post.php


class Post extends Model
{
    use HasFactory;
+    use \Illuminate\Database\Eloquent\SoftDeletes;
+ 
+     protected $fillable = [
+         'title',
+         'content',
+         'author_user_id',
+     ];
+ 
+     public function author(): \Illuminate\Database\Eloquent\Relations\BelongsTo
+     {
+         return $this->belongsTo(\App\Models\User::class, 'author_user_id');
+     }
}

Run migrations

php artisan migrate

Create another user and post

php artisan tinker --execute="App\Models\User::make(['name' => 'test', 'email' => 'test@mail', 'password' => bcrypt('test')])->save();"
php artisan tinker --execute="App\Models\Post::make(['title' => 'First Post', 'content' => 'This is the first post', 'author_user_id' => 2])->save();"

Create resourse

php artisan make:filament-resource Post

Changes in app/Filament/Resources/PostResource.php

    public static function form(Form $form): Form
    {
        return $form
            ->schema([
-                 //
-           ]);
+                 Forms\Components\TextInput::make('title')
+                     ->label('Title')
+                     ->string()
+                     ->maxLength(100)
+                     ->required(),
+                 Forms\Components\Textarea::make('content')
+                     ->label('Content')
+                     ->string()
+                     ->maxLength(5000)
+                     ->rows(10)
+                     ->required(),
+                 Forms\Components\BelongsToSelect::make('author_user_id')
+                     ->label('Author')
+                     ->relationship('author', 'name')
+                     ->searchable(true)
+                     ->native(false)
+                     ->editOptionForm([
+                         Forms\Components\TextInput::make('name')
+                             ->label('Name')
+                             ->string()
+                             ->maxLength(255)
+                             ->required(),
+                     ])
+                     ->required(),
+             ])
+             ->columns(1);
    }

    public static function table(Table $table): Table
    {
        return $table
            ->columns([
-                 //
+                 Tables\Columns\TextColumn::make('title')
+                     ->searchable()
+                     ->sortable(),
+                 Tables\Columns\TextColumn::make('content')
+                     ->searchable()
+                     ->sortable(),
+                 Tables\Columns\TextColumn::make('author.name')
+                     ->label('Author')
+                     ->badge()
+                     ->color('info')
+                     ->searchable()
+                     ->sortable(),
+                 Tables\Columns\TextColumn::make('created_at')
+                     ->label('Created')
+                     ->sortable()
+                     ->toggleable()
+                     ->toggledHiddenByDefault(),
            ])
            ->filters([
                //
            ])
            ->actions([
                Tables\Actions\EditAction::make(),
            ])
            ->bulkActions([
                Tables\Actions\BulkActionGroup::make([
                    Tables\Actions\DeleteBulkAction::make(),
                ]),
            ]);
    }
  1. Go to http://127.0.0.1:8000/posts/1/edit. This work.

Delete user test@mail (soft deleted)

php artisan tinker --execute="App\Models\User::query()->where('email', 'test@mail')->first()?->delete();"
  1. Go to http://127.0.0.1:8000/posts/1/edit. This don't work
Filament\Forms\Components\Select::getEditOptionActionFormData(): Return value must be of type array, null returned

Restore user test@mail (soft deleted)

php artisan tinker --execute="App\\Models\\User::query()->withTrashed()->where('email', 'test@mail')->first()?->restore();"
  1. Go to http://127.0.0.1:8000/posts/1/edit. This work.

Reproduction repository

https://github.com/gpibarra/filament-issue-form-BelongsToSelect--with-editOptionForm

Relevant log output

No response

gpibarra avatar Apr 25 '24 16:04 gpibarra

@gpibarra Hi! The issue occurs because you are trying to view the author of a deleted post, but the author_user_id is not set to null. Therefore, you need to either delete all posts for this author or set the author_user_id to null using model events, for example.

In my opinion, this is not a Filament issue.

dmitry-udod avatar May 02 '24 13:05 dmitry-udod

In my opinion, filament should detect this situation (the field value has data, but the model is not loaded, in this case, because it is deleted with soft delete), in the function

https://github.com/filamentphp/filament/blob/a873b859c365804479b69f4ef573942abeeb2679/packages/forms/src/Components/Select.php#L413

does not detect this situation, then advances to ->fillForm($this->getEditOptionActionFormData()) and finally executes

https://github.com/filamentphp/filament/blob/a873b859c365804479b69f4ef573942abeeb2679/packages/forms/src/Components/Select.php#L1000


I think at some point I should check this, so as not to try to render an 'edit option' if the option is not available... and even. Likewise, the value to be displayed in these cases, in my opinion, should be a '' instead of the model id since the name cannot be loaded.


Finally, the edit-option actions at https://github.com/filamentphp/filament/blob/a873b859c365804479b69f4ef573942abeeb2679/packages/forms/src/Components/Select.php#L429 and create-option at https://github.com/filamentphp/filament/blob/a873b859c365804479b69f4ef573942abeeb2679/packages/forms/src/Components/Select.php#L275 should have a label so that this text can be changed by translations

gpibarra avatar May 02 '24 15:05 gpibarra

I agree that this isn't really a problem with Filament, an error is a good thing if the data doesn't make sense (records are related to deleted records), and I can't imagine why an observer cannot be used to remove the bad data there. Feel free to push back with other reasoning and I can reconsider and reopen if appropriate.

danharrin avatar May 13 '24 10:05 danharrin

I agree that the data doesn't make sense. The question is whether filament should throw an exception or not, or whether it should hide the edit button in this case

gpibarra avatar May 13 '24 12:05 gpibarra

Having the exception in your error tracker could point you to an actual problem with your app, instead of staying silent about it

danharrin avatar May 13 '24 13:05 danharrin