[Mercadopago] Add support for mercadopago gateway. #185
Conversation
Manually picked up code from various branches/PRs related to mercadopago The branches/PRs are couple years old, and messy.
|
Once this PR is merged, I'll close other mercadopago related PRs, mentioning this PR. |
|
@mandarvaze Can you sync the branch with latest |
Done. |
Codecov Report
@@ Coverage Diff @@
## dev #185 +/- ##
==========================================
+ Coverage 77.39% 80.70% +3.30%
==========================================
Files 14 16 +2
Lines 376 456 +80
==========================================
+ Hits 291 368 +77
- Misses 85 88 +3
Continue to review full report at Codecov.
|
|
@mandarvaze Also check if the docs are generating properly. |
| | `customer_id` | Unique customer id issued by the gateway. For new customer it must skipped. Type- string| | ||
| | `order_type` | `"mercadopago"` or `"mercadolibre"` as per the order. Type- string | | ||
| | `installments` | No of installments for payment. Type- integer | | ||
|
|
There was a problem hiding this comment.
"For eg. https://www.mercadopago.com.ar/developers/en/reference/payments/_payments/post/"
check the documentation to get how mercadopago accepts the params.
There was a problem hiding this comment.
@mandarvaze Add PCI and Non PCI section and implementation with info.
### I'm not PCI-DSS compliant
No worries, both `authorize/3` and `purchase/3` accept a
"payment-source-identifier" (a `string`) instead of a `CreditCard.t`
struct. You'll have to generate this identifier using [Stripe.js and
Elements][stripe-js] client-side.
### I'm PCI-DSS compliant
In that case, you need not use [Stripe.js or Elements][stripe-js] and can
directly accept the client's card info and pass the `CreditCard.t` struct to
this module's functions.
| """ | ||
| @spec void(String.t(), keyword) :: {:ok | :error, Response} | ||
| def void(payment_id, opts) do | ||
| # url = "#{@base_url}/v1/payments/#{payment_id}?access_token=#{opts[:config][:access_token]}" |
There was a problem hiding this comment.
@mandarvaze remove this commented code.
| defp respond({:ok, %HTTPoison.Response{body: body, status_code: status_code}}, opts) do | ||
| body = body |> Poison.decode!() | ||
|
|
||
| case body["cause"] do |
There was a problem hiding this comment.
@mandarvaze success and failure should be mocked.
| def bad_card_purchase_response do | ||
| {:error, | ||
| %HTTPoison.Error{ | ||
| reason: "Bad Card for Purchase", |
There was a problem hiding this comment.
@mandarvaze this should capture the real bad response from the gateway.
|
@mandarvaze update the README file with the addition of this new gateway. |
Done. |
WalkthroughA new payment gateway integration for MercadoPago has been added to the Gringotts payment processing framework. The implementation includes a dedicated module for handling MercadoPago payment operations such as authorization, capture, purchase, void, and refund. Comprehensive documentation and configuration details are provided. To ensure reliability, a suite of unit and integration tests has been introduced, along with a mock module that simulates MercadoPago API responses for various scenarios. The tests validate both successful and error cases for all gateway operations, covering both internal logic and real API interaction patterns. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MercadopagoGateway
participant MercadoPagoAPI
Client->>MercadopagoGateway: authorize(amount, card, opts)
MercadopagoGateway->>MercadoPagoAPI: Create card token
MercadoPagoAPI-->>MercadopagoGateway: Card token response
MercadopagoGateway->>MercadoPagoAPI: Authorization request
MercadoPagoAPI-->>MercadopagoGateway: Authorization response
MercadopagoGateway-->>Client: {:ok | :error, Response}
Client->>MercadopagoGateway: capture(payment_id, amount, opts)
MercadopagoGateway->>MercadoPagoAPI: Capture request
MercadoPagoAPI-->>MercadopagoGateway: Capture response
MercadopagoGateway-->>Client: {:ok | :error, Response}
Client->>MercadopagoGateway: purchase(amount, card, opts)
MercadopagoGateway->>MercadoPagoAPI: Create card token
MercadoPagoAPI-->>MercadopagoGateway: Card token response
MercadopagoGateway->>MercadoPagoAPI: Purchase request
MercadoPagoAPI-->>MercadopagoGateway: Purchase response
MercadopagoGateway-->>Client: {:ok | :error, Response}
Client->>MercadopagoGateway: void(payment_id, opts)
MercadopagoGateway->>MercadoPagoAPI: Void request
MercadoPagoAPI-->>MercadopagoGateway: Void response
MercadopagoGateway-->>Client: {:ok | :error, Response}
Client->>MercadopagoGateway: refund(amount, payment_id, opts)
MercadopagoGateway->>MercadoPagoAPI: Refund request
MercadoPagoAPI-->>MercadopagoGateway: Refund response
MercadopagoGateway-->>Client: {:ok | :error, Response}
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
test/gateways/mercadopago_test.exs (1)
225-233: Update after rename of network‑error helperIf you accept the earlier typo fix, remember to update the mock call here:
- MockResponse.netwok_error_non_existent_domain() + MockResponse.network_error_non_existent_domain()Otherwise the test will raise
undefined function.🧰 Tools
🪛 GitHub Check: test
[failure] 225-225:
test network error type non existent domain (Gringotts.Gateways.MercadopagoTest)
🧹 Nitpick comments (9)
lib/gringotts/gateways/mercadopago.ex (4)
84-90: Remove duplicate alias & consolidate imports
CreditCardis aliased twice (84and90). The second alias supersedes the first and also introducesMoney&Response, so the first alias is redundant.-alias Gringotts.CreditCard
140-141: Unused pattern binds pollute compile output
currencyandexponentare never referenced, triggering warnings in CI. Either prefix them with_or match only the value you need:-{currency, value, exponent} = Money.to_integer(amount) +{_currency, value, _exponent} = Money.to_integer(amount)Repeat the same change in
capture/3,purchase/3andrefund/3.Also applies to: 174-175, 196-197, 243-244
🧰 Tools
🪛 GitHub Actions: Elixir CI
[warning] 140-140: Variables 'currency' and 'exponent' are unused; prefix with underscore if intentional.
316-329: Dead code:customer_params/3is unusedThe helper never gets called and is warning‑listed by Dialyzer.
If you intend to support stored customers, integrate it in
authorize_params/5; otherwise delete to reduce noise.🧰 Tools
🪛 GitHub Actions: Elixir CI
[warning] 316-316: Function 'customer_params/3' is unused.
148-151: Missing JSON library dependency
Poison.encode!/1andPoison.decode!/1raise at runtime unless
{:poison, "~> 5.0"}(or similar) is added tomix.exs. The
ecosystem has generally moved toJason, which is faster and comes with
Elixir ≥ 1.11.If possible, switch to Jason:
- |> Poison.encode!() + |> Jason.encode!()and likewise in
respond/2.Also applies to: 351-353
test/support/mocks/mercadopago_mock.ex (1)
223-225: Typo in function name
netwok_error_non_existent_domain/0is misspelled (“netwok”). The test
suite imports it correctly but the typo hampers discoverability.-def netwok_error_non_existent_domain do +def network_error_non_existent_domain doRemember to update the call‑site in
test/gateways/mercadopago_test.exs.test/gateways/mercadopago_test.exs (1)
76-99: Duplicate/unused module attributes create noisy warnings
@opts_store,@opts_store_without_validation,@opts_store_no_profile
and others are defined twice (see lines 57‑74 and 89‑99) yet never used
in this spec module. These duplications clutter the test module and
trip static‑analysis checks.Please remove the duplicates and keep only the fixtures that are
actually referenced by the tests.test/integration/gateways/mercadopago_test.exs (3)
57-66: Fix typo in variable namesThe variable names have a typo: "cutomer" should be "customer". Consistent spelling improves code readability and maintainability.
- @new_cutomer_good_opts [ + @new_customer_good_opts [ order_id: 123_126, config: @config, installments: 1, order_type: "mercadopago" ] - @new_cutomer_bad_opts [ + @new_customer_bad_opts [ config: @bad_config, order_id: 123_127 ]Make sure to update references to these variables throughout the code:
case good do - true -> @new_cutomer_good_opts ++ [email: email] - _ -> @new_cutomer_bad_opts ++ [email: email] + true -> @new_customer_good_opts ++ [email: email] + _ -> @new_customer_bad_opts ++ [email: email] end
169-173: Consider adding an issue ticket for MercadoPago API inconsistencyThe comment documents a discrepancy between expected behavior (401 Unauthorized) and actual API behavior (404 with "invalid token" message). Consider creating an issue ticket to track this API inconsistency for future reference or potential follow-up with MercadoPago.
80-92: Consider adding more validation of response contentThe tests currently validate only the success flag and status code. Consider adding assertions for other important fields in the response, such as transaction IDs, timestamps, or other payment-specific fields to ensure complete response validation.
For example, add assertions like:
# For successful authorize/purchase responses assert response.id != nil assert response.created_at != nil assert response.status == "approved" || response.status == "authorized" # For refund responses assert response.refund_id != nilAlso applies to: 98-110, 112-134, 136-151, 153-216, 218-240
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/gringotts/gateways/mercadopago.ex(1 hunks)test/gateways/mercadopago_test.exs(1 hunks)test/integration/gateways/mercadopago_test.exs(1 hunks)test/support/mocks/mercadopago_mock.ex(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
lib/gringotts/gateways/mercadopago.ex (2)
lib/gringotts/credit_card.ex (1)
full_name(49-52)lib/gringotts/response.ex (1)
error(84-86)
🪛 Gitleaks (8.21.2)
test/integration/gateways/mercadopago_test.exs
17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
21-21: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
test/gateways/mercadopago_test.exs
9-9: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Actions: Elixir CI
lib/gringotts/gateways/mercadopago.ex
[warning] 140-140: Variables 'currency' and 'exponent' are unused; prefix with underscore if intentional.
[warning] 174-174: Variables 'currency' and 'exponent' are unused; prefix with underscore if intentional.
[warning] 196-196: Variables 'currency' and 'exponent' are unused; prefix with underscore if intentional.
[warning] 243-243: Variables 'currency' and 'exponent' are unused; prefix with underscore if intentional.
[warning] 261-261: Variable 'url_params' is unused; prefix with underscore if intentional.
[warning] 316-316: Function 'customer_params/3' is unused.
[warning] 150-351: Poison.encode!/1 and Poison.decode!/1 are undefined (module Poison is not available or yet to be defined).
test/gateways/mercadopago_test.exs
[warning] 106-120: Module attributes '@opts_customer_profile_args', '@opts_customer_profile', '@opts_store_no_profile', '@opts_store_without_validation', '@opts_store', and '@unstore_id' were set but never used.
[error] 190-190: Test failure: refund bad payment params.
🪛 GitHub Check: test
test/gateways/mercadopago_test.exs
[failure] 225-225:
test network error type non existent domain (Gringotts.Gateways.MercadopagoTest)
[failure] 152-152:
test authorize with bad card (Gringotts.Gateways.MercadopagoTest)
[failure] 123-123:
test purchase successful response with right params (Gringotts.Gateways.MercadopagoTest)
[failure] 215-215:
test void with bad transaction id (Gringotts.Gateways.MercadopagoTest)
[failure] 197-197:
test refund debit less than refund amount (Gringotts.Gateways.MercadopagoTest)
[failure] 132-132:
test purchase with bad card (Gringotts.Gateways.MercadopagoTest)
[failure] 163-163:
test capture successful response with right params (Gringotts.Gateways.MercadopagoTest)
[failure] 172-172:
test capture with bad transaction id (Gringotts.Gateways.MercadopagoTest)
[failure] 208-208:
test void successful response with right params (Gringotts.Gateways.MercadopagoTest)
[failure] 190-190:
test refund bad payment params (Gringotts.Gateways.MercadopagoTest)
🔇 Additional comments (4)
test/integration/gateways/mercadopago_test.exs (4)
68-78: Good use of random email generation to avoid test conflictsThe
new_email_optsfunction generates random email addresses, which helps prevent test interference when running tests concurrently or repeatedly. This is a good practice for integration tests.
94-96: Good test organization with setup blockThe setup block is effectively used to prepare dynamic test data for the new customer tests. This helps maintain test isolation and prevents potential conflicts between test runs.
Also applies to: 98-110
153-216: Comprehensive test coverage for purchase operationsThe purchase operation tests thoroughly cover both positive and negative cases for old and new customers, with good and bad cards, and good and bad configuration options. This comprehensive approach helps ensure the gateway's reliability.
119-134: Good testing of error cases with specific error messagesThe tests for invalid payment IDs and excessive amounts properly validate both the HTTP status codes and success status. This ensures that the gateway correctly interprets error responses from the MercadoPago API.
Also applies to: 226-239
| defp token_params(%CreditCard{} = card) do | ||
| %{ | ||
| expirationYear: card.year, | ||
| expirationMonth: card.month, | ||
| cardNumber: card.number, | ||
| securityCode: card.verification_code, | ||
| cardholder: %{name: CreditCard.full_name(card)} | ||
| } | ||
| end |
There was a problem hiding this comment.
Parameter casing does not match Mercado Pago API
The REST docs use snake_case (expiration_year, card_number, etc.).
Sending camelCase keys (expirationYear) may lead to 400‑responses.
Please revisit the payload and adapt the keys (or use Jason.encode!/2
with KeyEncoder if you need automatic conversion).
| @spec commit(atom, String.t(), String.t(), keyword, keyword) :: {:ok | :error, Response.t()} | ||
| defp commit(method, path, body, opts, url_params) do | ||
| headers = [{"content-type", "application/json"}, {"accept", "application/json"}] | ||
| url = "#{@base_url}#{path}" | ||
|
|
||
| res = | ||
| HTTPoison.request( | ||
| method, | ||
| url, | ||
| body, | ||
| headers, | ||
| opts ++ [params: [access_token: opts[:config][:access_token]]] | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
url_params argument is ignored – simplify commit/5
commit/5 receives url_params but rebuilds the list internally, rendering the parameter useless and causing an “unused variable” warning.
Option 1 – actually use the incoming url_params:
- res =
- HTTPoison.request(
- method,
- url,
- body,
- headers,
- opts ++ [params: [access_token: opts[:config][:access_token]]]
- )
+ res =
+ HTTPoison.request(
+ method,
+ url,
+ body,
+ headers,
+ opts ++ [params: url_params]
+ )Option 2 – drop the parameter from all call‑sites and the function signature.
Either way removes the warning and clarifies intent.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Elixir CI
[warning] 261-261: Variable 'url_params' is unused; prefix with underscore if intentional.
| defp create_token(%CreditCard{} = card, opts) do | ||
| url_params = [public_key: opts[:config][:public_key]] | ||
|
|
||
| body = | ||
| card | ||
| |> token_params() | ||
| |> Poison.encode!() | ||
|
|
||
| {state, res} = commit(:post, "/v1/card_tokens/#{opts[:customer_id]}", body, opts, url_params) | ||
|
|
||
| case state do | ||
| :error -> {state, res} | ||
| _ -> {state, res.id} | ||
| end | ||
| end |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
create_token/2 may hit an invalid endpoint for new customers
When opts[:customer_id] is nil (new customer) the path becomes
/v1/card_tokens/nil (double slash), whereas Mercado Pago expects
/v1/card_tokens. Consider:
-path = "/v1/card_tokens/#{opts[:customer_id]}"
+path =
+ if opts[:customer_id],
+ do: "/v1/card_tokens/#{opts[:customer_id]}/cards",
+ else: "/v1/card_tokens"Also, for existing customers Mercado Pago requires
/v1/customers/:id/cards, not /card_tokens/:id. Please verify against
the official docs.
🏁 Script executed:
#!/bin/bash
# Find usages of card_tokens in Mercadopago gateway
rg -n "/v1/card_tokens" -n lib/gringotts/gateways/mercadopago.ex
# Find usages of the customers endpoint in Mercadopago gateway
rg -n "/v1/customers" -n lib/gringotts/gateways/mercadopago.exLength of output: 229
🌐 Web query:
Mercado Pago API endpoints: /v1/card_tokens and /v1/customers/:id/cards – what are their official request paths and intended use for new vs. existing customers?
💡 Result:
Mercado Pago API Endpoints: /v1/card_tokens and /v1/customers/:id/cards
Official Request Paths
- Create Card Token (tokenize card data):
- Endpoint:
POST /v1/card_tokens
- Endpoint:
- Manage Customer Cards (save card to customer):
- Endpoint:
POST /v1/customers/{customer_id}/cards - Other actions (e.g., list or delete):
- List cards:
GET /v1/customers/{customer_id}/cards - Delete a card:
DELETE /v1/customers/{customer_id}/cards/{id}
- List cards:
- Endpoint:
Intended Use: New vs. Existing Customers
| Endpoint | Intended Use | Target User |
|---|---|---|
POST /v1/card_tokens |
Tokenize card information (temporary, secure) | All users (required for both new and existing customer flows) |
POST /v1/customers/{id}/cards |
Save/tokenize a new card to a customer profile | Existing customer (customer previously created with a Customer ID) |
Detailed Flow and Use Cases
For New Customers
- Tokenize Card Data:
- Use
POST /v1/card_tokensto securely tokenize the raw card information entered by the user.
- Use
- Create Customer:
- Create a customer profile via
POST /v1/customerswith at least an email address.
- Create a customer profile via
- Attach Card to Customer:
- Use
POST /v1/customers/{customer_id}/cardswith the card token from step 1 to add the card to the new customer profile. - This securely saves the card for future transactions linked to this customer6.
- Use
For Existing Customers
-
Tokenize New Card:
- Use
POST /v1/card_tokensto get a token for the new card details.
- Use
-
Add Card to Existing Customer:
- Use
POST /v1/customers/{customer_id}/cardswith the token to save the card to the existing customer’s account10.
- Use
-
List Saved Cards (optional):
- Retrieve all saved cards for a customer via
GET /v1/customers/{customer_id}/cardswhen you need to show available cards for payment7.
- Retrieve all saved cards for a customer via
Summary Table
| Step | New Customer | Existing Customer |
|---|---|---|
| Tokenize card | /v1/card_tokens |
/v1/card_tokens |
| Create Customer record | /v1/customers |
Already exists |
| Attach card to profile | /v1/customers/{customer_id}/cards |
/v1/customers/{customer_id}/cards |
Key Points
- /v1/card_tokens is always used to securely handle raw card data and create a short-lived card token, which must be used immediately in a payment or to save the card[6][9].
- /v1/customers/:id/cards is for saving a card to a specific customer profile, enabling payments with saved cards (one-click, recurring, etc.)610.
- For one-off payments without customer registration, only /v1/card_tokens and the payment endpoint are used.
References:
Citations:
- 1: https://www.mercadopago.com.ar/developers/en/reference
- 2: https://www.mercadopago.com.ar/developers/en/docs
- 3: https://www.mercadopago.com.ar/developers/en/reference/integrations_api/_point_integration-api_devices/get
- 4: https://www.mercadopago.com.co/developers/en/reference/payments/_payments/post
- 5: https://www.mercadopago.com.co/developers/en/docs/checkout-api/integration-configuration/other-payment-methods
- 6: https://www.mercadopago.com.br/developers/en/docs/checkout-api/cards-and-customers-management/create-customer-and-card
- 7: https://www.mercadopago.cl/developers/en/docs/checkout-api/cards-and-customers-management/receive-payments-with-saved-cards
- 8: https://www.mercadopago.com.ar/developers/en/reference/payments/_payments_search/get
- 9: https://www.mercadopago.com.co/developers/en/docs/checkout-api/integration-configuration/card/integrate-via-core-methods
- 10: https://www.mercadopago.cl/developers/en/docs/checkout-api/cards-and-customers-management/add-new-cards-to-customer
Fix endpoint usage in create_token/2
The current implementation always POSTs to /v1/card_tokens/#{opts[:customer_id]}, which is invalid for both new and existing customers. According to the Mercado Pago docs:
- New customers should use
POST /v1/card_tokensto tokenize the card. - Existing customers should first tokenize via
POST /v1/card_tokens, then attach the token withPOST /v1/customers/{customer_id}/cards.
Please update lib/gringotts/gateways/mercadopago.ex (lines 287–301) to something like:
defp create_token(%CreditCard{} = card, opts) do
url_params = [public_key: opts[:config][:public_key]]
body = card |> token_params() |> Poison.encode!()
- {state, res} = commit(:post, "/v1/card_tokens/#{opts[:customer_id]}", body, opts, url_params)
- case state do
- :error -> {state, res}
- _ -> {state, res.id}
- end
+ # Always tokenize first
+ {state, res} = commit(:post, "/v1/card_tokens", body, opts, url_params)
+ if state == :error do
+ {state, res}
+ else
+ token = res.id
+ case opts[:customer_id] do
+ # New customer: return token only
+ nil -> {:ok, token}
+ # Existing customer: attach token to profile
+ customer_id ->
+ attach_body = %{card_token: token} |> Poison.encode!()
+ commit(:post, "/v1/customers/#{customer_id}/cards", attach_body, opts, [])
+ end
+ end
end- Verify that
attach_bodyand parameters match yourcommit/5signature. - Ensure the function returns consistent
{status, resource_id}tuples.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| defp create_token(%CreditCard{} = card, opts) do | |
| url_params = [public_key: opts[:config][:public_key]] | |
| body = | |
| card | |
| |> token_params() | |
| |> Poison.encode!() | |
| {state, res} = commit(:post, "/v1/card_tokens/#{opts[:customer_id]}", body, opts, url_params) | |
| case state do | |
| :error -> {state, res} | |
| _ -> {state, res.id} | |
| end | |
| end | |
| defp create_token(%CreditCard{} = card, opts) do | |
| url_params = [public_key: opts[:config][:public_key]] | |
| body = | |
| card | |
| |> token_params() | |
| |> Poison.encode!() | |
| # Always tokenize first | |
| {state, res} = commit(:post, "/v1/card_tokens", body, opts, url_params) | |
| if state == :error do | |
| {state, res} | |
| else | |
| token = res.id | |
| case opts[:customer_id] do | |
| # New customer: return token only | |
| nil -> {:ok, token} | |
| # Existing customer: attach token to profile | |
| customer_id -> | |
| attach_body = %{card_token: token} |> Poison.encode!() | |
| commit(:post, "/v1/customers/#{customer_id}/cards", attach_body, opts, []) | |
| end | |
| end | |
| end |
| def bad_amount_purchase_response do | ||
| {:error, | ||
| %HTTPoison.Response{ | ||
| body: | ||
| "{\"id\": 20359978, \"date_created\": \"2019-07-10T10:47:58.000-04:00\", \"date_approved\": \"2019-07-10T10:47:58.000-04:00\", \"date_last_updated\": \"2019-07-10T10:47:58.000-04:00\", \"date_of_expiration\": null, \"money_release_date\": \"2019-07-24T10:47:58.000-04:00\", \"operation_type\": \"regular_payment\", \"issuer_id\": \"25\", \"payment_method_id\": \"visa\", \"payment_type_id\": \"credit_card\", \"status\": \"approved\"}", | ||
| headers: [ | ||
| {"Content-Type", "application/json"}, | ||
| {"Access-Control-Allow-Origin", "*"}, | ||
| {"Access-Control-Allow-Methods", "PUT,OPTIONS,POST,GET"}, | ||
| {"Date", "Mon, 25 Dec 2017 14:17:56 GMT"} | ||
| ], | ||
| request_url: "#{@base_url}/v1/payments", | ||
| status_code: 200 | ||
| }} | ||
| end |
There was a problem hiding this comment.
Error tuples must wrap HTTPoison.Error, not HTTPoison.Response
Gringotts.Gateways.Mercadopago.respond/2 pattern‑matches on
{:error, %HTTPoison.Error{…}}. Returning a %Response{} under
:error will crash with a function‑clause mismatch.
Change the helpers that currently do this (bad_amount_purchase_response/0,
bad_amount_authorize_response/0, etc.):
-{:error, %HTTPoison.Response{ … }}
+{:error, %HTTPoison.Error{reason: "bad amount", id: 0}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def bad_amount_purchase_response do | |
| {:error, | |
| %HTTPoison.Response{ | |
| body: | |
| "{\"id\": 20359978, \"date_created\": \"2019-07-10T10:47:58.000-04:00\", \"date_approved\": \"2019-07-10T10:47:58.000-04:00\", \"date_last_updated\": \"2019-07-10T10:47:58.000-04:00\", \"date_of_expiration\": null, \"money_release_date\": \"2019-07-24T10:47:58.000-04:00\", \"operation_type\": \"regular_payment\", \"issuer_id\": \"25\", \"payment_method_id\": \"visa\", \"payment_type_id\": \"credit_card\", \"status\": \"approved\"}", | |
| headers: [ | |
| {"Content-Type", "application/json"}, | |
| {"Access-Control-Allow-Origin", "*"}, | |
| {"Access-Control-Allow-Methods", "PUT,OPTIONS,POST,GET"}, | |
| {"Date", "Mon, 25 Dec 2017 14:17:56 GMT"} | |
| ], | |
| request_url: "#{@base_url}/v1/payments", | |
| status_code: 200 | |
| }} | |
| end | |
| def bad_amount_purchase_response do | |
| {:error, %HTTPoison.Error{reason: "bad amount", id: 0}} | |
| end |
| test "with bad card" do | ||
| with_mock HTTPoison, | ||
| request: fn _method, _url, _body, _headers, _options -> | ||
| MockResponse.bad_card_purchase_response() | ||
| end do | ||
| assert {:error, _response} = Merc.authorize(@amount, @bad_card, @opts) | ||
| end |
There was a problem hiding this comment.
Wrong mock used for “bad card” authorize test
The authorize negative‑path test still calls
bad_card_purchase_response/0, which returns a purchase‑specific error.
Use bad_card_authorize_response/0 to match the scenario:
- MockResponse.bad_card_purchase_response()
+ MockResponse.bad_card_authorize_response()This mismap currently causes the test failure flagged by CI.
🧰 Tools
🪛 GitHub Check: test
[failure] 152-152:
test authorize with bad card (Gringotts.Gateways.MercadopagoTest)
| alias Gringotts.{CreditCard, FakeMoney} | ||
| alias Gringotts.Gateways.Mercadopago, as: Merc |
There was a problem hiding this comment.
FakeMoney alias hides real behaviour & breaks compilation
Gringotts.FakeMoney is not defined in the project, causing compile
errors. Use Money (from ex_money) or create an explicit stub.
-alias Gringotts.{CreditCard, FakeMoney}
+alias Gringotts.CreditCard
+alias Money📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| alias Gringotts.{CreditCard, FakeMoney} | |
| alias Gringotts.Gateways.Mercadopago, as: Merc | |
| alias Gringotts.CreditCard | |
| alias Money | |
| alias Gringotts.Gateways.Mercadopago, as: Merc |
| access_token: "TEST-4543588471539213-040810-f4f850f89480ee1bd56e05a9aa0d6210-543713181", | ||
| public_key: "TEST-4508ea76-c56b-436a-9213-57934dfb2d86" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hardcoding test credentials in source code
The API keys and tokens are hardcoded in the test file. Even though these are test credentials (indicated by the "TEST-" prefix), it's a security best practice to store them in environment variables or a secure configuration system.
- @config [
- access_token: "TEST-4543588471539213-040810-f4f850f89480ee1bd56e05a9aa0d6210-543713181",
- public_key: "TEST-4508ea76-c56b-436a-9213-57934dfb2d86"
- ]
- @bad_config [
- access_token: "TEST-4543588471539213-111111-f4f850f89480ee1bd56e05a9aa0d6210-543713181",
- public_key: "TEST-4508ea76-c56b-436a-9999-57934dfb2d86"
- ]
+ @config [
+ access_token: System.get_env("MERCADOPAGO_TEST_ACCESS_TOKEN", "TEST-4543588471539213-040810-f4f850f89480ee1bd56e05a9aa0d6210-543713181"),
+ public_key: System.get_env("MERCADOPAGO_TEST_PUBLIC_KEY", "TEST-4508ea76-c56b-436a-9213-57934dfb2d86")
+ ]
+ @bad_config [
+ access_token: "TEST-4543588471539213-111111-f4f850f89480ee1bd56e05a9aa0d6210-543713181",
+ public_key: "TEST-4508ea76-c56b-436a-9999-57934dfb2d86"
+ ]Also applies to: 21-22
🧰 Tools
🪛 Gitleaks (8.21.2)
17-17: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Why?
Summary by CodeRabbit