Laravel-Backpack / CRUD

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

[Bug] Image Field saved in s3 doesn't show thumbnail in form after save. #5299

Open ericjanofski opened 1 year ago

ericjanofski commented 1 year ago

I was attempting to use an image field to save an image to s3. Here is the controller code I am using:

$this->crud->field('file_name')
            ->type('image')
            ->label('Image')
            ->withFiles([
                'disk' => 's3',
            ]);

The image saves fine to s3, and saves in the database as the file name. Unfortunately, it displays as a broken link in the edit form after saving. When I inspect the broken image, it shows the url is http://the-local-url.test/the-file-name.

I attempted to fix this by setting the path property to the bucket name, thinking that would affect the display:

  $this->crud->field('file_name')
            ->type('image')
            ->label('Image')
            ->withFiles([
                'disk' => 's3',
                'path' => env('AWS_BUCKET_PATH')
            ]);

Unfortunately that saves the file in the database as http://the-bucket-path/the-file-name

I think either I'm missing a setting somewhere or the Image field isn't properly setting the dispaly image path in the edit form.

Note: it displays fine as a column, so I'm thinking the column is checking the disk and displaying the bucket path properly while the field is not.

Any help would be appreciated, thanks so much!

welcome[bot] commented 1 year ago

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication channels:

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

-- Justin Case The Backpack Robot

ericjanofski commented 1 year ago

Correction, an image on s3 will display as an image in a column if the disk is set:

$this->crud->column('file_name')->type('image')->disk('s3');

I tried hacking that into the field definition, but no dice, still a broken link displaying in the image field after save:

$this->crud->field('file_name')
            ->type('image')
            ->label('Image')
            ->disk('s3')
            ->withFiles([
                'disk' => 's3',
            ]);
pxpm commented 1 year ago

Hey @ericjanofski we have a PR in PRO to fix the s3 image but it's not fully tested yet.

Can you pull it in your project and check if it fixes your issue too ?

You can get that branch with: composer require backpack/pro:"dev-fix-image-s3-adapter as 2.0.99" (it's updated with current main).

Let me know how it goes 🙏

Cheers

ericjanofski commented 1 year ago

Thank you for reaching out. Unfortunately, after running this composer change the same issue persists. The image filename is stored in the database, but the edit form displays a broken image due to it attempting to display an image with the local url instead of the s3 bucket url.

Any assistance would be appreciated.

I'm also having issues with validation and restricting file size so I'm creating a custom image field in case I cannot find a solution. The deadline for this project is looming!

Thanks again for your help.

pxpm commented 1 year ago

AFAIK you need to use a custom package like https://github.com/crazybooot/base64-validation to validate the image field (since it uses base64 strings).

Can you do some @dd() on the image field to check where it's failing ? We have code in place that should get you the asset disk url properly. Also worth checking if you have the latest Backpack version by running php artisan backpack:version .

Let me know how it goes.

Cheers

ericjanofski commented 1 year ago

Okay, I'm happy to keep digging. I'd rather use the built in field over a javascript custom field anyway! Can you share where I would put @dd()? If i add it in the create method it just gives me the field details.

protected function setupCreateOperation() { CRUD::setValidation(LegacyBallImageRequest::class);

    $image = $this->crud->field('image')
        ->type('image')
        ->label('Image')
        ->withFiles([
            'disk' => 's3',
        ]);

@dd($image); }

LARAVEL VERSION:

10.20.0.0

BACKPACK PACKAGE VERSIONS:

backpack/basset: 1.1.1 backpack/crud: 6.1.15 backpack/generators: v4.0.2 backpack/pagemanager: 3.2.0 backpack/pro: dev-fix-image-s3-adapter backpack/theme-tabler: 1.0.12

ericjanofski commented 1 year ago

Good morning, I'm still having this issue even with the branch: composer require backpack/pro:"dev-fix-image-s3-adapter as 2.0.99"

After I upload an image with PageManager I see this:

Screenshot 2023-08-30 at 6 31 13 AM

When I first upload it, it looks great as it's a base64 string. But after that the field isn't putting the bucket path in front of the image and it displays as broken.

AliAlAasm-RP commented 7 months ago

I have the same issue, v6 . Seems like there is a bug too where using temporary urls doesnt sign the url from s3 so I signed it myself and tried to throw it to the image type crud in value attribute but then if I have a path attribute it thinks I want to go local and concatenates my localhost url to the aws signed s3 url i put in value. If i remove my path it doesn't add the local url but then that doesn't work because i cant have it save in a folder on s3 , just to the root and thats unorganized. Is there a fix for this?

ericjanofski commented 7 months ago

Hey there, I'll let the laravel opine on whether or not it's a bug.

I resolved my s3 uploading needs by creating a custom field for images and a custom field for video. The custom field uses javascript to call a controller via the api which uploads the file to s3. I also have a solution for streaming large video files to s3 as well. Let me know if you want the code I'm using.

AliAlAasm-RP commented 7 months ago

Hey there, I'll let the laravel opine on whether or not it's a bug.

I resolved my s3 uploading needs by creating a custom field for images and a custom field for video. The custom field uses javascript to call a controller via the api which uploads the file to s3. I also have a solution for streaming large video files to s3 as well. Let me know if you want the code I'm using.

Ha , oh man its for sure a couple bugs in the image attribute i noticed , hope they can fix it . Sure if you don't mind sharing , I was going to basically publish the image blade and modify it to fix or make a custom field , not sure if thats what solved it for you?

ericjanofski commented 7 months ago

Yep, i made a custom field based on the image and file upload fields. I was wrong in saying I used an api route, I think I had trouble with that, so I used a web route to get to the controller.

Here's a repo with the pertinent files. it was an evolution. I did the image uploader first and then made the video uploader, so the custom attributes evolved..

https://github.com/ericjanofski/backpack-s3-integration

AliAlAasm-RP commented 7 months ago

Hey Eric I appreciate it man, its funny that even their temporary link attribute at least for me doesnt seem to work but its in their docs. Hopefully they fix it soon but this will help!

Yep, i made a custom field based on the image and file upload fields. I was wrong in saying I used an api route, I think I had trouble with that, so I used a web route to get to the controller.

Here's a repo with the pertinent files. it was an evolution. I did the image uploader first and then made the video uploader, so the custom attributes evolved..

https://github.com/ericjanofski/backpack-s3-integration

pxpm commented 6 months ago

Hey @AliAlAasm-RP and @ericjanofski thanks for sticking with us.

I went back to work on the branch I mentioned to @ericjanofski previously and I've identified some failing scenarios.

From my tests I think everything is working as expected now in backpack/crud:6.6.3 and backpack/pro:2.1.3.

I've just released the pro version with the fix so it should take a little bit of time for our private repo to pick up the changes, you can then get the updated versions with a composer update.

Notice that I used the new Uploaders in v6 to test this, so running your own implementations, you may need to adapt.

For reference I will leave here my test scenarios, and my notes from it.

I used MinIO locally to emulate s3. This is my .env config

FILESYSTEM_DRIVER=s3
AWS_ACCESS_KEY_ID=admin
AWS_SECRET_ACCESS_KEY=password
AWS_DEFAULT_REGION=us-east-1
AWS_BUCKET=images
AWS_ENDPOINT=http://localhost:9000
AWS_URL=http://localhost:9000
# This last one may be environment dependant, testing locally I couldn't get the files to upload if this was set to false.
AWS_USE_PATH_STYLE_ENDPOINT=true

My field:

[
  'label'        => 'Image',
  'name'         => 'image',
  'type'         => 'image',
  'withFiles' => [
      'disk' => 's3',
      'temporaryUrl' => true,
      'path' => 'testing',
  ]

The results: image

image

Let me know if you guys give this a go and is working as expected. Please remember that if you are overwriting the image file in your resources folder you will need to manually copy the contents from the vendor to apply the fix, or delete the resources file to use the vendor one.

Cheers

AliAlAasm-RP commented 6 months ago

I appreciate you following up on this, I’ll try the updates [logo.png]

Ali Al-Aasm • t: 519.916.1005 Ext:11• c: 519.991.0678 w: redpiston.com • e: @.*** • twitter: @redpiston

On Feb 13, 2024, at 11:35 AM, Pedro Martins @.***> wrote:

Hey @AliAlAasm-RPhttps://github.com/AliAlAasm-RP and @ericjanofskihttps://github.com/ericjanofski thanks for sticking with us.

I went back to work on the branch I mentioned to @ericjanofskihttps://github.com/ericjanofski previously and I've identified some failing scenarios.

From my tests I think everything is working as expected now in backpack/crud:6.6.3 and backpack/pro:2.1.3.

I've just released the pro version with the fix so it should take a little bit of time for our private repo to pick up the changes, you can then get the updated versions with a composer update.

Notice that I used the new Uploaders in v6 to test this, so running your own implementations, you may need to adapt.

For reference I will leave here my test scenarios, and my notes from it.

I used MinIO locally to emulate s3. This is my .env config

FILESYSTEM_DRIVER=s3 AWS_ACCESS_KEY_ID=admin AWS_SECRET_ACCESS_KEY=password AWS_DEFAULT_REGION=us-east-1 AWS_BUCKET=images AWS_ENDPOINT=http://localhost:9000 AWS_URL=http://localhost:9000

This last one may be environment dependant, testing locally I couldn't get the files to upload if this was set to false.

AWS_USE_PATH_STYLE_ENDPOINT=true

My field:

[ 'label' => 'Image', 'name' => 'image', 'type' => 'image', 'withFiles' => [ 'disk' => 's3', 'temporaryUrl' => true, 'path' => 'testing', ]

The results: image.png (view on web)https://github.com/Laravel-Backpack/CRUD/assets/7188159/15046af1-ea8d-4223-8931-0f0e54df0c5c

image.png (view on web)https://github.com/Laravel-Backpack/CRUD/assets/7188159/ba67dbbe-8818-4adb-8031-617c7e659f2a

Let me know if you guys give this a go and is working as expected. Please remember that if you are overwriting the image file in your resources folder you will need to manually copy the contents from the vendor to apply the fix, or delete the resources file to use the vendor one.

Cheers

— Reply to this email directly, view it on GitHubhttps://github.com/Laravel-Backpack/CRUD/issues/5299#issuecomment-1941965558, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABJPPWRRFQRHMNTHK2SM4WTYTOI45AVCNFSM6AAAAAA374I35OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBRHE3DKNJVHA. You are receiving this because you were mentioned.Message ID: @.***>

olaitanade commented 3 months ago

$this->crud->addField( [ 'label' => 'Logo', 'name' => 'logo', 'type' => 'image', 'withFiles' => [ 'disk' => 's3', 'temporaryUrl' => true, 'path' => 'uploads/images/races/', ], ] );

When I save the form, it causes indefinite loading which duplicates the image in s3 bucket. Please help

pxpm commented 3 months ago

Hey @olaitanade, how's your s3 filesystem setup ?