CC-73: Add Great War Support#7
Merged
Merged
Conversation
…aschal/combat-command-game-systems into add-list-submission-support
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces the Great War (4th Edition) game system and refactors list-building schema validation. The refactor replaces game-system-specific 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/battlefront/_shared/_internal/createGetForceDiagramData.ts`:
- Around line 42-43: The deprecated function createGetForceDiagramData uses
unsafe non-null assertions on data.series (references: series and
seriesDisplayName) which can crash if series is undefined; update the function
to check for existence of data.series and series[data.series] before indexing,
and either return a safe fallback object (e.g., with series: undefined and
seriesDisplayName: ''/null) or throw a clear runtime error indicating callers
must migrate to getForceDiagramSeries/getForceDiagramAlignment; ensure you
reference the symbols data.series, series, and series[data.series] when adding
the null checks so the function no longer dereferences undefined.
In `@src/battlefront/_shared/schema/listData.validators.ts`:
- Around line 242-277: The validator uses generic paths like ['formations']
which hide which array item failed; update validateFormation (and the analogous
unit validator) to accept an item index (e.g., formationIndex: number) from the
caller and include it in ctx.addIssue paths (e.g., ['formations',
formationIndex]) so errors point to the exact item; locate calls where
formations/units are iterated (the listData.ts iteration mentioned) and pass the
current index through to validateFormation/validateUnit to ensure all
ctx.addIssue calls include the index.
- Around line 242-277: In validateFormation, the error message for missing
sourceSeriesKey is misleading because sourceSeriesKey can be undefined either
when the sourceForceDiagram key is absent from context.forceDiagrams or when the
force diagram exists but has no series; update the logic to first check whether
context.forceDiagrams hasOwnProperty(sourceData.sourceForceDiagram) (or use the
in operator) and emit the "unrecognized force diagram" zod issue only when the
key is absent, then only attempt to read sourceSeriesKey and perform the era
comparison (and emit the era-mismatch issue) if a series value exists; if the
diagram key exists but series is undefined, skip validation instead of emitting
the unrecognized error.
- Around line 316-362: In validateUnit, change the logic around sourceSeriesKey
so you first detect whether sourceData.sourceForceDiagram is a known key in
context.forceDiagrams (e.g., using the `in` operator or hasOwnProperty) and only
emit "Unit source has an unrecognized force diagram." when that key is missing;
if the key exists but context.forceDiagrams[sourceForceDiagram]?.series is
undefined, skip the era compatibility check (do not emit the unrecognized error)
and only perform the era comparison when a series value is present (i.e., when
sourceSeriesKey is truthy) so that recognized force diagrams lacking series
metadata don't produce the misleading error.
In `@src/battlefront/greatWarV4/schema/gameSystemConfig.test.ts`:
- Line 9: Fix the import formatting by adding a space after the comma between
the two imported symbols; update the import line that currently reads "import {
GameSystemConfig,gameSystemConfig } from './gameSystemConfig';" to "import {
GameSystemConfig, gameSystemConfig } from './gameSystemConfig';" so both
GameSystemConfig and gameSystemConfig are properly separated.
In `@src/battlefront/greatWarV4/schema/gameSystemConfig.ts`:
- Around line 14-26: The schema declares pointsVersion as optional but
superRefine calls isPointsVersionValid which treats a missing value as invalid;
make pointsVersion required by removing z.optional around
createEnumSchema(PointsVersion) so the schema and validator agree (adjusting the
createEnumSchema usage for pointsVersion), ensuring the superRefine check on
isPointsVersionValid and the error path ['dynamicPointsVersion'] behave
consistently with the tests.
- Line 13: The points schema currently uses z.coerce.number() with no
constraints, allowing zero, negative or absurdly large values; update the schema
for the points field in gameSystemConfig (the "points" property) to enforce
valid values by adding validators such as .int().min(1) or .positive() and
(optionally) a sensible .max(...) threshold (e.g. 1_000_000) to prevent
unreasonable inputs; ensure you keep z.coerce.number() as the base so strings
are coerced but append the chosen chain (.int().min(1) or .positive().max(...))
to the points definition.
In `@src/battlefront/greatWarV4/schema/gameSystemConfig.validators.ts`:
- Around line 3-10: The function isPointsVersionValid currently uses an if/else
to return true/false; simplify it to a single return of the boolean expression
by returning !!values.pointsVersion (or Boolean(values.pointsVersion)) directly
in isPointsVersionValid, referencing the GameSystemConfig parameter to preserve
types and behavior.
In `@src/battlefront/greatWarV4/schema/listData.test.ts`:
- Around line 16-26: The test fixture validData declares ForceDiagram.American
and Faction.UnitedStates but uses the British unit constant Unit.GWB101; change
the unit references in the formation (id 'form00') and unit (id 'unit00') to an
American unit constant (e.g., Unit.GWU101 / GWU107 / GWU118) so the formation
and unit entries match the meta (ForceDiagram.American / Faction.UnitedStates)
and keep validData semantically consistent.
In `@src/common/schemas/listDataId.ts`:
- Around line 5-6: The local errorMap option in listDataId.ts is typed as
errorMap?: () => { message: string } which prevents callers from using Zod's
(issue, ctx) parameters; update the type of errorMap to match Zod's signature
(accepting issue and ctx) and import the appropriate Zod types (e.g., ZodIssue
and the error-map/context type from zod) so errorMap?: (issue: ZodIssue, ctx: /*
Zod error map ctx type */) => { message: string } and callers can access
issue/ctx in a type-safe way.
In `@src/common/static/gameSystems.ts`:
- Around line 7-17: Update the inline comment above the GreatWarV4 enum entry to
correct the spelling mistake: change "pervious" to "previous" in the explanatory
block comment that starts before GreatWarV4 = 'great_war_v4' so the comment
reads "previous versions" instead of "pervious versions".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 911ebda0-b3bc-45f8-a356-41118162e5a1
📒 Files selected for processing (51)
src/battlefront/_shared/_fixtures/getIssueMessages.test.tssrc/battlefront/_shared/_fixtures/getIssueMessages.tssrc/battlefront/_shared/_internal/createGetForceDiagramData.tssrc/battlefront/_shared/schema/commandCard.tssrc/battlefront/_shared/schema/formation.tssrc/battlefront/_shared/schema/listData.test.tssrc/battlefront/_shared/schema/listData.tssrc/battlefront/_shared/schema/listData.validators.tssrc/battlefront/_shared/schema/matchResultDetails.test.tssrc/battlefront/_shared/schema/registrationDetails.tssrc/battlefront/_shared/schema/unit.tssrc/battlefront/_shared/static/rankingFactors.tssrc/battlefront/_shared/types.tssrc/battlefront/flamesOfWarV4/index.tssrc/battlefront/flamesOfWarV4/schema/gameSystemConfig.tssrc/battlefront/flamesOfWarV4/schema/listData.test.tssrc/battlefront/flamesOfWarV4/schema/listData.tssrc/battlefront/flamesOfWarV4/schema/registrationDetails.tssrc/battlefront/flamesOfWarV4/static/forceDiagrams.tssrc/battlefront/greatWarV4/index.tssrc/battlefront/greatWarV4/schema/gameSystemConfig.test.tssrc/battlefront/greatWarV4/schema/gameSystemConfig.tssrc/battlefront/greatWarV4/schema/gameSystemConfig.validators.tssrc/battlefront/greatWarV4/schema/listData.test.tssrc/battlefront/greatWarV4/schema/listData.tssrc/battlefront/greatWarV4/schema/matchResultDetails.tssrc/battlefront/greatWarV4/static/alignments.tssrc/battlefront/greatWarV4/static/factions.tssrc/battlefront/greatWarV4/static/forceDiagrams.tssrc/battlefront/greatWarV4/static/missionNames.tssrc/battlefront/greatWarV4/static/missionPackUtils.tssrc/battlefront/greatWarV4/static/missionPackVersions.tssrc/battlefront/greatWarV4/static/pointsVersions.tssrc/battlefront/greatWarV4/static/units.tssrc/battlefront/greatWarV4/types.tssrc/battlefront/teamYankeeV2/index.tssrc/battlefront/teamYankeeV2/schema/listData.test.tssrc/battlefront/teamYankeeV2/schema/listData.tssrc/battlefront/teamYankeeV2/schema/registrationDetails.tssrc/battlefront/teamYankeeV2/static/forceDiagrams.tssrc/battlefront/teamYankeeV2/static/units.tssrc/common/_fixtures/gameSystemFixtures.tssrc/common/_internal/createEnumSchema.test.tssrc/common/_internal/createEnumSchema.tssrc/common/helpers/getGameSystem.tssrc/common/helpers/validateGameSystemConfig.tssrc/common/helpers/validateMatchResultDetails.tssrc/common/schemas/listDataId.tssrc/common/static/gameSystems.tssrc/common/static/tournamentPairingMethods.tssrc/common/types.ts
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.
Resolves CC-73
Summary by CodeRabbit
New Features
Documentation