Skip to content

Conversation

danielrenaud
Copy link
Contributor

@danielrenaud danielrenaud commented Jun 22, 2020

Problem

No way to get available stores. This is needed for something like a store switcher.
See issue: magento/magento2#28569

Solution

Introduce new query for fetching available stores. Based on the current store, it will return a list of StoreConfigs for other stores available under the same website.

Alternate solution:
In the linked issue it is suggested to return a new field "stores" under current storeConfig, with list of available stores. To me this seems a little confusing because the available stores is not part of the current store's configuration. However I mention that approach in case it is preferred.

Requested Reviewers

@paliarush
@DrewML

@DrewML
Copy link
Contributor

DrewML commented Jun 23, 2020

In the linked issue it is suggested to return a new field "stores" under current storeConfig, with list of available stores. To me this seems a little confusing because the available stores is not part of the current store's configuration

Agree with you on this one - I don't think it makes much sense for storeConfig to have the list of store views - would make more sense if it was websiteConfig, but the UI doesn't need to know about websites to the best of my knowledge.

@DrewML DrewML self-requested a review June 23, 2020 14:58
### Proposed schema
```graphql
type Query {
availableStores: [StoreConfig] @doc(description: "Get a list of available stores")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to name this something like availableStoreViews? For UI developers in a headless world, the GraphQL schema will often be their first introduction to some Magento concepts.

Referencing "store view" instead of just "store" will make it much easier to Google for the right concept (test googling magento stores vs magento store views)

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Often times in the frontend we refer to store views as stores interchangeably. I think using just "store" is more consistent; e.g. StoreConfig and the Store header we use for store code. it's a good point. maybe just changing the description will be enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just changing the description will be enough?

The one downside there is that most tools like GraphiQL don't cover the description in their search field, so searching for something like store view wouldn't net any results.

BUT, you do raise a good point re: consistency vs accuracy. My feelings here aren't too strong, so I'll leave this as a judgement call for you to make 👍

@DrewML
Copy link
Contributor

DrewML commented Jun 23, 2020

Had one question re: naming, but from a technical perspective this looks good to me 👍

@paliarush
Copy link
Contributor

We need to make sure that the list of websites is not exposed by default. Currently some merchants want to hide the fact that they are managing multiple websites, and it is controlled using setting in the admin panel.
@danielrenaud I am curious where this request is coming from? As far as I remember long time ago we discussed that storefront app will have the list of known store views hardcoded because the theme is hardcoded there and not configured in the Magento admin.

@danielrenaud
Copy link
Contributor Author

@paliarush This query will only expose other store views, under the same website as the current store (by Store code passed in header). So the list of websites would not be exposed.
This request is to allow for creating a store switcher.

@DrewML DrewML requested a review from paliarush July 13, 2020 21:33
@DrewML
Copy link
Contributor

DrewML commented Jul 13, 2020

@paliarush any more changes you want to see here?

@paliarush paliarush merged commit 43f4fac into magento:master Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants