Fix #25350 - Some JS removes State/Province field for United Kingdom for example

[Bartlomiejsz]

Description (*)

This is fix for #25350

Fixed Issues (if relevant)

1. Fixes Some JS removes State/Province field for United Kingdom for example #25350: Some JS removes State/Province field for United Kingdom for example

Manual testing scenarios (*)

1. Click Address Book
2. Select the United Kingdom as a country
3. Set Postcode to AB12 3CD
4. Set State/Province to Greater London
5. Set City to London
6. Save address.
7. Verify that you see "Greater London" in Default Shipping and Default Billing address
8. Click Change Billing Address

Expected result (*)

Greater London is in the State/Province input

Questions or comments

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)

[m2-assistant]

Hi @Bartlomiejsz. 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.3-develop instance - deploy vanilla Magento instance

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

[krzksz]

Hey @Bartlomiejsz, thanks for the contribution! I have trouble to understand the extensiveness of this change, I can see you are also refactoring helpers but was it really required to touch PHP files at all? I did some short debugging and following changes inside _updateRegion method solved the issue for me and cleaned up the code a bit:

1. Move regionInput.val(''); below if (this.options.regionJson[country]) { as this clearing input field every time.
2. Extract this._removeSelectOptions(regionList); above if...else because it is present in both of them anyway.
3. Remove $(regionList).find('option:selected').removeAttr('selected'); as all the options are removed by the above call anyway.

Can you please check if those changes would be ok for you once you apply them? If so, feel free to update the PR.

[Bartlomiejsz]

Hi @krzksz, I'll check those changes as soon as possible, now only regarding php files being changed - this indeed wasn't required in terms of functional changes here and in first version of PR those were not modified. I modified those later to fix failing static tests ($this shouldn't be used in phtml files)

[Bartlomiejsz]

Hi @krzksz, I applied changes you requested, please review :)

[Bartlomiejsz]

Hi @krzksz, you're right, added this one change. I also noticed one additional issue, if you select some country which have regions list as select (i.e. USA), select one of regions, then switch to another country with also regions select (like Poland). Then Region select becomes empty (even without default message to choose option). I modified then saving of current region in same manner as introduced in this PR for text input - per country.

[magento-engcom-team]

Hi @krzksz, thank you for the review.
ENGCOM-6318 has been created to process this Pull Request
✳️ @krzksz, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[engcom-Bravo]

Hello @krzksz. Would You please mark this PR with an appropriate label. Thank You.

[krzksz]

@engcom-Bravo Done.

[engcom-Bravo]

Hello @Bartlomiejsz. Thank You for Your contribution. In order for us to be able to further process Your PR please cover it with the appropriate auto-tests. Thank You.

[Bartlomiejsz]

Hi @engcom-Bravo, sorry but what exactly should be covered with tests here? In my previous PRs all javascript changes were marked as Auto Tests: Not Required, ViewModel I added is simply returning existing helpers and it's addition I already included in dev/tests/integration/testsuite/Magento/Customer/Block/Form/RegisterTest.php.

[engcom-Echo]

@magento run all tests

[Bartlomiejsz]

Hi @krzksz, since #26285 introduced ViewModel used for similar goal that I achieved with one created here, and both changes together resulted in conflicts, I rebased my changes onto 2.4-develop and instead of creating my own ViewModel I used one created in mentioned PR. Please review again if possible

[ptylek]

@Bartlomiejsz this branch has conflicts that must be resolved

[Bartlomiejsz]

@ptylek done, resolved