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

allow KeyValueContainer objects to be passed as key-value arrays #158

Merged
merged 1 commit into from
May 2, 2014
Merged

allow KeyValueContainer objects to be passed as key-value arrays #158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions Form/Type/SeoMetadataType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,20 @@ class SeoMetadataType extends AbstractType
*/
public function buildForm(FormBuilderInterface $builder, array $options)
{
$keyValueOptions = array(
'required' => false,
'value_type' => 'text',
'use_container_object' => true,
);

$builder
->add('title', 'text', array('required' => false))
->add('originalUrl', 'text', array('required' => false))
->add('metaDescription', 'textarea', array('required' => false))
->add('metaKeywords', 'textarea', array('required' => false))
->add('extraProperties', 'burgov_key_value', array('required' => false, 'value_type' => 'text'))
->add('extraNames', 'burgov_key_value', array('required' => false, 'value_type' => 'text'))
->add('extraHttp', 'burgov_key_value', array('required' => false, 'value_type' => 'text'))
->add('extraProperties', 'burgov_key_value', $keyValueOptions)
->add('extraNames', 'burgov_key_value', $keyValueOptions)
->add('extraHttp', 'burgov_key_value', $keyValueOptions)
;
}

Expand Down
28 changes: 22 additions & 6 deletions Model/SeoMetadata.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

namespace Symfony\Cmf\Bundle\SeoBundle\Model;

use Burgov\Bundle\KeyValueFormBundle\KeyValueContainer;
Copy link
Member

Choose a reason for hiding this comment

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

there should be an empty line between the namespace and use declarations

use Symfony\Cmf\Bundle\SeoBundle\Exception\InvalidArgumentException;

/**
* This class is a container for the metadata.
*
Expand Down Expand Up @@ -190,9 +193,9 @@ public function getTitle()
/**
* {@inheritDoc}
*/
public function setExtraProperties(array $extraProperties)
public function setExtraProperties($extraProperties)
{
$this->extraProperties = $extraProperties;
$this->extraProperties = $this->toArray($extraProperties);

return $this;
}
Expand Down Expand Up @@ -226,9 +229,9 @@ public function removeExtraProperty($key)
/**
* {@inheritDoc}
*/
public function setExtraNames(array $extraNames)
public function setExtraNames($extraNames)
{
$this->extraNames = $extraNames;
$this->extraNames = $this->toArray($extraNames);

return $this;
}
Expand Down Expand Up @@ -262,9 +265,9 @@ public function removeExtraName($key)
/**
* {@inheritDoc}
*/
public function setExtraHttp(array $extraHttp)
public function setExtraHttp($extraHttp)
{
$this->extraHttp = $extraHttp;
$this->extraHttp = $this->toArray($extraHttp);

return $this;
}
Expand Down Expand Up @@ -294,4 +297,17 @@ public function removeExtraHttp($key)
unset($this->extraHttp[$key]);
}
}

private function toArray($data)
{
if ($data instanceof KeyValueContainer) {
Copy link
Member

Choose a reason for hiding this comment

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

this requires people to use the BurgovKeyValueFormBundle when using this class, while it is only required when using the form type/admin. I would suggest to add a little check in here to make it optional:

if (class_exists('Burgov\Bundle\KeyValueFormBundle\KeyValueContainer') && $data instanceof KeyValueContainer) {
    // ...
}

Copy link
Member

Choose a reason for hiding this comment

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

@wouterj php is sloppy enough to accept a non-existing class in that place.

Copy link
Member

Choose a reason for hiding this comment

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

hmm, indeed... http://3v4l.org/BldOT

$data = $data->toArray();
}

if (!is_array($data)) {
throw new InvalidArgumentException(sprintf('Expected either an array or KeyValueContainer, got "%s"', is_object($data) ? getclass($data) : get_type($data)));
}

return $data;
}
}
14 changes: 8 additions & 6 deletions Model/SeoMetadataInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

namespace Symfony\Cmf\Bundle\SeoBundle\Model;

use Burgov\Bundle\KeyValueFormBundle\KeyValueContainer;

/**
* The interface for the SeoMetadata.
*
Expand Down Expand Up @@ -79,19 +81,19 @@ public function setTitle($title);
public function getTitle();

/**
* @param array
* @param array|KeyValueContainer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is it OK like this, or should I remove the use statement and use FQDN's for all three methods?

Copy link
Member

Choose a reason for hiding this comment

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

no, you did it the correct way :)

*/
public function setExtraProperties(array $extraProperties);
public function setExtraProperties($extraProperties);

/**
* @param array
* @param array|KeyValueContainer
*/
public function setExtraNames(array $extraNames);
public function setExtraNames($extraNames);

/**
* @param array
* @param array|KeyValueContainer
*/
public function setExtraHttp(array $extraHttp);
public function setExtraHttp($extraHttp);

/**
* @return array
Expand Down
9 changes: 5 additions & 4 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
"php": ">=5.3.3",
"symfony/framework-bundle": "~2.3",
"sonata-project/seo-bundle": "1.1.*@dev",
"doctrine/collections": "1.*",
"burgov/key-value-form-bundle": "1.0.*@dev"
"doctrine/collections": "1.*"
},
"require-dev": {
"symfony-cmf/testing": "dev-master",
Expand All @@ -28,13 +27,15 @@
"symfony-cmf/routing-bundle": "1.2.*@dev",
"symfony-cmf/core-bundle": "1.1.*@dev",
"matthiasnoback/symfony-dependency-injection-test": "0.*",
"matthiasnoback/symfony-config-test": "0.*"
"matthiasnoback/symfony-config-test": "0.*",
"burgov/key-value-form-bundle": "1.0.*"
},
"suggest": {
"sonata-project/doctrine-phpcr-admin-bundle": "To provide an admin extension for the seo metadata",
"doctrine/phpcr-bundle": "To persist the metadata in PHPCR ODM documents",
"doctrine/doctrine-bundle": "To persist the metadata in ORM entities",
"doctrine/orm": "2.4.*@dev"
"doctrine/orm": "2.4.*@dev",
"burgov/key-value-form-bundle": "To edit the seo metadata 'extra' fields"
},
"autoload":{
"psr-0":{
Expand Down