Skip to content

[OP-20347] Frontend typings: WP table and context menus - #25665

Open
myabc wants to merge 24 commits into
implementation/op-20349-eslint-typing-fix-hal-index-signaturefrom
implementation/op-20347-eslint-typing-fix-wp-tables-menus
Open

myabc wants to merge 24 commits into
implementation/op-20349-eslint-typing-fix-hal-index-signaturefrom
implementation/op-20347-eslint-typing-fix-wp-tables-menus

Conversation

@myabc

@myabc myabc commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Ticket

https://community.openproject.org/wp/OP-20347

What are you trying to accomplish?

Part of OP-20341, second of two pull requests for this ticket: removes ESLint typing violations from the work package table, fast table, card view and context menus, without changing behaviour. #25664 covers the work package views.

  • Types work package context menu actions, context menu locals and handlers.
  • Types the table configuration, timeline callbacks and the timeline cell map.
  • Types fast table and card view handlers.

wp-table, wp-fast-table, wp-card-view and op-context-menu go from 97 typing violations to 0; application-code typing violations drop from 531 to 434.

What approach did you choose and why?

Changes are annotations and boundary casts; no narrowing guards were added. WorkPackageAction is unchanged, so the typed hook signatures still hold. Group sums are cast to the resource type the display field service already receives at runtime.

Small behaviour-preserving edits: WorkPackageEmbeddedBaseComponent#refresh returns void like the abstract it overrides, since no caller reads the promise; a private helper loses its leading underscore.

When this and #25664 are both merged, the local casts of GroupObject.value and of the card view handler token here may become redundant, because #25664 types both at the source; they can be dropped then.

Stacked on #25661, which makes the HalResource index signature unknown. Independent of #25662, #25663 and #25664.

AI involvement

Directed – I specified the requirements and AI implemented most of it; I validated via testing rather than a full line-by-line review.

Merge checklist

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

Browser JavaScript diff (5 files, type-only changes omitted)

Each changed browser file (TypeScript and templates, excluding specs, test helpers and declaration files) is transpiled on its own with frontend/tsconfig.json, before and after; this is the emitted JavaScript that differs.

diff --git frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-rows-helpers.js
index 02146f0..cc7945f 100644
--- frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-rows-helpers.js
+++ frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/grouped-rows-helpers.js
@@ -12,7 +12,7 @@ export function groupIdentifier(group) {
     return `${groupByProperty(group)}-${value}`;
 }
 export function groupName(group) {
-    const { value } = group;
+    const value = group.value;
     if (value === null) {
         return '-';
     }
diff --git frontend/src/app/features/work-packages/components/wp-table/configuration-modal/tab-portal-outlet.js
index 4a5ed1b..8d3bbbf 100644
--- frontend/src/app/features/work-packages/components/wp-table/configuration-modal/tab-portal-outlet.js
+++ frontend/src/app/features/work-packages/components/wp-table/configuration-modal/tab-portal-outlet.js
@@ -20,7 +20,7 @@ export class TabPortalOutlet {
         this.detach();
         const instance = this.activateInstance(tab);
         this.outletElement.innerHTML = '';
-        this.outletElement.appendChild(this._getComponentRootNode(instance.componentRef));
+        this.outletElement.appendChild(this.getComponentRootNode(instance.componentRef));
         this.outletElement.dataset.tabName = tab.name;
         this.currentTab = instance;
     }
@@ -60,7 +60,7 @@ export class TabPortalOutlet {
             },
         };
     }
-    _getComponentRootNode(componentRef) {
+    getComponentRootNode(componentRef) {
         return componentRef.hostView.rootNodes[0];
     }
 }
diff --git frontend/src/app/features/work-packages/components/wp-table/embedded/wp-embedded-base.component.js
index 5dd9ee1..b0bafb0 100644
--- frontend/src/app/features/work-packages/components/wp-table/embedded/wp-embedded-base.component.js
+++ frontend/src/app/features/work-packages/components/wp-table/embedded/wp-embedded-base.component.js
@@ -68,7 +68,6 @@ let WorkPackageEmbeddedBaseComponent = class WorkPackageEmbeddedBaseComponent ex
         if (visible) {
             this.loadingIndicator = promise;
         }
-        return promise;
     }
     get isInitialized() {
         return !!this.configuration;
diff --git frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.js
index 687a561..de3233b 100644
--- frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.js
+++ frontend/src/app/shared/components/op-context-menu/handlers/op-columns-context-menu.directive.js
@@ -66,7 +66,7 @@ let OpColumnsContextMenu = class OpColumnsContextMenu extends OpContextMenuTrigg
                 hidden: !this.wpTableSortBy.isSortable(c),
                 linkText: this.I18n.t('js.work_packages.query.sort_descending'),
                 icon: 'icon-sort-descending',
-                onClick: (evt) => {
+                onClick: () => {
                     if (this.wpTableSortBy.isManualSortingMode) {
                         this.confirmDialog.confirm({
                             text: this.text.confirmDelete,
@@ -84,7 +84,7 @@ let OpColumnsContextMenu = class OpColumnsContextMenu extends OpContextMenuTrigg
                 hidden: !this.wpTableSortBy.isSortable(c),
                 linkText: this.I18n.t('js.work_packages.query.sort_ascending'),
                 icon: 'icon-sort-ascending',
-                onClick: (evt) => {
+                onClick: () => {
                     if (this.wpTableSortBy.isManualSortingMode) {
                         this.confirmDialog.confirm({
                             text: this.text.confirmDelete,
diff --git frontend/src/app/shared/components/op-context-menu/handlers/wp-view-dropdown-menu.directive.js
index c9ab34b..afe804e 100644
--- frontend/src/app/shared/components/op-context-menu/handlers/wp-view-dropdown-menu.directive.js
+++ frontend/src/app/shared/components/op-context-menu/handlers/wp-view-dropdown-menu.directive.js
@@ -35,7 +35,7 @@ let WorkPackageViewDropdownMenuDirective = class WorkPackageViewDropdownMenuDire
                 linkText: this.I18n.t('js.views.card'),
                 title: this.I18n.t('js.button_show_cards'),
                 icon: 'icon-view-card',
-                onClick: (evt) => {
+                onClick: () => {
                     this.isOpen = false;
                     this.wpDisplayRepresentationService.setDisplayRepresentation(wpDisplayCardRepresentation);
                     if (this.wpTableTimeline.isVisible) {
@@ -50,7 +50,7 @@ let WorkPackageViewDropdownMenuDirective = class WorkPackageViewDropdownMenuDire
                 linkText: this.I18n.t('js.views.list'),
                 title: this.I18n.t('js.button_show_table'),
                 icon: 'icon-view-list',
-                onClick: (evt) => {
+                onClick: () => {
                     this.isOpen = false;
                     this.wpDisplayRepresentationService.setDisplayRepresentation(wpDisplayListRepresentation);
                     if (this.wpTableTimeline.isVisible) {
@@ -65,7 +65,7 @@ let WorkPackageViewDropdownMenuDirective = class WorkPackageViewDropdownMenuDire
                 linkText: this.I18n.t('js.views.timeline'),
                 title: this.I18n.t('js.button_show_gantt'),
                 icon: 'icon-view-timeline',
-                onClick: (evt) => {
+                onClick: () => {
                     this.isOpen = false;
                     if (!this.wpTableTimeline.isVisible) {
                         this.wpTableTimeline.toggle();

@github-actions

Copy link
Copy Markdown

Warning

Flaky specs

  • rspec ./spec/features/roles/report_spec.rb[1:1]
  • rspec ./spec/features/roles/report_spec.rb[1:2]
  • rspec ./spec/features/roles/report_spec.rb[1:3]
🤖 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 #25665, linked for reference only):

- `rspec ./spec/features/roles/report_spec.rb[1:1]`
- `rspec ./spec/features/roles/report_spec.rb[1:2]`
- `rspec ./spec/features/roles/report_spec.rb[1:3]`

Treat this as a standalone task, unrelated to PR #25665. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #25665 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.

Turns off no-explicit-any and the no-unsafe-* rules for specs and the
test helpers that are kept out of the production bundle. Test doubles
and expectation helpers routinely work with loosely typed values, and
the typing effort under OP-20341 targets application code only.

The two no-unsafe exemptions that lived in the vitest spec block move
into the new block, which covers the helper files as well.

Removes the inline disables in specs and test setup that the exemption
makes redundant.

https://community.openproject.org/wp/OP-20342
Removes the api.v3 Result, Collection and Duration interfaces, the
Factory global and the op.QueryParams namespace. Nothing references
them, and most declared their members as any.

https://community.openproject.org/wp/OP-20342
Declares the tablesorter call and its language defaults, and the
metadata plugin slot the reporting page clears, instead of any. The
reporting controller no longer needs its inline disables for them.

https://community.openproject.org/wp/OP-20342
Adds a HookSignatures map for the hooks the core registers and calls,
so register and call check callback parameters and return typed
results instead of any. Arbitrary hook names still fall back to an
open signature, because plugins register and call hooks by name.
The fallback excludes the known names, so a mismatched callback or
argument for a known hook fails to compile; the spec pins that down.

The bulk context menu actions are now declared as WorkPackageAction,
which the typed hook result exposed as narrower than its use.

https://community.openproject.org/wp/OP-20342
Types the dynamic component output maps with ng-dynamic-component's
EventHandler, which is what ndcDynamicOutputs accepts, and the checksum
callback as a plain thunk. The unconstrained Function type accepted
any callable and let calls through unchecked.

https://community.openproject.org/wp/OP-20342
Deletes WorkPackageResource#updateLinkedResources. It has no callers
and calls an undeclared wpCacheService, so it only compiled because
of the resource's `any` index signature.

https://community.openproject.org/wp/OP-20344
Makes HalLink#$fetch and #$callable generic and gives CallableHalLink
its call signature and a HalLink-typed $link, so link functions no
longer rely on the global `Function.$link?:any` augmentation, which is
removed.

HalResource#$links is now a map of CallableHalLink and #$embedded a map
of unknown values. Resource-specific link interfaces declare their
action links as CallableHalLink<T> instead of `Promise<any>` methods,
and are intersected with the base maps on `$links`/`$embedded` so the
resource interfaces they are merged into keep the `any` index
signature until it is flipped separately.

WorkPackageResourceEmbedded drops its `HalResource|any` unions and the
link interface no longer inherits the embedded attributes. The unused
QueryFilterResource embedded declaration is removed.

Callers that relied on the untyped maps get boundary casts.

https://community.openproject.org/wp/OP-20344
Types HalResource#$source as HalSource and accepts an unknown source in
the constructor and $initialize, which already unwrap a HalResource
passed in place of a raw source. The initializer callback takes a
HalResource, $copy and $plain drop their `any`, and lazy() becomes
generic over the value it defines.

initializeHalProperties now reads the source through typed views of
`_links` and `_embedded` and passes the target maps to setupProperty
directly instead of looking them up by a computed `$`-name, removing
the builder's `any` casts. Runtime behaviour is unchanged.

Callers and specs that relied on the untyped source get boundary
casts.

https://community.openproject.org/wp/OP-20344
Types ResourceChangeset#schema and WorkPackageChangeset#schema as
ISchemaProxy, which is what both getters return. FormResource#schema
stays the embedded form schema and gets its own FormSchemaResource
type, a SchemaResource whose attributes are field schemas, and its
commit link becomes a CallableHalLink.

IFieldSchema is now derived from IOPFieldSchema instead of duplicating
it with `any`-typed allowedValues and options. Fields that load
allowed values through the link cast it at the boundary.

SchemaProxy dispatches its proxied methods by name through
proxyMethod, which keeps them bound to the handler and still honours
subclass overrides, instead of wrapping unbound method references in a
Function proxy. The payload helper checks own schema keys with
Object.hasOwn.

https://community.openproject.org/wp/OP-20344
Declares attributes and link callables that callers already read
through the `any` index signature: FormResource#payload, #commit and
#configureForm, QueryResource#star, #unstar, #icalUrl and #updatedAt,
TimeEntryResource#hours, WorkPackageCollectionResource#customFields
and #createWorkPackage, and WorkPackageResource#configureForm and
#bcfViewpoints. WorkPackageResource#description becomes Formattable and
RelationResource#type a string.

Class and interface declaration merges are replaced by member
declarations on the class (with `implements` for the exported shape),
which emit no code under useDefineForClassFields: false. Resource
state getters cast through unknown instead of `any`.

Callers whose values are now typed get boundary casts.

https://community.openproject.org/wp/OP-20344
Types the request data, created sources and registered classes in
HalResourceService, the HTTP client options and interceptor, and
ErrorResource's errors and details. HalResourceNotificationService
takes ErrorResource where it reads error attributes and treats raw
responses as unknown.

HalError#resource is nullable, so the two callers passing it to
showError assert it at the boundary, as the service already assumed.

HTTPClientParamMap keeps its `any` values: query and BCF callers pass
untyped objects as request params.

https://community.openproject.org/wp/OP-20344
Types the remaining `any` members of the query filter, filter schema,
schema dependency and grid resources, lets attachable resources read
the now typed addAttachment link directly, and casts the two schema
and parent reads that go through the resource index signature.

https://community.openproject.org/wp/OP-20344
Removes type assertions, eslint-disable directives and imports that
the typed HAL links, sources, schemas and resource attributes made
unnecessary, and marks the two now typed $load() calls whose promises
were already discarded with `void`.

https://community.openproject.org/wp/OP-20344
Declares the base link map as holding a callable link, an array of
links, or nothing, since the builder resolves array-valued HAL links
to arrays and resources only carry the links the API sent. The self
link is always present, as the builder creates one when missing.

Call sites that read a link they have already checked, or that the
resource always carries, cast it at the point of use.

https://community.openproject.org/wp/OP-20344
Casts the resource attributes that display and edit fields read by
name at the point of use, keeping today's values. These reads compiled
only because HalResource declared an any index signature, which the
next commit removes.

https://community.openproject.org/wp/OP-20349
Casts work package attributes and raw links that views and services
read without a declared type, keeping today's behaviour, ahead of the
HalResource index signature becoming unknown.

https://community.openproject.org/wp/OP-20349
Casts undeclared resource attributes read in grids, context menus,
modals, search, BCF, the team planner and the costs and GitLab plugin
modules, keeping today's behaviour, ahead of the HalResource index
signature becoming unknown.

https://community.openproject.org/wp/OP-20349
Casts the resource attribute a spec reads by name, since specs are
typechecked even though they are exempt from the typing lint rules.

https://community.openproject.org/wp/OP-20349
Declares the HalResource index signature as unknown instead of any.
The any signature let every undeclared attribute read skip type
checking, which a code comment had tracked as a source of bugs since
2019 (#31462). Reads of undeclared attributes now need a declaration
or a cast, as the preceding commits add.

The payload helper picks the current schema with ?? instead of ||; the
value is a schema resource or nullish, so the result is unchanged.

https://community.openproject.org/wp/OP-20349
Types the permitted action constants, the collected action lists and
the bulk action link inputs as WorkPackageAction and
WorkPackageResource instead of any. The bulk link keeps assuming that
every bulk action carries an href, now as an explicit non-null
assertion. The view context menu passes action links on as strings.

https://community.openproject.org/wp/OP-20347
Types the context menu locals token with OpContextMenuLocalsMap and
declares the service that OPContextMenuService injects into the
locals, so the menu component no longer reads it through any. Drops
unused any-typed click event parameters, types the permission action
names as strings and reads the status form through the typed form
schema.

https://community.openproject.org/wp/OP-20347
Types the configuration tab component classes with the CDK
ComponentType, the external query configuration locals token and
injector data with QueryConfigurationLocals, and the table component
text labels with their actual shape. Replaces the remaining any
annotations in the table configuration with unknown where the value is
only passed along.

Declares the embedded table refresh as void, matching the abstract
WorkPackagesViewBase contract. It no longer returns its load promise,
which no caller read; the loading indicator still receives it.

https://community.openproject.org/wp/OP-20347
Resolves WorkPackageTimelineCell in the global IGroupCellsMap
declaration through an import type, which the ambient file could not
see before. Makes the timeline container's common pipe generic, types
the selection mode callbacks as returning void, relation errors as
unknown and milestone date moves as CellDateMovement.

https://community.openproject.org/wp/OP-20347
Types group sums with the GroupObject sums record and casts it at the
display field boundary, where it has always been passed as a resource.
Casts the still untyped group value locally, reads the card view
handler token through a typed provider token, and stringifies the
caught scroll error explicitly.

https://community.openproject.org/wp/OP-20347
@myabc
myabc force-pushed the implementation/op-20349-eslint-typing-fix-hal-index-signature branch from ac71ea0 to 2705f0a Compare September 29, 2026 09:47
@myabc
myabc force-pushed the implementation/op-20347-eslint-typing-fix-wp-tables-menus branch from 948a147 to 6c19193 Compare September 29, 2026 09:47
@myabc myabc added eslint javascript Pull requests that update Javascript code needs review labels Sep 29, 2026
@myabc myabc added this to the 18.0.x milestone Sep 29, 2026
@github-actions github-actions Bot added the ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. label Sep 29, 2026
@myabc
myabc requested a lite review from Copilot September 29, 2026 10:17

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

The broad typing changes span many frontend boundaries and warrant final human review.

Review effort: Lite
Findings: None

What changed in this PR

This PR removes unsafe TypeScript typings across work-package tables, fast tables, card views, timelines, and context menus while preserving behavior.

Changes:

  • Replaces any with domain-specific types and generics.
  • Types callbacks, handlers, configuration, portals, and injection tokens.
  • Aligns refresh and helper signatures with existing contracts.
File Description
frontend/​src/​app/​shared/​components/​op-context-menu/​wp-context-menu/​wp-view-context-menu.directive.ts Types action links.
frontend/​src/​app/​shared/​components/​op-context-menu/​op-context-menu.types.ts Types menu locals and services.
frontend/​src/​app/​shared/​components/​op-context-menu/​op-context-menu.component.ts Uses typed service access.
frontend/​src/​app/​shared/​components/​op-context-menu/​handlers/​wp-view-dropdown-menu.directive.ts Removes unused callback parameters.
frontend/​src/​app/​shared/​components/​op-context-menu/​handlers/​op-settings-dropdown-menu.directive.ts Types loading and authorization actions.
frontend/​src/​app/​shared/​components/​op-context-menu/​handlers/​op-context-menu-trigger.directive.ts Types keyboard callback inference.
frontend/​src/​app/​shared/​components/​op-context-menu/​handlers/​op-columns-context-menu.directive.ts Removes unused callback parameters.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​wp-table.component.ts Types synchronization and localized text.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​wp-table-configuration.ts Types configuration values.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​typings.d.ts Types timeline cell maps.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​timeline/​wp-timeline.ts Types selection callbacks.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​timeline/​container/​wp-timeline-container.directive.ts Types observable pipelines and errors.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​timeline/​cells/​timeline-milestone-cell-renderer.ts Types date movement values.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​external-configuration/​external-query-configuration.service.ts Types component classes and locals.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​external-configuration/​external-query-configuration.constants.ts Types the locals injection token.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​embedded/​wp-embedded-base.component.ts Aligns the refresh return type.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​context-menu-helper/​wp-context-menu-helper.service.ts Types actions and bulk links.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​configuration-modal/​wp-table-configuration.modal.ts Types components and tabs.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​configuration-modal/​tabs/​highlighting-tab.component.ts Types selected attributes.
frontend/​src/​app/​features/​work-packages/​components/​wp-table/​configuration-modal/​tab-portal-outlet.ts Types tab components and portal roots.
frontend/​src/​app/​features/​work-packages/​components/​wp-fast-table/​handlers/​table-handler-registry.ts Types state transformers.
frontend/​src/​app/​features/​work-packages/​components/​wp-fast-table/​builders/​modes/​grouped/​grouped-rows-helpers.ts Types grouped values.
frontend/​src/​app/​features/​work-packages/​components/​wp-fast-table/​builders/​modes/​grouped/​group-sums-builder.ts Types group sums and display-field inputs.
frontend/​src/​app/​features/​work-packages/​components/​wp-card-view/​wp-card-view.component.ts Types handler registry injection.

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

@myabc
myabc marked this pull request as ready for review September 29, 2026 10:26
@myabc
myabc force-pushed the implementation/op-20349-eslint-typing-fix-hal-index-signature branch from 2705f0a to cdd9389 Compare September 29, 2026 18:29

This branch has not been deployed

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

Labels

ai: Directed 🪄 A human specified the requirements and AI implemented most of it; They validated via testing. eslint javascript Pull requests that update Javascript code needs review

Development

Successfully merging this pull request may close these issues.

2 participants