Fix get request with query

[derom]

WHY are these changes introduced?

Fixes #134

WHAT is this pull request doing?

Added test to catch the described error. Made limit optional, because it has default value (50)

Type of change

• Patch: Bug (non-breaking change which fixes an issue)
• Minor: New feature (non-breaking change which adds functionality)
• Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• I have added a changelog entry, prefixed by the type of change noted above
• I have added/updated tests for this change
• I have documented new APIs/updated the documentation for modified APIs (for public APIs)

[thecodepixi]

This makes sense to me. We should definitely be able to make REST requests without passing a limit, since the API will just apply its default limit to the request. Thanks for catching that.

[derom]

Thanks for review @thecodepixi
I've pushed some changes. limit in PageInfo is non-optional again. I realized that the limit is always known and we should just set the default value if a user didn't specify the limit. Also, unused type PageInfoParams was removed

Too many additional changes. Will re-review.

[thecodepixi]

Taking another look at this now, and I'm not sure we can entirely do away with `PageInfo, but have asked for additional opinions on that front.

[paulomarg]

I agree, it's a bit strange that it doesn't cause any tests to break, but we're definitely still using that type in the REST client class.

[derom]

@thecodepixi @paulomarg PageInfoParams was replaced by PageInfo in this commit 53b5e07
I didn't find any usages of that interface https://github.com/Shopify/shopify-node-api/search?q=PageInfoParams

[thecodepixi]

Ah OK, maybe that was my misunderstanding. You're proposing just removing PageInfoParams but not PageInfo?

[derom]

Yes, only PageInfoParams

[thecodepixi]

Yes, only PageInfoParams

Oh then yeah, if that's no longer needed I would think it's fine to remove.

[thecodepixi]

I think this looks pretty great at this point! Would love an additional review from @paulomarg but I give this a 👍

[paulomarg]

I like it too! 🚀

[derom]

Should I do anything else to merge this pr?

[thecodepixi]

Should I do anything else to merge this pr?

I gotchu!