Refactor task definition view

[ishika021]

This pull request refactors the Task Definition view in the OnTrack frontend to improve separation of concerns while maintaining existing UI behaviour.

Previously, the task definition state directly reused the inbox template without a clear boundary between inbox logic and definition-based task exploration. This update introduces a dedicated controller and template for the task definition state, while continuing to reuse the inbox UI to avoid duplication.

The task data source has been switched to a definition-specific query, allowing tasks to be explored and filtered by task definition rather than individual student submissions. Safe initialisation has been added to prevent undefined state issues when unit data loads asynchronously.

Key Changes

• Introduced a dedicated units/tasks/definition state controller
• Reused inbox.tpl.html via a definition-owned template
• Switched task loading to queryTasksForTaskExplorer
• Enabled task definition mode using a scoped flag
• Safely initialised task definition filters
• No backend changes required

[31Husain31]

Reviewer: Husainuddin Mohammed ... ID: 223380186
Hello Ishika! I've gone through the code and have some thoughts.

The template wrapper
I'm not sure we actually need the new definition.tpl.html file. It's basically just a div that wraps an ng-include of the inbox template. Since you're already using a separate controller to handle the different behavior, I think the old approach of directly referencing the inbox template was actually simpler. The separation is already happening at the controller level - adding another template layer just makes things harder to follow in my opinion.

This adds an extra layer without providing real value. The old approach of directly using templateUrl: "units/states/tasks/inbox/inbox.tpl.html" was simpler and achieved the same result. The separation of concerns is already happening at the controller level - you're using a different controller (TaskDefinitionStateCtrl) with different data sources.

Suggestion: Remove the definition.tpl.html file and go back to directly referencing the inbox template in the state config. The controller is where the real separation happens.

Initialization logic
This bit looks a bit off to me:

You're basically doing the same initialization twice - once manually and once in the watch. The ?= operator only assigns if it's undefined anyway, so these are kinda redundant. The watch should be enough on its own since it'll fire when the data is available.

Also, have you actually seen the "Cannot set property 'source' of undefined" error happen? If this is fixing a real bug you encountered that's great, but if not we might be over-engineering the solution.

The old code was clearer
The original version was more straightforward:

It would fail if the data wasn't there, which actually helps catch problems during dev. Your version hides those issues which could make debugging harder later.

Testing
Can you walk me through how you tested this? Specifically:

• What happens when taskDefinitions is empty or undefined?
• Does the dropdown work properly with the watch-based setup?
• Direct navigation to the route before unit data loads?

---

Overall
I think the main issue is we're adding complexity without a clear benefit. If there's a specific bug this fixes, can you share more details? Otherwise I'd suggest either keeping it simple or maybe just adding the safe initialization without the extra template file.
Let me know what you think - happy to chat more about this if you want!

[mannat2634]

Overall, this refactor seems sound and preserves existing behaviour. Introducing a dedicated controller and switching to a definition-based task query provides clearer separation of concerns at the logic level.

That said, the new definition.tpl.html currently only wraps an ng-include of the inbox template and doesn’t add functional value on its own. Since the behavioural separation is already handled by the controller, this extra template layer may be unnecessary unless definition-specific UI changes are planned.

• If it’s intended as a future extension point, a brief comment explaining that would help; otherwise, referencing the inbox template directly would keep things simpler.

The defensive initialization makes sense for common AngularJS scenarios like direct navigation and delayed unit loading, but there’s some redundancy between the immediate assignments and the $watch.

• This could be simplified to retain async safety while improving clarity and avoiding unnecessary masking of data flow issues.

Finally, it would be helpful to confirm the tested scenarios (e.g. empty or undefined taskDefinitions, direct navigation before unit data loads) in detail, since those cases are where these changes provide the most value.

[ishika021]

I’ve addressed the review feedback by removing the redundant definition.tpl.html and simplifying the Task Definition state to rely directly on the existing inbox template.
This keeps the behavioral separation in the controller while avoiding an unnecessary template layer.
Please let me know if any further adjustments are needed.