Skip to content

Conversation

@chintan-soni-cko
Copy link
Contributor

@chintan-soni-cko chintan-soni-cko commented Aug 21, 2023

Issue

PIMOB-2093

Proposed changes

  1. added capabilities of billing form address in prefilldata from payment form config
  2. Added unit tests for it

Test Steps

If there's any functionality change, please list a step by step guide of how to verify the changes, and/or upload a screen recording for any visible changes.

  1. Add billing form address in prefill data from payment form config. Refer main activity from sample app
  2. verify address summary should update. (attached video)
  3. Click on edit address summary and change the fields from the billing form, then Address summary should updated accordingly

Checklist

billling_recording_20230821_094805.webm

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • Reviewers assigned
  • I have performed a self-review of my code and manual testing
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if applicable)

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you choose the solution you did and what alternatives you considered, etc...

Copy link
Contributor

@fabio-insolia-cko fabio-insolia-cko left a comment

Choose a reason for hiding this comment

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

Nice addition of quality test here
Good job!

@chintan-soni-cko chintan-soni-cko merged commit 5eef4d4 into master Aug 21, 2023
Copy link
Contributor

@jheng-hao-lin-cko jheng-hao-lin-cko left a comment

Choose a reason for hiding this comment

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

Well done on all the extensive test cases 🔥 🔥 🔥

import com.checkout.frames.screen.billingaddress.billingaddressdetails.models.BillingAddress
import com.checkout.frames.screen.paymentform.model.BillingFormAddress

internal class BillingFormAddressToBillingAddressMapper : Mapper<BillingFormAddress?, BillingAddress> {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we can make a companion object containing a lazy instance so the manager can just take the INSTANCE, for example:

internal class BillingFormAddressToBillingAddressMapper : Mapper<BillingFormAddress?, BillingAddress> {

    override fun map(from: BillingFormAddress?) =
        BillingAddress(
        name = from?.name, phone = from?.phone, address = from?.address ?: BillingAddress().address
    )

    internal companion object {
        internal val INSTANCE by lazy { BillingFormAddressToBillingAddressMapper() }
    }
}

// then in the PaymentModule we can do this 
fun paymentStateManager(
    supportedCardSchemes: List<CardScheme>,
    prefillData: PrefillData?
): PaymentStateManager =
    PaymentFormStateManager(supportedCardSchemes, prefillData, BillingFormAddressToBillingAddressMapper.INSTANCE)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants