active_merchant icon indicating copy to clipboard operation
active_merchant copied to clipboard

Pin: Update 3DS to include new parameters

Open hudakh opened this issue 1 year ago • 28 comments

Pin: Update 3DS secure to include new parameters

We have found we cannot use 3DS secure with Pin as the parameters do not match those documented by Pin. This closes #4719.

Test Summary Local: 5476 tests, 77236 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Unit: 44 tests, 145 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed Remote: 21 tests, 65 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 100% passed

hudakh avatar Mar 01 '23 05:03 hudakh

Nice work @hudakh, changes look good to me and Unit/Remote tests pass :+1:

Still needs workflow approval and review from an AM maintainer.

AMHOL avatar Mar 07 '23 14:03 AMHOL

Nice work @hudakh, changes look good to me and Unit/Remote tests pass +1

Still needs workflow approval and review from an AM maintainer.

Thanks @AMHOL. I've cleaned up the commits and PR description in preparation for further review. Not sure if I need to assign to anyone.

hudakh avatar Mar 07 '23 23:03 hudakh

I haven't done in the past, normally someone just comes along and picks it up.

AMHOL avatar Mar 07 '23 23:03 AMHOL

@madpilot can this be marked as ready for review please? I am not sure how to proceed further to get this merged.

hudakh avatar Mar 20 '23 01:03 hudakh

Is there not somewhere else to do this? My last PR on this repo was 11 years ago. I'm not the best person to be approving anything.

madpilot avatar Mar 20 '23 02:03 madpilot

Is there not somewhere else to do this? My last PR on this repo was 11 years ago. I'm not the best person to be approving anything.

No worries. Your name was the only name listed for Pin under the contributors list.. I thought perhaps that would give you some super powers I do not have.

hudakh avatar Mar 20 '23 03:03 hudakh

@jessiagee Can you please assist here? I see you were the reviewer of the last Pin-related merge request. I am not sure of next steps, but I feel this won't progress if I don't prod someone.

hudakh avatar Mar 27 '23 00:03 hudakh

following this too. trying to pin @rachelkirk as well who might have looked at previous Pin-related PRs before.

Hey Rachel, would you be able to help or direct to someone who could help with progressing this PR?

raymzag avatar Mar 29 '23 06:03 raymzag

@hudakh if this is blocking you, you can pull the gem from your fork using the following in your Gemfile:

gem 'activemerchant', git: '[email protected]:hudakh/active_merchant.git', branch: '1-update-pin-gateway'
# or
gem 'activemerchant', git: '[email protected]:hudakh/active_merchant.git', ref: 'c716f13a9c18db7bb4550f175f42f981555af079'

Otherwise you'll have to wait for the next activemerchant release after this PR is merged.

AMHOL avatar Mar 29 '23 13:03 AMHOL

Hello! Could you please elaborate on how you will need the change in AM (i.e. do you need a new release or just a merge to master)? Additionally can you provide more details on why you are making this change?

aenand avatar Apr 17 '23 14:04 aenand

@aenand

@hudakh would need a merge/release in order to use this feature on the mainline.

The change updates the Pin gateway in order to add support for passing three_d_secure params to the charges endpoint as documented here, whilst retaining support for the old style 3D Secure passthrough options in order to maintain support for existing integrations.

AMHOL avatar Apr 17 '23 14:04 AMHOL

There are some rubocop linter failures. Please run bundle exec rubocop -a to autocorrect them!

aenand avatar Apr 20 '23 16:04 aenand

There are some rubocop linter failures. Please run bundle exec rubocop -a to autocorrect them!

I'm not familiar with running rubocop in different ruby versions, so I corrected the issues manually.

hudakh avatar Apr 23 '23 23:04 hudakh

To provide a cleaner slate for the maintenance of the library, this PR/Issue is being labeled stale after 60 days without activity. It will be closed in 14 days unless you comment with an update regarding its applicability to the current build. Thank you!

github-actions[bot] avatar Nov 15 '23 01:11 github-actions[bot]

Pending review.

AMHOL avatar Nov 15 '23 13:11 AMHOL

Sorry I lost this 🤦🏽 Looks good to me. Please squash the commits and add a changelog!

@hudakh are you available to do this?

AMHOL avatar Nov 30 '23 10:11 AMHOL

Hey, sorry I am on leave until February and don't have access to the repo until then. Happy for you to take over.

On Thu, 30 Nov 2023, 9:16 pm Andy Holland, @.***> wrote:

Sorry I lost this 🤦🏽 Looks good to me. Please squash the commits and add a changelog!

@hudakh https://github.com/hudakh are you available to do this?

— Reply to this email directly, view it on GitHub https://github.com/activemerchant/active_merchant/pull/4720#issuecomment-1833514342, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEM3DKB7MHRI3ZCMMSEMQATYHBPYNAVCNFSM6AAAAAAVLSVQCKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZTGUYTIMZUGI . You are receiving this because you were mentioned.Message ID: @.***>

hudakh avatar Nov 30 '23 11:11 hudakh

Hey, sorry I am on leave until February and don't have access to the repo until then. Happy for you to take over. On Thu, 30 Nov 2023, 9:16 pm Andy Holland, @.> wrote: Sorry I lost this 🤦🏽 Looks good to me. Please squash the commits and add a changelog! @hudakh https://github.com/hudakh are you available to do this? — Reply to this email directly, view it on GitHub <#4720 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEM3DKB7MHRI3ZCMMSEMQATYHBPYNAVCNFSM6AAAAAAVLSVQCKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZTGUYTIMZUGI . You are receiving this because you were mentioned.Message ID: @.>

Nice, happy to do that @hudakh if you invite me as a collaborator on https://github.com/hudakh/active_merchant/tree/1-update-pin-gateway, otherwise I will need to open a separate PR for it.

AMHOL avatar Nov 30 '23 11:11 AMHOL

Done @aenand

AMHOL avatar Nov 30 '23 12:11 AMHOL

Awesome, thank you so much!

On Thu, 30 Nov 2023, 11:07 pm Andy Holland, @.***> wrote:

Done @aenand https://github.com/aenand

— Reply to this email directly, view it on GitHub https://github.com/activemerchant/active_merchant/pull/4720#issuecomment-1833698562, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEM3DKHZ66HIYFLTZS3MFRLYHB4XDAVCNFSM6AAAAAAVLSVQCKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZTGY4TQNJWGI . You are receiving this because you were mentioned.Message ID: @.***>

hudakh avatar Nov 30 '23 12:11 hudakh

To provide a cleaner slate for the maintenance of the library, this PR/Issue is being labeled stale after 60 days without activity. It will be closed in 14 days unless you comment with an update regarding its applicability to the current build. Thank you!

github-actions[bot] avatar Jan 30 '24 01:01 github-actions[bot]

Rebased

AMHOL avatar Jan 30 '24 11:01 AMHOL

@aenand any update on this?

AMHOL avatar Feb 01 '24 09:02 AMHOL

@AMHOL i'm so sorry i missed this. it's saying there are merge conflicts, can you check those?

aenand avatar Feb 01 '24 15:02 aenand

@aenand I am not seeing any merge conflict issues?

Screenshot 2024-02-06 at 10 45 39

AMHOL avatar Feb 06 '24 02:02 AMHOL

@aenand is anything further needed to get this merged?

AMHOL avatar Feb 28 '24 16:02 AMHOL