Skip to content

Unify minimum/maximum error handlers with draft-04 compatibility#163

Open
Padmashree06 wants to merge 1 commit intohyperjump-io:mainfrom
Padmashree06:exclusive-min-max
Open

Unify minimum/maximum error handlers with draft-04 compatibility#163
Padmashree06 wants to merge 1 commit intohyperjump-io:mainfrom
Padmashree06:exclusive-min-max

Conversation

@Padmashree06
Copy link
Contributor

Description

This PR introduces unified error handlers for:

  • minimum, exclusiveMinimum and draft-04 minimum
  • maximum, exclusiveMaximum and draft-04 maximum

The handlers now select and report only the most restrictive constraint, producing a single validation error instead of multiple overlapping messages.

Additional Updates

  • Added normalization for the combined handlers
  • Included test cases.
  • Existing individual handlers are commented out in index.js to avoid overlap and can be removed if preferred.

Tests

All tests pass locally.

Fixes

Issue: #135

Copy link
Collaborator

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

The new error handlers look like the right idea, but there's a lot of cleanup that needs to be done everywhere else.

  • You need to rebase your branch and resolve conflicts
  • The old handlers need to be removed
  • There's no need for new normalization handlers
  • There should be no changes to hyperjump-json-schema.test.js

@Padmashree06
Copy link
Contributor Author

Thank you for the feedback.
I will do the necessary changes.

Regarding the changes to hyperjump-json-schema.test.js, I made those adjustments because I was running into some reference errors during testing. I’m sorry if modifying that file wasn’t the right approach. I’ll revert those changes and look into fixing the underlying issue properly instead.

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.

2 participants