Skip to content

fix(nestJs): icon is optional#197

Open
GaoNeng-wWw wants to merge 1 commit intoopentiny:devfrom
GaoNeng-wWw:feat/allow-empty-icon
Open

fix(nestJs): icon is optional#197
GaoNeng-wWw wants to merge 1 commit intoopentiny:devfrom
GaoNeng-wWw:feat/allow-empty-icon

Conversation

@GaoNeng-wWw
Copy link
Collaborator

@GaoNeng-wWw GaoNeng-wWw commented Mar 11, 2026

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Summary by CodeRabbit

  • Documentation

    • Added localized type validation error messages in English and Chinese.
  • New Features

    • Enhanced icon field validation with type-checking and corresponding error messaging.

@github-actions github-actions bot added the enhancement New feature or request label Mar 11, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2026

Walkthrough

This PR introduces a new TYPE_ERROR translation message for type validation across English and Chinese localization files, updates the TypeScript type definition, and modifies the CreateMenuDto to validate the icon field as a string type using the new message.

Changes

Cohort / File(s) Summary
I18n Type Definition
template/nestJs/src/.generate/i18n.generated.ts
Added TYPE_ERROR: string field to the validation section of I18nTranslations type.
I18n Translation Keys
template/nestJs/src/i18n/enUS/validation.json, template/nestJs/src/i18n/zhCN/validation.json
Added new TYPE_ERROR localization keys with message templates for English ("{property} should to be {type}") and Chinese ("{property} 应该为 {type}"). Fixed JSON syntax by adding trailing commas to preceding entries.
Menu DTO Validation
template/nestJs/src/menu/dto/create-menu.dto.ts
Updated icon field validation: replaced @IsNotEmpty with @IsString, imported IsString from class-validator, marked field as not required, and configured error message to use new TYPE_ERROR with type parameter set to 'string'.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A new TYPE_ERROR hops into view,
Translations in English and Chinese too,
The icon field now checks its string heart,
With validators playing their part,
A rabbit's delight—all locales aligned! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title accurately describes the main change: making the icon field optional in the NestJS template, which is the primary purpose reflected across all modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@GaoNeng-wWw GaoNeng-wWw changed the title feat(nestJs): icon is optional fix(nestJs): icon is optional Mar 11, 2026
@github-actions github-actions bot added bug Something isn't working and removed enhancement New feature or request labels Mar 11, 2026
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@template/nestJs/src/i18n/enUS/validation.json`:
- Line 5: Update the validation message for the "TYPE_ERROR" key in
validation.json to correct the English; replace the current value "{property}
should to be {type}" with a grammatically correct phrase such as "{property}
should be {type}" (or "{property} must be {type}") so API errors read properly.

In `@template/nestJs/src/menu/dto/create-menu.dto.ts`:
- Line 1: The DTO marks the icon field optional in Swagger but not in
validation/TypeScript, so update the CreateMenuDto (and the other DTO
occurrences noted) by importing and adding the `@IsOptional`() decorator before
`@IsString`(), change the property signature to be optional (icon?: string), and
ensure IsOptional is added to the import list so validation, TypeScript typing,
and Swagger metadata are consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2888c4fd-0541-4663-a004-c4fb1ae20514

📥 Commits

Reviewing files that changed from the base of the PR and between 5f431a7 and e08622e.

📒 Files selected for processing (4)
  • template/nestJs/src/.generate/i18n.generated.ts
  • template/nestJs/src/i18n/enUS/validation.json
  • template/nestJs/src/i18n/zhCN/validation.json
  • template/nestJs/src/menu/dto/create-menu.dto.ts

"IS_ARRAY": "{property} should to be array",
"NOT_EMPTY_HUMAN": "{name} should not to be empty"
"NOT_EMPTY_HUMAN": "{name} should not to be empty",
"TYPE_ERROR": "{property} should to be {type}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the English validation copy.

"should to be" reads incorrectly in API errors. Please change it to "should be {type}" or "must be {type}".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@template/nestJs/src/i18n/enUS/validation.json` at line 5, Update the
validation message for the "TYPE_ERROR" key in validation.json to correct the
English; replace the current value "{property} should to be {type}" with a
grammatically correct phrase such as "{property} should be {type}" (or
"{property} must be {type}") so API errors read properly.

@@ -1,4 +1,4 @@
import { IsNotEmpty } from 'class-validator';
import { IsNotEmpty, IsString } from 'class-validator';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
# Inspect ValidationPipe setup and whether missing properties are skipped globally.
rg -n -C3 'ValidationPipe|skipMissingProperties' --type ts

Repository: opentiny/tiny-pro

Length of output: 1019


🏁 Script executed:

#!/bin/bash
# Read the actual create-menu.dto.ts file to see the current state
cat -n template/nestJs/src/menu/dto/create-menu.dto.ts | head -60

Repository: opentiny/tiny-pro

Length of output: 1992


🌐 Web query:

nestjs-i18n I18nValidationPipe default skipMissingProperties behavior

💡 Result:

I18nValidationPipe does not change skipMissingProperties—it just extends Nest’s built-in ValidationPipe and accepts the same ValidationPipeOptions (minus exceptionFactory). (app.unpkg.com)

So the default behavior is the same as ValidationPipe / class-validator: missing (null/undefined) properties are not skipped unless you explicitly enable it, i.e. skipMissingProperties effectively defaults to false. (docs.nestjs.com)

To enable “skip missing properties” behavior:

app.useGlobalPipes(new I18nValidationPipe({ skipMissingProperties: true }));
``` ([docs.nestjs.com](https://docs.nestjs.com/techniques/validation?utm_source=openai))

Citations:

- 1: https://app.unpkg.com/nestjs-i18n%4010.4.9/files/dist/pipes/i18n-validation.pipe.d.ts
- 2: https://docs.nestjs.com/techniques/validation?utm_source=openai
- 3: https://docs.nestjs.com/techniques/validation?utm_source=openai

---



</details>

**`icon` is not actually optional without `@IsOptional()`.**

`required: false` only affects OpenAPI docs. Since `I18nValidationPipe` uses default NestJS behavior (`skipMissingProperties: false`), `@IsString()` will reject requests that omit `icon`. The field is marked optional in Swagger but required in both validation and TypeScript. Fix the inconsistency by adding `@IsOptional()`, making the property optional in the DTO signature, and keeping the Swagger metadata consistent.

<details>
<summary>Suggested fix</summary>

```diff
-import { IsNotEmpty, IsString } from 'class-validator';
+import { IsNotEmpty, IsOptional, IsString } from 'class-validator';

   `@ApiProperty`({
     description: '菜单图标',
     required: false,
   })
+  `@IsOptional`()
   `@IsString`({
-    message: i18nValidationMessage<I18nTranslations>('validation.TYPE_ERROR', {type: 'string'}),
+    message: i18nValidationMessage<I18nTranslations>('validation.TYPE_ERROR', { type: 'string' }),
   })
-  icon: string;
+  icon?: string;

Also applies to: 42-49

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@template/nestJs/src/menu/dto/create-menu.dto.ts` at line 1, The DTO marks the
icon field optional in Swagger but not in validation/TypeScript, so update the
CreateMenuDto (and the other DTO occurrences noted) by importing and adding the
`@IsOptional`() decorator before `@IsString`(), change the property signature to be
optional (icon?: string), and ensure IsOptional is added to the import list so
validation, TypeScript typing, and Swagger metadata are consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant