Skip to content

Fix SC ENR parameters form validation for materials and clusters#1069

Open
sBouzols wants to merge 1 commit intomainfrom
fix_enr_parameters_form_schema_checks
Open

Fix SC ENR parameters form validation for materials and clusters#1069
sBouzols wants to merge 1 commit intomainfrom
fix_enr_parameters_form_schema_checks

Conversation

@sBouzols
Copy link
Contributor

PR Summary

fix(): Add requiredWhenActive helper function and manage yup schema to allow parameters with inactive clusters and materials

…o allow parameters with unactive clusters and material

Signed-off-by: sBouzols <sylvain.bouzols@gmail.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 25, 2026

📝 Walkthrough

Walkthrough

Modified validation schemas and initial field values for short-circuit ICC cluster parameters. Introduced conditional validation requiring fields based on an active state, updated initial values from empty arrays to null, and removed error-message-hiding behavior from a form input component.

Changes

Cohort / File(s) Summary
Short-Circuit ICC Cluster Parameters
src/components/parameters/short-circuit/columns-definition.ts, src/components/parameters/short-circuit/short-circuit-icc-cluster-table-cell.tsx, src/components/parameters/short-circuit/short-circuit-parameters-utils.ts
Updated column initial values from empty arrays to null for cluster selection fields. Removed hideErrorMessage prop from DirectoryItemsInput. Added requiredWhenActive helper to conditionally enforce field requirements based on active boolean state; applied to multiple fields (alpha, u0, usMin, usMax, type, filters) in materials and clusters arrays.
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing form validation for SC ENR parameters related to materials and clusters.
Description check ✅ Passed The description is related to the changeset, explaining the addition of a requiredWhenActive helper and modifications to Yup validation schema for ENR parameters.
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.


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.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link

@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.

🧹 Nitpick comments (1)
src/components/parameters/short-circuit/short-circuit-parameters-utils.ts (1)

135-135: Redundant .nullable() before requiredWhenActive.

The .nullable() chained on the base schema is redundant here since requiredWhenActive already applies .nullable() in its otherwise branch. This also differs from the materials schema (line 88) which doesn't include .nullable() on the base.

Consider removing for consistency:

♻️ Suggested refactor
-                      type: requiredWhenActive(yup.string().oneOf(['GENERATOR', 'HVDC']).nullable()),
+                      type: requiredWhenActive(yup.string().oneOf(['GENERATOR', 'HVDC'])),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/parameters/short-circuit/short-circuit-parameters-utils.ts` at
line 135, The base schema unnecessarily chains .nullable() before calling
requiredWhenActive: remove the redundant .nullable() so the field is defined as
type: requiredWhenActive(yup.string().oneOf(['GENERATOR', 'HVDC'])), matching
the materials schema pattern; keep requiredWhenActive as the place that applies
.nullable() in its otherwise branch to avoid double-nullable behavior and ensure
consistency with the other schema.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/components/parameters/short-circuit/short-circuit-parameters-utils.ts`:
- Line 135: The base schema unnecessarily chains .nullable() before calling
requiredWhenActive: remove the redundant .nullable() so the field is defined as
type: requiredWhenActive(yup.string().oneOf(['GENERATOR', 'HVDC'])), matching
the materials schema pattern; keep requiredWhenActive as the place that applies
.nullable() in its otherwise branch to avoid double-nullable behavior and ensure
consistency with the other schema.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2b8a404c-e854-4d97-82be-aca53aa1c7ee

📥 Commits

Reviewing files that changed from the base of the PR and between a1bad06 and 8051f74.

📒 Files selected for processing (3)
  • src/components/parameters/short-circuit/columns-definition.ts
  • src/components/parameters/short-circuit/short-circuit-icc-cluster-table-cell.tsx
  • src/components/parameters/short-circuit/short-circuit-parameters-utils.ts
💤 Files with no reviewable changes (1)
  • src/components/parameters/short-circuit/short-circuit-icc-cluster-table-cell.tsx

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.

1 participant