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

Redesign Response struct #1

Closed
pkrawat1 opened this issue Dec 19, 2017 · 1 comment
Closed

Redesign Response struct #1

pkrawat1 opened this issue Dec 19, 2017 · 1 comment

Comments

@pkrawat1
Copy link
Member

pkrawat1 commented Dec 19, 2017

Current state of Response

We can add arbitrary fields in a Response thanks to Response.new().

Field type remarks
:success bool
:authorization ?? holds the id/token
:status_code string HTTP response code
:error_code string the gateway's response code, used for both error and success codes from gateway
:message string descriptive string from gateway or gringotts
:avs_result tuple of string {address_status, zip_code_status}
:cvc_result string cvc_status
:params ?? raw response

Improvements

  1. [Deprecate] The :success field seems redundant as all API calls return the usual {:ok, Response} or {:error, Response}
  2. [Rename] The token/ID returned for each transaction should go into a key named :id instead of :authorization IMO.
  3. [Addition] We should keep a separate key for a token named :token (:authorization is also fine).
  4. [Format] We should also create a list of error atoms which are uniform for all gateways This will help users to pattern match on these atoms. Curently we have just 2 tags: :ok and :error, but we could,
    • return tuples with different kinds of tags, like :ok, :error, :declined, :invalid, etc.
    • return just :ok and :error tuples, but also add a reason or error_reason field:
    error_reason: {:server_error, "reason"} | {:invalid_format, "reason"} | {:network_error, "reason"}`
    where "reason" is the error msg string returned by the gateway.
  5. [Rename] :params to :raw
    • What should be the format for this? JSON (map), raw string, keyword-list?
    • We could make this a tuple, {:html, String.t}, {:json, Map.t} (already decoded), {:xml, ??}
  6. [Format] :avs_result is a tuple, a map would be better as the keys would be explicit,
    %{address: string, zip_code: string}

TL;DR

New definition:

Field (old) Field (new) type remarks
:success boolean (deprecated) will remove in future release
:authorization id string (renamed) transaction ID
:token string (new) The token obtained when card details are stored for future use
:status_code :http_code integer (new) HTTP response code
:error_code :gateway_code string (re-purposed) result code from gateway "as it is"
:reason {:tag, string} (only for error, nil otherwise) tags like :server_error, :fraud_risk, :avs_invalid
:message string (no change) textual description of result code if any
:avs_result map (converted to map) %{address: string, zip_code: string}
:cvc_result string (no change)
:params :raw {:tag, string} (renamed) html, string}, {:json, map}`
@oyeb
Copy link
Contributor

oyeb commented Jan 15, 2018

@aviabird/all I'm reviving this issue, we've stashed it since way too long.
Let's discuss this here quickly.

@oyeb oyeb changed the title Response module is not being used currently Redesign Response struct Jan 15, 2018
@aviabird aviabird deleted a comment from gopalshimpi Jan 18, 2018
@pkrawat1 pkrawat1 removed this from the Release 1.5.0 milestone Jan 25, 2018
oyeb added a commit that referenced this issue Feb 5, 2018
ashish173 pushed a commit that referenced this issue Apr 22, 2018
* Fixes #1: Introducing `Response.t`

with docs

* [monei] Adapted for new `Response.t`

* Refactored `commit`, `respond` for readability

* [monei] Updated test cases

* Corrected specs

* [bogus] Adapted for Response.t
ashish173 pushed a commit that referenced this issue Apr 22, 2018
* Fixes #1: Introducing `Response.t`

with docs

* [monei] Adapted for new `Response.t`

* Refactored `commit`, `respond` for readability

* [monei] Updated test cases

* Corrected specs

* [bogus] Adapted for Response.t
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants