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

Bugfix for regions with single quote in name #936

Merged
merged 1 commit into from
Jan 14, 2015
Merged

Bugfix for regions with single quote in name #936

merged 1 commit into from
Jan 14, 2015

Conversation

ddonnini
Copy link
Contributor

@ddonnini ddonnini commented Jan 8, 2015

Before this bugfix, changing the country of a customer address
use to generate a jQuery exception that breaks the options
population if a region of that country has a name containg
one of the css meta-characters.
For example, selecting France the option Côtes-d'Armor breaks
jQuery.

@lanken
Copy link
Contributor

lanken commented Jan 13, 2015

It's bad practice to change default flow of existing tests. Additionally in current case you overwrite France address at once without saving it.

The best way is to test customer with specific address separately.
The only changes you need to perform for this is to add one more variation to UpdateCustomerFrontendEntity test in https://github.com/magento/magento2/blob/develop/dev/tests/functional/tests/app/Magento/Customer/Test/TestCase/UpdateCustomerFrontendEntity/test.csv

"description";"customer/data/firstname";"customer/data/lastname";"customer/data/email";"address/data/firstname";"address/data/lastname";"address/data/company";"address/data/street";"address/data/city";"address/data/country_id";"address/data/region_id";"address/data/region";"address/data/telephone";"address/data/fax";"address/data/postcode";"constraint"
"Update customer information and add US address";"Jany %isolation%";"Doe %isolation%";"janydoe%isolation%@example.com";"Jany %isolation%";"Doe %isolation%";"Company %isolation%";"Some street %isolation%";"City %isolation%";"United States";"Colorado";"-";"555-888-111-999";"161-999-8888";"12345";"assertCustomerAddressSuccessSaveMessage, assertCustomerDefaultAddresses"
"Update customer information and add UK address";"Jonny %isolation%";"Doe %isolation%";"jonny%isolation%@example.com";"John %isolation%";"Doe %isolation%";"Company %isolation%";"Some street %isolation%";"City %isolation%";"United Kingdom";"-";"Region %isolation%";"0123456789-02134567";"5555-874-99634";"12345";"assertCustomerAddressSuccessSaveMessage, assertCustomerDefaultAddresses"
"Update customer information and add France address";"-";"-";"-";"Jean";"Reno";"Magento %isolation%";"18-20 Rue Maréchal Leclerc";"Quintin";"France";"Côtes-d'Armor";"-";"333-33-333-33";"-";"22800";"assertCustomerAddressSuccessSaveMessage, assertCustomerDefaultAddresses"

