Skip to content

feat(datagrid-web): add onBeforeExport and onAfterExport event actions - #2392

Open
r0b1n wants to merge 3 commits into
mainfrom
feat/datagrid-export-events
Open

feat(datagrid-web): add onBeforeExport and onAfterExport event actions#2392
r0b1n wants to merge 3 commits into
mainfrom
feat/datagrid-export-events

Conversation

@r0b1n

@r0b1n r0b1n commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds two optional action properties — On before export and On after export — to Data Grid 2, giving developers a logging/auditing hook into the export lifecycle
  • onBeforeExport fires fire-and-forget immediately before the first datasource page fetch; receives gridName, columnTitles, chunkSize, fileName, sheetName, and startTime
  • onAfterExport fires after the export resolves (success or abort); additionally receives exportedItemCount, status ("success" | "aborted"), and endTime
  • Both actions are optional and fully independent; the export flow is unchanged when neither is configured
  • fileName and sheetName are passed by the external export caller via exportData() options and default to empty strings when not provided

Changes

  • Datagrid.xml — two new <property type="action"> blocks with <actionVariables> in the Events group
  • DatagridProps.d.ts — updated manually to match XML (will be regenerated on build)
  • DSExportRequest.ts — exposed get loaded() and get limit() public getters (renamed private fields to _loaded / _limit)
  • ExportController.ts — added BeforeExportArgs / AfterExportArgs types; name constructor param; beforeexport / afterexport events in ControllerEvents; public on() method (returns Unsubscribe); emits both events in exportData()
  • useDataExport.ts — stores latest ActionValue props in refs; subscribes to beforeexport / afterexport once per controller lifetime via useEffect([entry]); effect cleanup unsubscribes automatically
  • ExportController.spec.ts — 5 new unit tests covering all event scenarios

Test plan

  • All existing unit tests pass (pnpm run test)
  • No lint errors
  • In Studio Pro: configure onBeforeExport and onAfterExport on a Data Grid 2, trigger an export, verify both microflows/nanoflows are called with correct variable values
  • Verify status is "aborted" when the user cancels mid-export
  • Verify export works normally when neither action is configured

@r0b1n
r0b1n requested a review from a team as a code owner August 19, 2026 07:35
@github-actions

This comment has been minimized.

@r0b1n
r0b1n force-pushed the feat/datagrid-export-events branch from e1fc392 to 5134ce1 Compare August 19, 2026 13:38
@github-actions

This comment has been minimized.

);
}, [columnsStore.visibleColumns, entry]);

