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] "upload_multiple" field within subfields of a "relationship" field does not allow files to be deleted #5392

Closed
daPhantom opened this issue Nov 30, 2023 · 4 comments

Comments

@daPhantom
Copy link

Bug report

What I did

Define a "upload_multiple" field as a subfield of a "relationship".

 CRUD::field([
    'name' => 'children',
    'type' => 'relationship',
    'subfields' => [
        [
            'name' => 'documents',
            'label' => 'Documents',
            'type' => 'upload_multiple',
            'withFiles' => [
                'disk' => 's3',
                'path' => 'upload',
            ],
        ],
    ]
]);

Then upload some files and try to remove a file.

What I expected to happen

File will be removed after save.

What happened

File still persists. Sometimes even empty files [{}] do pop up which break the column for upload_multiple.

What I've already tried to fix it

I have checked the JS part and also the \Backpack\CRUD\app\Library\Uploaders\MultipleFiles. It looks like the path to retrieve the deleted files is broken in case of subfields.

//uploadFiles method in MultipleFIles.php

$filesToDelete = CRUD::getRequest()->get('clear_'.$this->getName());

The get() key will resolve to 'clear_documents'. Instead the request is build like this, when using subfields and therefore the result will always be null.

"clear_children" => array:1 [▼
  0 => array:1 [▼
    "documents" => array:1 [▼
      0 => "upload/123-tcfb.pdf"
    ]
  ]
]
"_order_children" => array:1 [▼
  0 => array:1 [▼
    "documents" => "upload/456-tfdgh.pdf"
  ]
]

Is it a bug in the latest version of Backpack?

After I run composer update backpack/crud the bug... is it still there?
Yes. No new updates as of now.

Backpack, Laravel, PHP, DB version

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

PHP VERSION:

PHP 8.2.8 (cli) (built: Jul 28 2023 13:55:05) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.2.8, Copyright (c) Zend Technologies
with Zend OPcache v8.2.8, Copyright (c), by Zend Technologies

LARAVEL VERSION:

10.34.2.0

BACKPACK PACKAGE VERSIONS:

backpack/basset: 1.2.2
backpack/crud: 6.3.2
backpack/devtools: 2.0.3
backpack/generators: v4.0.2
backpack/permissionmanager: 7.1.1
backpack/pro: 2.0.18
backpack/revise-operation: 2.0.0
backpack/theme-tabler: 1.1.2

@daPhantom
Copy link
Author

When I debug the variables within MultipleFiles::uploadFiles after having deleted an upload in the UI I get the following request & result:

Request

//CRUD->getRequest()->request

Symfony\Component\HttpFoundation\InputBag Object
(
    [parameters:protected] => Array
        (
            [_token] => CENSORED
            [_method] => PUT
            [_http_referrer] => CENSORED
            [id] => 142
            [children] => Array
                (
                    [0] => Array
                        (
                            [id] => 5
                            [documents] => Array
                                (
                                    [0] => 
                                )

                        )

                )

            [clear_linePermits] => Array
                (
                    [0] => Array
                        (
                            [documents] => Array
                                (
                                    [0] => "upload/456-9BvW.pdf"
                                )

                        )

                )

            [_order_linePermits] => Array
                (
                    [0] => Array
                        (
                            [documents] => "upload/123-5PuU.pdf"
                        )

                )

            [_current_tab] => basis
            [_save_action] => save_and_back
        )
)

Result

$filesToDelete = CRUD::getRequest()->get('clear_'.$this->getIdentifier());
// null

$value = $value ?? CRUD::getRequest()->file($this->getName());
// [
//   0 => null
// ]

$previousFiles = $this->getPreviousFiles($entry) ?? [];
// [
//   0 => "upload/123-5PuU.pdf"
//   1  => "upload/456-9BvW.pdf"
// ]

Also the empty upload itself (0 => null), when only deleting something or saving an empty multiple_upload field seems odd to me. As mentioned this sometimes also causes actual empty JSON objects in the database which break the column field rendering since there is no file information.

I am also struggling with the request validation of upload_multiple. Maybe my setup is broken? According to the documentation validation and uploads within subfields should work... In general I want to have it as an optional field where you can upload multiple files but also leave it empty which should result in a null value or [] in the database.

I would also be available for a hands-on debugging session if possible to have both problems resolved.

Cheers,
Felix

@karandatwani92
Copy link
Contributor

Hello @daPhantom thanks for the feedback.

I feel your pain in finding the bug and appreciate the time you gave to debug & report.

I will assign my colleague @pxpm to take over from here.

Thanks again, Please report anything you found missing/confusing/wrong and we will try our best to address it.

Cheers 🙏

@karandatwani92 karandatwani92 added Possible Bug A bug that was reported but not confirmed yet. Size: S 1 day and removed triage labels Dec 1, 2023
@karandatwani92 karandatwani92 moved this to Todo in This week Dec 1, 2023
@karandatwani92 karandatwani92 added Bug and removed Possible Bug A bug that was reported but not confirmed yet. labels Dec 1, 2023
@karandatwani92
Copy link
Contributor

Hey @pxpm

I tested this and I can confirm where it stops working. I quickly tried this in the demo's monster CRUD:

[
 'name'      => 'postalboxes',
 'label'     => 'HasMany (1-n) <small>+ subfields</small>'.backpack_new_badge(),
 'subfields' => [
  [
  'name' => 'postal_name',
  'type' => 'upload_multiple',
  'withFiles' => true,
  ],
],
  'wrapper' => [
   'class' => 'form-group col-md-4',
 ],
 'tab'   => 'Relationship',
],           

Screenshot 2023-12-01 at 1 40 43 PM

From the above,

  • Remove a relationship object. Uploaded files get deleted from DB & storage.✅
  • Remove a single image from the upload_multiple subfield. Cross(X) has no effect. not even removed on UI.❌

@pxpm
Copy link
Contributor

pxpm commented Dec 8, 2023

Hey @daPhantom thanks for the report 🙏

I've fixed the issue in 6.4.2, you should be able to get the fix with composer update backpack/crud.

You should use the Validators we provide for a better experience validating upload fields. Have a look at https://backpackforlaravel.com/docs/6.x/custom-validation-rules#validuploadmultiple-for-upload_multiple-field-type

Let us know if you are still experience issues. If you find other issues you can open a new issue report and we will look into it.

Cheers

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

3 participants