Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] error on replacing repeatable subfield image when first saving it empty #5631

Closed
eleven59 opened this issue Aug 21, 2024 · 2 comments
Closed

Comments

@eleven59
Copy link

Bug report

What I did

  • I included an image field in a repeatable element.
  • I add a repeatable entry and save the page once without adding an image to its subfield, all is fine.
  • I then going back in to edit and select an image for that same subfield
  • The upload handler tries to delete the old (nonexistent) image, which throws an error (see below)

What I expected to happen

I expected that the entry would just be saved with the new image, and the old one only getting

What happened

This throws the following error:

League\Flysystem\Filesystem::delete(): Argument #1 ($location) must be of type string, null given, called in /<redacted>/vendor/laravel/framework/src/Illuminate/Filesystem/FilesystemAdapter.php on line 513

It has to do with this function in Backpack \ CRUD \ app \ Library \ Uploaders \ SingleBase64Image:

public function uploadRepeatableFiles($values, $previousRepeatableValues, $entry = null)
{
    foreach ($values as $row => $rowValue) {
        if ($rowValue) {
            if (Str::startsWith($rowValue, 'data:image')) {
                $base64Image = Str::after($rowValue, ';base64,');
                $finalPath = $this->getPath().$this->getFileName($rowValue);
                Storage::disk($this->getDisk())->put($finalPath, base64_decode($base64Image));
                $values[$row] = $previousRepeatableValues[] = $finalPath;
                continue;
            }
        }
    }

    $imagesToDelete = array_diff($previousRepeatableValues, $values);

    foreach ($imagesToDelete as $image) {
        Storage::disk($this->getDisk())->delete($image);
    }

    return $values;
}

As you can see this does not check anywhere whether an image actually exists in the spot where it tries to delete it.

What I've already tried to fix it

Updated all backpack packages, made a completely new model with repeatable field that only has an image. Repeated the steps under "What I did". The error remains.

Is it a bug in the latest version of Backpack?

After I run composer update backpack/crud the bug... is it still there?

Yes

Backpack, Laravel, PHP, DB version

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

### PHP VERSION:
8.3.10

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

### LARAVEL VERSION:
11.21.0.0

### BACKPACK PACKAGE VERSIONS:
backpack/basset: 1.3.5
backpack/crud: 6.7.30
backpack/generators: v4.0.5
backpack/pro: 2.2.14
backpack/settings: 3.1.1
backpack/theme-tabler: 1.2.11
@karandatwani92
Copy link
Contributor

Hey @eleven59

Thanks for pointing out the issue. We are already working on major refactoring of the Backpack Uploader to cover this and other uploader-related problems. It's a BIG PR, which may take some time. Sorry for the inconvenience.

You can subscribe to our newsletter for the Backpack Progress report and other updates.

@eleven59
Copy link
Author

Hi @karandatwani92,

That sounds like a great idea. I've fixed it by overwriting the uploader for now, so no rush in that regard for me. I fully trust you'll make something much better than my hacky solution here and will happily update my project whenever you're done with that.

Thanks for the quick reply, too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

2 participants