Feat/plugin simulation - #346
Open
HendrikThePendric wants to merge 21 commits into
Open
Conversation
Adds a dev-only Vite middleware in vite-extensions.config.mts that serves src/plugin-host/index.html at /plugin-host.html, transformed through server.transformIndexHtml with its entry module rewritten to an absolute /@fs path. This proves HTML can be served from outside Vite's root (.d2/shell/) on the same origin as the app and the plugin, which the rest of the plugin host simulator design depends on. The page content is a placeholder for now; later tasks build the real host page. Also adds an eslint import guard so app code cannot accidentally pull the dev-only plugin host (or dashboard-plugin entry points) into the app bundle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces nine loosely-typed `filters` props (Record<string, unknown>, Record<string, string>, Record<'relativePeriodDate', string>) across PluginWrapper, its line-list/pivot-table plugins, their analytics hooks, and the dashboard plugin with a single HostFilters type. It models the two hosts that populate this prop: the InterpretationModal in @dhis2/analytics (relativePeriodDate) and the dashboard (ou, pe, yourDimensions). Types only, no behavior change. Also removes YOUR_DIMENSION_TYPES and YourDimensionType, which were unused and meant something narrower (ORGANISATION_UNIT_GROUP_SET only) than the new yourDimensions field, to avoid confusion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`pnpm test` reported 68 failing files and `pnpm lint` failed outright, both caused by the gitignored `opensrc/` reference clones that `npx opensrc` creates. Exclude that directory from vitest and stylelint so both suites report only this repo's own code. Also: - gitignore `.playwright-cli` session artifacts, which tripped Prettier - allow `*.im.dhis2.org` in the sandbox network allowlist - extend the plugin-host import guard to bare directory imports, which the existing patterns missed because they all required a nested segment - include `*.mts` in tsconfig, so vite-extensions.config.mts is type-checked Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds /plugin-host.html, a dev-only page that embeds the real dashboard plugin in an iframe via @dhis2/app-runtime's Plugin component, with a picker over a fixed set of visualizations. This lets the plugin be exercised end to end (real DHIS2 API, real analytics data) from a Cypress spec that reads into the iframe. Getting this working surfaced two pre-existing gaps in the plugin bundle that made it impossible to render real data before: - dashboard-plugin.tsx never wrapped its tree in a Redux Provider, so any plugin code using the app's Redux hooks (e.g. LineListPlugin's useAppDispatch) threw immediately. Added AppCachedDataQueryProvider and StoreProvider, matching what AppWrapper sets up for the main app. - dashboard-plugin.tsx fetched the visualization with useDataQuery, which freezes its query definition on first render; switching to a different visualization id was silently ignored. Replaced it with useRtkQuery, which re-keys correctly on argument changes and also drops the no-restricted-imports eslint escape hatch. - .pluginWrapper had no inline-size rule, so inside a flex ancestor (the app shell's wrapper div) it collapsed to 0 width. PivotTable measures its parent and renders nothing at 0x0. Added inline-size to match the existing block-size treatment. The visualization picker is a native <select>, not a DHIS2 UI SingleSelectField: that component has no underlying <select> element for Cypress's cy.select() to target. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The "swaps the rendered visualization" test only asserted that a table thead was visible after the second selection, which is true whether or not the swap happened, since the first visualization's thead is already visible at that point. Reverting the useRtkQuery fix in dashboard-plugin.tsx back to useDataQuery still passed all three tests. Compare the thead th count before and after the switch instead. Column count is the discriminator, not header text, because it doesn't depend on translations or label edits and both fixtures are LINE_LIST with substantially different column sets (7 vs 21, confirmed against the API). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds org unit, period and dimension selects plus a relativePeriodDate field to the dev-only plugin host, so a developer can exercise the HostFilters contract without a real dashboard. buildHostFilters is the pure function that turns control state into HostFilters; only relativePeriodDate reaches the analytics request today, the other filters are forwarded to the plugin and deliberately ignored pending a product decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The app applies no dashboard filters (matching line-listing-app), but until now gave no indication of that. Add a dismissible notice, shown whenever the host passes an ou, pe, or yourDimensions filter, that tells the user those filters were ignored. relativePeriodDate is excluded from the trigger since it is actually applied. The notice mirrors dashboard-app's Cover + IconInfo24 + message + button treatment, with wording that covers pivot tables as well as line lists. It renders alongside both plugin types in PluginWrapper and resets its dismissal whenever the filters change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…never applied getBaseRequestIdentity folded the whole dashboard filters object into the request identity, even though only relativePeriodDate ever reaches the analytics request. So an ou, pe, or yourDimensions change (never applied) still remounted the canvas, discarding the user's sort and page, and refetched byte-identical data. Narrow the identity to relativePeriodDate in both getBaseRequestIdentity implementations, and derive a filters value in PluginWrapper containing only that field to pass down to the plugins, so their fetch effects don't retrigger on the other fields either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README claimed the plugin simply "will not render a visualization" when opened without a host. Tested behavior is different: it throws (No handler found for post message: getPropsFromParent, then a TypeError reading id of undefined) and shows a "There was a problem loading this plugin" error screen. Describe that instead, and note it's expected and harmless, so it doesn't look like a real bug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…spinner Review round 1 on the filters-not-applied notice found five issues: - The notice's near-opaque Cover sat on top of the canvas spinner (both were unconditional siblings), fully hiding it while a freshly-mounted visualization loaded with an unapplied filter set. Fixed by adding an isLoading prop that suppresses the notice without unmounting it, so its dismissal state survives the load and it reappears afterwards only if still applicable. - Added role="status" and aria-live="polite" so screen readers announce the notice; Cover doesn't forward arbitrary props, so this goes on the inner message div instead. - Added the yourDimensions-key-present-but-empty-array case to hasUnappliedFilters's test coverage. - query-tools.spec.ts now asserts ou/pe/yourDimensions do NOT change the request identity, so the relativePeriodDate-only narrowing is actually guarded by a test instead of being reachable-in-theory-only (plugin-wrapper.tsx already narrows before calling it). - The Cypress relativePeriodDate-only-doesn't-warn test now computes the year from new Date() instead of hardcoding 2026, following the pattern in rename.cy.ts/save.cy.ts/delete.cy.ts/interpretations.cy.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Repo owner decided both strings: the message now names "Event visualizations" instead of listing line list and pivot table separately, and the button reads "Show without filters" for parity with dashboard-app's existing (already translated) string. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds a free-text visualization-id input to the plugin host page so tests can drive an unknown id, since the fixture picker only offers known-good visualizations. Uses this to cover two behaviours only reachable through a real iframe: the canvas error renders with a working Retry, and the iframe honours the fixed height the host passes instead of growing with content, letting taller content scroll inside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The retry test only re-checked the error text after clicking Retry, which was already true before the click, so a Retry button wired to a no-op would still pass. Intercepts the eventVisualizations fetch (where a bad id actually fails) and asserts the request count increases after the click, which fails against a stubbed no-op handler and passes against the real one. Also adds a Clear button next to the pasted-id field: once it holds text it silently overrides the dropdown, with no visible way back short of clearing it by hand. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
src/dashboard-plugin.tsx had no test of its own even though three of the five bugs found on this branch lived in it (no Redux Provider, a frozen useDataQuery, plus a CSS bug in a component it renders). This adds a fast Vitest spec covering the part below the iframe: raw API response through normalizeApiSavedVisualization and the metadata store, into PluginWrapper. The third test switches visualization ids on an existing render and checks the rendered column count updates -- this is the regression test for the stale-visualization bug (useDataQuery freezes its query on first render). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The component config had no specPattern, so Cypress fell back to its
default `**/*.cy.{js,jsx,ts,tsx}` and matched every spec under
`opensrc/` — 52 files from the line-listing-app and dashboard-app
sources that `npx opensrc` fetches for reference. Those are e2e specs,
so running them as component tests fails or hangs.
The component suite now runs 9 specs / 61 tests in ~9s.
The e2e config was never affected; it already scopes to
`cypress/e2e/**/*.cy.ts`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every other tool skips `opensrc/` — the third-party sources `npx opensrc` fetches for reference — because of an explicit allowlist or ignore entry. Prettier skipped it only because it happens to honour .gitignore, which this file's own comment records as surprising behaviour that changed once .prettierignore was added. Make it explicit like the rest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lter toggle Product decided dashboard filters are never applied to Event visualizations, so the host page's second visualization picker and its elaborate org unit/period/dimension/relativePeriodDate controls only obscured that. Replace them with a single visualization id input and a single filter checkbox that toggles a fake, unresolved org unit filter just to trigger the "not applied" notice. Deletes filter-controls.tsx, build-filters.ts, its spec, and the fixtures file they used. Updates the three plugin-host Cypress specs to drive the page through the new controls, drops the relativePeriodDate coverage (kept as a unit-tested case in hasUnappliedFilters), and adds a cy.intercept-forced 500 on the analytics request as a second error variant. Updates the README section to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PluginWrapper accepts isInDashboard but nothing ever set it to true, even in the real dashboard, leaving legend-key.tsx's dashboard-specific branch unreachable in production. The dashboard plugin entry point is exactly where it belongs, so it's now hardcoded there. Adds a regression test asserting the plugin entry passes isInDashboard, via a thin spy wrapper around the real PluginWrapper so the other tests in the file still render the actual line list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
query.params.dimension.includes('qrur9Dvnyt5:GE:5:LE:10') never matched:
dimension strings are stage-prefixed (e.g.
"Zj7UnCAulEk.qrur9Dvnyt5:GE:5:LE:10"), and Array.prototype.includes needs
an exact element match. Every test in this file silently got
analyticsResponse2 regardless of which visualization it requested.
Switches to the same .some(d => d.includes(...)) substring check already
used in dashboard-plugin.spec.tsx. Confirmed the swap now works correctly
(visualization 1 gets its own 7-column response, not visualization 2's
11-column one); none of the existing assertions here check response
identity, so all four tests still pass unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Plugin` from @dhis2/app-service-plugin memoizes the props it forwards on their values, then re-notifies the iframe whenever that memo changes. The host page passed `visualization` as a fresh object literal, so it changed identity on every host render and resent `updated` even when nothing meaningful had changed — re-triggering the plugin's analytics fetch. dashboard-app's IframePlugin memoizes its whole prop bag; this matches it, so the simulator imitates the real parent instead of over-notifying it. Verified: before, typing a trailing space into the id field (a state change that leaves the trimmed id and the filter toggle untouched) fired a fresh, byte-identical analytics request. After, it fires none. This does not remove the duplicate fetch on a *legitimate* prop change, such as toggling the filter. That one is upstream: PluginLoader rewraps `alertsAdd` into a new closure on every `updated` message, and @dhis2/app-service-data's DataProvider builds a new DataEngine on every render, so the plugin's fetch callback is invalidated whenever the host sends anything at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
🚀 Deployed on https://pr-346.event-visualizer.netlify.dhis2.org |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements DHIS2-XXXX
Description
text
Quality checklist
Add N/A to items that are not applicable and check them.
ToDos
Known issues
Screenshots
supporting images