woocommerce-ios icon indicating copy to clipboard operation
woocommerce-ios copied to clipboard

Orders Details: Use common class for Country and State selector in Order Details

Open iamgabrielma opened this issue 2 years ago • 2 comments

Part of https://github.com/woocommerce/woocommerce-ios/issues/6667

Description

CountrySelectorCommand and StateSelectorCommand share the majority of its code. To avoid the downsides of having duplicated code, we're abstracting these into one common class.

Changes:

  • Create a common AddressSelectorCommandProtocol that Country and StateOfACountry conforms to (These are typealiases of Networking.Country and Networking.StateOfACountry respectively)
  • Collapse CountrySelectorCommand and StateSelectorCommand into a common AddressSelectorCommand class. Its implementation details use the common AddressSelectorCommandProtocol rather than the specifics of each model.
  • Now that CountrySelectorViewModel and StateSelectorViewModel use AddressSelectorCommand, these could be collapsed as well, but will be part of a different PR.
  • Expanded setUp and tearDown methods to CountrySelectorViewModelTests

Testing instructions

We have not changed the functionality, but the following flow can be used to test the changes where these selectors are used:

  1. Go to Orders > + > Create Order > Fill the minimum necessary fields
  2. Tap on Add Customer Details > Fill the minimum necessary fields
  3. Confirm that the Country and State fields works as expected by saving new or editing existing data, for example:
  • Select US > Select a predefined State from the list. Save changes.
  • Select a non-US country, write down something in State, or leave it empty. Save Changes.
  • Tap on Add a different shipping address and fill out Country and State again with different values. Save changes.
  • When tapping Country, confirm that the search bar at the top works by filtering countries.
  • When selecting US as Country and tapping State, confirm that the search bar at the top works by filtering states.
  1. Confirm how the Order reflects all changes upon saving.

iamgabrielma avatar Sep 13 '22 11:09 iamgabrielma

You can test the changes from this Pull Request by:

  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr7713-4caaa7c on your iPhone
If you need access to App Center, please ask a maintainer to add you.

wpmobilebot avatar Sep 13 '22 11:09 wpmobilebot

Warnings
:warning: PR is not assigned to a milestone.

Generated by :no_entry_sign: dangerJS

peril-woocommerce[bot] avatar Sep 13 '22 13:09 peril-woocommerce[bot]

This one from our pairing session fell through the cracks, sorry for that! I just fixed the .xcodeproj conflicts and is ready to be reviewed when you have the time 🙇

iamgabrielma avatar Dec 12 '22 11:12 iamgabrielma

@iamgabrielma nice one, more than 60 lines of code and we avoid duplicated code 👏 I left some non-blocking remarks in the code, let me know what you think. :shipit:

toupper avatar Dec 12 '22 11:12 toupper