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

Add CRUD form option for to-one associations #5353

Merged

Conversation

michaelKaefer
Copy link
Contributor

@michaelKaefer michaelKaefer commented Aug 5, 2022

Without this PR the association field renders a a dropdown and you cannot edit the associated entity but just select an already existing entity. If you want to edit an associated entity you cannot use AssociationField and so you have to create a custom EA field. Creating a custom EA field doesn't allow using EA fields and could sometimes kind of duplicate the CRUD form of the associated entity. This PR would enable you to use the CRUD form for AssociationField if it is a to-one association (for to-many associations CollectionField can be used).

I createad a reproducer: https://github.com/michaelKaefer/ea-reproducer/tree/association-field-use-crud-form.

Similar to #5210.

Copy link

@devzenfr devzenfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature would be really useful, thanks for your hard work!
I tried to make your pull request work inside my project but I still have some issues. I don't know if this is only related to my project and my use of your code.

src/Field/Configurator/AssociationConfigurator.php Outdated Show resolved Hide resolved
src/Resources/views/crud/form_theme.html.twig Outdated Show resolved Hide resolved
@michaelKaefer michaelKaefer force-pushed the association-field-use-crud-form branch 4 times, most recently from 7d71063 to 21d5961 Compare August 12, 2022 21:17
@michaelKaefer
Copy link
Contributor Author

@devzenfr Thank you for finding and fixing this errors! I think the PR is finished now.

@devzenfr
Copy link

@devzenfr Thank you for finding and fixing this errors! I think the PR is finished now.

Can confirm that it looks great. I'm now using a fork including your PR and everything is working smoothly. Nice job 🙏

@michaelKaefer michaelKaefer force-pushed the association-field-use-crud-form branch from 19d26d3 to cbee1c9 Compare August 14, 2022 10:49
@michaelKaefer michaelKaefer force-pushed the association-field-use-crud-form branch from cbee1c9 to acd06da Compare August 14, 2022 10:54
@ahmedyakoubi
Copy link

hey @michaelKaefer , thank you for your work . is this PR going to be merged soon ?

@michaelKaefer
Copy link
Contributor Author

Hi @ahmedyakoubi , no sorry, I don't know if and when it gets merged.

@javiereguiluz
Copy link
Collaborator

I apologize to @michaelKaefer for not having merged this on time. I'm going to try to merge it soon.

@ahmedyakoubi
Copy link

thank you @javiereguiluz and @michaelKaefer .

@javiereguiluz javiereguiluz added this to the 4.x milestone Nov 5, 2022
@javiereguiluz javiereguiluz merged commit c864e22 into EasyCorp:4.x Nov 5, 2022
@javiereguiluz
Copy link
Collaborator

This is finally merged 🙏

Michael, thanks a lot for this contribution. Your code was very easy to follow and implemented the feature in a nice and concise way. I also thank you for the reproducer (https://github.com/michaelKaefer/ea-reproducer/tree/association-field-use-crud-form) because it allowed me to play with the feature very easily.

While merging I did some tweaks. First, I reworded the docs a bit because I had some troubles while trying to fully understand the purpose of the feature. If I did some mistakes, we can fix it in a follow-up PR.

Second, I renamed the option to renderAsEmbeddedForm() (instead of the original useCrudForm()). As any naming, everything is debatable, but I felt that the original name was a bit generic and the new one is a bit more clear, although is longer. As always, if folks think that the new name is worse, we can revert the change.

Here are the tweaks: ae79fa8

@michaelKaefer michaelKaefer deleted the association-field-use-crud-form branch November 8, 2022 08:21
@michaelKaefer
Copy link
Contributor Author

Thank you, the docs are a lot better now, good to understand!

kiler129 added a commit to kiler129/EasyAdminBundle that referenced this pull request Dec 5, 2022
Functionality introduced in EasyCorp#5353
rendered sub-CRUD controller with context of the primary CRUD controller. With
invalid context some functions, like default fields generation, aren't working
properly. See EasyCorp#5512
kiler129 added a commit to kiler129/EasyAdminBundle that referenced this pull request Dec 23, 2022
Functionality introduced in EasyCorp#5353
(AssociationType) and in EasyCorp#5210
(CollectionType) rendered sub-CRUD controller with context of the primary CRUD
controller. With invalid context, some functions, like default fields generation,
aren't working properly. See EasyCorp#5512
kiler129 added a commit to kiler129/EasyAdminBundle that referenced this pull request Dec 23, 2022
Functionality introduced in EasyCorp#5353
(AssociationType) and in EasyCorp#5210
(CollectionType) rendered sub-CRUD controller with context of the primary CRUD
controller. With invalid context, some functions, like default fields generation,
aren't working properly. See EasyCorp#5512
kiler129 added a commit to kiler129/EasyAdminBundle that referenced this pull request Dec 23, 2022
Functionality introduced in EasyCorp#5353
(AssociationType) and in EasyCorp#5210
(CollectionType) rendered sub-CRUD controller with context of the primary CRUD
controller. With invalid context, some functions, like default fields generation,
aren't working properly. See EasyCorp#5512
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants