Skip to content

Major changes (ResponseObject, edited response hash)#22

Open
florrain wants to merge 6 commits into
masterfrom
major_changes
Open

Major changes (ResponseObject, edited response hash)#22
florrain wants to merge 6 commits into
masterfrom
major_changes

Conversation

@florrain
Copy link
Copy Markdown
Collaborator

@florrain florrain commented Dec 3, 2014

@12and1studio as usual 🚢
@evantahler another pull request you're welcome to review && comment ! Also maybe you would have other methods to add in the ResponseObject ? 👍
@dandemeyere we'll use it after a refact in the thredUP app 👓 👍

⚠️ ⚠️ ⚠️ These are major changes which will be part of the next major release, probably v1.0.0.

TODO

  • Global error handler
  • Ability of disabling the calls from the gem configuration. Useful in a Rails staging env for example. (config.enabled= false)
  • The documentation is really not up-to-date. Both wiki and readme need to be updated.
  • Add a CHANGELOG.md + a guide for contributers
  • Catch and beautify throttle errors

Response object

Because our code often repeats the same basic operations on the hash, we'd like to make it more simple. So instead of using the response as a hash, this would force all our users to handle the response as a dev-friendly object. Yes it is.

Examples

Before

  def handle_errors(response)
    raise "Responsys error - #{response[:error][:code]} - #{response[:error][:message]}" if response.is_a?(Hash) && response.has_key?(:status) && response[:status] == "failure"
    response
  end

After

  def embrace_this_new_handle_errors(response)
    raise "Responsys error - #{response.error_desc}"  if response.error?
    response
  end

Methods on the ResponseObject thay may be interesting:

  • success?
  • error?
  • error_code
  • error_message
  • error_desc
  • data
  • response_hash

Response hash

Also because it was the best occasion to do it, previously the response hash was:

{ status: "success", data: [{ EMAIL_PERMISSION_STATUS_: "I" }] }

But because it makes more sense, the status is now a boolean:

{ success: true, data: [{ EMAIL_PERMISSION_STATUS_: "I" }] }

Helpers

Are a little refactored.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do you want a return here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

How could I miss that, thanks! Same thing in the next one

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants