producthunt / producthunt-api

Product Hunt API
311 stars 28 forks source link

comments#index ordering does not work when per_page is set #35

Closed mikejarema closed 9 years ago

mikejarema commented 9 years ago

Here's an error case on a post with 4 comments. Listing the comments, 1 per page, ascending yields the same result as listing the comments, 1 per page, descending.

As per the API docs, I believe I'm using per_page and order correctly.

Here we are showing comments on a post, 1 per page, in descending order (correct result, shows most recent first):

$ curl -H "Authorization: Bearer MYTOKEN" "https://api.producthunt.com/v1/posts/3372/comments?per_page=1&order=desc"

{
    "comments": [
        {
            "id": 52080,
            "body": "Quick note -- http://namevine.com now supports username lookups on Instagram, Wordpress, Tumblr, Blogger and Github. \r\n\r\nAre there any additional social platforms that matter when setting up your startup's presence online?",
           ...
        }
    ]
}

Next we are showing comments on the same post, 1 per page, in ascending order (incorrect result, should show oldest first):

$ curl -H "Authorization: Bearer MYTOKEN" "https://api.producthunt.com/v1/posts/3372/comments?per_page=1&order=asc"

{
    "comments": [
        {
            "id": 52080,
            "body": "Quick note -- http://namevine.com now supports username lookups on Instagram, Wordpress, Tumblr, Blogger and Github. \r\n\r\nAre there any additional social platforms that matter when setting up your startup's presence online?",
           ...
        }
    ]
}

I'm trying to build out a ruby gem for the official PH API, hunting_season and hit this bug in my test suite.

mscoutermarsh commented 9 years ago

@mikejarema thanks! Looking into it.

mikejarema commented 9 years ago

@mscoutermarsh great -- did you manage to reproduce?

mscoutermarsh commented 9 years ago

@mikejarema not yet, will update when we do. :+1:

mscoutermarsh commented 9 years ago

hey @mikejarema. I was able to reproduce. I'll let you know when a fix is deployed.

mscoutermarsh commented 9 years ago

@mikejarema fix is deployed. Could you confirm all good for you?

mikejarema commented 9 years ago

The calls I referenced in this issue have been resolved, but I'm now running into issues when adding the older param. I'll post separately as a new issue.

mikejarema commented 9 years ago

Scratch the above comment, all looks good now, thanks for the quick xmas fix!

mscoutermarsh commented 9 years ago

ok phew! good to hear. :)

Thanks for finding that bug. Let me know if you run into anything else. Really appreciate it :smile: