[AGILE-360] Review suggestions: signal inputs and scoped chart plugins

[myabc]

Note

👨🏻 For @EinLama's consideration: merge/cherry-pick or throw away as necesasary.

Ticket

https://community.openproject.org/wp/AGILE-360

What are you trying to accomplish?

Companion PR to #24796 with two concrete suggestions from the review, offered as commits to cherry-pick or merge into the feature branch if you agree with them.

1. Signal inputs on WorkPackageOverviewGraphComponent (dc42957bfb6)

The component is registered via createCustomElement, and Angular Elements already maps dasherised attribute names onto inputs and applies input transforms. So the manual getAttribute(...) ?? getAttribute('data-...') parsing in ngOnInit (including the block added for show-group-by-options) can go entirely: booleanAttribute covers global-scope/show-group-by-options, a small JSON transform covers initial-filters, and groupBy becomes a model() since the select writes to it.

On the Rails side the callers now pass plain attributes instead of inputs: (which JSON-encodes into data-* attributes that Angular Elements never observes) — the same way the burndown widget in #24796 already passes chart-data.

2. Chart-scoped datalabels instead of global registration (b8489ccd3cd)

The plugin leak fixed by 38a2dbbc94b is that withDefaultRegisterables(ChartDataLabels, ...) put datalabels into Chart.js's global registry, so every chart on the page got it. Registering everything globally in init-chartjs.ts and then flipping Chart.defaults.set('plugins.datalabels', { display: false }) plus an explicit display: true opt-in is two global overrides to cure one global leak.

ng2-charts' BaseChartDirective has a plugins input for exactly this: inline plugins are scoped to that chart instance and never touch the registry. The genuinely shared pieces (registerables, PrimerColorsPlugin) are provided once at the root via provideCharts, which is how ng2-charts expects to be configured — registration still runs lazily in the first BaseChartDirective constructor.

This also removes the init-chartjs.ts import from main.ts: esbuild only splits on dynamic import(), so a static side-effect import from the entry file pins chart.js to the main chunk for good. Today chart.js lands in main anyway because app.module.ts statically imports every chart component, but that is an accident we want to undo, and chart setup should live inside the chart dependency graph rather than the app entry point.

What approach did you choose and why?

See above — each commit message carries the reasoning. Verified with ngc (AOT template check), tsc, ESLint, and the widget component specs. One thing worth a visual check before merging: datalabels still rendering on the WP graph and absent on the burndown chart, since the inline [plugins] binding is the one behavioural change not covered by compile or specs.

Merge checklist

• Added/updated tests
• Added/updated documentation in Lookbook (patterns, previews, etc)
• Tested major browsers (Chrome, Firefox, Edge, ...)

[github-actions]

Caution

The provided work package version does not match the core version

Details:

• Work package URL: https://community.openproject.org/wp/AGILE-360
• Work package version: 18.0.0
• Core version: 17.8.0

Please make sure that:

• The work package version OR your pull request target branch is correct

[Copilot]

Pull request overview

This PR refines the AGILE-360 work package overview graph integration by (1) switching the custom element to Angular signal-based inputs (so Rails callers can pass plain attributes instead of data-* inputs:) and (2) scoping Chart.js datalabels usage to only the relevant chart instances (removing the prior global Chart.js side-effect initialization).

Changes:

• Update WorkPackageOverviewGraphComponent to use input()/model() signals and consume attributes directly (global-scope, initial-filters, show-group-by-options).
• Remove global Chart.js initialization (init-chartjs.ts) and register shared Chart.js pieces via provideCharts, while passing datalabels as an inline per-chart plugin.
• Adjust Rails widget renderers + specs to pass/read plain attributes instead of data-* dataset attributes.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
modules/backlogs/spec/components/backlogs/sprint_reports/widgets/work_package_graph_spec.rb	Updates spec expectations to read plain attributes (global-scope, initial-filters, etc.).
modules/backlogs/app/components/backlogs/sprint_reports/widgets/work_package_graph.rb	Switches widget rendering to plain attributes and JSON-serializes initial-filters.
frontend/src/main.ts	Drops the global side-effect import of Chart.js initialization.
frontend/src/app/shared/components/work-package-graphs/overview/wp-overview-graph.template.html	Updates template bindings to call signal inputs (showGroupByOptions(), chartOptions()).
frontend/src/app/shared/components/work-package-graphs/overview/wp-overview-graph.component.ts	Migrates inputs to signals and removes manual attribute parsing in ngOnInit.
frontend/src/app/shared/components/work-package-graphs/embedded/wp-embedded-graph.html	Passes plugins into the chart directive for per-chart plugin scoping.
frontend/src/app/shared/components/work-package-graphs/embedded/wp-embedded-graph.component.ts	Scopes chartjs-plugin-datalabels to this chart instance via a local plugins array.
frontend/src/app/core/setup/init-chartjs.ts	Deletes the prior global Chart.js registration/defaults side-effect module.
frontend/src/app/app.module.ts	Registers Chart.js registerables + Primer colors plugin via provideCharts(...).
app/components/projects/settings/versions/work_packages_graph_widget_component.rb	Switches widget rendering to plain attributes and JSON-serializes initial-filters.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Warning

Flaky specs

• rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]
• rspec ./modules/wikis/spec/features/admin/internal_provider_spec.rb[1:1]
• rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24892, linked for reference only):

- `rspec ./modules/gantt/spec/features/timeline/timeline_dates_spec.rb[1:2:1]`
- `rspec ./modules/wikis/spec/features/admin/internal_provider_spec.rb[1:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #24892. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24892 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @myabc to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @myabc, and request a review from @myabc.
On every commit, set @myabc as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.