Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to directory.graphqls to include more configation settings #462

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions design-documents/graph-ql/coverage/directory.graphqls
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Copyright © Magento, Inc. All rights reserved.
# See COPYING.txt for license details.

type Query {
currency: Currency @resolver(class: "Magento\\DirectoryGraphQl\\Model\\Resolver\\Currency") @doc(description: "The currency query returns information about store currency.") @cache(cacheable: false)
countries: [Country] @resolver(class: "Magento\\DirectoryGraphQl\\Model\\Resolver\\Countries") @doc(description: "The countries query provides information for all countries. Sorted by Top Destination and locale specific country name.") @cache(cacheable: false)
country (id: String): Country @resolver(class: "Magento\\DirectoryGraphQl\\Model\\Resolver\\Country") @doc(description: "The countries query provides information for a single country.") @cache(cacheable: false)
}

type Currency {
base_currency_code: String
Copy link
Contributor

Choose a reason for hiding this comment

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

there is a different issue with Currency
https://github.com/magento/architecture/pull/474/files
see what I proposed here
https://github.com/magento/architecture/pull/474/files#r556999960
I am looking at other APIs and Currency object is simple usually name, code and symbol

Copy link
Author

Choose a reason for hiding this comment

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

Agreed that should be fixed, if there is a conclusion in the other PR I can update this one.

base_currency_symbol: String
default_display_currecy_code: String @deprecated(reason: "Symbol was missed. Use `default_display_currency_code`.")
default_display_currency_code: String
default_display_currecy_symbol: String @deprecated(reason: "Symbol was missed. Use `default_display_currency_symbol`.")
default_display_currency_symbol: String
available_currency_codes: [String]
exchange_rates: [ExchangeRate]
}

type ExchangeRate {
currency_to: String
rate: Float
}

type Country {
id: String
two_letter_abbreviation: String @doc(description: "Country code according to ISO 3166-1 alpha-2")
three_letter_abbreviation: String @doc(description: "Country code according to ISO 3166-1 alpha-3")
name: String @doc(description: "Name of the country in the locale of the current store")
full_name_locale: String @deprecated(reason: "Use `name`")
Copy link
Contributor

Choose a reason for hiding this comment

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

everywhere else we have fullname used in customers. Of course that's not quite right but would make this an outlier

full_name_english: String @deprecated(reason: "Use `name`")
Comment on lines +30 to +32
Copy link
Author

Choose a reason for hiding this comment

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

@cpartica Something like this?

Suggested change
name: String @doc(description: "Name of the country in the locale of the current store")
full_name_locale: String @deprecated(reason: "Use `name`")
full_name_english: String @deprecated(reason: "Use `name`")
fullname: String @doc(description: "Name of the country in the locale of the current store")
full_name_locale: String @deprecated(reason: "Use `fullname`")
full_name_english: String @deprecated(reason: "Use `fullname`")

region_required: Boolean @doc(description: "Is a the region for this country")
postcode_required: Boolean @doc(description: "Is a postcode required for this country")
eu: Boolean @doc(description: "Country is a member of the European Union")
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of having a specific region for EU, wouldn't it be better to have a "Zone" with value as EU or APAC EMEA, etc
There are other zones besides EU

Copy link
Author

Choose a reason for hiding this comment

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

In the backend of Magento there is a specific configuration field that allows you to check EU countries. But I agree that zone would be better, but that would require modifications on the other layers as well.

available_regions: [Region] @doc(description: "Only returns regions if region_required StoreConfig.allow_optional_state is set. Sorted by localized name.")
}

type Region {
id: Int
code: String @doc(description: "Retuns the region code if a region has a code, else returns the non localized name")
name: String @doc(description: "Locale specific region name.")
}

type StoreConfig {
default_country: String @doc(description: "Default country used for country fields")
allow_optional_state: String @doc(description: "Allow a customer to enter a state when it isn't required for a country")
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the path in system config for this?

Copy link
Author

Choose a reason for hiding this comment

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

Uhh, don't know the exact path, but it's this field:

Schermafbeelding 2021-01-14 om 12 43 37

}