Improve page number validation for entityIssueDetails

[jpherr]

Description

We are receiving URLs with alphanumeric reference numbers instead of page numbers. We do not know where these originate from at this stage but they are currently generating 500 errors, which is:

• not a good experience for users
• flooding Sentry with unhandled 500 errors

This change updates Valibot validation to assert that a number is passed. When a number is not passed, then we return a 400 Bad Request error, which is more appropriate and provides better feedback to both us for debugging, and to the user.

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update

Related Tickets & Documents

• Closes #2626
  Fix ValiError for Dev/Staging config#2626

QA Instructions, Screenshots, Recordings

Problem URL: https://provide.staging.planning.data.gov.uk/organisations/local-authority:LIV/tree-preservation-zone/missing%20associated%20entity/tree-preservation-order/entity/TPO335.G19

• Previously triggered unhandled 500 Error.
  • Logged Error to Sentry
• Now returns handled 400 Error (Bad Request)
  • Does NOT log Error to Sentry

Before

Browser:

Sentry:

After

Added/updated tests?

We encourage you to keep the code coverage percentage at 80% and above.

• Yes
• No, and this is why: Please replace this line with details on why tests have not been included
• I need help with writing tests

QA sign off

• Code has been checked and approved
• Design has been checked and approved
• Product and business logic has been checked and proved

[optional] Are there any post-deployment tasks we need to perform?

[optional] Are there any dependencies on other PRs or Work?

Summary by CodeRabbit

• Bug Fixes

  • Strengthened page-number validation to accept only whole numeric values and enforce a minimum of 1, reducing invalid input acceptance.

• Tests

  • Added unit tests covering numeric-string parsing to numbers and rejecting non-numeric page-number inputs to ensure validation behaves as expected.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: db4be39b-1cdd-44dd-bd83-ee105904cc15

📥 Commits

Reviewing files that changed from the base of the PR and between a9c576c and a48902c.

📒 Files selected for processing (2)

• src/middleware/entityIssueDetails.middleware.js
• test/unit/middleware/entityIssueDetails.middleware.test.js

🚧 Files skipped from review as they are similar to previous changes (1)

• test/unit/middleware/entityIssueDetails.middleware.test.js

---

Walkthrough

The PR tightens IssueDetailsQueryParams.pageNumber validation to accept only digit strings, convert them to numbers, and assert numeric/integer types; tests were added to confirm valid parsing and rejection of non-numeric input.

Changes

PageNumber query validation

Layer / File(s)	Summary
PageNumber validation schema with type checks src/middleware/entityIssueDetails.middleware.js	pageNumber validation now requires digit-only strings (v.regex(/^\d+$/)), uses Number(s) transform, then v.number() and v.integer() before v.minValue(1); default remains '1'.
Schema validation tests test/unit/middleware/entityIssueDetails.middleware.test.js	Adds valibot imports and a test suite that verifies numeric strings parse to numbers via v.parse and that non-numeric pageNumber input causes a ValiError (checked with isValiError).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

bug

Suggested reviewers

• tombrooks248

Poem

🐰
I nibbled at strings of digits bright,
Turned them to numbers with tidy might,
I hopped through tests to make things right,
No stray letters now in sight,
Validation snug — what a delight!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: strengthening validation for the pageNumber parameter in the entityIssueDetails middleware by requiring digit-only input and enforcing number/integer type checks.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 2626-fix-valierror-for-devstaging

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	66.58%	7232 / 10862
🔵	Statements	66.58%	7232 / 10862
🔵	Functions	63.93%	296 / 463
🔵	Branches	77.57%	1000 / 1289

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/middleware/entityIssueDetails.middleware.js	100%	74.07%	100%	100%

Generated in workflow #1493 for commit a48902c by the Vitest Coverage Report Action

[coderabbitai]

Actionable comments posted: 2

🤖 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/middleware/entityIssueDetails.middleware.js`:
- Line 29: The pageNumber pipeline currently uses v.transform(parseInt) which
permits mixed-format strings like "2abc" or "2.5"; update the pipeline for the
pageNumber field to first enforce a digits-only string (e.g., via a regex or
v.pattern) before converting to a number, then transform to a Number and
validate with v.integer() and v.minValue(1). In other words, modify the
pageNumber validator (the v.optional(v.pipe(v.string(), v.transform(...),
v.number(), v.integer(), v.minValue(1)))) so it inserts a strict digits-only
check (e.g., pattern /^\d+$/) prior to the transform step and only then
parse/convert to an integer.

In `@test/unit/middleware/entityIssueDetails.middleware.test.js`:
- Around line 274-280: The test for non-numeric pageNumber currently only
asserts inside the catch block so it will silently pass if
v.parse(IssueDetailsQueryParams, ...) does not throw; update the test to
explicitly assert that an exception is thrown (e.g., wrap the parse call in an
expect(...).toThrow or add an explicit fail() after the parse) while still
verifying the caught error satisfies isValiError(error); target the v.parse call
with IssueDetailsQueryParams and the test case that uses pageNumber:
'TPO335.G19'.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 58cdafe6-4522-4b72-99b0-1422c185f20e

📥 Commits

Reviewing files that changed from the base of the PR and between c1d0b66 and a9c576c.

📒 Files selected for processing (2)

• src/middleware/entityIssueDetails.middleware.js
• test/unit/middleware/entityIssueDetails.middleware.test.js

[gibahjoe]

LGTM