[Admin] Fix user fields disappearing on validation fails #6926

tuka217 · 2016-11-29T10:38:59Z

Q	A
Bug fix?	yes
New feature?	no
BC breaks?	yes
Related tickets	fixes #6725
License	MIT

pamil · 2016-11-29T11:00:05Z

src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php

+    }
+
+    /**
+     * @Then I should not be able to specify it password


pamil · 2016-11-29T11:00:51Z

src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php

+    {
+        Assert::false(
+            $this->createPage->hasPasswordField(), 
+            'There should not be password field, but it is.'


Please, nooooo, no no no, nope nope, this is the most annoying message while debugging :( :( :( 🌮

So how looks proper message?

The step does say what behaviour is expected and if it's failing, it says that expected behaviour didn't occur. No message is the best message 🌯 🎉

pamil · 2016-11-29T11:00:59Z

src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php

+    {
+        Assert::true(
+            $this->createPage->isOpen(),
+            'The customer creation page should be open, but it is not.'


pamil · 2016-11-29T11:01:13Z

src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php

+    {
+        Assert::false(
+            $this->createPage->hasCheckedCreateOption(),
+            'The create account option should not be selected, but it is.'


Zales0123 · 2016-11-30T13:12:27Z

features/user/managing_customers/adding_customer_account_after_failed_creation_action.feature

+        And I do not specify any information
+        And I try to add it
+        Then I should still be on the customer creation page
+        And I should be able to specify it password


I should be able to specify its password

Zales0123 · 2016-11-30T13:13:09Z

features/user/managing_customers/adding_customer_account_after_failed_creation_action.feature

+        Given I am logged in as an administrator
+
+    @ui @javascript
+    Scenario: Trying add new customer with an account without required information


Shouldn't be Trying to add...?

Zales0123 · 2016-11-30T13:14:07Z

features/user/managing_customers/being_able_to_create_account_for_existing_customer.feature

+Feature: Create account option availability
+    In order to correctly administrate customers
+    As an Administrator
+    I want not see create account option if the customer account already exist


Zales0123 · 2016-11-30T13:16:29Z

src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php

+    }
+
+    /**
+     * @Given /^I should not be able to select create account option$/


Unneeded /^ and $/

and @Then

Zales0123 · 2016-11-30T13:17:16Z

src/Sylius/Behat/Context/Ui/Admin/ManagingCustomersContext.php

+    }
+
+    /**
+     * @Given I do not choose create account option


Zales0123 · 2016-11-30T13:18:09Z

src/Sylius/Bundle/AdminBundle/Resources/private/js/app.js

@@ -14,7 +14,7 @@
            'containerSelector': '#sylius_calculator_container'
        });

-        $('#sylius_customer_create_user').change(function(){
+        $('#sylius_customer_createUser').change(function(){


Why changing it?

Because we use camelCase to named field in FormType. So we have sylius_nameOfEntity_nameOfField. Before this field was in twig template and it had the id "sylius_customer_create_user".

Zales0123 · 2016-11-30T13:19:05Z

src/Sylius/Bundle/CoreBundle/Form/EventSubscriber/AddUserFormSubscriber.php


-            return;
-        }
+        if(null === $form->get('createUser')->getViewData()) {


Missing space after if.

Zales0123 · 2016-11-30T13:20:00Z

src/Sylius/Bundle/CoreBundle/spec/Form/EventSubscriber/AddUserFormSubscriberSpec.php

+        $form->get('createUser')->willReturn($createUserCheckForm);
+        $createUserCheckForm->getViewData()->willReturn(null);
+
+


Double blank line.

michalmarcinkowski · 2016-11-30T20:13:25Z

src/Sylius/Bundle/ApiBundle/Form/EventSubscriber/AddUserFormSubscriber.php

@@ -9,7 +9,7 @@
 * file that was distributed with this source code.
 */

-namespace Sylius\Bundle\UserBundle\Form\EventSubscriber;
+namespace Sylius\Bundle\ApiBundle\Form\EventSubscriber;


Why you moved this to ApiBundle?

Is it truly a part of UserBundle? Before we use this subscriber for Core CustomerType. After this fix this type has new subscriber, but the old one is still needed in ApiBundle.

And probably should be fixed for api, but I'm not sure that it should be done in this PR.

Having two separate form types is the only valid solution, since we expect a different behavior on API and Web, so that's correct! 👍

…h account after fialed create acction [AdminBundle] Fix customer _form file

[AdminBundle] Fix Customer _form and app.js

michalmarcinkowski · 2016-12-01T08:48:12Z

src/Sylius/Bundle/ApiBundle/Resources/config/routing/customer.yml

@@ -19,6 +19,8 @@ sylius_api_customer_show:
        _controller: sylius.controller.customer:showAction
        _sylius:
            serialization_groups: [Detailed]
