eugenegujing opened a new pull request, #5640:
URL: https://github.com/apache/texera/pull/5640

   <!-- PR title: feat(workflow-operator): add not-blank validation messages -->
   
   ### What changes were proposed in this PR?
   
   Extends the not-blank validation pattern introduced for basic chart 
operators in #4006 to **all visualization operators**, and gives every backend 
`assert` a human-readable message.
   
   **Why this matters.** The frontend (AJV + `required` + the autofill 
attribute enum) already blocks most empty required fields in normal UI usage, 
but three gaps remained where users or API callers hit a bare `assertion 
failed` with no explanation:
   
   | Gap | Example | Fix |
   |---|---|---|
   | Bare asserts are the only backend check for execution paths that bypass 
frontend validation (direct API calls, agent-generated workflows) | 
`assert(value.nonEmpty)` in `PieChartOpDesc` | Every assert now carries a 
message, e.g. `assert(value.nonEmpty, "Value Column cannot be empty")` |
   | Empty lists pass AJV's `required` check (it only verifies key presence) | 
`FigureFactoryTableOpDesc.columns = []` ran and crashed | `@NotEmpty(message = 
...)` on required list attributes → schema gains `minItems: 1` |
   | Numeric constraints existed only in backend asserts, invisible to the 
frontend | `rowHeight = 10` passed the form, then `assert(rowHeight >= 30)` 
crashed | `@DecimalMin` on `FigureFactoryTable` `fontSize`/`rowHeight` → schema 
gains `minimum` |
   
   **Changes in detail:**
   
   - Added messages to all 46 previously bare asserts across 23 visualization 
operators, splitting compound asserts (e.g. `assert(x.nonEmpty && y.nonEmpty)`) 
per field so the error names the exact missing attribute.
   - Added `@NotNull(message = ...)` to required string attributes lacking it; 
with the generator's `useMinLengthForNotNull`, the schema gains `minLength: 1` 
so the frontend rejects empty strings as well. Annotation messages are 
identical to their assert messages (same convention as #4006). An 
annotation-only sweep over 20 more operator/config files brings every required 
visualization attribute under a constraint.
   - Fixed two null defaults (`ImageVisualizerOpDesc.binaryContent`, 
`ScatterMatrixChartOpDesc.selectedAttributes`) that threw a 
`NullPointerException` before the assert could produce its message.
   - Added two missing guards for fields that were interpolated unchecked: 
`IcicleChartOpDesc.manipulateTable()` (`value`) and 
`RadarChartOpDesc.createPlotlyFigure()` (`valueColumns`).
   - No optional field gained a new required-ness constraint, so existing saved 
workflows are unaffected.
   
   **Notes for reviewers:**
   
   - Messages quote each field's `@JsonSchemaTitle` verbatim; a few titles are 
terse (`x`, `r`, `theta` in contourPlot/quiverPlot/polarChart), producing 
messages like `x cannot be empty`. Improving those titles is left out of scope 
— happy to adjust if preferred.
   - `LineChartOpDesc.lines` reuses its pre-existing assert message (`At least 
one line must be configured`) instead of inventing a second phrasing.
   - Non-visualization operators also have required attributes without 
constraint annotations (~55 files); those fields need per-field semantic 
review, so they are left for a follow-up issue.
   
   ### Any related issues, documentation, discussions?
   
   Closes #4053. Follows the approach of #4006 (and #3692).
   
   ### How was this PR tested?
   
   Added 14 new spec files and extended 9 existing ones, covering every touched 
operator with positive (all fields set → template renders the configured 
columns), negative (empty field → `AssertionError` whose message names the 
field and contains "cannot be empty"), and boundary cases (`rowHeight = 10` → 
"at least 30", `fontSize = -1` → "non-negative", exact boundary values pass).
   
   ```
   sbt "WorkflowOperator/testOnly 
org.apache.texera.amber.operator.visualization.*"
   ```
   
   168 tests, 31 suites, all passed. Also ran `sbt scalafixAll` and `sbt 
scalafmtAll` (clean).
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Co-authored-by: Claude Code (Claude Fable 5)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to