You should also slightly change \Magento\Customer\Test\Block\Form\CustomerForm::fill() (this changes hasn't been delivered to mainline yet)

    public function fill(FixtureInterface $customer, Element $element = null)
    {
        /** @var CustomerInjectable $customer */
        if ($customer->hasData()) {
            return parent::fill($customer, $element);
        }
    }

@ddonnini
Copy link
Contributor Author

Cool, thanks, I'll change it as soon as possible

@ddonnini
Copy link
Contributor Author

@lanken is it better now?
Thanks again.

@lanken
Copy link
Contributor

lanken commented Jan 14, 2015

Yes, it's much better. The main idea is right, but could you, please, revert changes in

  • application.yml.dist
  • install_data.yml.dist
  • phpunit.xml.dist
    It's better to use not dist files, in such way you wouldn't commit them by mistake.

Also, revert changes in dev/tests/functional/tests/app/Magento/Customer/Test/Repository/Address.php as you deleted getAddressFR() method.

In file dev/tests/functional/tests/app/Magento/Customer/Test/TestCase/UpdateCustomerFrontendEntity/test.csv you should only add additional line, but don't delete existing ones.

@ddonnini
Copy link
Contributor Author

Maybe it's quite necessary a new pull request when everything is ok, without all my mistakes, what do you think?

@vpelipenko
Copy link
Contributor

Probably you can force push the source branch and leave there only one commit with correct fixes, it is not necessary to create new pull request.

@Vinai
Copy link
Contributor

Vinai commented Jan 14, 2015

You might find this useful regardign squashing commits: http://stackoverflow.com/a/5201642/485589

Before this bugfix, changing the country of a customer address
use to generate a jQuery exception that breaks the options
population if a region of that country has a name containg
one of the css meta-characters.
For example, selecting France the option Côtes-d'Armor breaks
jQuery.
@ddonnini
Copy link
Contributor Author

Voillà, thanks to all!

@vpelipenko
Copy link
Contributor

CR: passed
Builds: green
Resolution: OK to merge
Ticket: MAGETWO-32688

vpelipenko added a commit that referenced this pull request Jan 14, 2015
Bugfix for regions with single quote in name
@vpelipenko vpelipenko merged commit dad5614 into magento:develop Jan 14, 2015
magento-team added a commit to buskamuza/magento2-community-edition that referenced this pull request Jan 20, 2015
* Various improvements:
    * Updated Copyright Notice and provided reference to the license file
    * Updated test framework to support stores other than default
    * Removed version information from theme.xml files leaving it only in composer.json files
* Fixed bugs:
    * Fixed an issue where coupon code was reported to be invalid if it has been removed from reorder in backend and then re-applied
    * Fixed an issue where the 'Guide to Using Sample Data' link was incorrect in the web setup UI
    * Fixed an issue where the link to System Requirements in bootstrap.php was incorrect
    * Fixed an issue where Compiler could not verify case sensitive dependency
    * Fixed an issue where the Recently Compared Products and Recently Viewed Products widgets were not displayed in sidebars
    * Fixed an issue where the Orders and Returns widget type contained unnecessary tab
    * Fixed an issue where an image added to a CMS page using the WYSIWYG editor was displayed as a broken link after turning off the allow_url_fopen parameter in php.ini
    * Fixed an issue where it was impossible to log in to the backend from the first attempt after changing Base URL
    * Fixed an issue where it was impossible to set back the default English (United States) interface locale for the admin user after changing it so an other value
    * Fixed an issue where it was possible to execute malicious JavaScript code in the context of website via the Sender Email parameter
    * Fixed an issue where the Product Stock Alert email was sent to a customer from a store view different than a customer account was created in
    * Fixed an issue where the "Server cannot understand Accept HTTP header media type" error message was not informative enough
    * Fixed an issue where unit tests did not work as expected after installing Magento 2
    * Fixed an issue where the password change email notification was sent after saving admin account settings even if password was not changed
    * Fixed an issue where static tests failed as a result of adding  API functional tests
    * Fixed API functional tests after merging pull request [#927](magento/magento2#927)
    * Fixed an issue where the Edit button was present for invoiced orders
    * Fixed an issue where function _underscore did not work with keys like SKeyName ('s_key_name')
    * Fixed an issue where a fatal error occurred when browsing categories if web server did not have write permissions for media/catalog/product
* Github requests:
    * [#792](magento/magento2#792) -- Failed to set ini option "session.save_path" to value
    * [#796](magento/magento2#796) -- install.log cannot be created with open_basedir restriction
    * [#823](magento/magento2#823) -- Installation bug
    * [#920](magento/magento2#920) -- "web setup wizard is not accessible" error message but the setup wizard is actually accessible
    * [#829](magento/magento2#829) -- [API] OAuth1.0 request token request failing / Consumer key has expired
    * [#658](magento/magento2#658) -- Inline translate malfunctioning
    * [#950](magento/magento2#950) -- Fix for the missed trailing end of line in indexer.php usage help text
    * [#932](magento/magento2#932) -- Migration tool - not all input has comments
    * [#959](magento/magento2#959) -- Replace UTF8 'en dash' with minus in error message
    * [#911](magento/magento2#911) -- Fix test assertion and slight cleanup refactoring
    * [#936](magento/magento2#936) -- Bugfix for regions with single quote in name
    * [#902](magento/magento2#902) -- Add integration test for View\Page\Config\Reader\Html
    * [#925](magento/magento2#925) -- Failed test due to Class not following the naming conventions
    * [#943](magento/magento2#943) -- magento2-925 Failed Test due to Class not following the naming conventions
    * [#968](magento/magento2#968) -- Apply pattern matching datasource config files
    * [#949](magento/magento2#949) -- Added 'status' command for cache cli script / Also improved readability
* PHP 5.6 in composer.json:
    * Added PHP 5.6.0 to the list of required PHP versions in all composer.json files
    * Updated Travis CI configuration to include PHP 5.6 builds
* Framework improvements:
    * Removed TODOs in the Integration and Authorization modules
    * Removed leading backslash from the 'use' statement throughout the code base
magento-team added a commit to buskamuza/magento2-community-edition that referenced this pull request Jan 20, 2015
* Various improvements:
    * Updated Copyright Notice and provided reference to the license file
    * Updated test framework to support stores other than default
    * Removed version information from theme.xml files leaving it only in composer.json files
* Fixed bugs:
    * Fixed an issue where coupon code was reported to be invalid if it has been removed from reorder in backend and then re-applied
    * Fixed an issue where the 'Guide to Using Sample Data' link was incorrect in the web setup UI
    * Fixed an issue where the link to System Requirements in bootstrap.php was incorrect
    * Fixed an issue where Compiler could not verify case sensitive dependency
    * Fixed an issue where the Recently Compared Products and Recently Viewed Products widgets were not displayed in sidebars
    * Fixed an issue where the Orders and Returns widget type contained unnecessary tab
    * Fixed an issue where an image added to a CMS page using the WYSIWYG editor was displayed as a broken link after turning off the allow_url_fopen parameter in php.ini
    * Fixed an issue where it was impossible to log in to the backend from the first attempt after changing Base URL
    * Fixed an issue where it was impossible to set back the default English (United States) interface locale for the admin user after changing it so an other value
    * Fixed an issue where it was possible to execute malicious JavaScript code in the context of website via the Sender Email parameter
    * Fixed an issue where the Product Stock Alert email was sent to a customer from a store view different than a customer account was created in
    * Fixed an issue where the "Server cannot understand Accept HTTP header media type" error message was not informative enough
    * Fixed an issue where unit tests did not work as expected after installing Magento 2
    * Fixed an issue where the password change email notification was sent after saving admin account settings even if password was not changed
    * Fixed an issue where static tests failed as a result of adding  API functional tests
    * Fixed API functional tests after merging pull request [#927](magento/magento2#927)
    * Fixed an issue where the Edit button was present for invoiced orders
    * Fixed an issue where function _underscore did not work with keys like SKeyName ('s_key_name')
    * Fixed an issue where a fatal error occurred when browsing categories if web server did not have write permissions for media/catalog/product
* Github requests:
    * [#792](magento/magento2#792) -- Failed to set ini option "session.save_path" to value
    * [#796](magento/magento2#796) -- install.log cannot be created with open_basedir restriction
    * [#823](magento/magento2#823) -- Installation bug
    * [#920](magento/magento2#920) -- "web setup wizard is not accessible" error message but the setup wizard is actually accessible
    * [#829](magento/magento2#829) -- [API] OAuth1.0 request token request failing / Consumer key has expired
    * [#658](magento/magento2#658) -- Inline translate malfunctioning
    * [#950](magento/magento2#950) -- Fix for the missed trailing end of line in indexer.php usage help text
    * [#932](magento/magento2#932) -- Migration tool - not all input has comments
    * [#959](magento/magento2#959) -- Replace UTF8 'en dash' with minus in error message
    * [#911](magento/magento2#911) -- Fix test assertion and slight cleanup refactoring
    * [#936](magento/magento2#936) -- Bugfix for regions with single quote in name
    * [#902](magento/magento2#902) -- Add integration test for View\Page\Config\Reader\Html
    * [#925](magento/magento2#925) -- Failed test due to Class not following the naming conventions
    * [#943](magento/magento2#943) -- magento2-925 Failed Test due to Class not following the naming conventions
    * [#968](magento/magento2#968) -- Apply pattern matching datasource config files
    * [#949](magento/magento2#949) -- Added 'status' command for cache cli script / Also improved readability
* PHP 5.6 in composer.json:
    * Added PHP 5.6.0 to the list of required PHP versions in all composer.json files
    * Updated Travis CI configuration to include PHP 5.6 builds
* Framework improvements:
    * Removed TODOs in the Integration and Authorization modules
    * Removed leading backslash from the 'use' statement throughout the code base
magento-team pushed a commit that referenced this pull request Mar 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants