spatie / laravel-medialibrary

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

Probably copying from MediaCollection to singleFile MediaCollection works weird #3603

Closed drnkwtr closed 5 months ago

drnkwtr commented 6 months ago

I registered 2 media collections:

<?php
  public function registerMediaCollections(): void
  {
      $this->addMediaCollection('images');
      $this->addMediaCollection('thumbnail')
          ->singleFile();
  }

Then uploading some images by

<?php
  public function uploadImages(Request $request, $id)
  {
      $files = $request->allFiles()['images'];
      foreach($files as $uploaded_file) {
          Modification::find($id)
              ->addMedia($uploaded_file)
              ->toMediaCollection('images');
      }

And then selecting thumbnail by this (copied image replaces thumbnail for newer)

<?php
  public function selectThumbnail($mod_id, $file_id)
  {
      Media::find($file_id)->copy(Modification::find($mod_id),'thumbnail');

      return back();
  }

But for some reason copying to another collection works weird. I can copy file with incrementing id's, but can't with decrementing id's.

Example: uploaded 4 files with id=1, id=2, id=3, id=4. Selecting thumbnail with id=1 - ok, selecting id=2 - ok, selecting id=3 - ok, but now i can't choose thumbnail with file id=1 and id=2, only higher than selected id now

Is it only my fault or actually bug?

UPD: Seems like that problem is described here: issue

drnkwtr commented 6 months ago
    public function selectThumbnail($mod_id, $file_id)
    {
        $media = Media::find($file_id)->copy(Modification::find($mod_id),'thumbnail');
        $media->order_column = 1;
        $media->save();
        return back();
    }

This is the fix of this issue. I think, new media should be copied with order_column depends on count of collection medias.

patinthehat commented 6 months ago

@drnkwtr I'd be happy to take a look at a PR with failing unit tests that show the issue, as all of the tests are currently passing. Thanks! :+1: