-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Pin: Update 3DS to include new parameters #4720
Conversation
Nice work @hudakh, changes look good to me and Unit/Remote tests pass 👍 Still needs workflow approval and review from an AM maintainer. |
7cbb05c
to
c716f13
Compare
I haven't done in the past, normally someone just comes along and picks it up. |
@madpilot can this be marked as ready for review please? I am not sure how to proceed further to get this merged. |
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. |
@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. |
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? |
@hudakh if this is blocking you, you can pull the gem from your fork using the following in your Gemfile: gem 'activemerchant', git: 'git@github.com:hudakh/active_merchant.git', branch: '1-update-pin-gateway'
# or
gem 'activemerchant', git: 'git@github.com:hudakh/active_merchant.git', ref: 'c716f13a9c18db7bb4550f175f42f981555af079' Otherwise you'll have to wait for the next activemerchant release after this PR is merged. |
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 |
@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 |
There are some rubocop linter failures. Please run |
I'm not familiar with running rubocop in different ruby versions, so I corrected the issues manually. |
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! |
Pending review. |
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.
Sorry I lost this 🤦🏽 Looks good to me. Please squash the commits and add a changelog!
@hudakh are you available to do this? |
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. |
fdaa875
to
7d653f4
Compare
Done @aenand |
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
<#4720 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEM3DKHZ66HIYFLTZS3MFRLYHB4XDAVCNFSM6AAAAAAVLSVQCKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZTGY4TQNJWGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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! |
7d653f4
to
8744058
Compare
Rebased |
@aenand any update on this? |
@AMHOL i'm so sorry i missed this. it's saying there are merge conflicts, can you check those? |
@aenand I am not seeing any merge conflict issues? |
@aenand is anything further needed to get this merged? |
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! |
Still not stale. |
35e2620
to
4de3c00
Compare
Updated @aenand, it was the CHANGELOG entry. |
…t#4720) Co-authored-by: Huda <huda@haesemathematics.com.au>
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