Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions app/Application/Exceptions/EntriesAlreadyInvoiced.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ public static function forEntry(int $entryId): self
return self::forEntries([$entryId]);
}

/**
* An edit that would move time an invoice was already raised for.
*
* @param list<string> $fields
*/
public static function forLockedFields(int $entryId, array $fields): self
{
return new self(
"Time entry {$entryId} is already on an invoice: ".implode(', ', $fields).' cannot be changed.',
);
}

public static function forOtherInvoice(int $entryId, int $invoiceId): self
{
return new self("Time entry {$entryId} is already stamped with invoice {$invoiceId}.");
Expand Down
20 changes: 20 additions & 0 deletions app/Application/Exceptions/TaskLocked.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Modules\TasksProjects\Application\Exceptions;

/**
* A fully invoiced task, while the company locks invoiced tasks.
*
* The lock is a company setting rather than a rule of the data model: the time
* on the task is already history either way, and what the setting protects is
* the description and the status the invoice was raised against.
*/
final class TaskLocked extends TasksProjectsException
{
public static function forTask(int $taskId): self
{
return new self("Task {$taskId} is invoiced and this company locks invoiced tasks.");
}
}
25 changes: 25 additions & 0 deletions app/Application/Exceptions/TimerMismatch.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

declare(strict_types=1);

namespace Modules\TasksProjects\Application\Exceptions;

/**
* The timer the caller asked to stop is not the timer that is running.
*
* Stopping is addressed to a task, so a stale tab that still shows yesterday's
* play button would otherwise stop whatever happens to be running now. Either
* nothing runs or it runs somewhere else; both are the same mistake to the UI,
* which reloads the running timer and shows it where it really is.
*/
final class TimerMismatch extends TasksProjectsException
{
public static function forTask(int $taskId, ?int $runningTaskId): self
{
if ($runningTaskId === null) {
return new self("No timer is running, so task {$taskId} cannot be stopped.");
}

return new self("The running timer is on task {$runningTaskId}, not on task {$taskId}.");
}
}
44 changes: 34 additions & 10 deletions app/Application/Rounding.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,32 +10,56 @@
/**
* Billing increments, applied when a time entry is saved rather than when it is
* invoiced, so what the user sees on the timesheet is what gets billed.
*
* The direction is a company setting. `nearest` is the historic behaviour and
* stays the default; `up` and `down` are the two ways a firm that bills in
* blocks wants the block chosen, and neither reads as "nearest" to a client.
*/
final class Rounding
{
public const NEAREST = 'nearest';

public const UP = 'up';

public const DOWN = 'down';

/** @var list<string> */
public const DIRECTIONS = [self::NEAREST, self::UP, self::DOWN];

/**
* Round a duration to the nearest multiple of the increment.
* Round a duration to a multiple of the increment.
*
* Zero stays zero, because an entry with no time is not worth an increment.
* Anything above zero but below one increment rounds up: a two minute call
* on a fifteen minute increment bills a quarter of an hour, never nothing.
* Zero stays zero whichever way the company rounds, because an entry with
* no time is not worth an increment. Rounding `nearest` then treats a spell
* shorter than one increment as a whole one: a two minute call on a fifteen
* minute increment bills a quarter of an hour, never nothing. Rounding
* `down` is the one direction that may answer zero for real work, which is
* exactly what a firm asking to round down is asking for.
*/
public static function roundMinutes(int $minutes, int $increment): int
public static function roundMinutes(int $minutes, int $increment, string $direction = self::NEAREST): int
{
if (! in_array($increment, ModuleSettings::ROUNDING_INCREMENTS, true)) {
throw new InvalidArgumentException(
"Rounding increment {$increment} is not one of ".implode(', ', ModuleSettings::ROUNDING_INCREMENTS).'.',
);
}

if ($minutes <= 0) {
return 0;
if (! in_array($direction, self::DIRECTIONS, true)) {
throw new InvalidArgumentException(
"Rounding direction {$direction} is not one of ".implode(', ', self::DIRECTIONS).'.',
);
}

if ($minutes < $increment) {
return $increment;
if ($minutes <= 0) {
return 0;
}

return (int) round($minutes / $increment) * $increment;
return match ($direction) {
self::UP => (int) ceil($minutes / $increment) * $increment,
self::DOWN => intdiv($minutes, $increment) * $increment,
default => $minutes < $increment
? $increment
: (int) round($minutes / $increment) * $increment,
};
}
}
36 changes: 36 additions & 0 deletions app/Application/TaskLock.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

declare(strict_types=1);

namespace Modules\TasksProjects\Application;

use Modules\TasksProjects\Application\Exceptions\TaskLocked;
use Modules\TasksProjects\Support\ModuleSettings;

/**
* The "lock invoiced tasks" setting, in one place.
*
* A company that bills by task often wants the task to stop moving once the
* invoice is out, so what the client was billed for still reads the way it was
* billed. The rule is the same wherever a task changes, so the check lives here
* rather than being spelled out again in each writer.
*/
final class TaskLock
{
public function __construct(
private readonly ModuleSettings $settings,
private readonly TaskTimeSummary $summary,
) {}

/** @throws TaskLocked when the company locks invoiced tasks and this one is invoiced */
public function guard(int $companyId, int $taskId): void
{
if (! $this->settings->lockInvoicedTasks($companyId)) {
return;
}

if ($this->summary->forTask($companyId, $taskId)['invoiced'] === TaskTimeSummary::INVOICED) {
throw TaskLocked::forTask($taskId);
}
}
}
56 changes: 55 additions & 1 deletion app/Application/TaskService.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@

namespace Modules\TasksProjects\Application;

use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Collection;
use Illuminate\Database\Eloquent\ModelNotFoundException;
use Illuminate\Database\Query\Builder as QueryBuilder;
use Illuminate\Database\QueryException;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\DB;
Expand Down Expand Up @@ -47,10 +49,11 @@ public function __construct(
private readonly BoardOrderingService $board,
private readonly TaskStatusService $statuses,
private readonly ProjectService $projects,
private readonly TaskLock $lock,
) {}

/**
* @param array{project_id?: int, assignee_id?: int, task_status_id?: int, customer_id?: int, due_before?: string, due_after?: string, search?: string, sort_by?: string, sort_order?: string} $filters
* @param array{project_id?: int, assignee_id?: int, task_status_id?: int, customer_id?: int, invoiced?: bool, due_before?: string, due_after?: string, search?: string, sort_by?: string, sort_order?: string} $filters
* @return Collection<int, Task>
*/
public function listFor(int $companyId, array $filters = []): Collection
Expand All @@ -75,6 +78,10 @@ public function listFor(int $companyId, array $filters = []): Collection
$query->where('name', 'like', '%'.$filters['search'].'%');
}

if (isset($filters['invoiced'])) {
$this->filterByInvoiced($query, $companyId, (bool) $filters['invoiced']);
}

$sorts = $this->sorts();
[$key, $order] = $this->sortFor($filters, $sorts, self::DEFAULT_SORT_KEY, self::DEFAULT_SORT_ORDER);

Expand Down Expand Up @@ -165,6 +172,7 @@ public function update(int $companyId, int $id, array $attributes): Task
{
return DB::transaction(function () use ($companyId, $id, $attributes): Task {
$task = $this->findForCompany($companyId, $id);
$this->lock->guard($companyId, (int) $task->id);

if (array_key_exists('project_id', $attributes)) {
$project = $attributes['project_id'] === null
Expand Down Expand Up @@ -201,6 +209,7 @@ public function delete(int $companyId, int $id): void
{
DB::transaction(function () use ($companyId, $id): void {
$task = $this->findForCompany($companyId, $id);
$this->lock->guard($companyId, (int) $task->id);

$invoiced = TimeEntry::query()
->forCompany($companyId)
Expand All @@ -224,6 +233,7 @@ public function move(int $companyId, int $taskId, int $statusId, ?int $beforeId
{
return DB::transaction(function () use ($companyId, $taskId, $statusId, $beforeId, $afterId): Task {
$task = $this->findForCompany($companyId, $taskId);
$this->lock->guard($companyId, (int) $task->id);
$status = $this->statuses->findForCompany($companyId, $statusId);

$position = $this->board->positionFor($companyId, (int) $status->id, $beforeId, $afterId);
Expand All @@ -250,6 +260,50 @@ private function applyStatus(Task $task, TaskStatus $status): void
$task->closed_at = null;
}

/**
* Narrow the list to tasks whose billable time has, or has not, reached an
* invoice.
*
* The state belongs to the entries, so it is asked of them rather than
* cached on the task: uninvoiced means at least one billable entry is still
* unbilled, and invoiced means a stamped entry exists and no unbilled one
* does. Both are `exists` subqueries, which MySQL, PostgreSQL and SQLite
* all plan off the `(company_id, task_id)` index and all spell the same.
*
* @param Builder<Task> $query
*/
private function filterByInvoiced(Builder $query, int $companyId, bool $invoiced): void
{
if (! $invoiced) {
$query->whereExists($this->billableEntries($companyId, stamped: false));

return;
}

$query->whereExists($this->billableEntries($companyId, stamped: true))
->whereNotExists($this->billableEntries($companyId, stamped: false));
}

/** A correlated subquery over the stopped, billable entries of the task. */
private function billableEntries(int $companyId, bool $stamped): callable
{
$entries = (new TimeEntry)->getTable();
$tasks = (new Task)->getTable();

return static function (QueryBuilder $sub) use ($companyId, $stamped, $entries, $tasks): void {
$sub->selectRaw('1')
->from($entries)
->whereColumn($entries.'.task_id', $tasks.'.id')
->where($entries.'.company_id', $companyId)
->whereNull($entries.'.running_user_id')
->where($entries.'.billable', true);

$stamped
? $sub->whereNotNull($entries.'.invoice_id')
: $sub->whereNull($entries.'.invoice_id');
};
}

/** @param array<string, mixed> $attributes */
private function customerFor(?Project $project, array $attributes): ?int
{
Expand Down
Loading
Loading