Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Dont override extras #118

Merged
merged 3 commits into from

4 participants

@bchoquet-heliopsis

Since CoreExtension is loaded with a low priority in MenuFactory, prior extensions setting extras at item building time currently get their values overriden.

NB : Sorry about the messed up PRs. This is the one.

@stof
Owner

you don't need to create a new PR each time. You can simply force the push to the branch

@bchoquet-heliopsis

I thought that was what I did yesterday and got puzzled, sorry again.

@bchoquet-heliopsis

Not even a review ?

@Nek-
Owner

@stof anything against this PR ?

@bchoquet-heliopsis

I fixed code style according to PSR-2

@Nek-
Owner

Weird, why you PR can't pass tests ? o_o

@bchoquet-heliopsis

Well it does : https://travis-ci.org/KnpLabs/KnpMenu/builds/17988387
First commit did not but i fixed it

@Nek-
Owner

I had something weird frome here, now it's ok :)

Thank you very much.

@Nek- Nek- merged commit 835ee51 into KnpLabs:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 7, 2014
  1. @bchoquet-heliopsis
Commits on Jan 8, 2014
  1. @bchoquet-heliopsis

    fixed tests

    bchoquet-heliopsis authored
Commits on Jan 31, 2014
  1. @bchoquet-heliopsis

    CS fixes

    bchoquet-heliopsis authored
This page is out of date. Refresh to see the latest.
View
19 src/Knp/Menu/Factory/CoreExtension.php
@@ -50,10 +50,27 @@ public function buildItem(ItemInterface $item, array $options)
->setLinkAttributes($options['linkAttributes'])
->setChildrenAttributes($options['childrenAttributes'])
->setLabelAttributes($options['labelAttributes'])
- ->setExtras($options['extras'])
->setCurrent($options['current'])
->setDisplay($options['display'])
->setDisplayChildren($options['displayChildren'])
;
+
+ $this->buildExtras($item, $options);
+ }
+
+ /**
+ * Configures the newly created item's extras
+ * Extras are processed one by one in order not to reset values set by other extensions
+ *
+ * @param ItemInterface $item
+ * @param array $options
+ */
+ private function buildExtras(ItemInterface $item, array $options)
+ {
+ if (!empty($options['extras'])) {
+ foreach ($options['extras'] as $key => $value) {
+ $item->setExtra($key, $value);
+ }
+ }
}
}
View
93 tests/Knp/Menu/Tests/Factory/CoreExtensionTest.php
@@ -0,0 +1,93 @@
+<?php
+/**
+ * @author: bchoquet
+ */
+
+namespace Knp\Menu\Tests\Factory;
+
+
+use Knp\Menu\Factory\CoreExtension;
+use Knp\Menu\MenuFactory;
+use Knp\Menu\MenuItem;
+
+class CoreExtensionTest extends \PHPUnit_Framework_TestCase
+{
+ public function testBuildOptions()
+ {
+ $extension = $this->getExtension();
+ $item = $this->createItem( 'test' );
+
+ $options = $extension->buildOptions( array() );
+
+ $this->assertArrayHasKey( 'uri', $options );
+ $this->assertArrayHasKey( 'label', $options );
+ $this->assertArrayHasKey( 'attributes', $options );
+ $this->assertArrayHasKey( 'linkAttributes', $options );
+ $this->assertArrayHasKey( 'childrenAttributes', $options );
+ $this->assertArrayHasKey( 'labelAttributes', $options );
+ $this->assertArrayHasKey( 'extras', $options );
+ $this->assertArrayHasKey( 'current', $options );
+ $this->assertArrayHasKey( 'display', $options );
+ $this->assertArrayHasKey( 'displayChildren', $options );
+ }
+
+ public function testBuildItemsSetsExtras()
+ {
+ $item = $this->createItem( 'test' );
+ $item->setExtra( 'test1', 'original value' );
+ $extension = $this->getExtension();
+ $options = $extension->buildOptions(
+ array(
+ 'extras' => array(
+ 'test1' => 'options value 1',
+ 'test2' => 'options value 2',
+ )
+ )
+ );
+
+ $extension->buildItem( $item, $options );
+
+ $extras = $item->getExtras();
+
+ $this->assertEquals( 2, count( $extras ) );
+
+ $this->assertArrayHasKey( 'test1', $extras );
+ $this->assertEquals( 'options value 1', $item->getExtra( 'test1' ) );
+
+ $this->assertArrayHasKey( 'test2', $extras );
+ $this->assertEquals( 'options value 2', $item->getExtra( 'test2' ) );
+ }
+
+ public function testBuildItemDoesNotOverrideExistingExtras()
+ {
+ $item = $this->createItem( 'test' );
+ $item->setExtra( 'test1', 'original value' );
+ $extension = $this->getExtension();
+ $options = $extension->buildOptions(
+ array(
+ 'extras' => array(
+ 'test2' => 'options value',
+ )
+ )
+ );
+
+ $extension->buildItem( $item, $options );
+
+ $this->assertArrayHasKey( 'test1', $item->getExtras() );
+ $this->assertEquals( 'original value', $item->getExtra( 'test1' ) );
+ }
+
+ private function getExtension()
+ {
+ return new CoreExtension();
+ }
+
+ private function createItem( $name )
+ {
+ $factory = new MenuFactory();
+ $item = new MenuItem( $name, $factory );
+
+ return $item;
+ }
+}
+
Something went wrong with that request. Please try again.