feat(timer): start with project and task pickers, save and stop with a description

app/Application/TimerService.php

@@ -45,13 +45,32 @@ public function running(int $companyId, int $userId): ?TimeEntry
             ->first();
     }
 
-    /** @throws TimerAlreadyRunning when the user already has a timer in this company */
-    public function start(int $companyId, int $userId, int $taskId, ?string $description = null): TimeEntry
-    {
+    /**
+     * Put the caller's clock on a task.
+     *
+     * Starting the task the clock is already on is the same wish twice rather
+     * than a conflict, so it applies whatever the caller sent and answers the
+     * entry that is already running. That is what makes "create this task and
+     * start it" correct when `auto_start_tasks` started the clock on create.
+     *
+     * @throws TimerAlreadyRunning when the user's timer is on another task
+     */
+    public function start(
+        int $companyId,
+        int $userId,
+        int $taskId,
+        ?string $description = null,
+        ?bool $billable = null,
+    ): TimeEntry {
         $task = $this->tasks->findForCompany($companyId, $taskId);
+        $running = $this->running($companyId, $userId);
 
-        if ($this->running($companyId, $userId) !== null) {
-            throw TimerAlreadyRunning::forUser($userId, $companyId);
+        if ($running !== null) {
+            if ((int) $running->task_id !== (int) $task->id) {
+                throw TimerAlreadyRunning::forUser($userId, $companyId);
+            }
+
+            return $this->applyDetails($running, $description, $billable);
         }
 
         try {
@@ -64,7 +83,7 @@ public function start(int $companyId, int $userId, int $taskId, ?string $descrip
                 'ended_at' => null,
                 'duration_minutes' => 0,
                 'description' => $description,
-                'billable' => (bool) $task->billable,
+                'billable' => $billable ?? (bool) $task->billable,
                 'rate' => 0,
                 'amount' => 0,
                 'currency_id' => $this->currencyFor($task),
@@ -82,13 +101,23 @@ public function start(int $companyId, int $userId, int $taskId, ?string $descrip
     /**
      * Close the running entry: derive the elapsed minutes, round them to the
      * company increment, resolve the rate and cache the amount.
+     *
+     * The description and the billable flag are what the stop dialog collected,
+     * and each is applied only when it was sent: a client that stops without a
+     * body keeps whatever the start recorded.
      */
-    public function stop(int $companyId, int $userId): TimeEntry
-    {
+    public function stop(
+        int $companyId,
+        int $userId,
+        ?string $description = null,
+        ?bool $billable = null,
+    ): TimeEntry {
         $entry = $this->requireRunning($companyId, $userId);
         $endedAt = Carbon::now();
         $startedAt = $entry->started_at ?? $endedAt;
 
+        $this->writeDetails($entry, $description, $billable);
+
         $entry->ended_at = $endedAt;
         $entry->running_user_id = null;
         $entry->duration_minutes = Rounding::roundMinutes(
@@ -115,8 +144,13 @@ public function stop(int $companyId, int $userId): TimeEntry
      *
      * @throws TimerMismatch when the caller's timer is not on this task
      */
-    public function stopOn(int $companyId, int $userId, int $taskId): TimeEntry
-    {
+    public function stopOn(
+        int $companyId,
+        int $userId,
+        int $taskId,
+        ?string $description = null,
+        ?bool $billable = null,
+    ): TimeEntry {
         $task = $this->tasks->findForCompany($companyId, $taskId);
         $running = $this->running($companyId, $userId);
 
@@ -127,7 +161,7 @@ public function stopOn(int $companyId, int $userId, int $taskId): TimeEntry
             );
         }
 
-        return $this->stop($companyId, $userId);
+        return $this->stop($companyId, $userId, $description, $billable);
     }
 
     /** Throw away the running entry without recording any time. */
@@ -136,6 +170,35 @@ public function discard(int $companyId, int $userId): void
         $this->requireRunning($companyId, $userId)->delete();
     }
 
+    /** Apply the caller's details to a running entry and save if anything moved. */
+    private function applyDetails(TimeEntry $entry, ?string $description, ?bool $billable): TimeEntry
+    {
+        $this->writeDetails($entry, $description, $billable);
+
+        if ($entry->isDirty()) {
+            $entry->save();
+        }
+
+        return $entry;
+    }
+
+    /**
+     * Write onto an entry whatever the caller actually sent.
+     *
+     * Null is "the caller did not say", never "clear it", so a stop that
+     * carries only a description leaves the billable flag the start chose.
+     */
+    private function writeDetails(TimeEntry $entry, ?string $description, ?bool $billable): void
+    {
+        if ($description !== null) {
+            $entry->description = $description;
+        }
+
+        if ($billable !== null) {
+            $entry->billable = $billable;
+        }
+    }
+
     private function requireRunning(int $companyId, int $userId): TimeEntry
     {
         $entry = $this->running($companyId, $userId);

app/Http/Controllers/TimerController.php

@@ -9,15 +9,18 @@
 use Modules\TasksProjects\Application\TimerService;
 use Modules\TasksProjects\Http\Requests\StartTaskTimerRequest;
 use Modules\TasksProjects\Http\Requests\StartTimerRequest;
+use Modules\TasksProjects\Http\Requests\StopTimerRequest;
 use Modules\TasksProjects\Http\Resources\TimeEntryResource;
 use Modules\TasksProjects\Support\Abilities;
 use Modules\TasksProjects\Support\Authorizes;
 
 /**
  * The caller's own running timer, one per company.
  *
- * A second start is a conflict rather than a validation error, because the
- * first timer is still perfectly valid; the UI offers to stop it.
+ * A start on another task while one is running is a conflict rather than a
+ * validation error, because the first timer is still perfectly valid; the UI
+ * offers to stop it. A start on the task already being timed is not a conflict
+ * at all: it applies the details it carries and answers the running entry.
  *
  * The same timer is reachable two ways. `timer/start` and `timer/stop` name the
  * task in the body and are what the timesheet and the header chip use; the
@@ -58,15 +61,21 @@ public function start(StartTimerRequest $request): TimeEntryResource
             $context->userId,
             (int) $validated['task_id'],
             $validated['description'] ?? null,
+            self::flag($request, 'billable'),
         ));
     }
 
-    public function stop(Request $request): TimeEntryResource
+    public function stop(StopTimerRequest $request): TimeEntryResource
     {
         $context = $this->context($request);
         $this->authorize($context, Abilities::VIEW_OWN_TIME);
 
-        return new TimeEntryResource($this->timer->stop($context->companyId, $context->userId));
+        return new TimeEntryResource($this->timer->stop(
+            $context->companyId,
+            $context->userId,
+            $request->validated()['description'] ?? null,
+            self::flag($request, 'billable'),
+        ));
     }
 
     /** Start the caller's clock on one task, straight from its row or card. */
@@ -81,17 +90,24 @@ public function startOnTask(StartTaskTimerRequest $request, int $id): TimeEntryR
             $context->userId,
             $id,
             $request->validated()['description'] ?? null,
+            self::flag($request, 'billable'),
         ));
     }
 
     /** Stop the caller's clock, but only while it is running on this task. */
-    public function stopOnTask(Request $request, int $id): TimeEntryResource
+    public function stopOnTask(StopTimerRequest $request, int $id): TimeEntryResource
     {
         $context = $this->context($request);
         $this->authorize($context, Abilities::VIEW_TASK);
         $this->authorize($context, Abilities::VIEW_OWN_TIME);
 
-        return new TimeEntryResource($this->timer->stopOn($context->companyId, $context->userId, $id));
+        return new TimeEntryResource($this->timer->stopOn(
+            $context->companyId,
+            $context->userId,
+            $id,
+            $request->validated()['description'] ?? null,
+            self::flag($request, 'billable'),
+        ));
     }
 
     /** Throw the running entry away without recording any time. */
@@ -104,4 +120,15 @@ public function destroy(Request $request): JsonResponse
 
         return response()->json(['success' => true]);
     }
+
+    /**
+     * A boolean the caller sent, or null when they said nothing about it.
+     *
+     * The service treats null as "leave it alone", so an omitted flag has to
+     * stay distinguishable from a flag that was sent as false.
+     */
+    private static function flag(Request $request, string $key): ?bool
+    {
+        return $request->has($key) ? $request->boolean($key) : null;
+    }
 }

app/Http/Requests/StartTaskTimerRequest.php

@@ -4,14 +4,18 @@
 
 namespace Modules\TasksProjects\Http\Requests;
 
-/** Starting the clock from a task row: the task is the URL, the note is the body. */
+/**
+ * Starting the clock from a task row: the task is the URL, the details are the
+ * body. `billable` overrides the task's own flag, for a start dialog that asked.
+ */
 final class StartTaskTimerRequest extends ModuleRequest
 {
     /** @return array<string, list<string>> */
     public function rules(): array
     {
         return [
             'description' => ['sometimes', 'nullable', 'string'],
+            'billable' => ['sometimes', 'boolean'],
         ];
     }
 }

app/Http/Requests/StartTimerRequest.php

@@ -12,6 +12,7 @@ public function rules(): array
         return [
             'task_id' => ['required', 'integer', 'min:1'],
             'description' => ['nullable', 'string'],
+            'billable' => ['sometimes', 'boolean'],
         ];
     }
 }

app/Http/Requests/StopTimerRequest.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Modules\TasksProjects\Http\Requests;
+
+/**
+ * Stopping the clock: the note and the billable flag the dialog collected.
+ *
+ * Both are optional, because a stop from an older client sends nothing at all
+ * and must still close the entry with whatever the start recorded.
+ */
+final class StopTimerRequest extends ModuleRequest
+{
+    /** @return array<string, list<string>> */
+    public function rules(): array
+    {
+        return [
+            'description' => ['sometimes', 'nullable', 'string', 'max:2000'],
+            'billable' => ['sometimes', 'boolean'],
+        ];
+    }
+}