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.

Sign up for GitHub

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

Jump to bottom

[Fix] Do not modify current list of countries with require states during setup upgrade #16885

Merged
merged 3 commits into from Sep 28, 2018
Merged

[Fix] Do not modify current list of countries with require states during setup upgrade #16885

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80384fc
[Fix] Do not modify current list of countries with require states dur…
jalogut Jul 17, 2018
80001a1
[Fix] Set country codes in right places
jalogut Sep 6, 2018
875c394
[Fix] Add required countries only for IN and HR while keeping previou…
jalogut Sep 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/code/Magento/Directory/Model/ResourceModel/Country/Collection.php
View file
Open in desktop
Expand Up @@ -64,6 +64,8 @@ class Collection extends \Magento\Framework\Model\ResourceModel\Db\Collection\Ab
private $allowedCountriesReader;

/**
* @deprecated
*
* @var string[]
* @since 100.1.0
*/
Expand Down Expand Up @@ -349,6 +351,8 @@ public function setForegroundCountries($foregroundCountries)
}

/**
* @deprecated use \Magento\Directory\Helper\Data::getCountriesWithStatesRequired() instead
*
* Get list of countries with required states
*
* @return \Magento\Directory\Model\Country[]
Expand Down
11 changes: 0 additions & 11 deletions app/code/Magento/Directory/Setup/InstallData.php
View file
Open in desktop
Expand Up @@ -842,16 +842,5 @@ public function install(ModuleDataSetupInterface $setup, ModuleContextInterface
'value' => 1
]
);

$countries = $this->directoryData->getCountryCollection()->getCountriesWithRequiredStates();
$setup->getConnection()->insert(
$setup->getTable('core_config_data'),
[
'scope' => 'default',
'scope_id' => 0,
'path' => Data::XML_PATH_STATES_REQUIRED,
'value' => implode(',', array_keys($countries))
]
);
}
}
144 changes: 65 additions & 79 deletions app/code/Magento/Directory/Setup/UpgradeData.php
View file
Open in desktop
Expand Up @@ -32,7 +32,7 @@ public function __construct(Data $directoryData)
}

/**
* Upgrades data for Directry module.
* Upgrades data for Directory module.
*
* @param ModuleDataSetupInterface $setup
* @param ModuleContextInterface $context
Expand All @@ -41,10 +41,10 @@ public function __construct(Data $directoryData)
public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface $context)
{
if (version_compare($context->getVersion(), '2.0.1', '<')) {
$this->addCountryRegions($setup, $this->getDataForCroatia());
$this->addCountryRegions($setup, 'IN', $this->getDataForCroatia());
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe IN and HR should be swapped here

}
if (version_compare($context->getVersion(), '2.0.2', '<')) {
$this->addCountryRegions($setup, $this->getDataForIndia());
$this->addCountryRegions($setup, 'HR', $this->getDataForIndia());
}
}

Expand All @@ -56,27 +56,27 @@ public function upgrade(ModuleDataSetupInterface $setup, ModuleContextInterface
private function getDataForCroatia()
{
return [
['HR', 'HR-01', 'Zagrebačka županija'],
['HR', 'HR-02', 'Krapinsko-zagorska županija'],
['HR', 'HR-03', 'Sisačko-moslavačka županija'],
['HR', 'HR-04', 'Karlovačka županija'],
['HR', 'HR-05', 'Varaždinska županija'],
['HR', 'HR-06', 'Koprivničko-križevačka županija'],
['HR', 'HR-07', 'Bjelovarsko-bilogorska županija'],
['HR', 'HR-08', 'Primorsko-goranska županija'],
['HR', 'HR-09', 'Ličko-senjska županija'],
['HR', 'HR-10', 'Virovitičko-podravska županija'],
['HR', 'HR-11', 'Požeško-slavonska županija'],
['HR', 'HR-12', 'Brodsko-posavska županija'],
['HR', 'HR-13', 'Zadarska županija'],
['HR', 'HR-14', 'Osječko-baranjska županija'],
['HR', 'HR-15', 'Šibensko-kninska županija'],
['HR', 'HR-16', 'Vukovarsko-srijemska županija'],
['HR', 'HR-17', 'Splitsko-dalmatinska županija'],
['HR', 'HR-18', 'Istarska županija'],
['HR', 'HR-19', 'Dubrovačko-neretvanska županija'],
['HR', 'HR-20', 'Međimurska županija'],
['HR', 'HR-21', 'Grad Zagreb']
'HR-01' => 'Zagrebačka županija',
'HR-02' => 'Krapinsko-zagorska županija',
'HR-03' => 'Sisačko-moslavačka županija',
'HR-04' => 'Karlovačka županija',
'HR-05' => 'Varaždinska županija',
'HR-06' => 'Koprivničko-križevačka županija',
'HR-07' => 'Bjelovarsko-bilogorska županija',
'HR-08' => 'Primorsko-goranska županija',
'HR-09' => 'Ličko-senjska županija',
'HR-10' => 'Virovitičko-podravska županija',
'HR-11' => 'Požeško-slavonska županija',
'HR-12' => 'Brodsko-posavska županija',
'HR-13' => 'Zadarska županija',
'HR-14' => 'Osječko-baranjska županija',
'HR-15' => 'Šibensko-kninska županija',
'HR-16' => 'Vukovarsko-srijemska županija',
'HR-17' => 'Splitsko-dalmatinska županija',
'HR-18' => 'Istarska županija',
'HR-19' => 'Dubrovačko-neretvanska županija',
'HR-20' => 'Međimurska županija',
'HR-21' => 'Grad Zagreb',
];
}

Expand All @@ -88,79 +88,65 @@ private function getDataForCroatia()
private function getDataForIndia()
{
return [
['IN', 'AN', 'Andaman and Nicobar Islands'],
['IN', 'AP', 'Andhra Pradesh'],
['IN', 'AR', 'Arunachal Pradesh'],
['IN', 'AS', 'Assam'],
['IN', 'BR', 'Bihar'],
['IN', 'CH', 'Chandigarh'],
['IN', 'CT', 'Chhattisgarh'],
['IN', 'DN', 'Dadra and Nagar Haveli'],
['IN', 'DD', 'Daman and Diu'],
['IN', 'DL', 'Delhi'],
['IN', 'GA', 'Goa'],
['IN', 'GJ', 'Gujarat'],
['IN', 'HR', 'Haryana'],
['IN', 'HP', 'Himachal Pradesh'],
['IN', 'JK', 'Jammu and Kashmir'],
['IN', 'JH', 'Jharkhand'],
['IN', 'KA', 'Karnataka'],
['IN', 'KL', 'Kerala'],
['IN', 'LD', 'Lakshadweep'],
['IN', 'MP', 'Madhya Pradesh'],
['IN', 'MH', 'Maharashtra'],
['IN', 'MN', 'Manipur'],
['IN', 'ML', 'Meghalaya'],
['IN', 'MZ', 'Mizoram'],
['IN', 'NL', 'Nagaland'],
['IN', 'OR', 'Odisha'],
['IN', 'PY', 'Puducherry'],
['IN', 'PB', 'Punjab'],
['IN', 'RJ', 'Rajasthan'],
['IN', 'SK', 'Sikkim'],
['IN', 'TN', 'Tamil Nadu'],
['IN', 'TG', 'Telangana'],
['IN', 'TR', 'Tripura'],
['IN', 'UP', 'Uttar Pradesh'],
['IN', 'UT', 'Uttarakhand'],
['IN', 'WB', 'West Bengal']
'AN' => 'Andaman and Nicobar Islands',
'AP' => 'Andhra Pradesh',
'AR' => 'Arunachal Pradesh',
'AS' => 'Assam',
'BR' => 'Bihar',
'CH' => 'Chandigarh',
'CT' => 'Chhattisgarh',
'DN' => 'Dadra and Nagar Haveli',
'DD' => 'Daman and Diu',
'DL' => 'Delhi',
'GA' => 'Goa',
'GJ' => 'Gujarat',
'HR' => 'Haryana',
'HP' => 'Himachal Pradesh',
'JK' => 'Jammu and Kashmir',
'JH' => 'Jharkhand',
'KA' => 'Karnataka',
'KL' => 'Kerala',
'LD' => 'Lakshadweep',
'MP' => 'Madhya Pradesh',
'MH' => 'Maharashtra',
'MN' => 'Manipur',
'ML' => 'Meghalaya',
'MZ' => 'Mizoram',
'NL' => 'Nagaland',
'OR' => 'Odisha',
'PY' => 'Puducherry',
'PB' => 'Punjab',
'RJ' => 'Rajasthan',
'SK' => 'Sikkim',
'TN' => 'Tamil Nadu',
'TG' => 'Telangana',
'TR' => 'Tripura',
'UP' => 'Uttar Pradesh',
'UT' => 'Uttarakhand',
'WB' => 'West Bengal',
];
}

/**
* Add country regions data to appropriate tables.
*
* @param ModuleDataSetupInterface $setup
* @param string $countryId
* @param array $data
* @return void
*/
private function addCountryRegions(ModuleDataSetupInterface $setup, array $data)
private function addCountryRegions(ModuleDataSetupInterface $setup, string $countryId, array $data)
{
/**
* Fill table directory/country_region
* Fill table directory/country_region_name for en_US locale
*/
foreach ($data as $row) {
$bind = ['country_id' => $row[0], 'code' => $row[1], 'default_name' => $row[2]];
foreach ($data as $code => $name) {
$bind = ['country_id' => $countryId, 'code' => $code, 'default_name' => $name];
$setup->getConnection()->insert($setup->getTable('directory_country_region'), $bind);
$regionId = $setup->getConnection()->lastInsertId($setup->getTable('directory_country_region'));
$bind = ['locale' => 'en_US', 'region_id' => $regionId, 'name' => $row[2]];
$bind = ['locale' => 'en_US', 'region_id' => $regionId, 'name' => $name];
$setup->getConnection()->insert($setup->getTable('directory_country_region_name'), $bind);
}
/**
* Upgrade core_config_data general/region/state_required field.
*/
$countries = $this->directoryData->getCountryCollection()->getCountriesWithRequiredStates();
$setup->getConnection()->update(
$setup->getTable('core_config_data'),
[
'value' => implode(',', array_keys($countries))
],
[
'scope="default"',
'scope_id=0',
'path=?' => Data::XML_PATH_STATES_REQUIRED
]
);
}
}
4 changes: 0 additions & 4 deletions app/code/Magento/Directory/etc/di.xml
View file
Open in desktop
Expand Up @@ -34,10 +34,6 @@
<type name="Magento\Directory\Model\ResourceModel\Country\Collection" shared="false">
<arguments>
<argument name="helperData" xsi:type="object">DirectoryHelperDataProxy</argument>
<argument name="countriesWithNotRequiredStates" xsi:type="array">
<item name="FR" xsi:type="string">FR</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the reasoning behind making this change?

<item name="DE" xsi:type="string">DE</item>
</argument>
</arguments>
</type>
<preference for="Magento\Directory\Model\Country\Postcode\ConfigInterface" type="Magento\Directory\Model\Country\Postcode\Config" />
Expand Down