8000 Define FormType class instead of a classic `type` by Pierstoval · Pull Request #385 · EasyCorp/EasyAdminBundle · GitHub
[go: up one dir, main page]

Skip to content

Define FormType class instead of a classic type #385

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

Closed
wants to merge 5 commits into from
Closed

Define FormType class instead of a classic type #385

wants to merge 5 commits into from

Conversation

Pierstoval
Copy link
Contributor

Before:

capture d ecran 2015-07-08 a 11 15 13
capture d ecran 2015-07-08 a 11 16 31

After:

capture d ecran 2015-07-08 a 11 15 00
capture d ecran 2015-07-08 a 11 14 39

If you set up correct cascade options on your entities, it works.

BONUS: I added a new InvalidConfigurationException class to futurely manage all configuration errors and show them in the back-end directly. It'll allow us to replace all different exceptions in the controller with this one when they depend on configuration.

@Pierstoval Pierstoval changed the title Form type definition Define FormType class instead of a classic type Jul 8, 2015
@javiereguiluz
Copy link
Collaborator

@Pierstoval do you still want this feature? Will it work for any case?

@Pierstoval
Copy link
Contributor Author

Yep, I'd like to, but I must rebase and make many checks before this.
Also, I'd like to add a @service feature that will retrieve the form type as service so you could have 3 ways of using a form type:

  • Classic form type (type name that will be retrieved from symfony, but it has to be registered)
  • The class name (new instance created on the fly)
  • Service name (service retrieved from the container).

I think it could cover all possible uses cases.

@Pierstoval
Copy link
Contributor Author

I updated my PR but it's still a WIP, I don't have time to work on it right now so I'm gonna take time next week, just don't merge this PR unless you know what you're doing :)

$formType = new $metadata['fieldType'];
} elseif (strpos($metadata['fieldType'], '@') === 0) {
// If the type starts with "@", we try to retrive the associated service.
$formType = $this->get(substr($metadata['fieldType'], 1));
Copy link
Contributor

Choose a reason for hiding this comment

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

@Pierstoval : I don't understand: why not using the form type alias instead of @form_type_service ? You won't even need this right ? It should already be possible to use our own form types here, no ?

Copy link
10000 Contributor Author

Choose a reason for hiding this comment

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

It allows you to use a form type as a service without having it to be registered as a global type in Symfony (= no tag in your service definition).
I thought it could be useful to have services that are not bound to the Form component, because it can be useful later if we want to support "EasyAdmin plugins" ( = bundles that add features to EasyAdmin, mostly entities configuration)

Copy link
Contributor

Choose a reason for hiding this comment

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

😕 Ok. I don't know, it feels odd to me. Other bundles can provide real form types, it makes no difference to me.

On another note: Remember that in sf 2.8, form type aliases would be deprecated in favor of always using the form type FQCN. So, the first option about creating a form instance from a classname would eventually be problematic.
Also, I don't see much benefits to this possibility, as it cannot be instantiated with arguments, whereas the service is much more powerful.
Offering too many ways to achieve the same thing, but with constraints is rarely a good idea :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, specifying the class name is a constraint.
In this deprecation, you would then register form types with services only or by class names (i.e. with or without DIC).

But actually I'm totally lost in the Form component, it's a part of Symfony that is far too complex for me (brainplosion each time I have to struggle with complex forms...).

So, what to do then? As of Symfony 2.8, class names will be "natively" supported, so do you think I have to cancel this PR, or at least add a version condition for Symfony < 2.8 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this PR makes sense if trying to use our own form types by specifying its alias in the easyadmin configuration isn't fully supported (I don't remember if it is actually). For symfony 2.8 and up, it would not make any difference, as we could use both form alias (until it is removed from symfony) and class FQCN (if sf 2.8+). That would be handled be the form component itself.

All we have to ensure here, is that there isn't any restriction made by EasyAdmin on the usable form types.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, I tried, and this is working like a charm in 2.8 with both aliases and FQCN without this PR (You have to declare the form type as service of course).
Therefore, I think there is nothing more to do here =)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's close the PR then ;)

@javiereguiluz
Copy link
Collaborator
AF08

Closing it after reading your comments. Honestly, I can't understand the Symfony Form component, so I can't review this or add anything else to your discussion. Thanks for helping me with this!

@Pierstoval Pierstoval deleted the form_type_definition branch October 13, 2015 19:36
@Pierstoval
Copy link
Contributor Author

We'll review something similar once Symfony 2.3 will be no longer supported, because the Form component has endured many updates since 2.3, and I don't want to break too many things :)

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

Successfully merging this pull request may close these issues.

3 participants
0