Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly search media conversion by name #3715

Conversation

StyxUA
Copy link
Contributor

@StyxUA StyxUA commented Oct 20, 2024

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 timvandijck self-assigned this Nov 7, 2024
@timvandijck
Copy link
Member

Thanks for the PR, looks like great work!

@timvandijck timvandijck merged commit 9b24c13 into spatie:main Nov 8, 2024
9 checks passed
@StyxUA StyxUA deleted the fix/conversion-collection-finds-proper-conversion branch November 10, 2024 11:07
@timvandijck timvandijck mentioned this pull request Nov 15, 2024
timvandijck added a commit that referenced this pull request Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants