Laravel-Backpack / CRUD

Build custom admin panels. Fast!
https://backpackforlaravel.com
MIT License
3.18k stars 897 forks source link

[Bug] Upload subfield contains wrong url in repeteable relationship field #5355

Open CamusMX7 opened 1 year ago

CamusMX7 commented 1 year ago

Bug report

What I did

I have had this problem pending for a few months, I had reported it with the title "Upload field contains wrong url" previously and some solutions were applied. I followed what the updated documentation says at: https://backpackforlaravel.com/docs/6.x/crud-uploaders#handling-uploads-in-relationship-fields

What I expected to happen

That the files will be loaded and all the urls will correctly point to the corresponding file for each repeatable element.

What happened

Everything seemed to work fine when I tried it with 1 element, but when I tried to add more the URL points to something temporary.

Captura de pantalla 2023-10-25 222758

What I've already tried to fix it

nothing

Backpack, Laravel, PHP, DB version

LARAVEL VERSION:

10.24.0.0

BACKPACK PACKAGE VERSIONS:

backpack/basset: 1.2.0 backpack/crud: 6.2.1 backpack/generators: v4.0.2 backpack/permissionmanager: 7.0.1 backpack/pro: 2.0.13 backpack/theme-coreuiv2: 1.2.1 backpack/theme-coreuiv4: 1.1.1 backpack/theme-tabler: 1.1.1

CamusMX7 commented 1 year ago

Good afternoon, @tabacitu is there anything new on this topic? or any temporary alternative solution?

Thank you

karandatwani92 commented 1 year ago

Hey @CamusMX7,

I'm so sorry🙏, I missed replying to you. Answering this was my responsibility.

I tried a similar thing and it worked for me:

Code

CRUD::field('items')->subfields([
            [
                'name'    => 'description',
                'label'    => 'file_upload_demo',
                'type'    => 'upload',
                'withFiles' => true,
                'wrapper' => [
                    'class' => 'form-group col-md-8',
                ],
            ],
            [
                'name'       => 'quantity',
                'type'       => 'number',
                'attributes' => ['step' => 'any'],
                'wrapper'    => [
                    'class' => 'form-group col-md-2',
                ],
            ],
            [
                'name'       => 'unit_price',
                'type'       => 'number',
                'attributes' => ['step' => 'any'],
                'wrapper'    => [
                    'class' => 'form-group col-md-2',
                ],
            ],
        ])->reorder('order')->hint('<small class="float-right">Create/update/delete InvoiceItem entries over a <code>hasMany</code> relationship (1-n).</small>');
    }

Relationships

Invoice model
class Invoice extends Model
{
public function items()
    {
        return $this->hasMany(\App\Models\PetShop\InvoiceItem::class);
    }
}
InvoiceItem model
class InvoiceItem extends Model
{
    public function invoice()
    {
        return $this->belongsTo(\App\Models\PetShop\Invoice::class);
    }
}

Preview

I created two items here & accessed their URLs too. It worked! image

let me know if I missing something.

CamusMX7 commented 1 year ago

@karandatwani92 Thank you for your response. I'm a bit desperate, because it's a problem I've been exposing since July. It should be noted that I am working with a BelongsToMany relationship. I was first using the "repeatable" component as I specified when I reported a bug:

https://github.com/Laravel-Backpack/CRUD/issues/5232

When I was finally understood my DB is like this:

- castings: id, others
- locutores: id, others
- casting_locutor: casting_id, locutor_id, audioc

Pedro stayed to correct it, but I had no more news. When I asked why I saw the topic closed. I saw that there was a new article, which I mentioned here:

https://backpackforlaravel.com/docs/6.x/crud-uploaders#handling-uploads-in-relationship-fields

I changed the component from repeatable type to relationship type but I had the problem that I presented here. Now, I did an update today and I no longer have that problem BUT, now I have the same initial problem (from my July post). The folder of the repeated URL.

url repetida

My field and subield definition is:

CRUD::field([
    'name'          => 'locutors',
    'type'          => "relationship",
    'pivotSelect'=> [
        // 'attribute' => "title", // attribute on model that is shown to user
        'placeholder' => 'Selecciona un locutor',
        'wrapper' => [
            'class' => 'form-group col-md-6',
        ],
        // by default we call a $model->all(). you can configure it like in any other select
        'options' => function($model) {
            return $model->where('type', 'primary');
        },
        // note that just like any other select, enabling ajax will require you to provide an url for the field
        // to fetch available options. You can use the FetchOperation or manually create the enpoint.
        'ajax' => true,
        'data_source' => backpack_url('casting/fetch/locutor'),
        'attributes' => [
            'required'  => 'required',
            'class'     => 'selector_locutor'
        ],
    ],
    'subfields'   => [
                     [   // Upload
                        'name'      => 'audioc',
                        'label'     => 'Audio mp3 menor a 4MB',
                        'type'      => 'upload',
                        'withFiles' => [
                            'disk' => 'public', // the disk where file will be stored
                            'path' => 'castings', // the path inside the disk where file will be stored
                            //'fileNamer' => function($file, $uploader) { return 'the_file_name.png'; },
                        ],
                        'wrapper' => [
                            'class' => 'form-group col-md-4',
                        ]
                        ],
                        [   // select_from_array
                            'name'        => 'email_visto',
                            'label'       => "Email visto",
                            'type'        => 'select_from_array',
                            'options'     => (new \App\Models\Casting)->cat_si_no(),
                            'allows_null' => false,
                            'wrapper' => [
                                'class' => 'form-group col-md-2',
                            ],
                            'default'     => 0,
                            // 'allows_multiple' => true, // OPTIONAL; needs you to cast this to array in your model;
                            'attributes' => array('readonly' => 'readonly', 'disabled' => 'disabled')
                        ]
    ],
]);

If I remove the pathin withFiles everything works fine, but it is essential to move the files to the corresponding folder.

I think the two behaviors (the bug I initially reported and this one) are related and the difference has been when doing updates, but I have followed all the steps in the documentation, the recommendations and there is always a problem with this component. It has been quite a headache these months. Please, I really need to solve this because although it seems something simple it is fundamental in what I am developing and it is the only thing I need to deliver before the end of the month.

CamusMX7 commented 11 months ago

Hello @pxpm , Is there any news on this topic?

Last weekend I upgraded to the latest version of the system in production, but they are reporting the incorrect URL problem again:

error en carga de archivos

It is not uploading correctly (On upgrades it seems to jump between the two problems, one is the incorrect upload throwing a temporary link and the other is it uploads them correctly but replicates the folder containing the files).

Sorry for the insistence, but it is a problem that comes from the middle of the year and affects a lot.

cheers

CamusMX7 commented 10 months ago

Hello, I hope it will be a good start of the year. @pxpm Is there any news about this Bug? or is there any way where I can ask to know if this problem is going to be taken care of?

pxpm commented 10 months ago

Hey @CamusMX7 sorry it took me so much time to get back here.

I hope I have good news.

I've reworked the relationship uploaders and we should have a stable uploading process in relationships now too.

I tried the quick fix way, but it was not working as we found out. 😞

Here is the PR that fixes the issues https://github.com/Laravel-Backpack/CRUD/pull/5420

I'am giving some more tests to the PR but it should be merged shortly.

It's merged and tagged in Backpack/CRUD 6.5.2 and PRO 2.0.21.

Updated docs here: https://github.com/Laravel-Backpack/docs/pull/537

Let me know if you are still experiencing issues.

Cheers

CamusMX7 commented 10 months ago

Hello @pxpm ,

Thanks for the update. It seems that there are no more problems with the temporary path and the files are uploaded correctly.
Only that it follows the wrong path, for example if the folder is "castings" and the path should be "/storage/castings/file.mp3" the component shows the path "/storage/castings/castings/file.mp3" (like the example of the image where I marked the URL with the repeated folder).

