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 missing class option #7785

Merged
merged 2 commits into from
Mar 31, 2022
Merged

Conversation

VincentLanglet
Copy link
Member

@VincentLanglet VincentLanglet commented Mar 30, 2022

Subject

When using the ModelHiddenType, the option class is required.

Changelog

### Fixed
- Correctly pass the class to the ModelHiddentType when using child admin without parentAssociationMapping.

@VincentLanglet VincentLanglet requested review from jordisala1991 and a team March 31, 2022 17:28
@jordisala1991
Copy link
Member

Can you add a failing tests? How did you encounter the error?

@VincentLanglet
Copy link
Member Author

Can you add a failing tests? How did you encounter the error?

Honestly I dont really know fully how this is working.
A colleague was configuring a childAdmin but he was giving a wrong fieldName.
So he had an error message "option class is missing in ModelHiddenType".
By manually passing a class, he then got the message about the fact he were using a wrong fieldName, which was better to understand his mistake.

When the childAdmin is correctly configured, no error is thrown maybe because the option was set by something like
the AbstractFormContractor::getDefaultOptions.

I had thought if we should pass

'field_options' => [
     'model_manager' => $this->getModelManager(),
     'class' => $this->getClass(),
],

or

'field_options' => [
     'model_manager' => $this->getParent()->getModelManager(),
     'class' => $this->getParent()->getClass(),
],

But since we're trying to add a filter about the parent entity, I just tried and the second value was the right one.

Not sure about adding a test with a wrong config in order to expect we see an error message rather than another...

@jordisala1991
Copy link
Member

I am asking because I use a lot the parent child thing (most of my admins are nested), and I never seen an error like this

@VincentLanglet
Copy link
Member Author

I would say it's because you are using them correctly ^^

Try to modify a addChild call in your config to give a wrong fieldname ; it was the way for me to produce this error

@VincentLanglet VincentLanglet merged commit 29fc2f7 into sonata-project:4.x Mar 31, 2022
@VincentLanglet VincentLanglet deleted the addClass branch March 31, 2022 18:58
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.

2 participants