[InstallerBundle] Simplification and testing of sylius:install:setup command #3814

CoderMaggie · 2016-01-05T09:16:11Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	yes
Deprecations?	no
Fixed tickets
License	MIT

Removed multiple currencies, languages, countries and channels configuration.

Now when the currency is chosen it is set as base.

Added Behat testing of the command.

pjedrzejewski · 2016-01-05T09:23:53Z

features/installer/install_command.feature

@@ -0,0 +1,21 @@
+@installer
+Feature: Sylius Install Feature
+  In order to install Sylius


CoderMaggie · 2016-01-05T15:00:30Z

src/Sylius/Bundle/InstallerBundle/Behat/CliContext.php

+    public function __construct()
+    {
+        parent::__construct();
+        $this->application = new Application($this->getKernel());


$this->getKernel() returns null, anybody can suggest me, why? Any solutions? :D

Seems like you've encountered an error! :)

Contexts are instantiated in ContextFactory, exactly at line 80. They are initialized just one line below and KernelInterface instance is set by Symfony2Extension context initializer.

To solve this you can register @BeforeScenario hook (which has some downsides, especially that it would be called before every scenario in every suite that has CliContext). One another solution is to create application and register that command on I run a command "sylius:install:setup" step, which is cleaner and makes executing commands more isolated.

Thanks @pamil !

michalmarcinkowski · 2016-01-07T11:14:52Z

behat.yml.dist

@@ -118,6 +118,19 @@ default:
            filters:
                tags: "@homepage"

+        installer:
+            contexts:
+                - Behat\MinkExtension\Context\MinkContext


Do we need MinkContext in CLI?

pjedrzejewski · 2016-01-08T17:53:36Z

behat.yml.dist

@@ -118,6 +118,19 @@ default:
            filters:
                tags: "@homepage"

+        installer:


I'd call the suite cli.

michalmarcinkowski · 2016-02-25T12:28:48Z

behat.yml.dist

@@ -3,4 +3,4 @@

 imports:
    - etc/behat/profiles.yml
-    - etc/behat/suites.yml
+    - etc/behat/suites.yml


Should be reverted

michalmarcinkowski · 2016-02-25T12:42:38Z

src/Sylius/Bundle/InstallerBundle/Command/SetupCommand.php

-            $channel->setCode($code);
-            $channel->setName($code);
+            $channel->setCode('DEFAULT');
+            $channel->setName('DEFAULT');
            $channel->setColor(null);


Is it required?

CoderMaggie · 2016-02-25T14:36:57Z

@michalmarcinkowski updated.

michalmarcinkowski · 2016-02-26T15:53:58Z

src/Sylius/Bundle/InstallerBundle/Command/SetupCommand.php

-            $countryManager->persist($country);
-        }
+        $currency->setExchangeRate(1);
+        $currency->setBase(true);


When I run this command twice with different currencies I will have two base currencies in the database... We should validate that (in a separate PR).

lchrusciel · 2016-03-03T09:57:25Z

src/Sylius/Bundle/InstallerBundle/Command/SetupCommand.php

-            $code = trim($code);
-            $name = Intl::getRegionBundle()->getCountryName($code);
+        $code = trim($code);
+        $name = Intl::getCurrencyBundle()->getCurrencyName($code);


Maybe extract this line to service and inject it here?

[InstallerBundle] Simplification and testing of sylius:install:setup command

pjedrzejewski · 2016-03-08T15:02:36Z

Nice work Magda, thank you! 👍

CoderMaggie force-pushed the sylius-install-setup-command branch 3 times, most recently from d934a69 to 5f70ec2 Compare January 5, 2016 09:22

pjedrzejewski added the Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). label Jan 5, 2016

pjedrzejewski reviewed Jan 5, 2016
View reviewed changes

CoderMaggie force-pushed the sylius-install-setup-command branch 2 times, most recently from e8f3725 to 80025b9 Compare January 5, 2016 14:59

CoderMaggie reviewed Jan 5, 2016
View reviewed changes

michalmarcinkowski changed the title ~~[InstallerBundle] [WIP] Simplification and testing of sylius:install:setup command~~ [WIP][InstallerBundle] Simplification and testing of sylius:install:setup command Jan 7, 2016

