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

added a second parameter to the logic that checks the error that is retu... #28

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

germs12
Copy link

@germs12 germs12 commented Apr 28, 2015

...rned from responsys

This was causing some errors and @HannahLehman worked HOURS to try and resolve it. A little digging later we discovered the response was checking for the wrong value. Not wanting to break any existing functionality, I added a second check on the new value being returned.

👍

!(response[:status] == "failure" && (response[:error][:code] == "RECORD_NOT_FOUND" || response[:error][:code] == "LIST_NOT_FOUND"))
end

def list_exists?(list)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wouldn't let this method in the code if it's not used.

@florrain
Copy link
Collaborator

Hey!

Correct, that's a case that was not covered. I'm curious what was the error you had before this fix?

Added a few comments, I'll give another look after you go over them.

Also if you could add tests that would be awesome!

Thanks

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