Migrate Backlogs to JavaScript classes, type improvements #19907

myabc · 2025-08-13T15:44:17Z

⚠️ This PR is based off of #19429. Please review/merge that PR first.

Ticket

https://community.openproject.org/wp/66575

What are you trying to accomplish?

Migrates backlogs from Crockford Objects to JavaScript classes.

Screenshots

What approach did you choose and why?

Merge checklist

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

frontend/src/stimulus/controllers/dynamic/backlogs/model.ts

+              let newInput = self.findFactory(typeId, statusId, 'status_id');
+              newInput = self.prepareInputFromFactory(newInput, fieldId, 'status_id', fieldOrder, maxTabIndex);
+              // @ts-expect-error TS(2683): 'this' implicitly has type 'any' because it does n... Remove this comment to see the full error message
+              newInput = self.replaceStatusForNewType(input, newInput, $(this).parent().find('.status_id').val(), editor);


To address the issue, the assignment to newInput on line 193 should be removed, leaving only the function call to self.replaceStatusForNewType(...). This change will not alter the functional behavior of the code but will prevent confusion and clarify that the method is called for its side effect. No additional imports or definitions are necessary. The edit should be made only to the relevant line in the callback within the input.change handler.

Copilot

Pull Request Overview

This PR refactors the legacy backlog JavaScript code from a prototype-based object creation pattern (using the deprecated RB.Object.create() factory) to modern ES6 classes with TypeScript types. The migration modernizes the codebase by:

Converting old-style RB.Factory.initialize() patterns to direct class instantiation using new
Replacing function-based inheritance with ES6 class-based inheritance
Adding proper TypeScript type annotations for jQuery event handlers
Removing @ts-expect-error comments for resolved type issues
Improving code organization with proper imports and exports

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`frontend/src/typings/shims.d.ts`	Adds jQuery plugin type declarations for `colorcontrast` method
`frontend/src/stimulus/controllers/dynamic/backlogs/common.ts`	Converts RB utility objects to classes and adds TypeScript interfaces
`frontend/src/stimulus/controllers/dynamic/backlogs/model.ts`	Converts Model from factory pattern to ES6 class with proper constructor
`frontend/src/stimulus/controllers/dynamic/backlogs/work_package.ts`	Converts WorkPackage to abstract ES6 class extending Model
`frontend/src/stimulus/controllers/dynamic/backlogs/task.ts`	Converts Task to ES6 class extending WorkPackage
`frontend/src/stimulus/controllers/dynamic/backlogs/story.ts`	Converts Story to ES6 class with mixin pattern for EditableInplace
`frontend/src/stimulus/controllers/dynamic/backlogs/sprint.ts`	Converts Sprint to ES6 class with mixin pattern
`frontend/src/stimulus/controllers/dynamic/backlogs/impediment.ts`	Converts Impediment to ES6 class extending Task
`frontend/src/stimulus/controllers/dynamic/backlogs/taskboard.ts`	Converts Taskboard to ES6 class with proper constructor and methods
`frontend/src/stimulus/controllers/dynamic/backlogs/backlog.ts`	Converts Backlog to ES6 class replacing factory initialization
`frontend/src/stimulus/controllers/dynamic/backlogs/burndown.ts`	Converts Burndown to ES6 class
`frontend/src/stimulus/controllers/dynamic/backlogs/editable_inplace.ts`	Converts mixin to TypeScript higher-order function pattern
`frontend/src/stimulus/controllers/dynamic/backlogs/master_backlog.ts`	Updates to use direct class instantiation
`frontend/src/stimulus/controllers/dynamic/backlogs/show_main.ts`	Updates to use direct class instantiation
`frontend/src/assets/sass/backlogs/_taskboard.sass`	Updates comment to reference new class constructor

Copilot · 2025-11-05T23:07:26Z

frontend/src/stimulus/controllers/dynamic/backlogs/backlog.ts

+    });
+
+    // Observe menu items
+    this.$.find('.add_new_story').click(this.handleNewStoryClick);


The method handleNewStoryClick is passed as a callback without binding its context. When jQuery calls this callback, this will refer to the clicked DOM element, not the Backlog instance. This causes this.newStory() on line 136 to fail. Bind the method using an arrow function or .bind(this): this.$.find('.add_new_story').click((e) => this.handleNewStoryClick(e))