michalmarcinkowski reviewed Jan 7, 2016
View reviewed changes

CoderMaggie force-pushed the sylius-install-setup-command branch 2 times, most recently from 0b73745 to cfc27f7 Compare January 8, 2016 15:54

pjedrzejewski reviewed Jan 8, 2016
View reviewed changes

behat.yml.dist

@@ -118,6 +118,19 @@ default:

filters:

tags: "@homepage"

installer:

Copy link

Member

pjedrzejewski Jan 8, 2016

Choose a reason for hiding this comment

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

I'd call the suite cli.

CoderMaggie force-pushed the sylius-install-setup-command branch from 6208d17 to bbab89b Compare January 12, 2016 10:17

CoderMaggie changed the title ~~[WIP][InstallerBundle] Simplification and testing of sylius:install:setup command~~ [InstallerBundle] Simplification and testing of sylius:install:setup command Jan 12, 2016

CoderMaggie force-pushed the sylius-install-setup-command branch 2 times, most recently from 635a8b2 to 688cc7a Compare January 15, 2016 07:31

CoderMaggie mentioned this pull request Jan 15, 2016

[Currency] Add isBase flag on CurrencyInterface #3887

Merged

CoderMaggie mentioned this pull request Feb 25, 2016

[DX] Describe fixtures loading command with Behat scenarios #4302

Merged

michalmarcinkowski reviewed Feb 25, 2016
View reviewed changes

CoderMaggie force-pushed the sylius-install-setup-command branch from 0089965 to 9a8cf8e Compare February 25, 2016 12:42

michalmarcinkowski reviewed Feb 25, 2016
View reviewed changes

CoderMaggie force-pushed the sylius-install-setup-command branch 4 times, most recently from 29fdd5a to 194a7f2 Compare February 25, 2016 14:36

michalmarcinkowski reviewed Feb 26, 2016
View reviewed changes

michalmarcinkowski added Awaiting Review and removed Awaiting Review labels Mar 1, 2016

CoderMaggie force-pushed the sylius-install-setup-command branch from 194a7f2 to 91c584e Compare March 1, 2016 13:56

lchrusciel reviewed Mar 3, 2016
View reviewed changes

CoderMaggie force-pushed the sylius-install-setup-command branch from 91c584e to 019c822 Compare March 8, 2016 07:37

Magdalena Banasiak added 5 commits March 8, 2016 09:22

sylius:install:setup simplified

65220c3

Behat tests of installer command

6a63dd8

sylius:setup:install admin data validation testing

efb5a7a

Setting chosen Currency as base during setup

4da28df

Setting default channel and transfer to new Behats

c138004

CoderMaggie force-pushed the sylius-install-setup-command branch from 019c822 to c138004 Compare March 8, 2016 08:24

pjedrzejewski pushed a commit that referenced this pull request Mar 8, 2016

Merge pull request #3814 from TheMadeleine/sylius-install-setup-command

f08576f

[InstallerBundle] Simplification and testing of sylius:install:setup command

pjedrzejewski merged commit f08576f into Sylius:master Mar 8, 2016

CoderMaggie deleted the sylius-install-setup-command branch March 14, 2017 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[InstallerBundle] Simplification and testing of sylius:install:setup command #3814

[InstallerBundle] Simplification and testing of sylius:install:setup command #3814

CoderMaggie commented Jan 5, 2016

pjedrzejewski Jan 5, 2016

CoderMaggie Jan 5, 2016

pamil Jan 5, 2016

CoderMaggie Jan 6, 2016

michalmarcinkowski Jan 7, 2016

pjedrzejewski Jan 8, 2016

michalmarcinkowski Feb 25, 2016

michalmarcinkowski Feb 25, 2016

CoderMaggie commented Feb 25, 2016

michalmarcinkowski Feb 26, 2016

lchrusciel Mar 3, 2016

pjedrzejewski commented Mar 8, 2016

[InstallerBundle] Simplification and testing of sylius:install:setup command #3814

[InstallerBundle] Simplification and testing of sylius:install:setup command #3814

Conversation

CoderMaggie commented Jan 5, 2016

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

CoderMaggie commented Feb 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjedrzejewski commented Mar 8, 2016