-
Notifications
You must be signed in to change notification settings - Fork 152
128 extend store config coverage #130
Changes from 5 commits
a00df27
5ad0774
3872187
efe6091
f82bd47
9d7f217
b84844f
8355529
43b178e
1a8fef4
8f936ac
56d6801
b85ec3f
2d3b311
8b2731f
af69563
fbff94b
d49ef93
b2d22d0
b6c6cba
157546b
820157a
7159ad2
14cf51a
516a22d
1b41bfe
1ae2b99
bee1824
487113f
ee9adad
7ab6968
5d99845
6b492d5
4a3e4eb
2fda32c
12236ce
b620cf4
7bcd0ec
f6f673f
fec5e1f
a9343b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\StoreGraphQl\Model\Resolver\Store; | ||
|
||
use Magento\Framework\App\Config\ScopeConfigInterface; | ||
use Magento\Store\Model\ScopeInterface; | ||
|
||
/** | ||
* Extends default StoreConfigInterface for GraphQL request processing. | ||
*/ | ||
class ExtendedStoreConfigDataProvider | ||
{ | ||
/** | ||
* @var ScopeConfigInterface | ||
*/ | ||
private $scopeConfig; | ||
|
||
/** | ||
* @var array | ||
*/ | ||
private $extendedConfigs; | ||
|
||
/** | ||
* @param ScopeConfigInterface $scopeConfig | ||
* @param array $extendedConfigs | ||
*/ | ||
public function __construct( | ||
ScopeConfigInterface $scopeConfig, | ||
array $extendedConfigs | ||
) { | ||
$this->scopeConfig = $scopeConfig; | ||
$this->extendedConfigs = $extendedConfigs; | ||
} | ||
|
||
/** | ||
* Get data from ScopeConfig by path's defined in DI config | ||
* @return array | ||
*/ | ||
public function getExtendedConfigs() | ||
{ | ||
$extendedConfigsData = []; | ||
foreach ($this->extendedConfigs as $key => $path) { | ||
$extendedConfigsData[$key] = $this->scopeConfig->getValue($path, ScopeInterface::SCOPE_STORE); | ||
} | ||
|
||
return $extendedConfigsData; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
use Magento\Framework\GraphQl\Query\Resolver\ValueFactory; | ||
use Magento\Framework\GraphQl\Query\ResolverInterface; | ||
use Magento\Framework\GraphQl\Schema\Type\ResolveInfo; | ||
use Magento\StoreGraphQl\Model\Resolver\Store\ExtendedStoreConfigDataProvider; | ||
use Magento\StoreGraphQl\Model\Resolver\Store\StoreConfigDataProvider; | ||
|
||
/** | ||
|
@@ -24,21 +25,29 @@ class StoreConfigResolver implements ResolverInterface | |
*/ | ||
private $storeConfigDataProvider; | ||
|
||
/** | ||
* @var ExtendedStoreConfigDataProvider | ||
*/ | ||
private $extendedStoreConfigsDataProvider; | ||
|
||
/** | ||
* @var ValueFactory | ||
*/ | ||
private $valueFactory; | ||
|
||
/** | ||
* @param StoreConfigDataProvider $storeConfigsDataProvider | ||
* @param ExtendedStoreConfigDataProvider $extendedStoreConfigsDataProvider | ||
* @param ValueFactory $valueFactory | ||
*/ | ||
public function __construct( | ||
StoreConfigDataProvider $storeConfigsDataProvider, | ||
ExtendedStoreConfigDataProvider $extendedStoreConfigsDataProvider, | ||
ValueFactory $valueFactory | ||
) { | ||
$this->valueFactory = $valueFactory; | ||
$this->storeConfigDataProvider = $storeConfigsDataProvider; | ||
$this->extendedStoreConfigsDataProvider = $extendedStoreConfigsDataProvider; | ||
} | ||
|
||
/** | ||
|
@@ -52,7 +61,10 @@ public function resolve( | |
array $args = null | ||
) : Value { | ||
|
||
$storeConfigData = $this->storeConfigDataProvider->getStoreConfig(); | ||
$storeConfigData = array_merge( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should use just one config provider and the list of accessible fields should be filtered according to the extensible di.xml There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @paliarush |
||
$this->storeConfigDataProvider->getStoreConfig(), | ||
$this->extendedStoreConfigsDataProvider->getExtendedConfigs() | ||
); | ||
|
||
$result = function () use ($storeConfigData) { | ||
return !empty($storeConfigData) ? $storeConfigData : []; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
<?xml version="1.0"?> | ||
<!-- | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
--> | ||
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd"> | ||
<type name="Magento\StoreGraphQl\Model\Resolver\Store\ExtendedStoreConfigDataProvider"> | ||
<arguments> | ||
<argument name="extendedConfigs" xsi:type="array"> | ||
<!-- Begin Store Configuration Web Default Pages --> | ||
<item name="front" xsi:type="string">web/default/front</item> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, good point. |
||
<item name="cms_home_page" xsi:type="string">web/default/cms_home_page</item> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) |
||
<item name="no_route" xsi:type="string">web/default/no_route</item> | ||
<item name="cms_no_route" xsi:type="string">web/default/cms_no_route</item> | ||
<item name="cms_no_cookies" xsi:type="string">web/default/cms_no_cookies</item> | ||
<item name="show_cms_breadcrumbs" xsi:type="string">web/default/show_cms_breadcrumbs</item> | ||
<!-- End Store Configuration Web Default Pages --> | ||
<!-- Begin Design Configuration HTML Head Section--> | ||
<item name="head_shortcut_icon" xsi:type="string">design/head/head_shortcut_icon</item> | ||
<item name="default_title" xsi:type="string">design/head/default_title</item> | ||
<item name="title_prefix" xsi:type="string">design/head/title_prefix</item> | ||
<item name="title_suffix" xsi:type="string">design/head/title_suffix</item> | ||
<item name="default_description" xsi:type="string">design/head/default_description</item> | ||
<item name="default_keywords" xsi:type="string">design/head/default_keywords</item> | ||
<item name="head_includes" xsi:type="string">design/head/includes</item> | ||
<item name="demonotice" xsi:type="string">design/head/demonotice</item> | ||
<!-- End Design Configuration HTML Head Section --> | ||
<!-- Begin Design Configuration Header Section--> | ||
<item name="header_logo_src" xsi:type="string">design/header/header_logo_src</item> | ||
<item name="logo_width" xsi:type="string">design/header/logo_width</item> | ||
<item name="logo_height" xsi:type="string">design/header/logo_height</item> | ||
<item name="welcome" xsi:type="string">design/header/welcome</item> | ||
<item name="logo_alt" xsi:type="string">design/header/logo_alt</item> | ||
<!-- End Design Configuration Header Section --> | ||
<!-- Begin Design Configuration Footer Section--> | ||
<item name="copyright" xsi:type="string">design/footer/copyright</item> | ||
<item name="absolute_footer" xsi:type="string">design/footer/absolute_footer</item> | ||
<!-- End Design Configuration Footer Section --> | ||
</argument> | ||
</arguments> | ||
</type> | ||
</config> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -90,4 +90,31 @@ public function testGetStoreConfig() | |
); | ||
$this->assertEquals($storeConfig->getSecureBaseMediaUrl(), $response['storeConfig']['secure_base_media_url']); | ||
} | ||
|
||
/** | ||
* @magentoConfigFixture default_store web/default/front 'test_page' | ||
* @magentoConfigFixture default_store design/head/default_title 'Test Title' | ||
* @magentoConfigFixture default_store design/footer/copyright 'Test Copyright' | ||
*/ | ||
public function testExtendedStoreConfig() | ||
{ | ||
$front = 'test_page'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please check those values? Looks like they are not same in database:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should skip/rewrite this test until config fixture is supported. |
||
$default_title = 'Test Title'; | ||
$copyright = 'Test Copyright'; | ||
|
||
$query = <<<QUERY | ||
{ | ||
storeConfig{ | ||
front, | ||
default_title, | ||
copyright | ||
} | ||
} | ||
QUERY; | ||
$response = $this->graphQlQuery($query); | ||
$this->assertArrayHasKey('storeConfig', $response); | ||
$this->assertEquals($front, $response['storeConfig']['front']); | ||
$this->assertEquals($default_title, $response['storeConfig']['default_title']); | ||
$this->assertEquals($copyright, $response['storeConfig']['copyright']); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"hidrate" should be "hydrate"