refactor: migrate units/states/tasks/tasks.coffee from CoffeeScript to Angular

[Sujay-Deakin]

Description

Migrates units/states/tasks/tasks.coffee from CoffeeScript/AngularJS to TypeScript/Angular
as part of the ongoing Angular migration effort.

Changes

• Created tasks.component.ts with UnitsTasksStateComponent replacing UnitsTasksStateCtrl
• Created tasks.component.html with <ui-view> as the router outlet
• Created tasks.component.scss (no custom styles needed)
• Registered UnitsTasksStateComponent in doubtfire-angular.module.ts
• Downgraded component in doubtfire-angularjs.module.ts for hybrid compatibility
• Removed old import 'build/src/app/units/states/tasks/tasks.js' from doubtfire-angularjs.module.ts
• Deleted old tasks.coffee

Bug Fixes Applied During Migration

• Null-safe parsing of taskKey from URL params
• Force taskKeyString to a string before passing to newTaskService
• Avoid redundant $state.go when task is null
• Cleaner taskKey assignment using optional chaining
• Safer preventDefault condition with state name null checks

Testing

• Component registers correctly in the Angular module
• Downgraded directive available to AngularJS hybrid app
• taskData object initialises with correct default values
• taskKey syncs correctly from URL params on load

[Thirus224849242]

The migration appears incomplete. The last commit restores the full AngularJS state config and UnitsTasksStateCtrl controller inline in doubtfire-angularjs.module.ts, which means routing is still handled by AngularJS and the new Angular component is never actually used. The tasks.coffee logic has been relocated rather than migrated.

There is also a behaviour regression in setTaskKeyFromUrlParams. Falling back to an empty string when taskKeyString is null means taskKeyFromString is always called on init, potentially setting taskKey to an unexpected value instead of null. The original only called this function when taskKey was truthy, so a null guard should be added before calling the service.

It would also be worth confirming that TaskService from src/app/api/services/task.service actually exposes taskKeyFromString. The Angular component uses TaskService while the AngularJS controller uses newTaskService, and these may not be the same. (I might be wrong).

Finally, the transitionService.onStart hook uses empty criteria, so it fires on every state transition in the app rather than just those relevant to this state. It should be scoped to units/tasks to avoid unintended side effects.

The defensive null checks and removal of the old import are good. The routing side may need changes.

[Sujay-Deakin]

Hi Thirumal, thanks for the detailed feedback! I've addressed all four points in the latest commit:

1. Routing now uses the Angular component; removed the UnitsTasksStateCtrl controller entirely. The state config now uses the template '', which loads the Angular component directly, so routing is handled entirely by Angular.

2. Null guard added to setTaskKeyFromUrlParams; removed the ?? '' fallback. The service is now only called when taskKeyString is truthy, matching the original behaviour.

3. TaskService vs newTaskService; confirmed they are the same service. newTaskService in AngularJS is just a downgraded version of TaskService, as seen in doubtfire-angularjs.module.ts

4. Scoped the transitionService.onStart hook; changed from {} to {to: 'units/tasks.**'} so it only fires for transitions into units/tasks child states.

Tested locally, inbox and task explorer both load correctly after these changes. Let me know if anything else needs addressing!

[sakethsram8888]

Reviewed and tested locally
looks like there are issues with the code and the app accessing the browser, it doesnt load

[sheshankarvapally]

I reviewed PR #493 locally against the latest 10.0.x branch.

Initially, the frontend build was blocked by a TypeScript compile error in tasks.component.ts, but after the latest fix commit was pushed, I refreshed my local PR branch from upstream and confirmed the updated commit was checked out.

I then ran npm start again and confirmed that localhost now starts successfully. I tested the relevant Units/Tasks flow, including loading the app, opening the task-related area, checking the browser console, and confirming that the previous compile blocker is resolved.

I only observed a browser accessibility warning related to aria-hidden focus handling, but no blocking red console error from this PR flow.

Approved from my side. Nice work addressing the review feedback and fixing the build issue.

[sakethsram8888]

reviewed the migration changes and the follow-up fixes from the review comments. The routing cleanup, taskKey null guard, and scoped transitionService.onStart changes all look correct now.

Tested locally as well, inbox/task navigation and URL param syncing worked without issues, and I didn’t notice any regressions. Nice work on the migration and on addressing the feedback thoroughly.