Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

[WIP] Create custom behavior for RouteNotFoundException #177

Closed
wants to merge 58 commits into from

Conversation

ElectricMaxxx
Copy link
Member

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? no
Fixed tickets #10
License MIT
Doc PR need to be done

Todo:

  • implement the create() methods of the base matcher i created (parent, ancestor)
  • bring the BestMatcherPresentation to use the custom exception.html.twig i created
  • test for BestMatcherPresentation
  • tests for both base matcher
  • finish frontend test by crawling for some given stuff

$queryBuilder = $routeRepository->createQueryBuilder('r');
$queryBuilder->getChildren();
$queryBuilder->where('r.id = '.$parentUri);
$routes = $queryBuilder->getQuery();
Copy link
Member Author

Choose a reason for hiding this comment

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

Some query profs out there to help me to optimize/repair those queries.

@ElectricMaxxx
Copy link
Member Author

@dbu in generall this PR is very far away from ready. Just wanted to ask for some help with the querries. Got my first esxperiences with the orm querybuilder and never used the odm one. What i plan is:

  • config options for the possible best matches: parent, ancestor nodes ..., more?
  • do the queries depending on the config
  • pass the result route + content to the template
  • use the methods of twig's ExceptionController to get template

Not sure atm if it would be enough to simple set/overwrite the ExceptionController on twig.exception_controller or if i need to fetch kernel.exception events.

@ElectricMaxxx
Copy link
Member Author

Maybe i would provide a "you can go to" collection in configuration or We would just keep that in the template.

@ElectricMaxxx
Copy link
Member Author

Yea i changed a little bit:

Instead of putting the "best matchers" into the configuration by some shortcut or something else. I decided to create tags for them.Those BestMatchers are passed to a chain into the BestMatcherPresentation which is just an other implementation of Twigs ExceptionController. It does nothing more than looping through the chain of matchers and collect some RouteCollections, which can enrich a template for showing the Error.

Todo: see description on the top.

@@ -1,6 +1,7 @@
Changelog
=========

* **2014-02-08**: Custom exception controller for error handling
Copy link
Member

Choose a reason for hiding this comment

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

we use european dates, not the confused american ones ;-) so its YYYY-MM-DD

@ElectricMaxxx
Copy link
Member Author

Beside the implementation of both matcher i would like to server, i can't test the Frontend. For some reason the TwigBundle:Exception:exception.html.twig can't be overwritten by setting it up in the follwing folder: ./Tests/Resources/app/config/Resources/TwigBundle/views/Exception/exception.html.twig @wouterj do you have got a hint for me? Is that fould not usable as global Resources of an application?

@wouterj
Copy link
Member

wouterj commented Aug 10, 2014

@wouterj do you have got a hint for me?

It should be ./Tests/Resources/app/Resources/TwigBundle/views/Exception/exception.html.twig

@ElectricMaxxx
Copy link
Member Author

But it is: Tests/Resources/app/Resources/TwigBundle/views/Exception/exception.html.twig or here:
https://github.com/symfony-cmf/SeoBundle/blob/custom-exception-or-errors/Tests/Resources/app/Resources/TwigBundle/views/Exception/exception.html.twig

That was my reason to ask. @wouterj does the TestKernel miss some kind of ovewriting bundle templates? Or any hints how to test that?

public function create(Request $request)
{
// TODO: Implement create() method.
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Somebody (@dantleech, @lsmith77) any hints to build a good and fast query for getting all ancestors?
parent -> all children?

Copy link
Member

Choose a reason for hiding this comment

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

you don't want to use a query but instead want to use getChildren() .. there is a method for this both on the PHPCR as well as the PHPCR ODM level. In both cases you first need to fetch the parent (node or document).

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

In ErrorHandling/AncestorBestMatcher.php:

+/**

yes ..

}

$uriAsArray = array_shift($uriAsArray);
$parentUri = implode('/', $uriAsArray);
Copy link
Member Author

Choose a reason for hiding this comment

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

is there a nice reg. expression for that 7 lines?

Copy link
Member

Choose a reason for hiding this comment

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

PathHelper from phpcr-utils offers a method to get the parent path

Copy link
Member Author

Choose a reason for hiding this comment

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

Bingo :-)

@ElectricMaxxx
Copy link
Member Author

Will check the BestMatcher with the WebTests only, or? Cause writing some Unit-Tests for them would be an overkill (Mocking Registry, manager ...), and i would just rewrite the method behavior with mocks -> TestCodeSmell.

But need to get rid of the problem with the template loading first.

@dbu
Copy link
Member

dbu commented Aug 12, 2014

imho functional tests that test the behaviour of this bundle and not
some third party functionality are telling more than unit tests. unit
tests are best for algorithms or to track down specific bugs. so +1

@ElectricMaxxx
Copy link
Member Author

i just wanted to do a git rebase master but something went wrong.

@ElectricMaxxx
Copy link
Member Author

think i have fixed the template problem. Need to write some better tests now and then this one is ready.

@lsmith77
Copy link
Member

lsmith77 commented Oct 4, 2014

ping

@wouterj
Copy link
Member

wouterj commented Oct 4, 2014

Something went badly wrong here. I can't fix it with a rebase.

@ElectricMaxxx since you know which commits belong to this PR and which don't, you can create a new branch based on master and cherry-pick the commits of this PR into that new branch and start a new PR.

@ElectricMaxxx
Copy link
Member Author

Ok. But the most important: the test go green.

@lsmith77
Copy link
Member

lsmith77 commented Oct 4, 2014

btw .. I fixed the tests in master

@ElectricMaxxx
Copy link
Member Author

created follow up in #192

@ElectricMaxxx ElectricMaxxx deleted the custom-exception-or-errors branch February 14, 2015 12:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants