128 extend store config coverage #130

VitaliyBoyko · 2018-07-24T18:40:34Z

Added extended params to store config.

Fixed Issues (if relevant)

Extend GraphQL store config implementation #128: Extend GraphQL store config implementation

Added configs:
front : "Default Web URL"
cms_home_page : "CMS Home Page"
no_route : "Default No-route URL"
cms_no_route : "CMS No Route Page"
cms_no_cookies : "CMS No Cookies Page"
show_cms_breadcrumbs : "Show Breadcrumbs for CMS Pages"
head_shortcut_icon : "Favicon Icon"
default_title : "Default Page Title"
title_prefix : "Page Title Prefix"
title_suffix : "Page Title Suffix"
default_description : "Default Meta Description"
default_keywords : "Default Meta Keywords"
head_includes : "Scripts and Style Sheets"
demonotice : "Display Demo Store Notice"
header_logo_src : "Logo Image"
logo_width : "Logo Attribute Width"
logo_height : "Logo Attribute Height"
welcome : "Welcome Text"
logo_alt : "Logo Image Alt"
absolute_footer : "Footer Miscellaneous HTML"
copyright : "Copyright"

…nto 128-extend-store-config-coverage

vkublytskyi · 2018-07-26T14:00:45Z

app/code/Magento/StoreGraphQl/etc/graphql/di.xml

+            <argument name="extendedConfigs" xsi:type="array">
+                <!-- Begin Store Configuration Web Default Pages -->
+                <item name="front" xsi:type="string">web/default/front</item>
+                <item name="cms_home_page" xsi:type="string">web/default/cms_home_page</item>


Such implementation introduces an implicit dependency on CMS and Theme modules. Since both of these modules will require GraphQL coverage I think it's better to move declarations of exporting config path to corresponding modules (CmsGraphQl and ThemeGraphQl)

misha-kotov · 2018-07-26T15:50:54Z

app/code/Magento/StoreGraphQl/Model/Resolver/Store/StoreConfigDataProvider.php

@@ -54,8 +45,7 @@ public function __construct(
     */
    public function getStoreConfig() : array
    {
-        $storeId = $this->storeResolver->getCurrentStoreId();
-        $store = $this->storeRepository->getById($storeId);
+        $store = $this->storeManager->getStore();
        $storeConfig = current($this->storeConfigManager->getStoreConfigs([$store->getCode()]));

        return $this->hidrateStoreConfig($storeConfig);


"hidrate" should be "hydrate"

nuzil · 2018-07-25T19:40:19Z

app/code/Magento/StoreGraphQl/etc/graphql/di.xml

+        <arguments>
+            <argument name="extendedConfigs" xsi:type="array">
+                <!-- Begin Store Configuration Web Default Pages -->
+                <item name="front" xsi:type="string">web/default/front</item>


Just as proposition, and I would like to have an opinion of @paliarush or @vrann, maybe define some standard at least in Magento code, then items names will look like:
web_default_front - which is equal to web/default/front ? Then for developers will be easier to find items without looking into documentation.

Yes, good point.
Would be nice to avoid having coupling between di.xml and schema.graphqls and have just one declaration in schema.graphqls, but unfortunately config_path_with_underscore cannot be reliably converted to config/path/with_underscore.

nuzil · 2018-07-26T19:25:15Z

dev/tests/api-functional/testsuite/Magento/GraphQl/Store/StoreConfigResolverTest.php

+     */
+    public function testExtendedStoreConfig()
+    {
+        $front = 'test_page';


Can you please check those values? Looks like they are not same in database:

Magento\GraphQl\Store\StoreConfigResolverTest::testExtendedStoreConfig Failed asserting that two strings are equal. --- Expected +++ Actual @@ @@ -'test_page' +'cms'

@magentoConfigFixture does not work at the moment in Web API tests, @melnikovi is working on the fix

I think we should skip/rewrite this test until config fixture is supported.

paliarush · 2018-07-27T18:12:03Z

app/code/Magento/StoreGraphQl/Model/Resolver/StoreConfigResolver.php

@@ -52,7 +61,10 @@ public function resolve(
        array $args = null
    ) : Value {

-        $storeConfigData = $this->storeConfigDataProvider->getStoreConfig();
+        $storeConfigData = array_merge(


We should use just one config provider and the list of accessible fields should be filtered according to the extensible di.xml

Hi @paliarush
God point
But, the decision with one config provider doesn't right here.
We getting configs from different sources, StoreConfigDataProvider gets data from StoreConfigInterface unlike ExtendedStoreConfigDataProvider which gets data from ScopeConfigInterface.
It is not possible to get all StoreConfigInterface fields from ScopeConfigInterface.
For example base_url not present in core_config_data and defines in app/code/Magento/Store/Model/Store.php:607.

…raphQL modules

paliarush · 2018-08-02T00:42:03Z

app/code/Magento/CmsGraphQl/composer.json

+        "magento/framework": "*"
+    },
+    "suggest": {
+        "magento/module-graph-ql": "*",


Should we have dependency on CMS module (or at least suggest, if there are no hard dependencies)?

Hi
I guess Travis will fail in this case with the "redundant dependency" error.

…t and config tests

…QL-172

…L-172

-- add block_rollback fixture

…https://github.com/magento/graphql-ce into 128-extend-store-config-coverage Signed-off-by: vitaliyboyko <[email protected]> Conflicts: app/code/Magento/CmsGraphQl/README.md app/code/Magento/CmsGraphQl/composer.json app/code/Magento/CmsGraphQl/etc/schema.graphqls app/code/Magento/StoreGraphQl/Model/Resolver/Store/StoreConfigDataProvider.php app/code/Magento/StoreGraphQl/Model/Resolver/StoreConfigResolver.php composer.lock

Signed-off-by: vitaliyboyko <[email protected]>

magento-cicd2 · 2018-09-11T10:09:17Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ VitaliyBoyko
✅ TomashKhamlai
✅ naydav
❌ skmomemo

Signed-off-by: vitaliyboyko <[email protected]>

…nfig-fixture' of https://github.com/magento/graphql-ce into 128-extend-store-config-coverage

Signed-off-by: vitaliyboyko <[email protected]>

…re-config-coverage # Conflicts: # app/code/Magento/BundleGraphQl/Model/Resolver/BundleItemLinks.php # app/code/Magento/StoreGraphQl/Model/Resolver/Store/StoreConfigDataProvider.php # app/code/Magento/StoreGraphQl/Model/Resolver/StoreConfigResolver.php # composer.lock # dev/tests/api-functional/testsuite/Magento/GraphQl/Cms/CmsBlockTest.php

…https://github.com/magento/graphql-ce into 128-extend-store-config-coverage

Signed-off-by: vitaliyboyko <[email protected]>

VitaliyBoyko · 2018-09-12T17:22:23Z

Implementation has been moved to other pr:
#182

…g on errors #130

MC-4767

vitaliyboyko added 3 commits July 24, 2018 18:31

graphql-ce-128: added extended store config dataprovider

a00df27

Merge branch '2.3-develop' of https://github.com/magento/graphql-ce i…

5ad0774

…nto 128-extend-store-config-coverage

graphql-ce-128: fixed return types

3872187

VitaliyBoyko assigned VitaliyBoyko and nuzil Jul 24, 2018

VitaliyBoyko requested a review from nuzil July 24, 2018 18:42

graphql-ce-128: added test for extended configs

efe6091

VitaliyBoyko requested a review from vrann July 25, 2018 07:33

magento-engcom-team added this to the Release: 2.3.0 milestone Jul 25, 2018

magento-engcom-team added 2.3-develop Partner: Atwix partners-contribution labels Jul 25, 2018

graphql-ce-128: fixed the way to define current store

f82bd47

vkublytskyi self-requested a review July 26, 2018 13:55

vkublytskyi reviewed Jul 26, 2018

View reviewed changes

misha-kotov reviewed Jul 26, 2018

View reviewed changes

nuzil suggested changes Jul 26, 2018

View reviewed changes

paliarush reviewed Jul 27, 2018

View reviewed changes

zetlen mentioned this pull request Jul 30, 2018

Implement WebAPI endpoint for CRUD on core configuration data magento/pwa-studio#131

Closed

GraphQL-128: refactored extended configs, added CmsGraphQl and ThemeG…

9d7f217

…raphQL modules

paliarush reviewed Aug 2, 2018

View reviewed changes

paliarush approved these changes Aug 2, 2018

View reviewed changes

vitaliyboyko added 3 commits August 2, 2018 06:17

GraphQL-128: static fixes, removed test

b84844f

GraphQL-128: added new modules to root composer json

8355529

GraphQL-128: updated composer.lock

43b178e

nuzil approved these changes Aug 20, 2018

View reviewed changes

TomashKhamlai added 2 commits August 21, 2018 10:47

Test coverage for CMS page

1a8fef4

Fix problems reported by testCodeStyle

8f936ac

naydav added requires-tests PRs which require GraphQL tests requires-documentation labels Aug 21, 2018

Valeriy Nayda and others added 13 commits September 5, 2018 14:31

GraphQL-152: Allow scalars as resolver return type

b6c6cba

Merge branch 'GraphQL-165' into GraphQL-152-165

157546b

GraphQL-152: Allow scalars as resolver return type

820157a

GraphQL-128: added magentoApiConfig fixture annotation support in tes…

7159ad2

…t and config tests

GraphQL-128: fixed static issue

14cf51a

GraphQL-167: added support for @magentoApiConfigFixture

516a22d

Merge remote-tracking branch 'origin/2.3-develop' into GraphQL-152-165

1b41bfe

Merge remote-tracking branch 'origin/graphql-ce-151-block' into Graph…

1ae2b99

…QL-172

Merge remote-tracking branch 'origin/graphql-ce-151-page' into GraphQ…

bee1824

…L-172

Merge remote-tracking branch 'origin/graphql-ce-158' into GraphQL-172

487113f

GraphQL-172: GraphQL modules delivery

ee9adad

-- add block_rollback fixture

GraphQL-128: updated lock file

5d99845

Signed-off-by: vitaliyboyko <[email protected]>

vitaliyboyko added 3 commits September 11, 2018 10:14

GraphQL-128: removed accidentally added file

6b492d5

Signed-off-by: vitaliyboyko <[email protected]>

Merge branches '128-extend-store-config-coverage' and '167-magento-co…

4a3e4eb

…nfig-fixture' of https://github.com/magento/graphql-ce into 128-extend-store-config-coverage

GraphQL-128: uskiped and fixed inventory test

2fda32c

Signed-off-by: vitaliyboyko <[email protected]>

naydav added the test-coverage label Sep 11, 2018

vitaliyboyko and others added 6 commits September 11, 2018 14:39

GraphQL-128: removed unused injection

12236ce

Signed-off-by: vitaliyboyko <[email protected]>

GraphQL-130: Extend store config coverage

7bcd0ec

GraphQL-130: Extend store config coverage

f6f673f

GraphQL-130: Extend store config coverage

fec5e1f

Merge branches '128-extend-store-config-coverage' and '2.3-develop' of …

a9343b8

…https://github.com/magento/graphql-ce into 128-extend-store-config-coverage

VitaliyBoyko pushed a commit that referenced this pull request Sep 12, 2018

GraphQL-128: rework of #130

6cfd876

Signed-off-by: vitaliyboyko <[email protected]>

VitaliyBoyko closed this Sep 12, 2018

VitaliyBoyko deleted the 128-extend-store-config-coverage branch September 12, 2018 17:22

magento-engcom-team pushed a commit that referenced this pull request Feb 15, 2019

ENGCOM-3278: magento-engcom/import-export-improvements#75: fix stopin…

e131ff7

…g on errors #130

magento-engcom-team pushed a commit that referenced this pull request Jul 26, 2019

Merge pull request #130 from magento-pangolin/MC-4767

f87bba6

MC-4767

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

128 extend store config coverage #130

128 extend store config coverage #130

VitaliyBoyko commented Jul 24, 2018

vkublytskyi Jul 26, 2018

misha-kotov Jul 26, 2018

nuzil Jul 25, 2018

paliarush Jul 27, 2018

nuzil Jul 26, 2018

paliarush Aug 2, 2018

paliarush Aug 2, 2018

paliarush Jul 27, 2018

VitaliyBoyko Jul 27, 2018 •

edited

Loading

paliarush Aug 2, 2018

VitaliyBoyko Aug 2, 2018

magento-cicd2 commented Sep 11, 2018 •

edited

Loading

VitaliyBoyko commented Sep 12, 2018

128 extend store config coverage #130

128 extend store config coverage #130

Conversation

VitaliyBoyko commented Jul 24, 2018

Fixed Issues (if relevant)

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

VitaliyBoyko Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magento-cicd2 commented Sep 11, 2018 • edited Loading

VitaliyBoyko commented Sep 12, 2018

VitaliyBoyko Jul 27, 2018 •

edited

Loading

magento-cicd2 commented Sep 11, 2018 •

edited

Loading