-
Notifications
You must be signed in to change notification settings - Fork 121
[HACK Week] Address map search in order details: analytics, enable feature flag, unit tests #15964
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
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
6db916a
Track two events for address map picker.
jaclync f6a243a
Update release notes.
jaclync ebf54c6
Mock map local search to enable unit testing `selectLocation` and `up…
jaclync 7d757ee
Remove unused import.
jaclync 5af9ce9
Enable address map search for all.
jaclync 64ed9be
Track `locale` property for `order_detail_edit_address_map_picker_tap…
jaclync f59700d
Merge branch 'trunk' into hack/WOOMOB-943-address-map-picker-analytics
jaclync 18f3811
Update test case names for better readability.
jaclync 85c27c5
Merge branch 'trunk' into hack/WOOMOB-943-address-map-picker-analytics
jaclync File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
190 changes: 190 additions & 0 deletions
190
...erceTests/ViewRelated/Orders/Order Details/Addresses/AddressMapPickerViewModelTests.swift
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,190 @@ | ||
| import Testing | ||
| import MapKit | ||
| import Contacts | ||
| import Yosemite | ||
| @testable import WooCommerce | ||
|
|
||
| struct AddressMapPickerViewModelTests { | ||
| private let mockCountryByCode: (String) -> Country? | ||
|
|
||
| init() { | ||
| mockCountryByCode = { countryCode in | ||
| switch countryCode { | ||
| case "US": | ||
| return Country(code: "US", name: "USA", states: [ | ||
| StateOfACountry(code: "CA", name: "Cali") | ||
| ]) | ||
| default: | ||
| return nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Initialization Tests | ||
|
|
||
| @available(iOS 17, *) | ||
| @Test func initialization_with_empty_fields_sets_properties_with_default_values() { | ||
| // Given | ||
| let emptyFields = AddressFormFields() | ||
|
|
||
| // When | ||
| let sut = AddressMapPickerViewModel(fields: emptyFields, countryByCode: mockCountryByCode) | ||
|
|
||
| // Then | ||
| #expect(sut.searchResults.isEmpty) | ||
| #expect(sut.annotations.isEmpty) | ||
| #expect(!sut.hasValidSelection) | ||
| } | ||
|
|
||
| // MARK: - Selection Tests | ||
|
|
||
| @available(iOS 17, *) | ||
| @Test func selectLocation_updates_annotations_and_hasValidSelection() async { | ||
| // Given | ||
| let fields = AddressFormFields() | ||
| let mockSearchProvider = MockAddressMapLocalSearchProvider.withBasicCoordinates() | ||
| let sut = AddressMapPickerViewModel(fields: fields, countryByCode: mockCountryByCode, searchProvider: mockSearchProvider) | ||
| let searchCompletion = MockMKLocalSearchCompletion() | ||
|
|
||
| // When | ||
| await sut.selectLocation(searchCompletion) | ||
|
|
||
| // Then | ||
| #expect(sut.annotations.count == 1) | ||
| #expect(sut.hasValidSelection == true) | ||
| } | ||
|
|
||
| // MARK: - Address Field Updates Tests | ||
|
|
||
| @available(iOS 17, *) | ||
| @Test func updateFields_with_no_selected_place_does_not_modify_fields() { | ||
| // Given | ||
| let sut = AddressMapPickerViewModel(fields: .init(), countryByCode: mockCountryByCode) | ||
| var updatedFields = AddressFormFields() | ||
| updatedFields.address1 = "Original Address" | ||
| updatedFields.city = "Original City" | ||
|
|
||
| // When | ||
| sut.updateFields(&updatedFields) | ||
|
|
||
| // Then | ||
| #expect(updatedFields.address1 == "Original Address") | ||
| #expect(updatedFields.city == "Original City") | ||
| } | ||
|
|
||
| @available(iOS 17, *) | ||
| @Test func updateFields_when_country_not_found_in_countryByCode_sets_country_and_state_as_strings() async { | ||
| // Given | ||
| let mockSearchProvider = MockAddressMapLocalSearchProvider.withFrenchAddress() | ||
| let sut = AddressMapPickerViewModel(fields: .init(), countryByCode: mockCountryByCode, searchProvider: mockSearchProvider) | ||
| let searchCompletion = MockMKLocalSearchCompletion() | ||
|
|
||
| await sut.selectLocation(searchCompletion) | ||
|
|
||
| // When | ||
| var updatedFields = AddressFormFields() | ||
| sut.updateFields(&updatedFields) | ||
|
|
||
| // Then | ||
| #expect(updatedFields.address1 == "Tour Eiffel") | ||
| #expect(updatedFields.city == "Paris") | ||
| #expect(updatedFields.postcode == "75007") | ||
| #expect(updatedFields.country == "FR") | ||
| #expect(updatedFields.state == "Île-de-France") | ||
| #expect(updatedFields.selectedCountry == nil) // Country is not found in countryByCode dictionary | ||
| #expect(updatedFields.selectedState == nil) | ||
| } | ||
|
|
||
| @available(iOS 17, *) | ||
| @Test func updateFields_when_country_is_found_in_countryByCode_sets_selected_country_and_state() async { | ||
| // Given | ||
| let mockSearchProvider = MockAddressMapLocalSearchProvider.withUSAddress() | ||
| let sut = AddressMapPickerViewModel(fields: .init(), countryByCode: mockCountryByCode, searchProvider: mockSearchProvider) | ||
| let searchCompletion = MockMKLocalSearchCompletion() | ||
|
|
||
| await sut.selectLocation(searchCompletion) | ||
|
|
||
| // When | ||
| var updatedFields = AddressFormFields() | ||
| sut.updateFields(&updatedFields) | ||
|
|
||
| // Then | ||
| #expect(updatedFields.address1 == "1 Apple Park Way") | ||
| #expect(updatedFields.city == "Cupertino") | ||
| #expect(updatedFields.postcode == "95014") | ||
| #expect(updatedFields.country == "USA") | ||
| #expect(updatedFields.state == "Cali") | ||
| #expect(updatedFields.selectedCountry?.code == "US") | ||
| #expect(updatedFields.selectedState?.code == "CA") | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Mock Classes | ||
|
|
||
| final private class MockMKLocalSearchCompletion: MKLocalSearchCompletion {} | ||
|
|
||
| final private class MockAddressMapLocalSearchProvider: AddressMapLocalSearchProviding { | ||
| private let mockPlacemark: MKPlacemark | ||
|
|
||
| init(mockPlacemark: MKPlacemark) { | ||
| self.mockPlacemark = mockPlacemark | ||
| } | ||
|
|
||
| func search(for completion: MKLocalSearchCompletion) async throws -> MKLocalSearch.Response { | ||
| let mockMapItem = MKMapItem(placemark: mockPlacemark) | ||
| let mockResponse = MockMKLocalSearchResponse(mapItems: [mockMapItem]) | ||
| return mockResponse | ||
| } | ||
| } | ||
|
|
||
| final private class MockMKLocalSearchResponse: MKLocalSearch.Response { | ||
| private let _mapItems: [MKMapItem] | ||
|
|
||
| init(mapItems: [MKMapItem]) { | ||
| self._mapItems = mapItems | ||
| super.init() | ||
| } | ||
|
|
||
| override var mapItems: [MKMapItem] { _mapItems } | ||
| } | ||
|
|
||
| private extension MockAddressMapLocalSearchProvider { | ||
| static func withFrenchAddress() -> MockAddressMapLocalSearchProvider { | ||
| let postalAddress = CNMutablePostalAddress() | ||
| postalAddress.street = "Tour Eiffel" | ||
| postalAddress.city = "Paris" | ||
| postalAddress.postalCode = "75007" | ||
| postalAddress.country = "France" | ||
| postalAddress.isoCountryCode = "FR" | ||
| postalAddress.state = "Île-de-France" | ||
|
|
||
| let placemark = MKPlacemark( | ||
| coordinate: CLLocationCoordinate2D(latitude: 48.8584, longitude: 2.2945), | ||
| postalAddress: postalAddress | ||
| ) | ||
|
|
||
| return MockAddressMapLocalSearchProvider(mockPlacemark: placemark) | ||
| } | ||
|
|
||
| static func withUSAddress() -> MockAddressMapLocalSearchProvider { | ||
| let postalAddress = CNMutablePostalAddress() | ||
| postalAddress.street = "1 Apple Park Way" | ||
| postalAddress.city = "Cupertino" | ||
| postalAddress.postalCode = "95014" | ||
| postalAddress.country = "United States" | ||
| postalAddress.isoCountryCode = "US" | ||
| postalAddress.state = "CA" | ||
|
|
||
| let placemark = MKPlacemark( | ||
| coordinate: CLLocationCoordinate2D(latitude: 37.3349, longitude: -122.0090), | ||
| postalAddress: postalAddress | ||
| ) | ||
|
|
||
| return MockAddressMapLocalSearchProvider(mockPlacemark: placemark) | ||
| } | ||
|
|
||
| static func withBasicCoordinates() -> MockAddressMapLocalSearchProvider { | ||
| let placemark = MKPlacemark(coordinate: CLLocationCoordinate2D(latitude: 37.7749, longitude: -122.4194)) | ||
| return MockAddressMapLocalSearchProvider(mockPlacemark: placemark) | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing the reference here caught my eye, is there a reason for the implementation to accept an inout and modify the reference rather than returning a copy? (
func updateFields(_ fields: AddressFormFields) -> AddressFormFields). Then the tests would look something like:Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used
inouthere for direct mutation without copying the struct, which works well with SwiftUI's@Bindingbinding patterns. If using the functional approach (creating a new struct and updating the existing), because the map picker only updates a subset of address properties, other fields like names/email/phone need to be preserved manually with caution. Lemme know what you think!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, thanks! The scenario where we only have to update the subset of properties while preserving the rest definitely makes for a good use case of mutating the object directly to avoid errors.