fix(api): enum validation messages named no allowed values - #363
Closed
modernitconsultants wants to merge 1 commit into
Closed
fix(api): enum validation messages named no allowed values#363modernitconsultants wants to merge 1 commit into
modernitconsultants wants to merge 1 commit into
Conversation
Post a folio charge with type "beverage" and the API answers
type must be one of the following values:
with nothing after the colon. The caller is told they are wrong and never
told what right looks like.
class-validator's IsEnum is for TypeScript enum objects. It validates with
Object.keys(entity).map(k => entity[k]), which happens to read an array's
values, so the check works; but it builds its message from
Object.entries(entity).filter(([k]) => isNaN(parseInt(k))), which drops every
key of an array because array keys are all numeric. So the allowed-value list
comes out empty. IsIn is the decorator for a list of allowed values, and
interpolates it.
Every enum decorator in the API was affected: 88 across 64 files, covering
accounting, folio, payment, reservation, rate plan, housekeeping, media,
groups, waitlist and the booking-requests package.
Validation behaviour does not change. Both decorators compare the same value
against the same list.
apps/api/src/validation-messages.spec.ts reads class-validator's metadata
registry rather than the source, so it covers the copies PartialType and
OmitType generate, which no grep sees. It asserts a rendered message too, and
that is what caught the first version of the metadata assertion: it filtered
on `type`, which is 'customValidation' for every ValidateBy decorator, so it
matched nothing and passed with an @IsEnum deliberately planted.
|
Maintainer note: we can’t push to your fork from the bot ( git fetch https://github.com/TelivityAI/haip.git cursor/enum-validation-booking-requests-guard-4a4f
git push https://github.com/modernit-com-au/haip.git FETCH_HEAD:enum-validation-messagesThat lands one hygiene commit on this PR (booking-requests DTOs in the enum-message guardrail). Optional — #363 is already mergeable without it. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // contributes no metadata, which would make this test pass by not looking. | ||
| // `!**/*.spec.ts` matters: one DTO directory holds its own spec, and pulling | ||
| // it in here would run that file's `describe` blocks inside this one. | ||
| const dtoModules = import.meta.glob(['./modules/**/dto/**/*.ts', '!**/*.spec.ts'], { eager: true }); |
There was a problem hiding this comment.
Maintainer hygiene (optional — not blocking merge): the enum-message guardrail only eager-loads apps/api/src/modules/**/dto/**, so a future @IsEnum([...]) in packages/booking-requests wouldn’t trip it.
Suggested fix already written + green locally on cursor/enum-validation-booking-requests-guard-4a4f (commit 1cc3595). Happy for you to ignore or cherry-pick:
git fetch https://github.com/TelivityAI/haip.git cursor/enum-validation-booking-requests-guard-4a4f
git cherry-pick 1cc3595
git pushOr a maintainer with fork write can fast-forward this branch to that tip. Still 4 test cases — just broader coverage.
telivity-otaip
added a commit
that referenced
this pull request
Sep 9, 2026
Maintainer hygiene on #363: the metadata registry check only loaded apps/api modules/**/dto, so a future @IsEnum([...]) in packages/booking-requests could regress unnoticed. Eager-load those HTTP DTOs too and assert ListBookingRequestsDto registers. Co-authored-by: telivity-otaip <telivity-otaip@users.noreply.github.com>
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
1 participant
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.
The bug
Post a folio charge with
type: "beverage"and the API answers:Nothing after the colon. The caller is told they are wrong and never told what right looks like.
Why
@IsEnum(['room','tax',…])reads correctly and validates correctly. class-validator'sIsEnumis for TypeScript enum objects:Object.keys(entity).map(k => entity[k]), which happens to read an array's values, so the check worksObject.entries(entity).filter(([k]) => isNaN(parseInt(k))), which drops every key of an array, because array keys are all numericSo the allowed-value list comes out empty.
@IsIn([...])is class-validator's decorator for a list of allowed values, and interpolates it.Scope
88 decorators across 64 files — every enum-shaped 400 the API returns. Accounting, folio, payment, reservation, rate plan, housekeeping, media, groups, waitlist, and the booking-requests package.
Validation behaviour does not change: both decorators compare the same value against the same list. The diff is 88 one-word substitutions plus the import specifier, and no logic.
The test
apps/api/src/validation-messages.spec.tsreads class-validator's metadata registry rather than the source. A grep only finds the spelling it was told to look for; the registry holds what actually runs, including the copiesPartialTypeandOmitTypegenerate.It also asserts a rendered message, and that is what saved it. The first version of the metadata assertion filtered on
m.type === 'isEnum'— buttypeis'customValidation'for every decorator built onValidateBy, so it matched nothing and went green with an@IsEnumdeliberately planted. Both arms were then re-checked against that planted decorator and both name the offending site.Checks run
pnpm lintandpnpm typecheckare clean (0 errors). The new spec passes.The README/test-stats counts were derived, not measured, and I want to flag that plainly. This machine has no Postgres, Redis or Docker, so
pnpm testcannot run here andsync-test-count.mjscannot produce a count. The change adds one spec file with four cases and modifies no existing spec, so 2245/268 → 2249/269. I applied that with the script's own exportedapplyCountsandbuildStatsDocumentso the bytes match what a real run writes, andisReadmeOutOfDateagrees. If yourTestjob disagrees, that delta is the thing to look at.Aside, not fixed here
channex-review.mapper.spec.tsfails on any machine east of about UTC+7. Its fixturereceived_at: '2024-08-05T07:35:21.000000'carries no timezone, sonew Date()reads it as local andstayDatelands a day early. Green in CI because CI runs UTC. Happy to send a separate PR if you want it.