spatie / laravel-medialibrary

Associate files with Eloquent models
https://spatie.be/docs/laravel-medialibrary
MIT License
5.78k stars 1.08k forks source link

Bug Report - Undefined Variable $model in Laravel Media Library Command #3383

Closed eisler closed 11 months ago

eisler commented 1 year ago

Bug Report

Summary:

When running the php artisan media-library:clean --dry-run command in a production environment, an error occurs due to an undefined variable $model. This error prevents the command from executing successfully.

Steps to Reproduce:

  1. Run the following command in a Laravel application with the Spatie Media Library package installed:
php artisan media-library:clean --dry-run
  1. When prompted with the question, "Do you really wish to run this command? (yes/no) [no]," respond with "yes."

Expected Results:

The media-library:clean command should run in dry-run mode without errors and provide a summary of the media files that would be deleted without actually deleting them.

Actual Results:

An ErrorException occurs with the message "Undefined variable $model" as shown below:

ErrorException

  Undefined variable $model

  at vendor/spatie/laravel-medialibrary/src/Conversions/ConversionCollection.php:65
     61▕          * In some cases the user might want to get the actual model
     62▕          * instance so conversion parameters can depend on model
     63▕          * properties. This will causes extra queries.
     64▕          */
  ➜  65▕         if ($model->registerMediaConversionsUsingModelInstance && $media->model) {
     66▕             $model = $media->model;
     67▕
     68▕             $model->mediaConversions = [];
     69▕         }

      +21 vendor frames
  22  artisan:37
      Illuminate\Foundation\Console\Kernel::handle()

Additional Information:

freekmurze commented 1 year ago

Feel free to PR a fix for this!

maximkou commented 1 year ago

@eisler Hi! Looks like, you/someone else have changed this file manually. Please, check it.

Why do I think so? See https://github.com/spatie/laravel-medialibrary/blob/10.7.10/src/Conversions/ConversionCollection.php#L63 (your package version source)

  1. $model variable defined always - without any conditions
  2. in your trace condition 65▕ if ($model->registerMediaConversionsUsingModelInstance && $media->model) { is on line 65, but in original repository this condition in line 63
eisler commented 1 year ago

Upgrading spatie/laravel-medialibrary (10.7.10 => 10.11.3)

I apologize for the incorrect version mentioned in the initial bug report. The correct version where this issue occurred is 10.11.3.

https://github.com/spatie/laravel-medialibrary/blob/10.11.3/src/Conversions/ConversionCollection.php#L65

maximkou commented 1 year ago

This is already fixed in 10.11.4