Cheers

pxpm commented 10 months ago

@CamusMX7 Is it possible that you are repeating castings both in your disk configuration and in your field definition ?

What I mean is, for a default public disk configuration, doing the storage:link your have:

'public' => [
            'driver'     => 'local',
            'root'       => storage_path('app/public'),

            'url'        => env('APP_URL').'/storage', // you maybe have `castings` here too ???

            'visibility' => 'public',
            'throw'      => false,
        ],

If I wanted to store the files in storage/castings I would define the field with:

->withFiles(['path' => 'castings', 'disk' => 'public']) // backpack uses the `public` disk by default

Let me know if that's not the case and I can investigate a little bit more.

Cheers

CamusMX7 commented 10 months ago

@pxpm This is not the case, because my definition in filesystem.php is:

'public' => [
            'driver' => 'local',
            'root' => storage_path('app/public'),
            'url' => env('APP_URL').'/storage',
            'visibility' => 'public',
            'throw' => false,
        ],

and in the controller:

CRUD::field([
    'name'          => 'locutors',
    'type'          => "relationship",
    'label'         => 'Locutores',
    'max_rows' => 10,
    'pivotSelect'=> [
        'placeholder' => 'Selecciona un locutor',
        'wrapper' => [
            'class' => 'form-group col-md-5',
        ],
        // by default we call a $model->all(). you can configure it like in any other select
        'options' => function($model) {
            return $model->where('type', 'primary');
        },
        // note that just like any other select, enabling ajax will require you to provide an url for the field
        // to fetch available options. You can use the FetchOperation or manually create the enpoint.
        'ajax' => true,
        'data_source' => backpack_url('casting/fetch/locutor'),
        'attributes' => [
            'required'  => 'required',
            'class'     => 'selector_locutor'
        ],
        'attribute' => 'nombreApellidos',
    ],
    'subfields'   => [
        [   // Upload
                        'name'      => 'audioc',
                        'label'     => 'Audio mp3 menor a 4MB',
                        'type'      => 'upload',
                        // 'upload'    => true,
                        //'disk'      => 'public', // if you store files in the /public folder, please omit this; if you store them in /storage or S3, please specify it;
                        // optional:
                        //'temporary' => 10 // if using a service, such as S3, that requires you to make temporary URLs this will make a URL that is valid for the number of minutes specified
                        'withFiles' => [
                            'disk' => 'public', // the disk where file will be stored
                            'path' => 'castings', // the path inside the disk where file will be stored
                            //'fileNamer' => function($file, $uploader) { return 'the_file_name.png'; },
                        ],
                        'wrapper' => [
                            'class' => 'form-group col-md-5',
                        ]
                        ]
    ],
]);

And in my relationship table (casting_locutor) I have

casting_id (BIGINT 20)
locutor_id (BIGINT 20)
audioc (VARCHAR 255)
created_at (TIMESTAMP)
updated_at(TIMESTAMP)

And the "audioc" field contains the correct URL (castings/file_name.mp3) but as I said, in the url of the subfield in the repeatable component it repeats "castings" (castings/castings/file_name.mp3)

As in the image I shared above:

285351876-fe9d4c59-1666-45c7-86bd-54852d35ecff

Cheers

pxpm commented 10 months ago

I am sorry @CamusMX7 but I am not able to reproduce your issue.

image

Can you show me your relation definition ?

This is my test scenario:

// db
Schema::create('monster_productdummy', function (Blueprint $table) {
            $table->increments('id');
            $table->integer('monster_id')->unsigned();
            $table->integer('product_id')->unsigned();
            $table->string('notes');
            $table->nullableTimestamps();
        });
