search

contao / core-bundle

[READ-ONLY] Contao Core Bundle
GNU Lesser General Public License v3.0
123 stars 58 forks source link

[WIP] CMF / Dynamic Routing #1653

Closed aschempp closed 5 years ago

aschempp commented 6 years ago

This is a work in progress how to implement the CMF Dynamic Router to the Contao code base. It currently does nothing more than routing, there are no new features like real controllers for pages etc.

Feel free to review and test it, but there's a lot still to be changed 😊

PS: this also incorporates/replaces https://github.com/contao/core-bundle/pull/1637

aschempp commented 6 years ago

I think this should be reviewed and merged pretty soon, because we will likely need multiple iterations on the concept. But that won't be possible without first getting something done (remember Security and Fragments) …

leofeyer commented 6 years ago

I cannot merge this with all checks failing though.

aschempp commented 6 years ago

😂 feel free to review/test first 😝

leofeyer commented 6 years ago

In order to merge this, we should first try to write a test for the existing routing logic. Then we can be sure that the new logic does not break anything.

@aschempp Can we port the FrontendTest class to test the routing in Contao 4.6?

aschempp commented 5 years ago

Can we port the FrontendTest class to test the routing in Contao 4.6?

That was my idea. We should at some point port all the tests, but in CMF-Router they will span multiple classes. I'm not sure if we should rather find a way to write functional tests, maybe by using the new Symfony Panther?

leofeyer commented 5 years ago

As a first step, I have created a test class that tests all variations of Frontend::getPageIdFromUrl() (see contao/contao@0bd65319f3e1528931d2ed114fa131a4b4020dc0). What else needs to be tested?

aschempp commented 5 years ago

Honestly I'm not sure this test will actually handle all cases. I tried to implement the same but I failed because you never realize which statement returns null or false. That's why I copied the whole method and changed the return statements to exceptions in https://github.com/contao/core-bundle/pull/1653/files#diff-c52fbc6c0eb3348578874e564aeaa1aeR73

Maybe we should start by writing down a list of supported and non-supported cases that should be tested? But we should probably brainstorm that on a phone call …

aschempp commented 5 years ago

see https://github.com/contao/contao/pull/95