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-28226: User can't add more fields after removing several of them #187

Merged
merged 1 commit into from
Dec 13, 2017

Conversation

webhdx
Copy link
Contributor

@webhdx webhdx commented Dec 12, 2017

JIRA: https://jira.ez.no/browse/EZP-28226

Description

This PR fixes Content Type Edit > Add Field Definition action which had been failing if first field definitions were removed. It was caused by assigning already existing identifier. I created a method which resolves new name making sure it doesn't exist in edited Content Type.

Remarks

It has to be ported upwards to 1.10 and master.

@webhdx webhdx added the Bug label Dec 12, 2017
@webhdx webhdx self-assigned this Dec 12, 2017
@micszo
Copy link
Member

micszo commented Dec 12, 2017

Cannot reload cache on v2 with this branch:
screen shot 2017-12-12 at 16 12 37

@webhdx
Copy link
Contributor Author

webhdx commented Dec 13, 2017

@micszo This needs to be rebased on master in order to work in v2. Please test on 1.7 first.

@micszo
Copy link
Member

micszo commented Dec 13, 2017

Retest OK with eZ Platform EE v1.7.
Moving on to v1.13.

@micszo
Copy link
Member

micszo commented Dec 13, 2017

Retest OK with eZ Platform EE v1.13.0-beta1 with this branch.

@micszo
Copy link
Member

micszo commented Dec 13, 2017

@webhdx is it possible to test with v2 before merge?

@webhdx
Copy link
Contributor Author

webhdx commented Dec 13, 2017

@micszo
Copy link
Member

micszo commented Dec 13, 2017

Retest OK with eZ Platform EE v2 with patch.

@lserwatka lserwatka merged commit 5ed6bb8 into 1.5 Dec 13, 2017
@lserwatka lserwatka deleted the content_type_edit_add_def branch December 13, 2017 10:10
@lserwatka
Copy link
Member

@webhdx could you merge this up?

@webhdx
Copy link
Contributor Author

webhdx commented Dec 13, 2017

@lserwatka Done.

@andrerom
Copy link
Contributor

andrerom commented Dec 14, 2017

@webhdx This one breaks unit test on 1.10 and up:

There was 1 failure:

1) EzSystems\RepositoryForms\Tests\Form\Processor\ContentTypeFormProcessorTest::testAddFieldDefinition
Expectation failed for method name is equal to <string:addFieldDefinition> when invoked 1 time(s)
Parameter 1 for invocation eZ\Publish\API\Repository\ContentTypeService::addFieldDefinition(eZ\Publish\Core\Repository\Values\ContentType\ContentTypeDraft Object (...), eZ\Publish\API\Repository\Values\ContentType\FieldDefinitionCreateStruct Object (...)) does not match expected value.
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 eZ\Publish\API\Repository\Values\ContentType\FieldDefinitionCreateStruct Object (
     'fieldTypeIdentifier' => 'ezstring'
-    'identifier' => 'new_ezstring_3'
+    'identifier' => 'new_ezstring_1'
     'names' => Array (...)
     'descriptions' => null
     'fieldGroup' => 'content'
     'position' => 1
     'isTranslatable' => null
     'isRequired' => null
     'isInfoCollector' => null
     'validatorConfiguration' => null
     'fieldSettings' => null
     'defaultValue' => null
     'isSearchable' => null
 )

/repository-forms/lib/Form/Processor/ContentTypeFormProcessor.php:119
/repository-forms/tests/RepositoryForms/Form/Processor/ContentTypeFormProcessorTest.php:159

@andrerom
Copy link
Contributor

andrerom commented Dec 14, 2017

And here on 1.5 it break PHP parsing on PHP 5.6:

PHP Parse error:  syntax error, unexpected ':', expecting ';' or '{' in /home/travis/build/ezsystems/repository-forms/lib/Form/Processor/ContentTypeFormProcessor.php on line 172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants