spatie / laravel-medialibrary

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

Correctly search media conversion by name #3715

Closed StyxUA closed 1 week ago

StyxUA commented 1 month ago

This PR fixes ConversionCollection's behavior in finding conversion by name.

If a subject model has multiple collections, and multiple conversions with the same name, but applied to different collections, then ConversionCollection incorrectly determines the conversion which should be applied to the media.

Example:

public function registerMediaCollections(): void
{
    $this->addMediaCollection('avatar')->acceptsMimeTypes(['image/png'])->singleFile();
    $this->addMediaCollection('signature')->acceptsMimeTypes(['image/jpeg'])->singleFile();
}

public function registerMediaConversions(?Media $media = null): void
{
    $this
        ->addMediaConversion('preview')
        ->fit(Fit::Crop, 50, 50)
        ->performOnCollections('avatar')
        ->format('png');

    $this
        ->addMediaConversion('preview')
        ->fit(Fit::Crop, 300, 100)
        ->performOnCollections('signature')
        ->format('jpeg');
}

Now, whereas conversion files are generated correctly, their URLs and paths are wrong, because ConversionCollection doesn't honor media collection, getting just the first conversion which name matches:

$media = $model->getFirstMedia('signature');

ConversionCollection::createForMedia($media)
    ->getByName('preview') // <-- issue occurs here, first conversion with this name is fetched
    ->getManipulations()
    ->toArray();

Result:

[
  "format" => [
    "png",
  ],
  "fit" => [
    Spatie\Image\Enums\Fit {#4033
      +name: "Crop",
      +value: "crop",
    },
    50,
    50,
  ],
]

Generated URL is wrong too, because internally the fetched conversion is used to determine file extension:

$media->getFirstMediaUrl('signature', 'preview');

Result:

https://.../media/2/c/test-preview.png <-- should be .jpeg

This PR fixes this behavior. Specifically, ConversionCollection::getByName() function first filters conversions by media's collection, and only then searches by conversion name.

Tests included.

timvandijck commented 1 week ago

Thanks for the PR, looks like great work!