TASK-2026-00398 : Timer in Task Management Tool

[hridyalakshmi]

Feature description

• When a user clicks Start on a task:
• Capture Project, Task ID, User, Start Time
• Create a record in Task Timer DocType
• Do not use browser local storage
• Active task display:
• Show in a fixed global header/top bar
• Visible across all pages/modules
• Display:
• Task ID / Task Name
• Start Time
• Running Duration (optional)
• Persist until the task is stopped
• Auto-remove after stopping
• Multiple running tasks:
• Show up to 3 active tasks
• If more than 3 → display “X tasks running”
• On clicking Stop:
• Delete the Task Timer record
• Prompt user to update Task Status (Completed / Working)
• Validation rules:
• If a task has an active timer → cannot mark as Completed
• Show message: “Please stop the timer before completing the task”
• Control multiple timers using Projects Setting:
• Ignore Employee Time Overlap = Disabled → Only 1 active task allowed
• Enabled → Multiple tasks allowed

Solution description

1.Start Task

• When a user clicks Start on a task:
• Capture and store:
• Project
• Task ID / Task Name
• User
• Start Time
• Create a new record in a dedicated DocType (e.g., Task Timer)
• Do not use browser local storage

2. Active Task Display (Global Header)

• Show active task(s) in a fixed header/top bar
• Display:
• Task ID / Task Name
• Start Time
• Running Duration (recommended)
• The task bar should:
• Be visible across all pages/modules
• Persist until the task is stopped
• Automatically disappear when no active tasks exist

3. Multiple Active Tasks Handling

• Display up to 3 running tasks individually
• If more than 3 tasks are active:
• Show summary text: “X tasks running”

4. Stop Task Action

• When a user clicks Stop:
• Prompt user to update Task Status (e.g., Working / Completed)
• Delete the corresponding Task Timer record

5. Validation Rules

• Prevent marking a task as Completed if a timer is active
• Show message:
  “Please stop the timer before completing the task”

6. Control Multiple Timers (Based on Settings)

• Use setting: “Ignore Employee Time Overlap”Disabled → Allow only one active task per user
• Enabled → Allow multiple active tasks

Output screenshots (optional)

Screencast.from.28-04-26.11.02.43.AM.IST.webm

Is there any existing behaviour change of other features due to this code change?

No.

Was this feature tested on these browsers?

• Chrome

[MhmdSinanKT]

🔴 Critical Issues

1. Architectural Problem: New DocType Creates Unnecessary Duplication

Issue:
The new Active Task Timer doctype duplicates information already present in the Task doctype:

• start_time (already exists in Task)
• project (referenced in Task)
• subject (already exists in Task)

Why This is a Problem:

• Creates a one-to-one relationship that should be a field update
• Requires additional database queries and joins
• Violates DRY principle
• Harder to maintain consistency between Task and Timer records
• Future developers will be confused about which field to use

Recommended Solution:
Add these fields directly to the Task doctype instead:

{
  "fieldname": "is_timer_active",
  "fieldtype": "Checkbox",
  "label": "Timer Active"
},
{
  "fieldname": "timer_started_by",
  "fieldtype": "Link",
  "label": "Timer Started By",
  "options": "User"
},
{
  "fieldname": "timer_start_time",
  "fieldtype": "Datetime",
  "label": "Timer Start Time"
}

Advantages:

• Single record per task
• No complex joins needed
• Simpler queries: frappe.db.get_list("Task", {"is_timer_active": 1})
• All task-related data in one place
• Easier to audit and maintain

If there's a conflict with the existing start_time field:

• That's a design decision that needs documentation
• Either migrate that field or rename it explicitly
• This should have been discussed in the PR description

---

2. Incomplete PR Description

Current State:
The PR description is just a blank template with no actual information.

What's Missing:

• Why was a new doctype needed vs. modifying Task?
• Detailed explanation of the timer functionality
• How does it handle concurrent timers?
• What happens on user logout/browser close?
• Design rationale and alternatives considered
• Screenshots/videos showing the UI in action
• List of all affected areas and behavior changes

Action Required:
Complete the description before this PR can be approved. The reviewer should not have to reverse-engineer the feature.

---

3. Missing Visual Demonstration

Issue:
No screenshots or screen recordings showing:

• The timer widget in the bottom-right corner
• What it looks like when active
• How the blinking animation appears
• How multiple concurrent timers are displayed
• Integration with the task management UI

Why It Matters:

• UI features must be visually verified
• The CSS includes complex animations that need to be visually validated
• Future maintainers need reference documentation

---

⚠️ Code Quality & Standards Issues

4. Silent Failure on Authentication

File: task_management_tool.py (Lines 282-284)

@frappe.whitelist()
def start_active_timer(task, project, subject, start_time):
    user = frappe.session.user
    if not user or user == 'Guest':
        return  # ❌ PROBLEM: Silent failure!

Issues:

• Returns None silently instead of raising an error
• Frontend doesn't know the operation failed
• User sees no feedback
• Logs are unclear

Fix:

@frappe.whitelist()
def start_active_timer(task, project, subject, start_time):
    user = frappe.session.user
    if not user or user == 'Guest':
        frappe.throw(_("User authentication required. Please login first."))

---

5. Missing Input Validation

File: task_management_tool.py (Lines 282-325)

Problem:
No validation of input parameters:

def start_active_timer(task, project, subject, start_time):
    # ❌ What if task is invalid?
    # ❌ What if start_time is malformed?
    # ❌ What if project doesn't exist?
    # ❌ No permission checks on task access

Required Validations:

