Repository navigation
Conversation
`runMigrations()` returns an empty array both when every migration has already been applied and when the configured `migrations` globs matched no files. The CLI derives its report from that array alone, so the second case is presented as `No pending migrations found` and `vendure migrate --run` exits 0 having applied nothing. Reporting it directly does not work: `log()` is a no-op whenever `VENDURE_RUNNING_IN_CLI` is set, which is why the condition is invisible today. Throwing would break the `create` scaffold, which configures a migrations glob before any migration file exists. So the condition is handed to the caller through a new `RunMigrationsOptions.onDiagnostic` callback. Zero loaded migration classes on its own is not evidence of a problem, it is also what a project looks like before its first migration is authored. The discriminator is the `migrations` table: zero classes loaded while the database has migrations on record can only mean the patterns stopped matching. That is also the dangerous case, a deploy pointed at `dist/migrations/*.js` that was never built. The schema-drift warning goes through the same callback. It was already computed on every `vendure migrate --run` and then discarded through the suppressed `log()`, and it is the check the issue actually asks for. The callback receives data rather than a pre-formatted string, both CLI entry points build their report from one function, and a warning no longer renders with a green success glyph. Based on the diagnosis and the callback seam from vendurehq#5039 by @alex-hahn. Relates to vendurehq#5001
`addRegisterCustomerCustomFieldsInput()` adds a `customFields` field to `RegisterCustomerInput` whenever the Customer entity has public, writable custom fields, so those fields can be set during registration. `CustomFieldProcessingInterceptor` never processed them. It builds its set of recognised inputs as `Create<Entity>Input` / `Update<Entity>Input`, and `RegisterCustomerInput` matches neither, so a `validate` function on such a field runs on `updateCustomer` but is silently skipped on `registerCustomerAccount`. A rejected value produced an account that was created successfully and carried an invalid custom field, with nothing reported. Adds a small map of inputs that carry custom fields without following that naming convention, currently just `RegisterCustomerInput` -> `Customer`, and routes it through the same validation and default-application path. Registration creates a Customer, so it is treated as a create input and defaults apply as they do elsewhere. `validateInput()` also had its own copy of the entity-name derivation as an inline regex; it now calls `getEntityNameFromInputType()` so there is one implementation. The two are equivalent for every existing input (`CreateProductInput` -> `Product`, `OrderLineCustomFieldsInput` unchanged). The existing `OrderLineCustomFieldsInput` special case is deliberately left as it is, to keep the diff additive. Note this is a behaviour change: a `validate` function that previously only ran on update will now also reject invalid values at registration. That is the intent of the fix, but it may surface validators that were never exercised on this path before.
Co-authored-by: Cursor <cursoragent@cursor.com>
Address review feedback on vendurehq#5174: - Only report `schema-out-of-sync` when the database has migration history. Against a database with nothing applied the schema builder returns the entire schema as pending, so a first `vendure migrate --run` on a freshly created project printed every CREATE TABLE as a warning where it previously printed one success line. - Cap the rendered drift queries at 10. The queries themselves stay on the diagnostic for programmatic consumers. - Only point at the working directory when a configured pattern is relative. The scaffolded config uses `path.join(__dirname, ...)`, where the cwd does not explain the failure. - Correct the `onDiagnostic` doc comment: suppression comes from `VENDURE_RUNNING_IN_CLI`, and passing the callback does not change it. - Use the singular in the CLI summary for a single migration. Relates to vendurehq#5001 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013bLmE6tqF198Z7TxHPy52e
A Promotion whose conditions pass but whose actions produce no adjustment anywhere is still attached to the Order, and PromotionService counts usageLimit / perCustomerUsageLimit from exactly those join rows. A customer who received nothing from a coupon still burns their allowed use. OrderCalculator now drops a Promotion from order.promotions at the end of applyPriceAdjustments unless it adjusted an OrderLine or a ShippingLine. Promotions whose actions define onActivate / onDeactivate are kept: their benefit is not expressed as an adjustment amount, and on the first pass the side effect has not run yet, so there is nothing to discount. Coupon codes are unaffected - a code the customer typed stays on the Order even when it discounts nothing.
The DefaultSearchPlugin search strategies treated `input.sort` as an
explicit sort request whenever it was truthy. Clients generated with
graphql-codegen commonly send `sort: {}` when the user has not selected
a sort order, and an empty object is truthy, so the sort branch was
taken, no ORDER BY field was added from it, and the `score DESC`
ordering was skipped entirely.
The effect was that term searches fell back to the deterministic
`productVariantId ASC` tiebreaker, so the relevance score, which is
still computed and returned in the SELECT, had no influence on the
result order. Exact matches could be returned below unrelated items.
`hasSortParameter()` now treats a sort object with no defined fields as
absent, so relevance ordering applies. Behaviour is unchanged when at
least one sort field is set. All three strategies (Postgres, MySQL /
MariaDB, SQLite / sql.js) shared the defect and are fixed together.
…rrel SubscribableJob is the return type of JobQueue.add() and is handed back by JobQueueService, but the class was not re-exported from packages/core/src/job-queue/index.ts, so plugin code had to import it from '@vendure/core/dist/job-queue/subscribable-job'. The same line also exports JobUpdate and JobUpdateOptions, which its updates() method uses. Six entities that carry an "@docsCategory entities" annotation, and so have a generated page under docs/reference/typescript-api/entities, were missing from packages/core/src/entity/index.ts: Region (region.mdx) ApiKey (api-key.mdx) HistoryEntry (history-entry.mdx) OrderHistoryEntry (order-history-entry.mdx) CustomerHistoryEntry (customer-history-entry.mdx) OrderModification (order-modification.mdx) Each was reachable only through a deep import into dist/, which pins consumers to core's internal file layout. Several are surrounded by siblings the barrel already exports: Country and Province are exported while their base class Region was not, and OrderModificationLine is exported while OrderModification was not. After this change every entity with a published reference page is exported from the barrel, with no exceptions. Undocumented entities are left alone. The five translation entities absent from the barrel (RegionTranslation, PromotionTranslation, PaymentMethodTranslation, ShippingMethodTranslation, ApiKeyTranslation) carry no @docsCategory annotation and have no reference page, and the barrel is already split on them - eight translation entities are exported and five are not, with nothing documenting the line. Settling that split is a separate call for maintainers, not drift to fix here. No symbol is moved, renamed or removed, and none of the added names collides with an existing export.
An empty `migrations` table does not mean an empty database. A schema built with `synchronize: true` has every table and no migration history, because TypeORM only writes that table when it runs a migration. The previous guard treated the two states as the same, so switching to `synchronize: false` and changing an entity produced "No pending migrations found" instead of the ALTER TABLE the developer needs to generate a migration for. The guard now asks whether any entity table exists. Also: - `describeDiagnostic()` no longer claims all patterns resolve against the working directory when the `migrations` config mixes absolute and relative patterns. - `connection.close()` and `resetConfig()` now run even if an `onDiagnostic` callback throws. Relates to vendurehq#5001 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…config Address review feedback on vendurehq#5180. `RegisterCustomerInput` is now registered in `createInputsWithCustomFields` in the constructor, so `hasCustomFields()` and `shouldApplyDefaults()` pick it up unchanged. `getEntityNameFromInputType()` maps it to `Customer`. This replaces the separate `nonStandardInputEntityNames` map, which stated "this is a create input" in a fourth place alongside the two sets and `OrderLineCustomFieldsInput`. The test config moves out of `custom-fields.e2e-spec.ts` into its own spec. Customer there has only the readonly `score` field, which is what makes `writeableNonLocalizedFields` emit the `customFields: JSON` fallback shape on `UpdateCustomerInput`, which in turn is what lets "throws on attempt to update readonly field when no other custom fields defined" reach the interceptor. Adding a writable Customer custom field generates `UpdateCustomerCustomFieldsInput` instead and breaks that test, which is the only coverage of the JSON-fallback readonly path. The new spec also covers what was missing: that a rejected value leaves no Customer behind, that a valid value is persisted on the entity, and that defaults are applied on registration now that it counts as a create input. Relates to vendurehq#5180 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Surcharge.order` used TypeORM's default `orphanedRowAction: 'nullify'`, so saving an Order set `orderId` to null on every persisted Surcharge row missing from that Order's `surcharges` array. Callers hold that array as a snapshot taken when the Order was loaded, so a Surcharge added since - by a blocking event handler, or by a concurrent request - was silently detached: no error, no event, no log. Omitting `surcharges` from the save in `applyPriceAdjustments` closes the path this was reported on, but the same hazard is present in every other full-Order save: `removeItemsFromOrder`, `addCustomerToOrder`, `PromotionService.addPromotionsToOrder`, `OrderModifier.cancelOrder`, and plugin code following the pattern documented on `ActiveOrderStrategy`. Setting `orphanedRowAction: 'disable'` on the relation fixes all of them at once. Nothing relies on the orphan behaviour: surcharges are deleted by `removeSurchargeFromOrder`, not detached. `orphanedRowAction` is applied when TypeORM builds the persistence subjects, so this changes no schema and needs no migration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Address Nigel review of vendurehq#5180. "does not create an account when validation fails" performed no registration of its own, so it only passed because the preceding test had attempted one. Run alone it asserted that an email nobody submitted has no Customer. Folded into the test that attempts the registration. Also guards the `items[0]` access in the defaults test with a `totalItems` assertion, and separates the new constructor comment from the pre-existing `OrderLineCustomFieldsInput` note so they no longer read as one block. Relates to vendurehq#5180 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three changes to the new createFromFileStream test: - Stream real PDF bytes under the .pdf filename. Plain text has no magic-byte signature, so content detection returned nothing and the file passed permission checks on the extension alone. If content detection is ever tightened, the test would fail with a MimeTypeError that looks unrelated to the behaviour it guards. Passing a Buffer also takes the same byte path as the real caller in AssetImporter, instead of the object-mode string path. - Read the Asset back with its channels relation and assert on that too. The returned entity carries the ids assigned in memory before the save, so on its own it cannot show the join row was written. The in-memory assertion stays, because it is what catches a missing ctx.channelId. - Move the ordering note so it covers both trailing describe blocks. Each creates an Asset, which advances the auto-increment id that the total-count and hardcoded-id assertions earlier in the file rely on. Relates to vendurehq#5204 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ils (vendurehq#5173) Co-authored-by: Michael Bromley <michael@michaelbromley.co.uk>
…ield logic (vendurehq#5336) Co-authored-by: Ihor Barmak <ihor@uplab.io>
The two diagnostic shapes carried structured data that nothing acted on: `patterns` and `cwd` decided whether to print one line, `queries` was sliced and prefixed. Both were only ever turned into text. Collapse them into a single `MigrationDiagnostic` carrying a `code` and the rendered `lines`, so the package exports two names rather than five. Adding a field later is easy; removing an exported type is not. Core now renders the wording where it detects the condition, and `describeDiagnostic` is gone. The ten-item cap moves with the rendering to the CLI, so a deploy script calling `runMigrations()` directly gets the full drift list back. `hasAnyEntityTable` loaded every entity table to answer a yes/no question, which `createSchemaBuilder().log()` then did again on its own query runner. `hasTable` on one core table answers it in a single statement. Relates to vendurehq#5001 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
vendurehq#5339) Co-authored-by: Michael Bromley <michael@michaelbromley.co.uk>
The Admin API `ProductVariant.stockLevels` field resolves one channel-filtered query per variant, so listing variants issues one query per row. vendurehq#5224 batched the sibling path behind `getAvailableStock` but left this one alone. `getStockLevelsForVariant` now reads through a per-request DataLoader held on the RequestContext. The batch function runs the same joins and channel filter with `productVariantId IN (...)` and groups the rows by variant, so the resolver output is unchanged. The loader is keyed to the ctx that supplies the channel filter, so one loader per ctx cannot mix channels. `cache: false` for the same reason as vendurehq#5224: batch, but do not memoize, so a write earlier in the request is not masked by a value read before it. Measured on the core e2e fixture, an Admin API page of 34 variants selecting stockLevels: 34 stock_level queries before, 1 after.
Member
Author
|
Superseded: opened against a stale fork master; reopening now that master is synced with upstream. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue (draft for vendurehq/vendure)
Title: Admin API
ProductVariant.stockLevelsissues one channel-filtered query per variantProductVariant.stockLevels(packages/core/src/api/resolvers/entity/product-variant-entity.resolver.ts:205) callsStockLevelService.getStockLevelsForVariant(packages/core/src/service/services/stock-level.service.ts:60), which builds a query builder joiningstockLocation.channelsand filtering onctx.channelId— one query per variant, no batching. vendurehq#5224 removed this stampede for the Shop APIstockLevelfield; the Admin APIstockLevelsfield still has it.Measured on the core e2e fixture:
productVariants(options: { take: 40 })selectingstockLevelsreturned 34 variants and issued 34stock_levelqueries, 48 for the whole request. An Admin UI product-detail page with 40 variants costs 40; an ERP or WMS sync paging 100 variants costs 100 extra queries per page.Expected: a page of variants costs a bounded number of queries.
Actual: it costs one per variant.
Fix (PR body draft for vendurehq/vendure)
getStockLevelsForVariantnow reads through a per-requestDataLoader<ID, StockLevel[]>held inRequestContextCacheService— the sibling of vendurehq#5224.Output is identical: same joins, same channel filter, same selected relations, with
productVariantId IN (:...ids)in place of= :id, then grouped by variant id. Public signature unchanged, no new public API. The loader lives on the ctx that supplies the channel filter, so one loader per ctx cannot mix channels.RequestContextCacheServiceis aWeakMap<RequestContext, Map>, so non-HTTP contexts work unchanged.cache: false, as in vendurehq#5224: batch without memoizing, so a write earlier in the request is not masked.Seven cases added to
stock-level.service.spec.ts: batching, grouping, empty result, relation selection, channel parameter, re-read on a later tick, and no batch shared across contexts or withgetAvailableStock. Stock e2e green (37 + 10 + 18);stock-control-multi-locationalready assertsstockLevelsper variant across a variant list, which is the parity proof.Same 34-variant page: 34 queries → 1.