From 862f58df38a5958eba1294490d360fafc2c00309 Mon Sep 17 00:00:00 2001 From: Dan Jackson Date: Mon, 11 Dec 2017 15:59:55 -0800 Subject: [PATCH 1/3] Add failing test in `STPAddressViewModelTest` to capture #840 Why does the last assert of `testIsValid_Zip` fail? An `STPAddressViewModel` with required billing fields == `Zip` pass a `nil` country to determine if it `isValid`, and therefore will *always* incorrectly report that countries without postal codes are invalid. --- Tests/Tests/STPAddressViewModelTest.m | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/Tests/Tests/STPAddressViewModelTest.m b/Tests/Tests/STPAddressViewModelTest.m index 4d898521276..8de61b7707a 100644 --- a/Tests/Tests/STPAddressViewModelTest.m +++ b/Tests/Tests/STPAddressViewModelTest.m @@ -18,6 +18,7 @@ @implementation STPAddressViewModelTest - (void)testInitWithRequiredBillingFields { STPAddressViewModel *sut = [[STPAddressViewModel alloc] initWithRequiredBillingFields:STPBillingAddressFieldsNone]; XCTAssertTrue([sut.addressCells count] == 0); + XCTAssertTrue(sut.isValid); sut = [[STPAddressViewModel alloc] initWithRequiredBillingFields:STPBillingAddressFieldsZip]; XCTAssertTrue([sut.addressCells count] == 1); @@ -126,7 +127,29 @@ - (void)testSetAddress { XCTAssertEqualObjects(sut.addressCells[8].contents, @"555-555-5555"); } -- (void)testIsValid { +- (void)testIsValid_Zip { + STPAddressViewModel *sut = [[STPAddressViewModel alloc] initWithRequiredBillingFields:STPBillingAddressFieldsZip]; + + STPAddress *address = [STPAddress new]; + + address.country = @"US"; + sut.address = address; + XCTAssertEqual(sut.addressCells.count, 1ul); + XCTAssertFalse(sut.isValid, @"US addresses have postalCode, require it when requiredBillingFields is .Zip"); + + address.postalCode = @"94016"; + sut.address = address; + XCTAssertTrue(sut.isValid); + + address.country = @"MO"; // in Macao, postalCode is optional + address.postalCode = nil; + sut.address = address; + XCTAssertEqual(sut.addressCells.count, 0ul); + XCTAssertTrue(sut.isValid, @"in Macao, postalCode is optional, valid without one"); +} + + +- (void)testIsValid_Full { STPAddressViewModel *sut = [[STPAddressViewModel alloc] initWithRequiredBillingFields:STPBillingAddressFieldsFull]; XCTAssertFalse(sut.isValid); sut.addressCells[0].contents = @"John Smith"; From a08471168b5b943232b587a55cde76475492d0e8 Mon Sep 17 00:00:00 2001 From: Dan Jackson Date: Mon, 11 Dec 2017 16:28:09 -0800 Subject: [PATCH 2/3] Add `addressFieldTableViewCountryCode` to the `STPAddress` created by `STPAddressViewModel` When there's no country table cell/text field, the country information is stored in `addressFieldTableViewCountryCode`. This fixes #840, because the address used to check `isValid` is created through this method. I think this is a desirable change overall. When `setAddress:` is called, it extracts the `STPAddress.country` value, and stores it into `addressFieldTableViewCountryCode`. It makes a lot of sense to me to reverse that when reading the `STPAddress` back out of the object. --- Stripe/STPAddressViewModel.m | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Stripe/STPAddressViewModel.m b/Stripe/STPAddressViewModel.m index 6735d447702..54295510e7e 100644 --- a/Stripe/STPAddressViewModel.m +++ b/Stripe/STPAddressViewModel.m @@ -338,6 +338,9 @@ - (STPAddress *)address { break; } } + // Prefer to use the contents of STPAddressFieldTypeCountry, but fallback to + // `addressFieldTableViewCountryCode` if nil (important for STPBillingAddressFieldsZip) + address.country = address.country ?: self.addressFieldTableViewCountryCode; return address; } From 9a1b3db690cb8fec1ddfaaf165511a7a8d865cec Mon Sep 17 00:00:00 2001 From: Dan Jackson Date: Mon, 11 Dec 2017 17:11:10 -0800 Subject: [PATCH 3/3] Hide `inputAccessoryToolbar` when there is no postal code for the current country I observed this bug while testing. The toolbar with 'Next' (above the number pad) was visible for countries without postal codes. When the paymentCell was completely filled, the 'Next' button enabled, but did nothing when tapped. Instead of special-casing on the `configuration.requiredBillingAddressFields`, just use the logic already built into `STPAddressViewModel` to know whether there'll be a field to change to. --- Stripe/STPAddCardViewController.m | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/Stripe/STPAddCardViewController.m b/Stripe/STPAddCardViewController.m index c6e2b6ee4a1..4374fc417c5 100644 --- a/Stripe/STPAddCardViewController.m +++ b/Stripe/STPAddCardViewController.m @@ -124,9 +124,7 @@ - (void)createAndSetupViews { self.inputAccessoryToolbar = [UIToolbar stp_inputAccessoryToolbarWithTarget:self action:@selector(paymentFieldNextTapped)]; [self.inputAccessoryToolbar stp_setEnabled:NO]; - if (self.configuration.requiredBillingAddressFields != STPBillingAddressFieldsNone) { - paymentCell.inputAccessoryView = self.inputAccessoryToolbar; - } + [self updateInputAccessoryVisiblity]; self.tableView.dataSource = self; self.tableView.delegate = self; @@ -316,6 +314,14 @@ - (void)updateDoneButton { ); } +- (void)updateInputAccessoryVisiblity { + // The inputAccessoryToolbar switches from the paymentCell to the first address field. + // It should only be shown when there *is* an address field. This compensates for the lack + // of a 'Return' key on the number pad used for paymentCell entry + BOOL hasAddressCells = self.addressViewModel.addressCells.count > 0; + self.paymentCell.inputAccessoryView = hasAddressCells ? self.inputAccessoryToolbar : nil; +} + - (void)setCustomFooterView:(UIView *)footerView { _customFooterView = footerView; [self.stp_willAppearPromise voidOnSuccess:^{ @@ -360,11 +366,13 @@ - (void)paymentCardTextFieldDidEndEditingCVC:(__unused STPPaymentCardTextField * - (void)addressViewModel:(__unused STPAddressViewModel *)addressViewModel addedCellAtIndex:(NSUInteger)index { NSIndexPath *indexPath = [NSIndexPath indexPathForRow:index inSection:STPPaymentCardBillingAddressSection]; [self.tableView insertRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationAutomatic]; + [self updateInputAccessoryVisiblity]; } - (void)addressViewModel:(__unused STPAddressViewModel *)addressViewModel removedCellAtIndex:(NSUInteger)index { NSIndexPath *indexPath = [NSIndexPath indexPathForRow:index inSection:STPPaymentCardBillingAddressSection]; [self.tableView deleteRowsAtIndexPaths:@[indexPath] withRowAnimation:UITableViewRowAnimationAutomatic]; + [self updateInputAccessoryVisiblity]; } - (void)addressViewModelDidChange:(__unused STPAddressViewModel *)addressViewModel {