Skip to content

[GSSoC'26] fix: make OTP required in verifyDeliverySchema#756

Open
vipul674 wants to merge 1 commit into
KanishJebaMathewM:mainfrom
vipul674:fix/verify-delivery-otp-required-742
Open

[GSSoC'26] fix: make OTP required in verifyDeliverySchema#756
vipul674 wants to merge 1 commit into
KanishJebaMathewM:mainfrom
vipul674:fix/verify-delivery-otp-required-742

Conversation

@vipul674

@vipul674 vipul674 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Description

  • Removed .optional() from OTP field in verifyDeliverySchema so Zod enforces presence with consistent validation errors
  • Removed redundant manual if (!otp) guard in handler since schema now handles required-field enforcement

Files Changed

  • backend/api/src/validation/requestSchemas.js: Removed .optional() from OTP inner schema
  • backend/api/src/routes/orderRoutes.js: Removed manual if (!otp) check (now handled by Zod)

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation

Difficulty & Label Request

Assessed difficulty: level1

Please apply the matching difficulty label for GSSoC '26 scoring.
If applicable, please also apply gssoc:approved after review.

Testing & Verification

Commands run: N/A (schema change only, validated by existing Zod validation flow)

Result: Schema now rejects missing OTP with structured error instead of generic 400

Known Limitations

  • None

GSSoC 2026 Compliance

This contribution was prepared with AI assistance and manually reviewed before submission.

Closes #742

Summary by CodeRabbit

  • Refactor
    • Strengthened delivery verification validation so an OTP is strictly required and must match the expected 6-digit format.
  • Bug Fixes
    • Updated the delivery verification flow so missing OTP values no longer fail immediately; they now follow the standard “OTP unavailable/expired” or “invalid OTP” handling, including any related retry/lockout behavior.

@vipul674

Copy link
Copy Markdown
Contributor Author

@KanishJebaMathewM I have submitted the fix. Please let me know if any changes are needed!

@github-actions

Copy link
Copy Markdown
Contributor

🎉 Thank you for your contribution! Your pull request has been received and will be reviewed shortly.

If you enjoy the project, please consider giving the repository a ⭐. You can also follow my GitHub profile to stay updated on future open-source projects.

Thanks for being part of the community! 🚀

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 62843e15-8b81-4408-b70f-1a3bfa49e8ad

📥 Commits

Reviewing files that changed from the base of the PR and between 709fc10 and f970be4.

📒 Files selected for processing (2)
  • backend/api/src/routes/orderRoutes.js
  • backend/api/src/validation/requestSchemas.js
💤 Files with no reviewable changes (2)
  • backend/api/src/validation/requestSchemas.js
  • backend/api/src/routes/orderRoutes.js

📝 Walkthrough

Walkthrough

verifyDeliverySchema now requires otp, and the POST /:id/verify-delivery handler no longer performs an early missing-OTP 400 check.

Changes

OTP Presence Validation Consolidation

Layer / File(s) Summary
Require OTP in schema and drop handler check
backend/api/src/validation/requestSchemas.js, backend/api/src/routes/orderRoutes.js
verifyDeliverySchema removes .optional() from otp, and the verify-delivery route no longer returns immediately when otp is missing.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related issues

Possibly related PRs

Suggested labels

level:beginner, type:bug

Poem

🐰 Zod now says, “Bring six digits, please,”
The handler no longer checks with ease.
One tiny hop, one cleaner gate,
OTP rules now հավ? wait — straight and great!

🚥 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 clearly and concisely describes the main change: making OTP required in verifyDeliverySchema.
Linked Issues check ✅ Passed The schema now requires OTP and the redundant manual missing-OTP check was removed, matching issue #742.
Out of Scope Changes check ✅ Passed The PR stays scoped to the OTP validation inconsistency fix and only touches the two relevant files.
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 unit tests (beta)
  • Create PR with unit tests

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

@vipul674 vipul674 force-pushed the fix/verify-delivery-otp-required-742 branch from 709fc10 to f970be4 Compare June 24, 2026 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: verifyDeliverySchema marks OTP as optional but handler treats it as required — schema does not enforce presence

1 participant