// field
[
                'name'    => 'dummyproducts',
                'label'   => 'BelongsToMany (n-n) <small>+ subfields for pivot table</small>'.backpack_new_badge(),
                'wrapper' => [
                    'class' => 'form-group col-md-4',
                ],
                'pivotSelect' => [
                    'wrapper' => [
                        'class' => 'form-group col-md-6',
                    ],
                ],
                'subfields' => [
                    [
                        'name'    => 'notes',
                        'type'    => 'upload',
                        'wrapper' => [
                            'class' => 'form-group col-md-6',
                        ],
                        'withFiles' => [
                            'disk' => 'public',
                            'path' => 'amazing',
                        ]
                    ],
                ],
                'tab'   => 'Relationship',
            ],

// relation
public function dummyproducts()
    {
        return $this->belongsToMany(\App\Models\Product::class, 'monster_productdummy')->withPivot('notes')->using(\App\Models\MonsterProductDummy::class);
    }

// pivot model
<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Relations\Pivot;

class MonsterProductDummy extends Pivot
{
    protected $table = 'monster_productdummy';
} 

Cheers

CamusMX7 commented 10 months ago

Hi @pxpm ,

I compared your definitions against mine and I don't see any difference that gives me a clue.

//casting model  - relation
public function locutors() {
        return $this->belongsToMany(Locutor::class)->withPivot('audioc')->using(CastingLocutor::class); 
    }
//pivot model
namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Database\Eloquent\Relations\Pivot;

class CastingLocutor extends Pivot
{
    use \Backpack\CRUD\app\Models\Traits\CrudTrait;
    use HasFactory;

    /**
     * Indicates if the IDs are auto-incrementing.
     *
     * @var bool
     */
    // public $incrementing = true;

    /**
     * The attributes that are mass assignable.
     *
     * @var array
     */
    protected $fillable = [
        // 'id',
        'casting_id',
        'locutor_id',
        'audioc'
    ];

    /**
     * The attributes that should be cast to native types.
     *
     * @var array
     */
    protected $casts = [
        // 'id' => 'integer',
        'casting_id' => 'integer',
        'locutor_id' => 'integer'
    ];
}

As I told you before, in my relation table I don't have "id key" my primary key is composed by the two ID's of the related tables.

BUT it really strikes me that in your image, the upload field does not show the folder, shouldn't it save in the database the complete path? I mean in your example: "amazing/inpainted-original-image-NBz7.jpg" (Attached image), because only show "inpainted-original-image-NBz7.jpg"

amazing_missing

If I play with the path in "withFiles" it changes and repeats. For example, in the image I shared above my path was ".../storage/castings/castings/1-1-XNua.mp3" if I change in the path:

'subfields'   => [
        [ 
                        'name'      => 'audioc',
                        'label'     => 'Audio mp3 menor a 4MB',
                        'type'      => 'upload',
                        'withFiles' => [
                            'disk' => 'public', 
                            'path' => 'example_change_path', //HERE
                        ]
                  ]
            ]

If I change the path now below in the browser it would show over the path ".../storage/example_change_path/castings/1-1-XNua.mp3" because in my field (controller), in the path now I have "example_change_path" but obviously in the database the path saved in the field is "castings/1-1-XNua.mp3" and if I now save a new file, in the database field it would be stored: "example_change_path/1-1-XNua.mp3". But because of this possible bug now the path in the form (at the bottom of the browser) shows ".../storage/example_change_path/example_change_path/1-1-XNua.mp3".

That's why I don't understand why your example image does not show the folder where the file is stored. In all the defined file fields I have (outside the repeatable component) it works correctly and in the database it stores the name of the folder and the name of the defined file (example: "folder/file.jpg").

Cheers

VesninAndrey commented 1 month ago

Only that it follows the wrong path, for example if the folder is "castings" and the path should be "/storage/castings/file.mp3" the component shows the path "/storage/castings/castings/file.mp3" (like the example of the image where I marked the URL with the repeated folder).

I had the same issue. And I've found the case. https://github.com/Laravel-Backpack/CRUD/blob/main/src/app/Library/Uploaders/Support/RegisterUploadEvents.php#L120 On this line we got parent entry, not a subfield one.

A quick solution is to null entry before add the upload field. $this->crud->entry = null;