Created Unit tests for getRoutesByName with defined idPrefix. #189
Conversation
Fixed getRoutesByName to work if $idPrefix is not defined. Added Tests to cover those cases.
|
@dbu Here are the tests and the fixes... As Requested. |
| } | ||
| } else { | ||
| $routes = $names; |
There was a problem hiding this comment.
the change in getRoutesByNames should not be needed. the loop is only needed if there actually is a prefix. what you propose ends up in a complicated way to copy $names into $routes.
…es the foreach loop. Removed the .idea file from .gitignore Added Back the missing getRoutesByNames skipped test.
|
@dbu Took care of those notes you requested. Have a good night. |
| @@ -60,7 +60,7 @@ public function setPrefix($prefix) | |||
| */ | |||
| public function getRouteCollectionForRequest(Request $request) | |||
| { | |||
| $url = $request->getPathInfo(); | |||
| $url = $request->getPathInfo(); | |||
|
@dbu fixed the code style fauxpa. |
|
| // $name is the route document path | ||
| if ($this->idPrefix && 0 === strpos($name, $this->idPrefix)) { | ||
| if ( '' === $this->idPrefix || 0 === strpos($name, $this->idPrefix) ) { |
There was a problem hiding this comment.
Spaces should be removed before and after the condition.
|
Please add the pr template to your pr description |
|
@dbu Fixed those notes. |
| foreach ($names as $name) { | ||
| if (0 === strpos($name, $this->idPrefix)) { | ||
| if ('' === $this->idPrefix || 0 === strpos($name, $this->idPrefix)) { |
There was a problem hiding this comment.
sorry, but you still need to undo the changes here, the existing code was correct.
There was a problem hiding this comment.
you can remove the idPrefix check here .. since its redundant
|
ok merging this .. will fix those last details myself. thank you for your work. just want to get this in quickly due to our upcoming stable release schedule |
Created Unit tests for getRoutesByName with defined idPrefix.
[Doctrine] [Phpcr] [RoutePrivder] Fixed LSmith's early optimizations, and added Unit tests to cover.