From bb26bf6ebcc124e323ff6524fb42d776b23fcbfa Mon Sep 17 00:00:00 2001
From: Styx <styx@hidev.it>
Date: Sun, 20 Oct 2024 12:52:12 +0200
Subject: [PATCH] Correctly filtering conversions which will be applied to
 media in different collections of the same subject.

---
 src/Conversions/ConversionCollection.php      | 17 ++--
 .../ConversionCollectionConversionsTest.php   | 99 +++++++++++++++++++
 2 files changed, 110 insertions(+), 6 deletions(-)
 create mode 100644 tests/Conversions/ConversionCollectionConversionsTest.php

diff --git a/src/Conversions/ConversionCollection.php b/src/Conversions/ConversionCollection.php
index 2b57a110c..9d4db3f7a 100644
--- a/src/Conversions/ConversionCollection.php
+++ b/src/Conversions/ConversionCollection.php
@@ -38,9 +38,11 @@ public function setMedia(Media $media): self
 
     public function getByName(string $name): Conversion
     {
-        $conversion = $this->first(fn (Conversion $conversion) => $conversion->getName() === $name);
+        $conversion = $this
+            ->getConversions($this->media->collection_name)
+            ->first(fn (Conversion $conversion) => $conversion->getName() === $name);
 
-        if (! $conversion) {
+        if (!$conversion) {
             throw InvalidConversion::unknownName($name);
         }
 
@@ -51,7 +53,7 @@ protected function addConversionsFromRelatedModel(Media $media): void
     {
         $modelName = Arr::get(Relation::morphMap(), $media->model_type, $media->model_type);
 
-        if (! class_exists($modelName)) {
+        if (!class_exists($modelName)) {
             return;
         }
 
@@ -89,14 +91,16 @@ public function getConversions(string $collectionName = ''): self
             return $this;
         }
 
-        return $this->filter(fn (Conversion $conversion) => $conversion->shouldBePerformedOn($collectionName));
+        return $this
+            ->filter(fn (Conversion $conversion) => $conversion->shouldBePerformedOn($collectionName))
+            ->values();
     }
 
     protected function addManipulationToConversion(Manipulations $manipulations, string $conversionName): void
     {
         /** @var Conversion|null $conversion */
         $conversion = $this->first(function (Conversion $conversion) use ($conversionName) {
-            if (! $conversion->shouldBePerformedOn($this->media->collection_name)) {
+            if (!$conversion->shouldBePerformedOn($this->media->collection_name)) {
                 return false;
             }
 
@@ -113,7 +117,7 @@ protected function addManipulationToConversion(Manipulations $manipulations, str
 
         if ($conversionName === '*') {
             $this->each(
-                fn (Conversion $conversion) => $conversion->addAsFirstManipulations(clone $manipulations)
+                fn (Conversion $conversion) => $conversion->addAsFirstManipulations(clone $manipulations),
             );
         }
     }
@@ -124,4 +128,5 @@ public function getConversionsFiles(string $collectionName = ''): self
             ->getConversions($collectionName)
             ->map(fn (Conversion $conversion) => $conversion->getConversionFile($this->media));
     }
+
 }
diff --git a/tests/Conversions/ConversionCollectionConversionsTest.php b/tests/Conversions/ConversionCollectionConversionsTest.php
new file mode 100644
index 000000000..049af4a67
--- /dev/null
+++ b/tests/Conversions/ConversionCollectionConversionsTest.php
@@ -0,0 +1,99 @@
+<?php
+
+use Spatie\Image\Enums\Fit;
+use Spatie\MediaLibrary\Conversions\Conversion;
+use Spatie\MediaLibrary\Conversions\ConversionCollection;
+use Spatie\MediaLibrary\MediaCollections\Models\Media;
+use Spatie\MediaLibrary\Tests\TestSupport\TestModels\TestModel;
+
+beforeEach(function () {
+    $this->model = (new class extends TestModel
+    {
+        public function registerMediaConversions(?Media $media = null): void
+        {
+            $this
+                ->addMediaConversion('preview')
+                ->fit(Fit::Crop, 50, 50)
+                ->performOnCollections('avatar')
+                ->format('png')
+                ->nonQueued();
+
+            $this
+                ->addMediaConversion('preview')
+                ->fit(Fit::Crop, 300, 100)
+                ->performOnCollections('signature')
+                ->format('jpeg')
+                ->nonQueued();
+
+            $this
+                ->addMediaConversion('web')
+                ->format('webp')
+                ->nonQueued();
+        }
+
+        public function registerMediaCollections(): void
+        {
+            $this->addMediaCollection('avatar')->acceptsMimeTypes(['image/png'])->singleFile();
+            $this->addMediaCollection('signature')->acceptsMimeTypes(['image/jpeg'])->singleFile();
+        }
+    })::create(['name' => 'testmodel']);
+
+    $avatarMedia = $this->model
+        ->addMedia($this->getTestPng())
+        ->preservingOriginal()
+        ->toMediaCollection('avatar');
+    $avatarMedia->save();
+
+    $signatureMedia = $this->model
+        ->addMedia($this->getTestJpg())
+        ->preservingOriginal()
+        ->toMediaCollection('signature');
+    $signatureMedia->save();
+
+    $this->avatarMedia = $avatarMedia->refresh();
+    $this->signatureMedia = $signatureMedia->refresh();
+});
+
+it('will apply correct conversions for media in different collections', function () {
+    $conversionCollection = ConversionCollection::createForMedia($this->avatarMedia);
+    /** @var ConversionCollection<int, Conversion> $conversions */
+    $conversions = $conversionCollection->getConversions('avatar');
+
+    expect($conversions->count())->toBe(2)
+        ->and($conversions->first()->getName())->toBe('preview')
+        ->and($conversions->first()->getResultExtension())->toBe('png')
+        ->and($conversions->first()->getManipulations()->toArray())->toMatchArray([
+            'fit'    => [Fit::Crop, 50, 50],
+            'format' => ['png'],
+        ])
+        ->and($conversions->last()->getName())->toBe('web')
+        ->and($conversions->last()->getResultExtension())->toBe('webp')
+        ->and($conversions->last()->getManipulations()->toArray())->toMatchArray([
+            'format' => ['webp'],
+        ]);
+
+    $conversionCollection = ConversionCollection::createForMedia($this->signatureMedia);
+    /** @var ConversionCollection<int, Conversion> $conversions */
+    $conversions = $conversionCollection->getConversions('signature');
+
+    expect($conversions->count())->toBe(2)
+        ->and($conversions->first()->getName())->toBe('preview')
+        ->and($conversions->first()->getResultExtension())->toBe('jpeg')
+        ->and($conversions->first()->getManipulations()->toArray())->toMatchArray([
+            'fit'    => [Fit::Crop, 300, 100],
+            'format' => ['jpeg'],
+        ])
+        ->and($conversions->last()->getName())->toBe('web')
+        ->and($conversions->last()->getResultExtension())->toBe('webp')
+        ->and($conversions->last()->getManipulations()->toArray())->toMatchArray([
+            'format' => ['webp'],
+        ]);
+});
+
+it('will generate correct filenames for media in different collections but with identically named conversions', function () {
+    expect($this->model->getFirstMediaUrl('avatar', 'preview'))->toEndWith('.png');
+    expect($this->model->getFirstMediaUrl('avatar', 'web'))->toEndWith('.webp');
+
+    expect($this->model->getFirstMediaUrl('signature', 'preview'))->toEndWith('.jpeg');
+    expect($this->model->getFirstMediaUrl('signature', 'web'))->toEndWith('.webp');
+});