Skip to content

feat(taskflow)!: Per state UI rendering order preservation - #246

Open
chathura-de-silva wants to merge 9 commits into
OpenNSW:mainfrom
chathura-de-silva:feat/236-per-state-ui-rendering-order-preservation
Open

chathura-de-silva wants to merge 9 commits into
OpenNSW:mainfrom
chathura-de-silva:feat/236-per-state-ui-rendering-order-preservation

Conversation

@chathura-de-silva

@chathura-de-silva chathura-de-silva commented Sep 28, 2026 •

Copy link
Copy Markdown

Summary

Previously render.jsons had no way to specify or preserve an order of UI sections. It was workarounded in the frontend side. This introduces a mechanism to specify a state dependant order and preserve it so that frontend can directly render it in the given order.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Other (please describe):

Changes Made

Rendered view (breaking)

  • view is now an ordered list of entries instead of a map keyed by section. Position in the list is render order; there is no separate order field.
  • Each entry carries:
    • id: the section's key in render.json (unique within a task, stable across states), for tracking entries on the client.
    • title: the section's title, which was previously dropped in the pipeline (omitted when the section has none).
    • rest...

Ordering rules

  • Visible sections are returned in the order of the state's layout.
  • Visible sections the layout doesn't list are appended after the listed ones, sorted by key(alphabetically).
  • A state with no order returns its sections sorted by key(alphabetically).
  • Layout entries that aren't visible in the current state are skipped; duplicates are placed once.
  • An invalid $ref (not of the form #/layouts/<name>, or naming an undefined layout) fails with an error instead of silently falling back.

Implementation

  • TaskRenderer.Render now builds the whole view: it projects sections via uiprojector, resolves the state's layout, orders the sections, and merges in each section's legal handles.
  • ZoneViewAssembler is now a thin wrapper: it calls Render with the task's state, data and the caller's claims, and wraps the result in ZoneView.
    The second parse of the config and mergeView were removed.
  • Both render endpoints (TaskManager.GetTaskRenderInfo and the zoneview assembler) go through Render, so both return the new list shape.
  • legalCommands and filterLegalHandles moved from `zone_as unchanged.

Tests

  • New renderer_test.go covering: layout ordering per state, shared layouts, the key-sorted fallback, unlisted sections going last, $ref errors, titles and legal handles, handle declaration order, payload shape per projector, omitted empty titles, the empty config, plus unit tests for resolveLayout and orderSlots.
  • zone_assembler_test.go trimmed to the assembler's own concerns. Tests now identify sections by id.

Docs

  • template-reference.md: new "The zoneview renderer" section documenting sections, layouts, states.order and the ordering rules.
  • frontend-guide.md: new "The zoneview convention" section and its fields.

The breaking label in the "Rendered view" section matches the reads view as a map, like the trader-app's zone renderer,needs updating alongside this PR.

Testing

  • I have tested this change locally
  • I have added tests that prove my fix is effective or that my feature works
  • I have tested edge cases
  • All existing tests pass

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have checked that there are no merge conflicts

Related Issues

Closes #236
Related to OpenNSW/nsw-srilanka#512
Related to https://github.com/LSFLK/lsf-govtech-tnsw/issues/150

Screenshots/Demo

(If applicable, add screenshots or GIFs to help explain your changes)

Additional Notes

Note

Breaking Change
Trader-Portals Task view will fail (on a core version bump without the relavant feature PR there)

OpenNSW/nsw-srilanka#504

Summary by CodeRabbit

  • New Features
    • Sections now appear in a defined order, with unlisted sections placed afterward.
    • Rendered sections include IDs and optional titles, and expose only actions available in the current state.
    • Empty configurations return an empty view; invalid layout references produce errors.
  • Documentation
    • Added guidance on configuring section layouts and understanding the frontend view format.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 648a7831-2d3e-4fd3-9123-0d6f71e28c64

📥 Commits

Reviewing files that changed from the base of the PR and between 9d0f524 and 18eb18e.

📒 Files selected for processing (7)
  • taskflow/docs/frontend-guide.md
  • taskflow/docs/template-reference.md
  • taskflow/renderer/zoneview/renderer.go
  • taskflow/renderer/zoneview/renderer_test.go
  • taskflow/renderer/zoneview/zone_assembler.go
  • taskflow/renderer/zoneview/zone_assembler_test.go
  • taskflow/renderer/zoneview/zoneview.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The zone view renderer now returns an ordered list of enriched components. State layouts determine section order, and current-state actions determine which handles are included. The assembler forwards the rendered view directly, and documentation describes the configuration and frontend format.

Changes

Zone view rendering

Layer / File(s) Summary
Layout and view contracts
taskflow/renderer/zoneview/zoneview.go, taskflow/docs/template-reference.md
Configuration types now represent named layouts and state order references. The docs describe layout configuration and fallback ordering.
Resolve, order, and enrich rendered sections
taskflow/renderer/zoneview/renderer.go, taskflow/renderer/zoneview/renderer_test.go, taskflow/docs/frontend-guide.md
The renderer resolves state layouts, emits sections in layout order, sorts unlisted sections, and includes titles and state-legal handles. Tests cover ordering, layout errors, payloads, handles, and empty configuration. The frontend guide describes the list format.
Forward the complete rendered view
taskflow/renderer/zoneview/zone_assembler.go, taskflow/renderer/zoneview/zone_assembler_test.go
The assembler now assigns the renderer’s complete view directly. Tests check component IDs and handles in the list-shaped view.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 18eb1

The view format changes as intended. No demonstrated issue blocks merging, though the consuming frontend’s compatibility should be confirmed during rollout.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 18eb1

The new view format requires consumers to understand an ordered list. Existing action and visibility controls appear preserved, but compatibility with deployed consumers and the update sequence are not established.

Retained concerns

  • Medium · architecture · inferred: The immediate map-to-list change can make a consumer expecting the previous zoneview shape unable to render task details or their action handles. The changed response path has no compatibility behavior; whether any deployed consumer still expects the map is unknown.
Security review details

Security Blast Radius

  • inferred — The demonstrated change affects task views and the presentation of their action handles. Available evidence does not establish an expansion to another tenant, data store, credential boundary, or service.

Trust Boundaries and Controls

  • observed — Caller claims reach the projector through ZoneViewAssembler. The renderer cannot add a layout entry that the projector did not emit, and it filters each emitted section's handles against current-state actions.

Resilience and Maintainability Implications

  • inferred — A consumer-format mismatch could prevent users from seeing task content or available actions, but no deployed mismatched consumer or resulting control failure is demonstrated.

Hardening Proposals

  • proposed — Confirm which deployed consumers parse zoneview responses and coordinate their migration with the list-format rollout, or provide a temporary compatibility path if consumers cannot update together.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 5 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #236 requires state-dependent section ordering, named layouts, and frontend-visible order preservation. The PR adds TaskTemplateConfig.Layouts, StateView.Order, and layout reference resoluti…
Out of Scope Changes check ✅ Passed The changed renderer, assembler, model types, tests, and documentation directly support Issue #236. The assembler refactor forwards the complete ordered view from TaskRenderer.Render, and the tests …
Title check ✅ Passed The title clearly identifies the main change: preserving UI rendering order for each task state. It is concise and relevant to the changeset.
Description check ✅ Passed The description follows the required template and covers the summary, change type, implementation, testing, checklist, related issues, screenshots, and additional notes. It contains minor wording and …
Full details: Docstring Coverage

Explanation

Docstring coverage is 60.87% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 5 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@chathura-de-silva
chathura-de-silva force-pushed the feat/236-per-state-ui-rendering-order-preservation branch from 18eb18e to 86d8bd5 Compare September 29, 2026 02:29

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

Don't we need a nsw-srilanka consumer that consumes the ordered list and retires the #504 ZONE_ORDER hack?

Gap: Trader portal still treats view as a map (types.ts, TraderZoneLayout, task handler tests). If we merge this PR and bump core without a portal update, task screens break.

What nsw-srilanka should do:

  1. Accept view as an ordered list (use each entry’s id).
  2. Render top to bottom in that list order.
  3. Remove the temporary ZONE_ORDER hack from #504 (status_awaiting / review_history hardcoding)

@chathura-de-silva

chathura-de-silva commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

Don't we need a nsw-srilanka consumer that consumes the ordered list and retires the #504 ZONE_ORDER hack?

Gap: Trader portal still treats view as a map (types.ts, TraderZoneLayout, task handler tests). If we merge this PR and bump core without a portal update, task screens break.

What nsw-srilanka should do:

  1. Accept view as an ordered list (use each entry’s id).
  2. Render top to bottom in that list order.
  3. Remove the temporary ZONE_ORDER hack from #504 (status_awaiting / review_history hardcoding)

It's already done and tested with local core. Waiting to this to merge so I could point and test it with the remote core repo.

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.

Per State UI Rendering Order Preservation

2 participants