+            form:
+                type: Sylius\Bundle\ApiBundle\Form\Extension\ApiCustomerType


We dont need to configure form for show action

Please fix it in a separate PR.

michalmarcinkowski · 2016-12-01T08:50:20Z

src/Sylius/Bundle/ApiBundle/Form/EventSubscriber/AddUserFormSubscriber.php

@@ -9,7 +9,7 @@
 * file that was distributed with this source code.
 */

-namespace Sylius\Bundle\UserBundle\Form\EventSubscriber;
+namespace Sylius\Bundle\ApiBundle\Form\EventSubscriber;


Having two separate form types is the only valid solution, since we expect a different behavior on API and Web, so that's correct! 👍

michalmarcinkowski · 2016-12-01T10:29:33Z

Great job Ania! 👍

tuka217 force-pushed the user-disappear branch from 8279b8d to a7e9a02 Compare November 29, 2016 10:44

pjedrzejewski added the Bug Fix label Nov 29, 2016

tuka217 force-pushed the user-disappear branch 2 times, most recently from 8766e8a to 947c293 Compare November 29, 2016 10:59

tuka217 changed the title ~~[WIP][Admin] Fix user fields disappearing on validation fails~~ [Admin] Fix user fields disappearing on validation fails Nov 29, 2016

pamil reviewed Nov 29, 2016

View reviewed changes

tuka217 changed the title ~~[Admin] Fix user fields disappearing on validation fails~~ [WIP][Admin] Fix user fields disappearing on validation fails Nov 29, 2016

tuka217 force-pushed the user-disappear branch 4 times, most recently from 409fef9 to bf6cf8a Compare November 30, 2016 13:03

tuka217 changed the title ~~[WIP][Admin] Fix user fields disappearing on validation fails~~ [Admin] Fix user fields disappearing on validation fails Nov 30, 2016

Zales0123 reviewed Nov 30, 2016

View reviewed changes

tuka217 force-pushed the user-disappear branch 3 times, most recently from 3728876 to e3b0daf Compare November 30, 2016 15:35

michalmarcinkowski reviewed Nov 30, 2016

View reviewed changes

tuka217 added 4 commits December 1, 2016 10:19

[Behat] Add scenarios with implementation about creating customer wit…

eb5777c

…h account after fialed create acction [AdminBundle] Fix customer _form file

[CoreBundle][UserBundle] Move AddUserFormSubsciber to CoreBundle

c63933c

[AdminBundle] Fix Customer _form and app.js

[ApiBundle] New CustomerType

829ceba

fix after review

21cf071

tuka217 force-pushed the user-disappear branch from e3b0daf to 21cf071 Compare December 1, 2016 09:19

michalmarcinkowski reviewed Dec 1, 2016

View reviewed changes

michalmarcinkowski merged commit 9af5c7a into Sylius:master Dec 1, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Admin] Fix user fields disappearing on validation fails #6926

[Admin] Fix user fields disappearing on validation fails #6926

tuka217 commented Nov 29, 2016 •

edited

Loading

pamil Nov 29, 2016

pamil Nov 29, 2016

tuka217 Nov 29, 2016

pamil Nov 29, 2016

pamil Nov 29, 2016

pamil Nov 29, 2016

Zales0123 Nov 30, 2016

Zales0123 Nov 30, 2016

Zales0123 Nov 30, 2016

Zales0123 Nov 30, 2016

Zales0123 Nov 30, 2016

Zales0123 Nov 30, 2016

Zales0123 Nov 30, 2016

tuka217 Nov 30, 2016 •

edited

Loading

Zales0123 Nov 30, 2016

Zales0123 Nov 30, 2016

michalmarcinkowski Nov 30, 2016

tuka217 Dec 1, 2016 •

edited

Loading

tuka217 Dec 1, 2016 •

edited

Loading

michalmarcinkowski Dec 1, 2016

michalmarcinkowski Dec 1, 2016

michalmarcinkowski Dec 1, 2016

michalmarcinkowski Dec 1, 2016

michalmarcinkowski commented Dec 1, 2016

		$form->get('createUser')->willReturn($createUserCheckForm);
		$createUserCheckForm->getViewData()->willReturn(null);

[Admin] Fix user fields disappearing on validation fails #6926

[Admin] Fix user fields disappearing on validation fails #6926

Conversation

tuka217 commented Nov 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuka217 Nov 30, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tuka217 Dec 1, 2016 • edited Loading

Choose a reason for hiding this comment

tuka217 Dec 1, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalmarcinkowski commented Dec 1, 2016

tuka217 commented Nov 29, 2016 •

edited

Loading

tuka217 Nov 30, 2016 •

edited

Loading

tuka217 Dec 1, 2016 •

edited

Loading

tuka217 Dec 1, 2016 •

edited

Loading