Add support Collection#products endpoint #609
Conversation
siddhantbajaj
force-pushed
the
collection-products
branch
from
6e58db9 to
da88994
Compare
|
Hey @siddhantbajaj I think this will have to wait until we have merged #600 so that we can appropriately manage the version which will support this. I will get back to you soon. |
siddhantbajaj
force-pushed
the
collection-products
branch
2 times, most recently
from
58552d3 to
e8152dc
Compare
|
Currently blocked by the ^^ PR, but I can use some code reviews till then. :-) |
|
I think you also need to add an api-test. Will this be a new entry in the API docs? It's neither a method in SmartCollections, neither in CustomCollections so you might need to add it. |
|
Yes, I already have a PR up for it - https://github.com/Shopify/shopify/pull/213687. Just added you for the review there as well. |
siddhantbajaj
force-pushed
the
collection-products
branch
from
e8152dc to
c95ac3c
Compare
| extension: false | ||
| ) | ||
|
|
||
| assert_raises NotImplementedError do |
There was a problem hiding this comment.
Weird.. I would have expected this to be a 404.
There was a problem hiding this comment.
Don't really have a strong opinion here, just something we are following in other parts of the project as well, where version is incorrect.
test/collection_test.rb
Outdated
|
|
||
| collection = ShopifyAPI::Collection.find(1) | ||
|
|
||
| fake( |
There was a problem hiding this comment.
Does this need to be here if the call is going to fail?
There was a problem hiding this comment.
Yes, we can remove this. 👍
|
|
||
| module ShopifyAPI | ||
| class Collection < Base | ||
| include Events |
There was a problem hiding this comment.
do we need to include Events?
There was a problem hiding this comment.
I think yes, we do need to keep support for Events. We will also have to create PR in Shopify/shopify to add support for Collection.events.
We are introducing a new endpoint which allow merchants to query for products for a particular collection without knowing its type.This PR adds support for this new endpoint in the shopify api gem.
NOTE: We currently have this change behind
unstable. I will create another PR to replaceunstablewith specific version these changes will go in.What should reviewers focus on?
ShopifyAPI::Collection.find(2).productsthis should return the products of the collection.Note: This PR is safe to rollback as the new endpoint is behind a version change which will probably be introduced in
2019/10version.