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

[Enhancement] - User can see Payment Methods screen #367

Merged
merged 23 commits into from
Nov 1, 2018

Conversation

Rcureton
Copy link
Contributor

@Rcureton Rcureton commented Oct 31, 2018

What

  • Added PaymentMethodsViewModel, PaymentMethodsActivity, PaymentMethodsViewHolderViewModel, PaymentMethodsViewHolder, and PaymentMethodsAdapter.
  • Imported the vector drawables for CreditCardTypes.
  • Added the ViewModel tests for PaymentMethodsViewModel and PaymentMethodsViewHolderViewModel
  • Added new userpayments.graphql file for Payment Queries and Mutations.
  • Added CustomTypeAdapter for Date.
  • Upgraded constraint_layout_version to 1.1.3

Story

User can see Payment Methods screen

See 👀

@Rcureton Rcureton requested a review from eoji October 31, 2018 20:19
@Rcureton Rcureton changed the base branch from master to settings-v3 October 31, 2018 20:52
Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!! Had a few comments.


</android.support.design.widget.AppBarLayout>

<ScrollView
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @dannyalright, we are wondering: should the whole screen scroll under the toolbar or only the list of cards. I'm leaning towards the list so we can always see the "Add a new card" row.

}
}).build())

this.cards.assertValueCount(1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this test, let's assert the proper list is emitted like this:

@Test
    fun testGetCards() {
        val node = UserPaymentsQuery.Node("", "5555", Date(), "9876",
                CreditCardState.ACTIVE, CreditCardPaymentType.CREDIT_CARD, CreditCardTypes.MASTERCARD)
        
        
        setUpEnvironment(environment().toBuilder().apolloClient(object : MockApolloClient() {
            override fun getStoredCards(): Observable<UserPaymentsQuery.Data> {

                return Observable.just(UserPaymentsQuery.Data(UserPaymentsQuery.Me("",
                        UserPaymentsQuery.StoredCards("", List(1
                        ) { _ -> node }))))
            }
        }).build())

        this.cards.assertValue(Collections.singletonList(node))
    }
}

@Rcureton
Copy link
Contributor Author

Rcureton commented Nov 1, 2018

Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GIF
CLOSE

init {
this.card.map { expiration -> expiration.expirationDate() }
.map {
val sdf = SimpleDateFormat("MM/yyyy", Locale.getDefault())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this a private variable so we don't recreate it every time card emits

Copy link
Contributor

@eoji eoji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TO QA 🚀

@Rcureton Rcureton merged commit 2436858 into settings-v3 Nov 1, 2018
@Rcureton Rcureton deleted the rc/payment-methods branch November 1, 2018 21:15
eoji added a commit that referenced this pull request Feb 6, 2019
* User can see Payment Methods screen (#367)
* [💳] Saving credit cards (#368)
* [🚫💳] Deleting saved cards (#373)
* Adding divider when cards are present with tests. (#393)
* updating stripe to 8.3.0 #453
* [💳] Only allowing Kickstarter accepted card types to be saved (#454)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants