Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EZP-26710 : Error on fielddescription without description #1842

Conversation

nicolas-bastien
Copy link
Contributor

https://jira.ez.no/browse/EZP-26710

Description

When you try to call FieldDefinition::getDescription($languageCode) on a FieldDefinition with no description you ends on an

Warning: array_key_exists() expects parameter 2 to be array, boolean given"

Step to reproduce

Go on the new content vie page form a content type folder
ezsystems/PlatformUIBundle#732

Test

Manually

@nicolas-bastien
Copy link
Contributor Author

@bdunogier tavis green ready for review (easy)

@@ -116,7 +116,7 @@ public function getDescriptions()
*/
public function getDescription($languageCode)
{
if (array_key_exists($languageCode, $this->descriptions)) {
if (is_array($this->descriptions) && array_key_exists($languageCode, $this->descriptions)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't the default value of descriptions be an array instead/as well since it's documented as string[] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same question as names are the same but as I don't master FielDefinition I did the less breaker way.

What I can do to make something cleaner is :

  • add a defaults variable to ValueObject
  • use it in the construct to set default properties
  • add name and description default to FieldDescription

Would you prefer that ?

/cc @bdunogier @andrerom

Copy link
Member

Choose a reason for hiding this comment

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

I'd also prefer if the property was made an array, since it is documented as such.

Check

$field->description = unserialize($row['ezcontentclass_attribute_serialized_description_list']);
, this is where FieldDefinition objects are initialized. This builds the Persistence (SPI) object, which is then mapped to API objects there.

@@ -116,7 +116,7 @@ public function getDescriptions()
*/
public function getDescription($languageCode)
{
if (array_key_exists($languageCode, $this->descriptions)) {
if (is_array($this->descriptions) && array_key_exists($languageCode, $this->descriptions)) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd also prefer if the property was made an array, since it is documented as such.

Check

$field->description = unserialize($row['ezcontentclass_attribute_serialized_description_list']);
, this is where FieldDefinition objects are initialized. This builds the Persistence (SPI) object, which is then mapped to API objects there.

@nicolas-bastien nicolas-bastien force-pushed the EZP-26710-Error-fielddescription-without-description branch from 163f6f6 to 92e5d56 Compare January 9, 2017 09:28
@nicolas-bastien
Copy link
Contributor Author

Close i nfavour of #1876 to ease QA validation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants