Skip to content

refactor: simplify booru tag DTO handling#133

Merged
AlejandroAkbal merged 3 commits into
mainfrom
auto-triage/326-api-dto-simplify
Mar 16, 2026
Merged

refactor: simplify booru tag DTO handling#133
AlejandroAkbal merged 3 commits into
mainfrom
auto-triage/326-api-dto-simplify

Conversation

@AlejandroAkbal
Copy link
Copy Markdown
Member

@AlejandroAkbal AlejandroAkbal commented Mar 16, 2026

Summary

  • remove redundant tag percent-decoding from the booru posts DTO and rely on actual Nest/Fastify request parsing
  • replace the old plainToInstance-only DTO tests with request-level validation against a real Fastify/Nest app
  • keep tag splitting/validation behavior intact while aligning coverage with runtime behavior

Validation

  • pnpm test -- booru/dto/booru-queries.dto.spec.ts
  • pnpm exec eslint src/booru/dto/booru-queries.dto.ts src/booru/dto/booru-queries.dto.spec.ts
  • pnpm exec tsc --noEmit --pretty false

Summary by CodeRabbit

  • Bug Fixes

    • Improved tag parsing and stricter per-tag validation for query-based searches; empty or malformed tags now produce validation errors.
  • Tests

    • Replaced unit-style checks with integration-style request tests that exercise query parsing, percent-decoding, pipe-splitting, repeated params, and error responses.
  • Refactor

    • Centralized and applied global validation configuration for consistent request validation across the app.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 16, 2026

Warning

Rate limit exceeded

@AlejandroAkbal has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 15 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 18a6fb4d-2592-408c-a0b0-9a906e610329

📥 Commits

Reviewing files that changed from the base of the PR and between 637e05c and a9f7dbf.

📒 Files selected for processing (1)
  • src/booru/dto/booru-queries.dto.spec.ts

Walkthrough

Refactors DTO tests from unit-style to integration-style using a NestJS test controller and TestModule; simplifies and tightens tags transformation/validation in booruQueryValuesPostsDTO; centralizes ValidationPipe configuration in src/common/validation.ts and applies it from src/main.ts.

Changes

Cohort / File(s) Summary
Test Integration
src/booru/dto/booru-queries.dto.spec.ts
Replaces plainToInstance tests with an integration-style NestJS test harness (Fastify adapter). Adds booruQueriesTestController with GET /dto-test/posts, initializes app with global ValidationPipe, issues HTTP requests to verify percent-decoding, pipe-splitting, repeated params, and error responses for empty tags.
DTO Transformation & Validation
src/booru/dto/booru-queries.dto.ts
Modifies tags Transform to uniformly handle arrays and single values and flatMap pipe-delimited splits; adds @IsString({ each: true }) to enforce per-element string validation.
Global Validation Wiring
src/common/validation.ts, src/main.ts
Adds APP_VALIDATION_PIPE_OPTIONS and createAppValidationPipe() to centralize ValidationPipe options (transform, whitelist, forbidNonWhitelisted) and replaces inline ValidationPipe usage in main.ts with app.useGlobalPipes(createAppValidationPipe()).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • fix: decode encoded booru tags #128 — Modifies booruQueryValuesPostsDTO.tags transformation and related test behavior (percent-decoding and error handling) overlapping the DTO/tag parsing changes in this PR.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: refactoring the booru tag DTO handling to simplify it. This is reflected across all modified files including the DTO transformation logic, validation configuration, and test restructuring.

✏️ 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
  • Commit unit tests in branch auto-triage/326-api-dto-simplify
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch auto-triage/326-api-dto-simplify
📝 Coding Plan
  • Generate coding plan for human review comments

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.

@AlejandroAkbal
Copy link
Copy Markdown
Member Author

Good catch. I tightened PR #133 so the request-level DTO test no longer hardcodes its own validation-pipe options.

It now reuses the same app validation setup that src/main.ts uses via a shared helper in src/common/validation.ts, so if the real bootstrap validation options change later, this test will track that instead of drifting.

Validation re-run:

  • pnpm test -- booru/dto/booru-queries.dto.spec.ts
  • pnpm exec eslint src/common/validation.ts src/main.ts src/booru/dto/booru-queries.dto.spec.ts
  • pnpm exec tsc --noEmit --pretty false

Copy link
Copy Markdown
Contributor

@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: 1

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

Inline comments:
In `@src/booru/dto/booru-queries.dto.spec.ts`:
- Line 8: Rename the test controller class from booruQueriesTestController to
PascalCase B ooruQueriesTestController to follow NestJS conventions, and update
any usages or references of booruQueriesTestController (e.g., the controller
reference later in the spec) to the new B ooruQueriesTestController identifier
so imports/instantiations compile and match other controllers like
AppController/BooruController/TestBooruErrorsController.
🪄 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: 4dfb624f-707f-4268-a3bf-ff427018c197

📥 Commits

Reviewing files that changed from the base of the PR and between ded312c and 637e05c.

📒 Files selected for processing (3)
  • src/booru/dto/booru-queries.dto.spec.ts
  • src/common/validation.ts
  • src/main.ts

Comment thread src/booru/dto/booru-queries.dto.spec.ts Outdated
@AlejandroAkbal
Copy link
Copy Markdown
Member Author

Final sweep update: I took the one real remaining nit from CodeRabbit.

  • Renamed the local DTO test controller to BooruQueriesTestController for Nest-style PascalCase consistency
  • Validation rerun:
    • pnpm test -- booru/dto/booru-queries.dto.spec.ts
    • pnpm exec eslint src/booru/dto/booru-queries.dto.spec.ts

Latest commit on this PR: a9f7dbf (test: rename booru DTO test controller).

I also re-checked UBW PR #7 in the same sweep: no actionable review comments there, and Copilot PR #8 has zero file changes, so there was nothing to adopt on the UBW side.

@AlejandroAkbal AlejandroAkbal merged commit c6464e0 into main Mar 16, 2026
1 check passed
@AlejandroAkbal AlejandroAkbal deleted the auto-triage/326-api-dto-simplify branch March 16, 2026 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant