fix(core,elasticsearch-plugin): Include channelId in productInStock cache key - #4214
Conversation
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThis PR scopes product/variant in-stock caching to the current channel by adding Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ac0df9c to
a2f6eac
Compare
abb49bb to
46b2bf9
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/plugin/default-search-plugin/indexer/indexer.controller.ts (1)
477-493:⚠️ Potential issue | 🟡 MinorInner query fetches all product variants regardless of channel.
The query at lines 479–485 retrieves variants by
productIdwithout filtering by the current channel. This meansproductInStockmay reflect stock from variants not assigned toctx.channel, even though the cache key is scoped per-channel. SincegetSaleableStockLevel(ctx, v)only checks stock by variant ID and doesn't validate channel membership, variants outside the current channel are included in the result. This is pre-existing behavior, but verify in your e2e tests that the product stock status reflects expected results for each channel independently.
🧹 Nitpick comments (1)
packages/elasticsearch-plugin/e2e/elasticsearch-plugin.e2e-spec.ts (1)
1570-1644: Test setup is well-structured and correctly models the multi-channel stock scenario.One minor note:
adminClientis left withSTOCK_CHANNEL_TOKENafterbeforeAll(line 1641). Since the test cases only useshopClient, this doesn't affect the current tests. However, if additional tests are appended after thisdescribeblock in the future, the leaked admin channel token could cause confusing failures. Consider resetting it at the end ofbeforeAllor adding anafterAll.♻️ Optional: reset adminClient token at end of beforeAll
// Reindex the second channel adminClient.setChannelToken(STOCK_CHANNEL_TOKEN); await adminClient.query<Codegen.ReindexMutation>(REINDEX); await awaitRunningJobs(adminClient); + + // Reset to default channel token to avoid leaking state + adminClient.setChannelToken(E2E_DEFAULT_CHANNEL_TOKEN); });
|
fixing the test so that it resets the adminClient |
…e leakage in e2e tests
Description
The
getProductInStockValue()/productVariantsStockcache inindexer.controller.tsused a key that didn't include the channel ID. In multi-channel setups, this caused staleproductInStockvalues from one channel to leak into another during reindexing — products appeared "in stock" in channels where they had no stock.Fixed by including
ctx.channelIdin the cache key in both:@vendure/elasticsearch-plugin(elastic-index-product-in-stock-{channelId}-...)@vendure/coredefault-search-plugin (productVariantsStock-{channelId}-...)Fixes vendurehq/community-plugins#1
Breaking changes
None.
Testing
Automated
E2e tests added for both plugins:
elasticsearch-plugin.e2e-spec.ts(multi-channel productInStock cache):inStockin Channel 1inStock: truein Channel 2inStock: falsein Channel 2default-search-plugin.e2e-spec.ts(multi-channel productInStock cache):All tests confirmed to fail without the fix and pass with it.
Running the tests
Manual local testing
Prerequisites
docker-compose up -d postgres_16)docker-compose up -d elasticsearch)Setup
packages/dev-server/dev-config.ts, enable one of:DefaultSearchPlugin.init({ bufferUpdates: false, indexStockStatus: true })ElasticsearchPlugin.init({ ... })cd packages/dev-server DB=postgres npm run populate DB=postgres npm run devScreenshots
Default Channel
Channel 2
Checklist
📌 Always:
👍 Most of the time: