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

modifyField() does not call the defined macros in the modifications #5352

Open
dimer47 opened this issue Oct 20, 2023 · 6 comments
Open

modifyField() does not call the defined macros in the modifications #5352

dimer47 opened this issue Oct 20, 2023 · 6 comments
Assignees
Labels
Minor Bug A bug that happens only in a very niche or specific use case. Priority: COULD

Comments

@dimer47
Copy link
Contributor

dimer47 commented Oct 20, 2023

Bug report

What I did

By using setFromDb() and modifying the type of a field to upload with the modifyField function the form does not apply the enctype.

What I expected to happen

The modifyField method should add the enctype to the form if it is an upload, upload_multiples, image, ... type.

What happened

Currently, the form does not send the file in POST request.

What I've already tried to fix it

If we rewrite the field with CRUD::field instead of modifying it with CRUD::modifyField it works and the enctype is defined

Is it a bug in the latest version of Backpack?

This is indeed the latest version of Backpack v6 and I confirm after running ``composer update backpack/crud``` the bug is still there.

Backpack, Laravel, PHP, DB version

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

BACKPACK PACKAGE VERSIONS:

backpack/basset: 1.2.1
backpack/crud: 6.2.4
backpack/devtools: 3.0.0
backpack/editable-columns: 3.0.2
backpack/generators: v4.0.2
backpack/logmanager: v5.0.1
backpack/pro: 2.0.16
backpack/settings: 3.1.0
backpack/theme-tabler: 1.1.1

The code:

    protected function setupCreateOperation()
    {
        CRUD::setValidation(AccountDocumentRequest::class);
        CRUD::setFromDb();

        // This not add enctype on form 
//        CRUD::modifyField('path', [
//            'type' => 'upload',
//            'withFiles' => true,
//            'tab' => trans('admin/account_address.tabs.general')
//        ]);

        // Redeclaration of field add enctype on form
        CRUD::field([
            'name'=>'path',
            'type' => 'upload',
            'withFiles' => true,
            'tab' => trans('admin/account_address.tabs.general')
        ]);
    }

@dimer47 dimer47 added the triage label Oct 20, 2023
@welcome
Copy link

welcome bot commented Oct 20, 2023

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:

  • Bug Reports, Feature Requests - Github Issues (here);
  • Quick help (How do I do X) - Gitter Chatroom;
  • Long questions (I have done X and Y and it won't do Z wtf) - Stackoverflow, using the backpack-for-laravel tag;
  • Showing off something you've made, asking for opinion on Backpack/Laravel matters - Reddit;

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

@dimer47
Copy link
Contributor Author

dimer47 commented Oct 20, 2023

The Backpack\CRUD\app\Library\CrudPanel\Traits\Fields@hasUploadFields check that the field has the upload attribute as true value. If we add it in the modifyField the enctype is added well. I think that it is related to the registered attribute macros function.

What do you think ?

@pxpm
Copy link
Contributor

pxpm commented Oct 20, 2023

Hey @dimer47 thanks for the report and the additional information.

I think I got the issue here. The upload => true was a requirement since forever, even before the Uploaders were a thing.

I am not sure about the real intent of it, I am guessing that by the time it was added (very long time ago), was to don't rely on the field types to add the enc-type.

Behind the scenes the Uploaders add this automatically in the field for you.

The Uploader macro is activated with withFiles attribute or method, when addField() or field() are called.
When you do setFromDb, the addField() method generated from your db columns will not have the macro.

The macros should not be ran on field modifications, they should only run when explicitly defined in the addField, or via macro method on the field instance.

Your scenario would work just fine if you do:

CRUD::setFromDb();
CRUD::field('path')->withFiles();

Let me know if it worked for you.

Cheers

@pxpm pxpm self-assigned this Oct 20, 2023
@pxpm pxpm removed the triage label Oct 20, 2023
@pxpm pxpm changed the title [Bug] Missing enctype on form using upload field in v6 Missing enctype on form using upload field in v6 Oct 20, 2023
@dimer47
Copy link
Contributor Author

dimer47 commented Oct 23, 2023

Hey pxpm 👋🏻

Thanks for your return,

I confirm that by doing CRUD::setFromDb(); and CRUD::field('path')->withFiles(); this works perfectly 👍. The enctype is correctly added to form.
Do you think it would be possible to add the macro in the modifyField method to prevent this bug?

Cheers

@pxpm
Copy link
Contributor

pxpm commented Oct 23, 2023

Maybe I can work around it by checking if the field don't contain the macro in the previous attributes, only in the ones provided by modifyField(), and in that case, run the macro.

It may work non-BC like that. I will investigate that possibility. I don't think at the moment we have a way to run a specific macro on the field, either run all field macros or specifically call the ->macro() to run it.

I can't prioritize this rigth now, but I will keep this open so I don't forget.

Thanks again for the report 🙏

@pxpm pxpm changed the title Missing enctype on form using upload field in v6 modifyField() does not call the defined macros in the modifications Oct 23, 2023
@pxpm pxpm added Priority: COULD Minor Bug A bug that happens only in a very niche or specific use case. labels Oct 23, 2023
@dimer47
Copy link
Contributor Author

dimer47 commented Oct 25, 2023

Thank you @pxpm for your feedback and your help.
Even if this problem is not corrected, I think it is a good idea to leave the issue open to help developers who notice the same problem.

Thanks again @pxpm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Minor Bug A bug that happens only in a very niche or specific use case. Priority: COULD
Projects
Status: No status
Development

No branches or pull requests

2 participants