Feat: Upgrade to V2 #6
Conversation
| @@ -1,6 +1,6 @@ | |||
|
|
|||
| generates: | |||
| ./src/generated/graphql-shop-api-types.ts: | |||
| ./src/ui/generated/graphql-shop-api-types.ts: | |||
There was a problem hiding this comment.
Place generated inside ui dir, because Admin UI Angular compilation can't access anything outside the ui directory
| @@ -19,25 +19,25 @@ | |||
| "build": "rimraf dist && tsc && copyfiles -u 1 'src/ui/**/*' dist/src/ && copyfiles -u 1 'generated/*' dist/src/generated", | |||
| "generate": "graphql-codegen", | |||
| "start": "ts-node test/dev-server.ts", | |||
| "test": "jest --preset=\"ts-jest\" --forceExit" | |||
| "test": "vitest run" | |||
There was a problem hiding this comment.
You can run yarn vitest to run Vitest in watch mode when developing locally 🦾
| */ | ||
| @Injectable() | ||
| @EventSubscriber() | ||
| export class ProductVariantSubscriber implements EntitySubscriberInterface<ProductVariant> { |
There was a problem hiding this comment.
ProductVariantSubscriber has been replaced by eventBus.ofType(StockMovementEvent), because in V2 multiple entities (StockLocations) can be responsible for the final saleableStock
| @@ -47,11 +54,54 @@ export class BackInStockService { | |||
| private productVariantService: ProductVariantService, | |||
| private translatorService: TranslatorService, | |||
| private eventBus: EventBus, | |||
| ) {} | |||
| @Inject(PLUGIN_INIT_OPTIONS) private options: BackInStockOptions | |||
There was a problem hiding this comment.
- BackInStockPlugin.options
+ @Inject(PLUGIN_INIT_OPTIONS) private options: BackInStockOptionsBackInStockPlugin.optionsusually works, but you could end up with weird bootstrap scenarios, where BackInStockPlugin.options is undefined. With @Inject we make NestJS responsible for resolving, which is safer.
src/service/back-in-stock.service.ts
Outdated
| // Listen for stock movements and update subscriptions | ||
| this.eventBus.ofType(StockMovementEvent).subscribe(async event => { | ||
| // Refetch variants, because variants in event does not have all properties fetched from DB | ||
| const variants = await this.productVariantService.findByIds(event.ctx, event.stockMovements.map(sm => sm.productVariant.id)); |
There was a problem hiding this comment.
event.stockMovements[0].productVariant only has a partial variant: {id: 1}, so we refetch the entire variant to be able to resolve saleableStockLevel
| where: { | ||
| id | ||
| }, | ||
| relations: { productVariant: true, channel: true, customer: true }, |
There was a problem hiding this comment.
New way of fetching relations in TypeOrm 0.3.0.
relations: ['productVariant'] is deprecated: https://github.com/typeorm/typeorm/releases/tag/0.3.0
| // SWC required to support decorators used in test plugins | ||
| // See https://github.com/vitest-dev/vitest/issues/708#issuecomment-1118628479 | ||
| // Vite plugin | ||
| swc.vite({ |
There was a problem hiding this comment.
Use swc in vitest instead of esbuild because of some Typescript decorator transpilation thing: nestjs/nest#9228
Didn't go into details to find out, but I saw that Vendure/Michael is also using it this way in @vendure/core.
|
LGTM) thank you! v2 🎉 |
Upgrade to support Vendure V2.
BREAKING CHANGE
Previous versions of this plugin only sent emails when stock went from <1 to >1, but the plugin now also sends an email when a customer subscribes to a variant that already has stock and the stock is being updated to anything above 1.
This is because previous versions listened for changes in the DB entity ProductVariant, where we could get the stock level before update and after update, thus enabling use to determine when stock went from <1 to >1.
Vendure V2 introduces the concept of stock locations and a custom StockLocationStrategy.
With this new change, the only way to correctly determine a variant's stock level, is to use
stockLevelService.getAvailableStockLevel(), but that doesn't give us the stock level before update. Hence, now the plugin sends an email on stock changes that change the stock to anything >1.This is IMO an acceptable change, considering this is the only scenario that changed:
This is technically correct, because the variant is in stock, the only thing is that it was already in stock.
Please let me know what you think of this solution.