gregraab commented 4 months ago

Bug report

What I did

Existing app with image field using S3 disk. We only store the S3 directory where the image file is located in the db and then upload multiple version of the file named thumb, small, med etc. Name of the db field is picture.

We then have an accessor on the model (getPictureAttribute) that builds up the necessary url to the image. This isn't exactly how it works, but equivalent code if we weren't storing multiple images for each record.

public function getPictureAttribute() {
   $base_url = config('backpack.aws_base_url'); //gets set to our url based on environment
   $base_path = $this->attributes['picture']; //the path only on S3 to the files, what we actually stored in the db
   return $picture  = $base_url . '/' . $base_path . '/' .  'small.jpg';
}

One thing to note. We have cname setup for the S3 bucket these are saved in. Example https://profiles.aws.com which points to https://ouridentifer-public-dev.s3.us-north-19.amazonaws.com

What I expected to happen

Expect it to act as it did previously. No documented changes that I can find. So in the above code what I would expect the image field to access the $picture accessor which evaluates to https://profiles.aws.com/PATH/small.jpg as expected when stepping through a debug session.

What happened

On the form, choose an image, crop etc and hit save. The file processes, uploads to S3 and saves as expected in the db (path only). Then I get a broken image link in the Image Field instead of the image that was just successfully uploaded.

The image link being used by the image field after upload is: https://ouridentifer-public-dev.s3.us-north-19.amazonaws.com/https://profiles.aws.com/PATH/small.jpg so it is grabbing the S3 bucket url and appending everything to that, where previously it would just use the returned picture attribute as expected.

What I've already tried to fix it

I can change the accessor to fix this, but that breaks all the other code that is written on top of that, and looking at the Laravel S3 docs, and Backpack docs I don't think this is intended behavior. And is definitely a non-documented change from what was in previous versions.

Is it a bug in the latest version of Backpack?

After I run composer update backpack/crud the bug... is it still there? This is found during an update to BP6 with everything updated. We did update from an earlier version of BP5 to the latest version of BP5 prior and there is a small chance this happened in that upgrade and was not caught in testing, but I don't believe that to be the case.

Backpack, Laravel, PHP, DB version

When I run php artisan backpack:version the output is:

PHP VERSION:

8.3.7

PHP EXTENSIONS:

Core, date, libxml, openssl, pcre, zlib, filter, hash, json, pcntl, random, Reflection, SPL, session, standard, sodium, mysqlnd, PDO, xml, bcmath, calendar, ctype, curl, dom, mbstring, FFI, fileinfo, ftp, gd, gettext, iconv, igbinary, imagick, imap, intl, ldap, exif, msgpack, mysqli, pcov, pdo_mysql, pdo_pgsql, pdo_sqlite, pgsql, Phar, posix, readline, redis, shmop, SimpleXML, soap, sockets, sqlite3, sysvmsg, sysvsem, sysvshm, tokenizer, xmlreader, xmlwriter, xsl, zip, memcached, swoole, Zend OPcache, xdebug

LARAVEL VERSION:

11.7.0.0

BACKPACK PACKAGE VERSIONS:

backpack/basset: 1.3.4 backpack/crud: 6.7.14 backpack/editable-columns: 3.0.9 backpack/generators: v4.0.5 backpack/permissionmanager: 7.2.1 backpack/pro: 2.2.0 backpack/revise-operation: 2.0.0 backpack/theme-coreuiv2: 1.2.3 //We are using coreuiv2 if that makes a difference, though I am thinking it shouldn't. backpack/theme-coreuiv4: 1.1.1 backpack/theme-tabler: 1.2.10

pxpm commented 4 months ago

Hello @gregraab sorry for the late reply. Can you provide me your field definition ?

Cheers

gregraab commented 4 months ago

$this->crud->addField([
    'name' => 'profile.picture',
    'label' => 'Profile Picture',
    'type' => 'image',
    'disk' => 's3-public',
    'crop' => true,
    'aspect_ratio' => 1.33,
    'tab' => 'My Info',
    'hint' => 'Kinda Long, Removed for clarity',
    'wrapper' => ['class' => 'col-md-6 mt-3 toolbar-override'],
])

gregraab commented 3 months ago

Hello @pxpm, were you able to look at this?

pxpm commented 3 months ago

Hey @gregraab

Since this is using your custom code to upload/retrieve the image I am not sure I can help much without debugging your actual code.

Try adding temporary => true to your field definition. If that does not work, go back to the previous version that was working, and copy the image.blade.php field file into your resources folder: https://backpackforlaravel.com/docs/6.x/crud-how-to#how-to-publish-a-column-field-filter-button-and-modify-it

After update again to v6, keeping that "old version" of the file so that Backpack use the old file instead of the new one. I am not sure we did any changes in the field since the version you are upgrading from (it's possible we did, but it's also possible we didn't). Some "underlying" package may have changed like the one that connects to s3 or even something internal at Laravel. The previous step may help to debug if it was any change we did to that file that broke your implementation, or if the problem lies elsewhere.

Alternatively you can try to use our internal uploaders we added in v6: https://backpackforlaravel.com/docs/6.x/crud-uploaders

Let me know how it goes.