Skip to content

Add Min Max Validation for Count Related Policies - #2860

Merged
Induwara04 merged 4 commits into
wso2:mainfrom
Induwara04:policy-issue
Jul 27, 2026
Merged

Add Min Max Validation for Count Related Policies#2860
Induwara04 merged 4 commits into
wso2:mainfrom
Induwara04:policy-issue

Conversation

@Induwara04

Copy link
Copy Markdown
Contributor

This pull request introduces improvements to the validation logic in the PolicyParameterEditor component, focusing on handling disabled parameter groups and enforcing logical constraints between minimum and maximum values. The main changes are grouped as follows:

Issue: #2500
Screenshot 2026-07-24 at 20 38 59

Validation logic improvements:

  • Added the isDisabledByAnyOf helper function to detect if a schema group is disabled via an anyOf entry with an enabled: false property, allowing the validator to skip required checks for disabled groups.
  • Updated validateRequiredFields to skip validation for nested objects that are marked as disabled according to the schema and current values.
  • Enhanced validateConstraints to check that, when both min and max properties are present and numeric, the maximum value is not less than the minimum value, and to report a clear error if this constraint is violated.

Documentation and code clarity:

  • Clarified the documentation for validateValueConstraints to better reflect that it recursively validates format constraints, not just presence.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 7f6b3191-0fd1-40bb-9058-08e763d179bf

📥 Commits

Reviewing files that changed from the base of the PR and between 95fafb4 and ad380af.

📒 Files selected for processing (2)
  • portals/ai-workspace/docker-compose.yaml
  • portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx

📝 Walkthrough

Walkthrough

The policy parameter editor now skips required-field validation for nested objects disabled through anyOf/enabled: false rules and reports errors when numeric min exceeds max. The ai-workspace container image uses the 1.0.0-rc-SNAPSHOT tag.

Changes

Policy parameter validation

Layer / File(s) Summary
Disabled nested object validation
portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx
Adds detection for disabled nested objects and conditionally skips recursive required-field checks.
Numeric min/max validation
portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx
Adds cross-field validation that reports max when finite numeric min exceeds max, and updates the validation documentation.

Workspace image configuration

Layer / File(s) Summary
Snapshot image tag
portals/ai-workspace/docker-compose.yaml
Updates the ai-workspace image reference to the 1.0.0-rc-SNAPSHOT tag.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: malinthaprasan, pubudu538, tgtshanika, thushani-jayasekera

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the feature but misses required template sections like Purpose, Goals, Tests, Security, Related PRs, and Test environment. Reformat the PR description to include all template headings, especially Purpose, Goals, Approach, User stories, tests, security checks, and test environment.
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: adding min/max validation for policy-related parameters.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx (1)

146-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression tests for the new validation branches.

Cover disabled objects with missing required children, disabled array items, an anyOf false branch without required: ['enabled'], min > max, min === max, and nested min/max paths.

Also applies to: 189-193, 300-331

🤖 Prompt for 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.

In
`@portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx`
around lines 146 - 161, Add regression tests for the validation logic around
isDisabledByAnyOf and the related branches at the referenced validation paths.
Cover disabled objects with missing required children, disabled array items,
anyOf false branches lacking required enabled, min greater than max, min equal
to max, and nested min/max paths, preserving expected validation outcomes for
each case.
🤖 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
`@portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx`:
- Around line 151-155: Update the supportsDisabled detection around
ParameterSchema.anyOf to identify any branch whose properties.enabled.const is
false, without requiring entry.required to include enabled. Preserve the
existing actual-value check and ensure branches lacking required are handled
correctly.
- Around line 189-193: Extend the disabled-object guard from the direct object
branch to the array-item validation path in PolicyParameterEditor, ensuring
validateRequiredFields skips object items where enabled is false or
isDisabledByAnyOf indicates they are disabled. Preserve validation for enabled
items and non-object array elements.

---

Nitpick comments:
In
`@portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx`:
- Around line 146-161: Add regression tests for the validation logic around
isDisabledByAnyOf and the related branches at the referenced validation paths.
Cover disabled objects with missing required children, disabled array items,
anyOf false branches lacking required enabled, min greater than max, min equal
to max, and nested min/max paths, preserving expected validation outcomes for
each case.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a5f72072-75d1-4809-b518-7799621e5384

📥 Commits

Reviewing files that changed from the base of the PR and between b1bb538 and 95fafb4.

📒 Files selected for processing (1)
  • portals/ai-workspace/src/pages/appShell/PolicyParameterEditor/PolicyParameterEditor.tsx

@Induwara04
Induwara04 merged commit b2e8c36 into wso2:main Jul 27, 2026
7 checks passed
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.

3 participants