Skip to content

Update feed query DTO#232

Merged
yamcodes merged 2 commits intomainfrom
231-limit-must-be-a-number-was-a-string
Jan 27, 2026
Merged

Update feed query DTO#232
yamcodes merged 2 commits intomainfrom
231-limit-must-be-a-number-was-a-string

Conversation

@yamcodes
Copy link
Collaborator

@yamcodes yamcodes commented Jan 27, 2026

Description

  • Make limit and offset optional
  • Parse string numeric values for limit and offset
  • Remove default values from DTO definition

Fixes #231

PR Checklist

  • Read the Developer's Guide in CONTRIBUTING.md
  • Use a concise title to represent the changes introduced in this PR
  • Provide a detailed description of the changes introduced in this PR, and, if necessary, some screenshots
  • Reference an issue or discussion where the feature or changes have been previously discussed
  • Add a failing test that passes with the changes introduced in this PR, or explain why it's not feasible
  • Add documentation for the feature or changes introduced in this PR to the docs; you can run them with bun docs

Summary by CodeRabbit

  • Improvements
    • Article feed query parameters limit and offset are now optional; sensible defaults are applied when omitted.
    • When provided, limit and offset are validated as numeric values within allowed ranges to prevent invalid queries.

✏️ Tip: You can customize this high-level summary in your review settings.

@yamcodes yamcodes linked an issue Jan 27, 2026 that may be closed by this pull request
4 tasks
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

ArticleFeedQueryDto's limit and offset query properties were made optional and their validations were changed to parse numeric strings (string.numeric.parse) with explicit MIN/MAX boundary checks; imports of DEFAULT_LIMIT/DEFAULT_OFFSET were removed.

Changes

Cohort / File(s) Summary
Query DTO Optional Parameters
src/articles/dto/article-feed-query.dto.ts
limit and offset changed from required to optional; validation switched to string.numeric.parse with number.integer checks and explicit MIN_LIMIT/MAX_LIMIT/MIN_OFFSET comparisons; removed DEFAULT_LIMIT/DEFAULT_OFFSET imports.

Sequence Diagram(s)

(omitted — changes are limited to DTO validation and don't introduce new multi-component control flow)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • Update feed query DTO #232 — Modifies the same DTO (src/articles/dto/article-feed-query.dto.ts) to make limit/offset optional and replace DEFAULT_*-based checks with numeric parsing/validation.

Poem

🐰
Hopping through queries, light and spry,
Limit and offset now wave goodbye,
Numbers parsed with gentle care,
Boundaries checked in the open air,
I nibble bugs and leave things fair.

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Update feed query DTO' is vague and generic, using a non-descriptive term that doesn't convey the meaningful change of making limit/offset optional or adding string parsing. Consider a more descriptive title like 'Make limit and offset optional in feed query DTO and parse numeric strings' to better communicate the primary change.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR addresses the core issue #231 by parsing numeric string values for limit (and offset), allowing the DTO to accept query parameters as strings and convert them to numbers.
Out of Scope Changes check ✅ Passed All changes directly address the linked issue requirements: making limit/offset optional, parsing numeric strings, and removing default values from the DTO.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

Copy link
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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/articles/dto/article-feed-query.dto.ts (1)

4-9: Update pagination docstring to remove default references.

The comment still mentions DEFAULT_LIMIT/DEFAULT_OFFSET, which are no longer applied in this DTO. Please update it to avoid misleading docs.

✏️ Suggested update
- * - limit: number of items per request (default: DEFAULT_LIMIT, min: MIN_LIMIT, max: MAX_LIMIT)
- * - offset: number of items to skip (default: DEFAULT_OFFSET, min: MIN_OFFSET)
+ * - limit: number of items per request (min: MIN_LIMIT, max: MAX_LIMIT)
+ * - offset: number of items to skip (min: MIN_OFFSET)

- Make limit and offset optional
- Parse string numeric values for limit and offset
- Remove default values from DTO definition
@yamcodes yamcodes merged commit 5478b13 into main Jan 27, 2026
8 of 9 checks passed
@yamcodes yamcodes deleted the 231-limit-must-be-a-number-was-a-string branch January 27, 2026 19:51
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.

limit must be a number (was a string)

1 participant