Suggested change

this.$.find('.add_new_story').click(this.handleNewStoryClick);

this.$.find('.add_new_story').click((e) => this.handleNewStoryClick(e));

Copilot · 2025-11-05T23:07:26Z

frontend/src/stimulus/controllers/dynamic/backlogs/backlog.ts

+  }
+
+  handleNewStoryClick(e:JQuery.Event) {
+    const toggler = $(this).parents('.header').find('.toggler');


The method uses $(this) expecting it to be the DOM element, but when called as a class method, this refers to the Backlog instance. This conflicts with the callback binding issue in line 76. Either change line 76 to bind properly and use this.$ here, or change the method to be a standalone function that receives the correct context.

Suggested change

const toggler = $(this).parents('.header').find('.toggler');

const toggler = $(e.currentTarget).parents('.header').find('.toggler');

Copilot · 2025-11-05T23:07:26Z

frontend/src/stimulus/controllers/dynamic/backlogs/taskboard.ts

+    this.$.find('#tasks .add_new.clickable').click(this.handleAddNewTaskClick);
+    this.$.find('#impediments .add_new.clickable').click(this.handleAddNewImpedimentClick);


The methods handleAddNewTaskClick and handleAddNewImpedimentClick are passed as callbacks without binding context. When jQuery invokes these callbacks, this will refer to the clicked DOM element, not the Taskboard instance. Both methods use $(this) expecting the DOM element, but also need to access instance methods. Use arrow functions to preserve context: this.$.find('#tasks .add_new.clickable').click((e) => this.handleAddNewTaskClick(e))

Suggested change

this.$.find('#tasks .add_new.clickable').click(this.handleAddNewTaskClick);

this.$.find('#impediments .add_new.clickable').click(this.handleAddNewImpedimentClick);

this.$.find('#tasks .add_new.clickable').click((e) => this.handleAddNewTaskClick(e));

this.$.find('#impediments .add_new.clickable').click((e) => this.handleAddNewImpedimentClick(e));

Copilot · 2025-11-05T23:07:27Z

frontend/src/stimulus/controllers/dynamic/backlogs/taskboard.ts

+    const row = $(this).parents('tr').first();
+    $('#taskboard').data('this').newImpediment(row);
+  }
+
+  handleAddNewTaskClick(e:JQuery.Event) {
+    const row = $(this).parents('tr').first();


These methods expect this to be the DOM element (lines 152, 157 use $(this)), which is inconsistent with ES6 class methods where this refers to the instance. This creates a context mismatch between the implementation and how the methods are registered as callbacks on lines 72-73.

Suggested change

const row = $(this).parents('tr').first();

$('#taskboard').data('this').newImpediment(row);

}

handleAddNewTaskClick(e:JQuery.Event) {

const row = $(this).parents('tr').first();

const row = $(e.currentTarget).parents('tr').first();

$('#taskboard').data('this').newImpediment(row);

}

handleAddNewTaskClick(e:JQuery.Event) {

const row = $(e.currentTarget).parents('tr').first();

Copilot · 2025-11-05T23:07:27Z

frontend/src/stimulus/controllers/dynamic/backlogs/story.ts

+      data += `&${this.$.find('.editor').serialize()}`;
+    }
+
+    //TODO: this might be unsave in case the parent of this story is not the


Corrected spelling of 'unsave' to 'unsafe'.

Suggested change

//TODO: this might be unsave in case the parent of this story is not the

//TODO: this might be unsafe in case the parent of this story is not the

Copilot · 2025-11-05T23:07:27Z

frontend/src/stimulus/controllers/dynamic/backlogs/model.ts

+              let newInput = self.findFactory(typeId, statusId, 'status_id');
+              newInput = self.prepareInputFromFactory(newInput, fieldId, 'status_id', fieldOrder, maxTabIndex);
+              // @ts-expect-error TS(2683): 'this' implicitly has type 'any' because it does n... Remove this comment to see the full error message
+              newInput = self.replaceStatusForNewType(input, newInput, $(this).parent().find('.status_id').val(), editor);


The value assigned to newInput here is unused.

Suggested change

newInput = self.replaceStatusForNewType(input, newInput, $(this).parent().find('.status_id').val(), editor);

self.replaceStatusForNewType(input, newInput, $(this).parent().find('.status_id').val(), editor);

myabc · 2025-11-05T23:14:36Z

@copilot please go through this draft conversion of a very old part of the codebase from "Crockford Objects"/custom Object factories (RB.Object.initialize) to modern JavaScript/TypeScript classes. Look for any possible errors in the conversion and correct please.

You may want to start with the suggestions that you provided as comments in this Pull Request:
#19907 (comment)

Do the following before committing or pushing:

npm run generate-typings
npm run build
npm run test

Although there are no test cases that cover these particular files, this will at least check TypeScript compiles.

Copilot · 2025-11-05T23:14:48Z

@myabc I've opened a new pull request, #20945, to work on those changes. Once the pull request is ready, I'll request review from you.

myabc mentioned this pull request Aug 13, 2025

[#65619] Migrate backlogs to TypeScript #19465

Closed

3 tasks

myabc force-pushed the impl/66574-backlogs-initial-typescript-conversion branch from dc48590 to 8225553 Compare August 13, 2025 16:13

myabc added javascript Pull requests that update Javascript code maintenance DO NOT MERGE labels Aug 13, 2025

myabc force-pushed the impl/66574-backlogs-initial-typescript-conversion branch 2 times, most recently from e9b3034 to 443c01a Compare August 13, 2025 18:34

Base automatically changed from impl/66574-backlogs-initial-typescript-conversion to dev August 15, 2025 09:38

myabc mentioned this pull request Nov 5, 2025

[#65618] Remove jQuery from core #19429

Merged

17 tasks

Rework Crockford Objects as JS/TS classes

7f853c6

myabc force-pushed the impl/66575-backlogs-js-classes branch from fe0ba07 to 7f853c6 Compare November 5, 2025 23:02

myabc changed the base branch from dev to code-maintenance/rid-jquery November 5, 2025 23:03

myabc requested a review from Copilot November 5, 2025 23:03

github-advanced-security bot found potential problems Nov 5, 2025

View reviewed changes

Copilot AI reviewed Nov 5, 2025

View reviewed changes

Copilot AI mentioned this pull request Nov 5, 2025

Fix context binding and runtime errors in Backlogs class migration #20945

Draft

3 tasks

Base automatically changed from code-maintenance/rid-jquery to dev November 6, 2025 12:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrate Backlogs to JavaScript classes, type improvements #19907

Migrate Backlogs to JavaScript classes, type improvements #19907

Uh oh!

myabc commented Aug 13, 2025 •

edited

Loading

Uh oh!

Check warning

Copilot Autofix

Copilot AI left a comment

Uh oh!

Copilot AI Nov 5, 2025

Uh oh!

Copilot AI Nov 5, 2025

Uh oh!

Copilot AI Nov 5, 2025

Uh oh!

Copilot AI Nov 5, 2025

Uh oh!

Copilot AI Nov 5, 2025

Uh oh!

Copilot AI Nov 5, 2025

Uh oh!

myabc commented Nov 5, 2025

Uh oh!

Copilot AI commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

	this.$.find('.add_new_story').click(this.handleNewStoryClick);
	this.$.find('.add_new_story').click((e) => this.handleNewStoryClick(e));

	const toggler = $(this).parents('.header').find('.toggler');
	const toggler = $(e.currentTarget).parents('.header').find('.toggler');

		this.$.find('#tasks .add_new.clickable').click(this.handleAddNewTaskClick);
		this.$.find('#impediments .add_new.clickable').click(this.handleAddNewImpedimentClick);

	//TODO: this might be unsave in case the parent of this story is not the
	//TODO: this might be unsafe in case the parent of this story is not the

	newInput = self.replaceStatusForNewType(input, newInput, $(this).parent().find('.status_id').val(), editor);
	self.replaceStatusForNewType(input, newInput, $(this).parent().find('.status_id').val(), editor);

Migrate Backlogs to JavaScript classes, type improvements #19907

Are you sure you want to change the base?

Migrate Backlogs to JavaScript classes, type improvements #19907

Uh oh!

Conversation

myabc commented Aug 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

What are you trying to accomplish?

Screenshots

What approach did you choose and why?

Merge checklist

Uh oh!

Check warning

Copilot Autofix

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

myabc commented Nov 5, 2025

Uh oh!

Copilot AI commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

myabc commented Aug 13, 2025 •

edited

Loading