magento/magento2#28569:Multi-store: Missing store codes in relation t… #28794

gallyamov · 2020-06-18T10:39:50Z

Description (*)

This is for #28569

Related Pull Requests

N/A

Fixed Issues (if relevant)

Fixes Multi-store: Missing store codes in relation to a group and website #28569

Manual testing scenarios (*)

Check fixed issue for testing steps and expected results.

Questions or comments

N/A

Contribution checklist (*)

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds are green)

…bsite - Added stores for storeConfig query

m2-assistant · 2020-06-18T10:39:53Z

Hi @gallyamov. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

gallyamov · 2020-06-18T10:40:23Z

@magento run all tests

gallyamov · 2020-06-18T12:38:19Z

@magento run Functional Tests B2B

gallyamov · 2020-06-18T14:24:06Z

@magento run Functional Tests B2B

swnsma

Hello @gallyamov!
Please check my comments.

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

app/code/Magento/StoreGraphQl/etc/schema.graphqls

…adding_stores_to_store_config

- Refactored code to use getList

- Added param type hint

gallyamov · 2020-06-22T14:19:07Z

@magento run all tests

danielrenaud · 2020-06-22T23:06:21Z

Hi @gallyamov
I'm not sure this is the best approach from a schema perspective. I've opened up an architectural proposal with an alternative approach: magento/architecture#390

danielrenaud · 2020-06-23T16:05:44Z

@gallyamov Can you please implement this using the schema defined here: magento/architecture#390
It shouldn't affect the underlying implementation much, mainly just the schema

…adding_stores_to_store_config

…ebsite - Implemented new availableStores query - Code refactor

gallyamov · 2020-06-24T12:52:15Z

@magento run all tests

danielrenaud · 2020-06-24T18:02:01Z

app/code/Magento/Store/Model/Service/StoreConfigManager.php

     */
-    protected function getStoreConfig($store)
+    public function getStoreConfig($store)


I don't think we need this change, and I would prefer to avoid it

I know we discussed this offline and thought it might not be a big deal.
However, now the class's api differs from it's interface, and updating the interface would be backwards incompatible.

you can cache values in getStoreConfigs, so it fetches data just once
then you can call getStoreConfigs([$store->getCode()]) instead of calling getStoreConfig($store)
It's the same thing

danielrenaud · 2020-06-24T18:06:51Z

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

    {
-        $storeConfig = current($this->storeConfigManager->getStoreConfigs([$store->getCode()]));
+        $storesConfigData = [];
+        $storeConfigs = $this->storeConfigManager->getStoreConfigs();


We should only return available stores for the current website. This is returning all stores for all websites

danielrenaud · 2020-06-24T18:12:54Z

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

+     * @magentoConfigFixture test_store store/information/name Test Store
+     * @throws Exception
+     */
+    public function testAvailableStoreConfigs(): void


I think this should be in a separate test file since it's testing a different query/resolver

…adding_stores_to_store_config

…ebsite - Created separate test for available stores Added website specific store output

gallyamov · 2020-06-25T16:42:50Z

@magento run all tests

cpartica · 2020-07-09T20:53:34Z

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

    {
-        $storeConfig = current($this->storeConfigManager->getStoreConfigs([$store->getCode()]));
+        $websiteStores = $this->storeWebsiteRelation->getWebsiteStores($websiteId, true);


you need a (int) typecast, and plz do that int $websiteID in methods, it's the preferred way

cpartica · 2020-07-09T20:57:23Z

@gallyamov, FYI "testDefaultWebsiteAvailableStoreConfigs" test is failing

…adding_stores_to_store_config

…ebsite - Fixed strict types and tests

gallyamov · 2020-07-10T10:33:18Z

@magento run all tests

magento-engcom-team · 2020-07-13T17:07:36Z

Hi @swnsma, thank you for the review.
ENGCOM-7810 has been created to process this Pull Request

…28794

m2-assistant · 2020-07-15T16:45:10Z

Hi @gallyamov, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

#28569:Multi-store: Missing store codes in relation to a group and we…

0b577a3

…bsite - Added stores for storeConfig query

gallyamov added the Project: GraphQL label Jun 18, 2020

ghost added the Progress: pending review label Jun 18, 2020

magento-engcom-team added Component: StoreGraphQl Release Line: 2.4 labels Jun 18, 2020

gallyamov requested a review from danielrenaud June 18, 2020 14:02

gallyamov marked this pull request as ready for review June 18, 2020 14:02

swnsma requested changes Jun 20, 2020

View reviewed changes

ghost assigned swnsma Jun 20, 2020

ghost added Progress: needs update and removed Progress: pending review labels Jun 20, 2020

gallyamov added 3 commits June 22, 2020 13:51

Merge branch '2.4-develop' of github.com:magento/magento2 into 28569_…

1973c26

…adding_stores_to_store_config

#28569: Missing store codes in relation to a group and website

5ab033d

- Refactored code to use getList

#28569: Missing store codes in relation to a group and website

ac5835b

- Added param type hint

gallyamov added 2 commits June 24, 2020 13:46

Merge branch '2.4-develop' of github.com:magento/magento2 into 28569_…

5ea15d1

…adding_stores_to_store_config

#28569: Multi-store: Missing store codes in relation to a group and w…

4fe5f8f

…ebsite - Implemented new availableStores query - Code refactor

danielrenaud suggested changes Jun 24, 2020

View reviewed changes

ghost assigned danielrenaud Jun 24, 2020

cpartica added the PAP Partners acceleration program label Jun 25, 2020

gallyamov added 2 commits June 25, 2020 13:39

Merge branch '2.4-develop' of github.com:magento/magento2 into 28569_…

48f8add

…adding_stores_to_store_config

#28569: Multi-store: Missing store codes in relation to a group and w…

d8643e2

…ebsite - Created separate test for available stores Added website specific store output

swnsma added Award: bug fix Award: category of expertise Award: test coverage Progress: accept and removed Progress: accept labels Jul 9, 2020

cpartica requested changes Jul 9, 2020

View reviewed changes

gallyamov added 2 commits July 10, 2020 13:18

Merge branch '2.4-develop' of github.com:magento/magento2 into 28569_…

b23125e

…adding_stores_to_store_config

#28569: Multi-store: Missing store codes in relation to a group and w…

0ccadf5

…ebsite - Fixed strict types and tests

danielrenaud approved these changes Jul 10, 2020

View reviewed changes

ghost added Progress: ready for testing Progress: testing in progress Progress: accept and removed Progress: ready for testing Progress: testing in progress labels Jul 10, 2020

swnsma self-requested a review July 13, 2020 17:06

swnsma approved these changes Jul 13, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: accept labels Jul 13, 2020

ghost added Progress: testing in progress Progress: accept and removed Progress: ready for testing Progress: testing in progress labels Jul 14, 2020

magento-engcom-team mentioned this pull request Jul 15, 2020

Multi-store: Missing store codes in relation to a group and website #28569

Closed

magento-engcom-team pushed a commit that referenced this pull request Jul 15, 2020

ENGCOM-7810: #28569:Multi-store: Missing store codes in relation t… #…

51866b2

…28794

magento-engcom-team merged commit fcc5772 into 2.4-develop Jul 15, 2020

okorshenko deleted the 28569_adding_stores_to_store_config branch August 25, 2020 16:15

magento/magento2#28569:Multi-store: Missing store codes in relation t… #28794

magento/magento2#28569:Multi-store: Missing store codes in relation t… #28794

Uh oh!

Conversation

gallyamov commented Jun 18, 2020

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Jun 18, 2020

Uh oh!

gallyamov commented Jun 18, 2020

Uh oh!

gallyamov commented Jun 18, 2020

Uh oh!

gallyamov commented Jun 18, 2020

Uh oh!

swnsma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gallyamov commented Jun 22, 2020

Uh oh!

danielrenaud commented Jun 22, 2020

Uh oh!

danielrenaud commented Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gallyamov commented Jun 24, 2020

Uh oh!

danielrenaud Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

danielrenaud Jun 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpartica Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

danielrenaud Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

danielrenaud Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

gallyamov commented Jun 25, 2020

Uh oh!

cpartica Jul 9, 2020

Choose a reason for hiding this comment

Uh oh!

cpartica commented Jul 9, 2020

Uh oh!

gallyamov commented Jul 10, 2020

Uh oh!

magento-engcom-team commented Jul 13, 2020

Uh oh!

m2-assistant bot commented Jul 15, 2020

Uh oh!

Uh oh!

danielrenaud commented Jun 23, 2020 •

edited

Loading

danielrenaud Jun 25, 2020 •

edited

Loading