KnpLabs / KnpRadBundle

Rapid Application Development for Symfony2 [UNMAINTAINED]
MIT License
291 stars 49 forks source link

fix form alias #199

Closed chrisyue closed 9 years ago

chrisyue commented 10 years ago

The dot . is not allowed in the form alias (or form type name), so the dot should be replaced by underscore in the compiler pass, otherwise the form type can't be found (in a situation that the form type is created manually and the entity is not in the same bundle as the form type). Of course, to make it work, the form type's getName method should return the proper alias name (eg. app_form_new_task_type)

I also suggest that the bundle name app should not hard coded in the form alias when FormTypeCreator try to find the proper form type with entity name, currently the form alias format is

sprintf('app_form_%s%s_type', $purpose, $shortEntityClassName);

it could be better if change it to:

sprintf('%s_%s%s', $entityBundleName, $purpose, $shortEntityClassName);

The form and type are also be removed because they seem pointless and tedious(it must be a FORM TYPE's alias)

I'll update this PR if you agree with that. :)

docteurklein commented 10 years ago

thanks for opening this pr :) I can only agree on all what you say. You can go ahead and apply modification you suggested. It will be a BC break we'll have to handle. Also, can you see why tests fail (aside from the behat features, those are unrelated)? Thanks!

chrisyue commented 10 years ago

Sure. however I am on vacation until 19th Oct. I'll PR again as soon as possible when I get home.

chrisyue commented 10 years ago

I have been busy these days so I didn't PR yet, sorry about that. I checked the behat test result yesterday and I don't know why it's failed, and I need more time to figure it out. Of course if someone can help me with that I'll appreciate :)

docteurklein commented 10 years ago

i think it's directly related to the form alias change: we verify the output, but interestint changed with thihis pr.

docteurklein commented 10 years ago

the behat suite failed for another unrelated reason. only phpsec suite has to be fixed. run it, you'll immediatly see the problem :-)

chrisyue commented 10 years ago

@docteurklein yes you are right and I've updated the code. Now the form type name should be {bundlename}_entityclassname(all lowercases).

for example: if I have an entity Star\Bundle\Craft\Entity\Zergling and a form App\Form\Type\ZerglingType(or App\Form\ZerglingType), the form's getName method should return starcraft_zergling(although the form is in bundle App but it's for entity Star\Bundle\Craft\Zergling), otherwise a form called starcraft_zergling will be automatically generated for the entity.

chrisyue commented 9 years ago

I've updated a new commit to remove some unneccesary code, and also updated the phpspec test code.

chrisyue commented 9 years ago

@docteurklein could this PR be merged?