[WCM] Deleting save and remove methods from repositories

[mdoutreluingne]

Fix #1327

In this PR I've removed the save and remove methods from the repositories, for the reasons explained here #1327.

( The add method has been renamed to save, here #1204 )

Reviews welcome!

[mdoutreluingne]

It seems that the red tests are linked to my PR. However, I want to correct the tests but I am unable them.

Locally I still have this error Compile Error: Cannot use Doctrine\ORM\EntityManagerInterface as EntityManagerInterface because the name is already in use that I am unable correct them .

Can you please help me?

[weaverryan]

Thanks for taking this on! I just dropped a hint about the failed test

[mdoutreluingne]

It looks like the other failed test came from it_generates_basic_crud.php, I'll try to fix that tomorrow

[mdoutreluingne]

Finally, the remaining failed test came from it_generates_crud_using_custom_repository on the assert assertFileEquals because the files WithCustomRepository.php and SweetFoodController.php were not identical due to the renderForm methods in WithCustomRepository.php for the route names app_sweet_food_new and app_sweet_food_edit.

When I ran the tests locally, I didn't get this error again. It's strange because after committing and pushing on the repository, the it_generates_crud_using_custom_repository test still fails, whereas locally I don't get any more errors concerning this test.

I noticed that this test also failed on the PRs(#1330, #1325).

Would you have any suggestions for correcting this test?

[weaverryan]

The tests can be sensitive. That's partly by design, but they can be a pain... especially when it comes to different Symfony versions. Here's what I'm thinking:

A) Duplicate WithCustomRepository.php to create a new WithCustomRepositoryLegacy.php that includes the renderForm() version.

B) In MakeCrudTest, dynamically choose WithCustomRepository.php vs WithCustomRepositoryLegacy.php in the assertFileEquals() by checking Kernel::VERSION_ID < 60200. Something like:

- sprintf('%s/fixtures/%s', \dirname(__DIR__), 'make-crud/expected/WithCustomRepository.php'),
+ sprintf('%s/fixtures/make-crud/expected/WithCustomRepository%s.php', \dirname(__DIR__), Kernel::VERSION_ID < 60200 ? 'Legacy' : ''),

Let me know if that helps :)

[mdoutreluingne]

Unfortunately, the tests failed globally (1 test failed when installing symfony/console), I don't know why it compares render() and renderForm() when WithCustomRepository.php is chosen dynamically.

One thing to note: the last 2 tests failed on the index and show route names, while the other 2 failed on the new and edit route names.

[weaverryan]

My code suggestion for checking the Symfony version was slightly wrong - I just pushed the fix for it. Let's see what the tests think now :)

[mdoutreluingne]

The tests were successful ! :)

@weaverryan Thanks for your help

I didn't understand why you replaced renderForm() by render() for the route name index and show in WithCustomRepositoryLegacy.php, which should normally be the version with renderForm()

[weaverryan]

I didn't understand why you replaced renderForm() by render() for the route name index and show in WithCustomRepositoryLegacy.php, which should normally be the version with renderForm()

Even back when renderForm() existed and was the correct solution, there was no reason to call renderForm() if you weren't passing a form into the template. So index and show have always generated as render().

Anyway, tests green now! Woo!

[weaverryan]

Thanks Maxime!