Fix taskKey small bugs

[ishika021]

This PR makes a few small, I've focused on improvements to the units/tasks state to improve robustness and avoid subtle edge-case bugs.

1. Safer handling of missing taskKey URL parameter

• Previously, taskKey from the URL was always passed into taskKeyFromString, even when it was undefined. This could lead to unexpected parsing behaviour on direct navigation or initial page load.
• Now, the code explicitly checks for the presence of taskKey and resets the selected task when it is missing.

2. Prevent redundant $state.go calls

• Updating the URL for the selected task previously triggered $state.go even when the taskKey hadn’t changed. This could cause unnecessary digest cycles and state churn.
• The update now short-circuits when the URL already reflects the current task selection.

3. More precise prevention of unnecessary state reloads

• The preventDefault logic on state changes has been tightened to only block transitions when the state, unit, and task key are all unchanged. This avoids accidentally blocking legitimate navigation where only the task selection changes.

4. Minor cleanup for clarity

Small refactors were made to simplify task key assignment and ensure consistent string handling of URL parameters, without changing user-facing behaviour.

These changes are intentionally small and non-functional, and are intended to improve stability, readability, and maintainability of the existing task state logic.

[31Husain31]

Hey! I've tested this PR locally. Since my backend suddenly unexpectedly crashed (database migrations failing - not related to your changes), I did what testing I could with the compiled code.

What I tested:
Compiled the CoffeeScript and checked the output - everything looks good:
[Screenshot 1: The compilation output showing all files compiled successfully]

[Screenshot 2: The compiled JavaScript showing the bug fixes in the code]

Verified all 4 fixes are in the compiled code:
[Screenshot 3: Bug fix verification showing null checks, early returns, and sameTask logic]

Ran TypeScript compilation check:
[Screenshot 4: TypeScript compilation results showing only 2 pre-existing errors]

My thoughts on the changes:
The fixes make sense:

Adding the null check for taskKeyString is good defensive programming
The early return to avoid redundant $state.go calls should help with performance
The sameTask check is actually pretty important - without it, you'd block legitimate task switches within the same unit, which seems like it would be annoying for users
The refactoring makes it easier to understand what's happening

One thing I noticed:
Line 52 in the CoffeeScript uses task?.taskKey() ? null - I initially thought this was invalid syntax but it's actually fine, it's CoffeeScript's existential operator and compiles correctly to proper null checking.

Conclusion:
Code looks solid. Would've liked to test it properly with the backend running (Reviewer #2 can do this) but the static analysis shows everything compiles and the logic makes sense. The changes are small, non-breaking, and fix actual edge cases.

[JeffySam]

I reviewed and tested this PR locally within the scope available on my setup. While I wasn’t able to run the full application end-to-end, I validated the changes through compilation checks and inspection of the generated output to ensure the logic behaves as intended.

What I tested:

Recompiled the CoffeeScript and reviewed the generated JavaScript — compilation completed cleanly and the output reflects the intended changes.

Manually inspected the compiled JS to confirm all four fixes are present.

Verified the key logic updates, including the added null checks, early return, and same-task handling.

Ran a TypeScript compilation check; only the existing known errors appeared, with no new issues introduced by this PR.

Thoughts on the changes:

The additional guard around taskKeyString is a solid defensive improvement.

The early return preventing redundant $state.go calls should help avoid unnecessary navigation.

The sameTask check is important — without it, valid task changes within the same unit could be incorrectly blocked.

The refactor improves clarity and makes the flow of the logic easier to reason about.

Minor note:

The CoffeeScript expression on line 52 (task?.taskKey() ? null) initially looks unusual like husain said, but it’s valid CoffeeScript syntax and compiles correctly to safe null checking.

Conclusion:
Even without full runtime testing, the changes compile cleanly and the logic is sound. The fixes are small, targeted, and address real edge cases without introducing breaking behaviour. Looks good to merge from a code review perspective