@frappe.whitelist()
def start_active_timer(task, project, subject, start_time):
    user = frappe.session.user
    if not user or user == 'Guest':
        frappe.throw(_("Authentication required"))
    
    # Validate task exists and user has access
    if not frappe.db.exists("Task", task):
        frappe.throw(_("Task {0} not found").format(task))
    
    # Check user has permission to access task
    if not frappe.has_permission("Task", "read", task):
        frappe.throw(_("No permission to access this task"))
    
    # Validate start_time format
    try:
        start_dt = frappe.utils.get_datetime(start_time)
        if start_dt > frappe.utils.now_datetime():
            frappe.throw(_("Start time cannot be in the future"))
    except:
        frappe.throw(_("Invalid start_time format"))

---

6. Permission Bypass Without Audit Trail

File: task_management_tool.py (Lines 315-316)

doc.flags.ignore_permissions = True
doc.save(ignore_permissions=True)

Issues:

• Bypasses permission checks silently
• No audit trail of who created the timer
• Security risk: any user can create timers for any task
• Two identical permission overrides (redundant)

---

7. Inconsistent Code Formatting

File: one_compliance.js (Lines 1-225)

Issues:

• Mix of tabs and spaces for indentation
• Inconsistent quote usage (' vs ")
• Long lines exceeding 100 characters without wrapping
• Missing JSDoc comments for functions

Action Required:

• Run through a code formatter (Prettier recommended)
• Use consistent tab width (2 or 4 spaces, not tabs)
• Add JSDoc comments for all public functions

---

8. Hardcoded Z-Index Creates Conflicts

File: one_compliance.css & one_compliance.js

#oc-timer-wrap {
    z-index: 2147483647;  /* ❌ Maximum possible z-index */
}

Problem:

• Uses the absolute maximum z-index value
• Will conflict with other system modals/popups
• Not maintainable

Fix:

#oc-timer-wrap {
    z-index: 1040;  /* Bootstrap modal is 1050 */
}

---

❓ Timer Logic & Counter Design Concerns

9. Missing Real-Time Counter Implementation

Issue:
The code stores start_time but there's no visible logic for:

• Calculating elapsed time
• Updating the display every second
• Formatting the countdown/timer display

Missing Code:

// This is MISSING from the code:
setInterval(function() {
    var timers = getActiveData();
    timers.forEach(function(timer) {
        var elapsed = calculateElapsed(timer.start_time);
        updateTimerDisplay(elapsed);  // ❌ Not defined
    });
}, 1000);

Questions:

1. Does the timer count up from start_time or count down?
2. How is elapsed time calculated and formatted?
3. What's the update frequency (every second, every 100ms)?
4. How does it sync if user opens multiple tabs?

Action Required:
Add the actual timer counter logic with:

• Elapsed time calculation function
• Display update mechanism
• Format specification (HH:MM:SS?)
• Real-time sync across browser tabs

---

10. Contradictory Counter Design

Issue:
The code is unclear about whether multiple concurrent timers are supported:

# Can have multiple timers per user?
existing_timer = frappe.db.sql("""
    SELECT task, subject FROM `tabActive Task Timer` 
    WHERE user = %s AND task != %s
""", (user, task), as_dict=True)

if existing_timer:
    frappe.throw(...)  # ❌ Prevents concurrent timers

Confusion:

• Design allows multiple timers in the DB schema
• But code prevents more than one per user
• Frontend displays multiple timers ("Timer is ON: 3 Tasks running")
• This is contradictory!

Action Required:
Clarify the desired behavior:

Option A: One Timer at a Time (Current Prevention Logic)

- Only one task can have an active timer per user
- Code matches intent ✓
- UI should show just one timer, not multiple

Option B: Multiple Concurrent Timers (Current DB Schema)

- User can time multiple tasks simultaneously
- Remove the prevention check
- Update UI to show all active timers

Choose one and make code consistent with that choice.

---

📋 Additional Concerns

11. No Error Handling in Callbacks

File: task_management_tool.js

frappe.call({
    method: "...",
    callback: (r) => {
        if (r.message) {  // ✓ Good
            // ...
        }
        // ❌ What if r.exc? No error handling!
    }
});

Add error handler:

frappe.call({
    method: "...",
    callback: (r) => {
        if (r.message) {
            // Success handling
        }
    },
    error: (r) => {
        frappe.msgprint({
            title: __("Error"),
            indicator: "red",
            message: __("Failed to start timer. Please try again.")
        });
    }
});

---

12. Missing Unit Tests

Issue:
No tests for the new backend methods:

• start_active_timer()
• stop_active_timer()
• get_active_timer()

Action Required:
Add tests covering:

• Normal operation
• Permission checks
• Concurrent timer prevention
• Edge cases (invalid inputs, missing tasks)

---

✅ What Looks Good

• ✓ Realtime pubsub integration (frappe.publish_realtime)
• ✓ LocalStorage fallback for offline support
• ✓ Responsive UI widget with animations
• ✓ Cross-tab synchronization via localStorage

---

📝 Summary

Priority	Issue	Action
🔴 Critical	New DocType vs. Task field	Redesign to add fields to Task
🔴 Critical	Incomplete PR description	Complete all sections
🔴 Critical	Timer counter logic missing	Implement elapsed time calculation
🟠 High	No visual proof (screenshots)	Add UI screenshots/videos
🟠 High	Silent authentication failure	Add proper error throwing
🟠 High	Missing input validation	Validate all parameters
🟠 High	Contradictory counter design	Clarify one-timer vs. multi-timer intent
🟡 Medium	Code formatting inconsistencies	Run through formatter
🟡 Medium	Permission bypass without audit	Add audit logging
🟡 Medium	Hardcoded z-index	Use reasonable value (1040)
🟡 Medium	No error handling in callbacks	Add error handlers
🟡 Medium	No unit tests	Add test coverage

This is not a rejection, but a detailed list of improvements needed before this feature is production-ready.

[Jumana-K]

Add naming rule for Active Task Timer