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

Return error message for unknown track status #301

Merged

Conversation

neslom
Copy link
Contributor

@neslom neslom commented Mar 21, 2016

Fixes #298

This is my first attempt at a contribution here, so I hope I didn't miss anything too obvious.. 😄

if _, err := c.List(trackID); err != nil {
return nil, err
}

Copy link
Member

Choose a reason for hiding this comment

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

Do we not need to do anything with the result of the List?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we just need to return an error here if the Track doesn't exist.

I looked into the API a bit and it looks like the /tracks/<trackID>/status endpoint returns the same response for both valid and invalid tracks, so we just need to first check that the track exists and return an error if it doesn't instead of parsing the response.

If there's no error here, then Status looks at the fetched problems and recent problems, so we don't need the List results.

Copy link
Member

Choose a reason for hiding this comment

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

/tracks/:track_id/status is the endpoint for a problem named "status" which we don't have.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I should have been more specific.

This is the endpoint I was referencing:

http://exercism.io/api/v1/tracks/elixir/status?key=<someKey>

Copy link
Member

Choose a reason for hiding this comment

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

OK, I think that the fix is to improve that endpoint so that it says whether or not the track exists. Are you comfortable in Ruby? If not, no worries, I can do this easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fork that repo right now 👍

@kytrinyx
Copy link
Member

The fix to the command looks good, I just have the one question about the change in the API.

@neslom
Copy link
Contributor Author

neslom commented Mar 23, 2016

Hey @kytrinyx, I submitted exercism/exercism#2806 to address this. I can remove the changes I made in the api package now 👍

@neslom neslom force-pushed the return-error-for-unknown-track-status branch from 6c87ab1 to 1ead254 Compare March 23, 2016 06:21
@kytrinyx
Copy link
Member

Oh, cool! Yeah, this is better.

@Tonkpils
Copy link
Contributor

Merging this since exercism/exercism#2806 has been merged

@Tonkpils Tonkpils merged commit 6af8bd5 into exercism:master Mar 29, 2016
@neslom
Copy link
Contributor Author

neslom commented Mar 29, 2016

Thanks @Tonkpils! I hope I can help out again!

Tonkpils added a commit that referenced this pull request Mar 29, 2016
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.

3 participants