-
Notifications
You must be signed in to change notification settings - Fork 49
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
Vendure Primary Collection Plugin #258
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need codegen for this plugin, so we can remove this file (and add it whenever we need it)
@@ -52,6 +52,7 @@ jobs: | |||
'vendure-plugin-stripe-subscription', | |||
'vendure-plugin-variant-bulk-update', | |||
'vendure-plugin-webhook', | |||
'vendure-plugin-primary-collection', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good one, I always forget this 👌
package.json
Outdated
@@ -98,7 +98,8 @@ | |||
"modify-customer-orders", | |||
"multiserver-db-sessioncache", | |||
"vendure-order-client", | |||
"all-plugins" | |||
"all-plugins", | |||
"vendure-plugin-primary-collection" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can change this to primary-collection
. It's used in commit scopes: E.g. git commit 'feat(primary-collection): initial commit'
. And the vendure-plugin-*
prefix makes it a bit tedious to do every time you commit :-)
expect(product.primaryCollection.name).toBe('Computers'); | ||
}); | ||
|
||
it("Should return 'Computers' as a primary collection for 'Cars'", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make the test data a little more intuitive for this one? 'Computer' as primary collection for a 'Car' sounds like an unexpected answer 😄. (Although these days most cars are sort of computers but...)
@@ -0,0 +1,26 @@ | |||
{ | |||
"name": "@pinelab/vendure-plugin-primary-collection", | |||
"version": "0.0.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make this 1.0.0 if you'd like. It looks finished enough 🦾
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add the changelog for v1.0.0 + date? There are examples in the other plugins
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super clean, nothing to add but a few tiny remarks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work as always, thanks!
Description
Extend
Vendure
'sshop-api
by addingprimaryCollection
field toProduct
gql type
. The primary collection of a product is the collection that is the highest placed collection inVendure
(Collection's are sortable inVendure
, and it's a good practice to sort by importance). To construct breadcrumbs and URL's it's useful to have a primary collection for each product, in case a product is part of multiple collections.Breaking changes
This PR won't introduce any breaking changes
Checklist
📌 Always:
⚡ Most of the time:
📦 For publishable packages:
package.json
to the next patch/minor/major?