useEffect(() => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i felt like this is an antipattern of synchronizing props from useExportData to ExportController.

we already have NanoEvent style of communicatings between events.
and DSExportRequest is emitting loadstart and loadend.
can we subscribe to this somewhere?
seems not quite right to break the nanoevents pattern and sync using useEffect for this.
Am I wrong?

r0b1n added 3 commits August 20, 2026 15:49
- Narrow AfterExportArgs.status to "success" | "aborted" union type
- Move onBeforeExport callback before handler(req) to match spec ordering
- Update openspec artifacts to replace filterCondition with fileName/sheetName
  and document the intentional removal of filterCondition
@r0b1n
r0b1n force-pushed the feat/datagrid-export-events branch from 5134ce1 to 5bace00 Compare August 20, 2026 13:49
@github-actions

Copy link
Copy Markdown
Contributor

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge


What was reviewed

File Change
packages/pluggableWidgets/datagrid-web/CHANGELOG.md New [Unreleased] entry for export event actions
packages/pluggableWidgets/datagrid-web/src/Datagrid.xml Two new onBeforeExport / onAfterExport action properties with <actionVariables>
packages/pluggableWidgets/datagrid-web/typings/DatagridProps.d.ts Updated to include typed ActionValue props and preview stubs
packages/pluggableWidgets/datagrid-web/src/features/data-export/DSExportRequest.ts Private loaded/limit renamed to _loaded/_limit; public getters added
packages/pluggableWidgets/datagrid-web/src/features/data-export/ExportController.ts BeforeExportArgs/AfterExportArgs types; beforeexport/afterexport NanoEvents; public on(); name constructor param; emit calls in exportData()
packages/pluggableWidgets/datagrid-web/src/features/data-export/useDataExport.ts Refs for latest ActionValue; two subscription effects wiring props to controller
packages/pluggableWidgets/datagrid-web/src/features/data-export/__tests__/ExportController.spec.ts New: 5 unit tests for ExportController callback behavior
packages/pluggableWidgets/datagrid-web/openspec/changes/datagrid-export-events/** OpenSpec artifacts (design, proposal, spec, tasks) — informational only

Skipped (out of scope): dist/, pnpm-lock.yaml, openspec/ documentation files

CI: Several checks (sigridci, Run code quality check, Plan E2E matrix, AI Code Review) were still IN_PROGRESS at review time. The completed checks (triage, Check SHA in GH Actions, Read versions file, Snyk code/license/security) all show SUCCESS. Confirm all checks are green before merging.


Findings

⚠️ Low — Missing unit tests for useDataExport wiring

File: packages/pluggableWidgets/datagrid-web/src/features/data-export/useDataExport.ts lines 52–76
Problem: The two new useEffect subscription blocks are untested. The canExecute guard, the new Big(...) wrapping for chunkSize/exportedItemCount, and the subscription cleanup (unsubscribe on unmount) have no unit-test coverage. The ExportController is well-tested, but the wiring layer that translates NanoEvent args into ActionValue.execute() calls is not.
Fix: Add a useDataExport.spec.ts (or extend an existing one) using RTL + actionValue() from @mendix/widget-plugin-test-utils. Verify that calling controller.emit("beforeexport", args) results in action.execute being called with the correct payload, and that the subscription is removed when the component unmounts.


⚠️ Low — Unnecessary optional chain on entry in subscription effects

File: packages/pluggableWidgets/datagrid-web/src/features/data-export/useDataExport.ts lines 52, 65
Problem: entry?.controller.on(...)entry is initialized via useState(() => createEntry(...)) which always returns a value, so entry is never undefined. The optional chain silently returns undefined from the effect, which suppresses the unsubscribe cleanup function. React handles undefined cleanup gracefully, but relying on that hides the fact that the cleanup is always expected to be a function here.
Fix:

// before:
return entry?.controller.on("beforeexport", args => { ... });

// after:
return entry.controller.on("beforeexport", args => { ... });

⚠️ Low — ExportController tests do not assert payload field values

File: packages/pluggableWidgets/datagrid-web/src/features/data-export/__tests__/ExportController.spec.ts lines 638–646
Problem: The onAfterExport success test only checks status: "success" via objectContaining. Fields like gridName, columnTitles, chunkSize, fileName, and sheetName are not asserted anywhere. The makeController() helper already sets up two named columns (Col1, Col2) and a grid name of "test-grid", so it's straightforward to assert that columnTitles is "Col1,Col2" and gridName is "test-grid".
Fix: Extend at least one test to check the full payload shape:

expect(onAfter).toHaveBeenCalledWith(expect.objectContaining({
    status: "success",
    gridName: "test-grid",
    exportedItemCount: 10
}));

⚠️ Low — CHANGELOG entry uses first-person voice

File: packages/pluggableWidgets/datagrid-web/CHANGELOG.md line 11
Problem: - We added two optional export event actions… — Keep a Changelog convention (and the existing entries in this file) uses past-tense imperative without "We" (e.g., - Added two optional export event actions…).
Fix: Drop "We":

- Added two optional export event actions — **On before export** and **On after export** — so developers can log export operations via a microflow or nanoflow. ...

Positives

  • AfterExportArgs extends BeforeExportArgs is clean DRY typing — no field duplication.
  • startTime = new Date() is captured once before beforeexport and reused in afterexport, exactly matching the spec requirement that both callbacks receive the identical timestamp.
  • The NanoEvents approach for beforeexport/afterexport is consistent with the existing sourcechange/abort/exportend pattern — no parallel mechanism introduced.
  • canExecute is correctly checked before every action.execute() call in useDataExport.ts.
  • The ref pattern (useRef updated every render, subscription created once on [entry]) is the correct solution for "always call latest ActionValue without resubscribing on each render".
  • XML actionVariable keys are all lowerCamelCase and align with the TypeScript BeforeExportArgs/AfterExportArgs types and the execute() call sites.
  • afterEach(jest.clearAllMocks) is present in the spec — no test pollution.
  • Five targeted ExportController tests cover the ordering constraint, status mapping, startTime identity, and the no-callback baseline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants