mirror of
https://github.com/solidtime-io/solidtime.git
synced 2026-10-07 21:33:18 +01:00
move goal progress calculation out of the resources
This commit is contained in:
@@ -14,6 +14,7 @@ use App\Http\Resources\V1\Goal\GoalResource;
|
||||
use App\Models\Goal;
|
||||
use App\Models\Organization;
|
||||
use App\Service\BillingContract;
|
||||
use App\Service\GoalProgressService;
|
||||
use App\Service\GoalsContract;
|
||||
use App\Service\TimezoneService;
|
||||
use Illuminate\Auth\Access\AuthorizationException;
|
||||
@@ -73,7 +74,7 @@ class GoalController extends Controller
|
||||
*
|
||||
* @operationId getGoals
|
||||
*/
|
||||
public function index(Organization $organization, GoalIndexRequest $request, GoalsContract $access): GoalCollection
|
||||
public function index(Organization $organization, GoalIndexRequest $request, GoalsContract $access, GoalProgressService $progressService): GoalCollection
|
||||
{
|
||||
// An organization goal is visible to the member it is for, who does not hold the :organization-type permission.
|
||||
// Either permission is enough, the access contract decides which goals come back.
|
||||
@@ -96,8 +97,9 @@ class GoalController extends Controller
|
||||
}
|
||||
|
||||
$goals = $query->paginate(config('app.pagination_per_page_default'));
|
||||
$progressByGoalId = $progressService->getCurrentProgressForGoals($goals->getCollection(), Carbon::now());
|
||||
|
||||
return new GoalCollection($goals);
|
||||
return new GoalCollection($goals, $progressByGoalId);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -107,7 +109,7 @@ class GoalController extends Controller
|
||||
*
|
||||
* @operationId getGoal
|
||||
*/
|
||||
public function show(Organization $organization, Goal $goal, GoalsContract $access): GoalResource
|
||||
public function show(Organization $organization, Goal $goal, GoalsContract $access, GoalProgressService $progressService): GoalResource
|
||||
{
|
||||
// Either permission is enough, see index
|
||||
$this->checkAnyPermission($organization, ['goals:view:own', 'goals:view:organization-type']);
|
||||
@@ -118,7 +120,7 @@ class GoalController extends Controller
|
||||
}
|
||||
$goal->load('member.user');
|
||||
|
||||
return new GoalResource($goal);
|
||||
return new GoalResource($goal, $progressService->getCurrentProgress($goal, Carbon::now()));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -128,7 +130,7 @@ class GoalController extends Controller
|
||||
*
|
||||
* @operationId createGoal
|
||||
*/
|
||||
public function store(Organization $organization, GoalStoreRequest $request, GoalsContract $access): JsonResponse
|
||||
public function store(Organization $organization, GoalStoreRequest $request, GoalsContract $access, GoalProgressService $progressService): JsonResponse
|
||||
{
|
||||
if ($request->getType() === GoalType::Personal) {
|
||||
$this->checkPermission($organization, 'goals:create:own');
|
||||
@@ -159,7 +161,7 @@ class GoalController extends Controller
|
||||
$goal->save();
|
||||
$goal->load('member.user');
|
||||
|
||||
return (new GoalResource($goal))
|
||||
return (new GoalResource($goal, $progressService->getCurrentProgress($goal, Carbon::now())))
|
||||
->response()
|
||||
->setStatusCode(201);
|
||||
}
|
||||
@@ -173,7 +175,7 @@ class GoalController extends Controller
|
||||
*
|
||||
* @operationId updateGoal
|
||||
*/
|
||||
public function update(Organization $organization, Goal $goal, GoalUpdateRequest $request, GoalsContract $access): GoalResource
|
||||
public function update(Organization $organization, Goal $goal, GoalUpdateRequest $request, GoalsContract $access, GoalProgressService $progressService): GoalResource
|
||||
{
|
||||
if ($goal->type === GoalType::Personal) {
|
||||
$this->checkPermission($organization, 'goals:update:own', $goal);
|
||||
@@ -212,7 +214,7 @@ class GoalController extends Controller
|
||||
$goal->save();
|
||||
$goal->load('member.user');
|
||||
|
||||
return new GoalResource($goal);
|
||||
return new GoalResource($goal, $progressService->getCurrentProgress($goal, Carbon::now()));
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -5,14 +5,42 @@ declare(strict_types=1);
|
||||
namespace App\Http\Resources\V1\Goal;
|
||||
|
||||
use App\Http\Resources\PaginatedResourceCollection;
|
||||
use App\Models\Goal;
|
||||
use App\Service\Dto\GoalProgressDto;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Http\Resources\Json\ResourceCollection;
|
||||
|
||||
class GoalCollection extends ResourceCollection implements PaginatedResourceCollection
|
||||
{
|
||||
/**
|
||||
* The resource that this resource collects.
|
||||
*
|
||||
* @var string
|
||||
* @var array<string, GoalProgressDto>
|
||||
*/
|
||||
public $collects = GoalResource::class;
|
||||
private array $progressByGoalId;
|
||||
|
||||
/**
|
||||
* @param array<string, GoalProgressDto> $progressByGoalId
|
||||
*/
|
||||
public function __construct($resource, array $progressByGoalId)
|
||||
{
|
||||
parent::__construct($resource);
|
||||
$this->progressByGoalId = $progressByGoalId;
|
||||
}
|
||||
|
||||
protected function collects(): ?string
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Transform the resource collection into an array.
|
||||
*
|
||||
* @return array<array<string, string|bool|int|null|array<string, string|bool|int|null|array<int, string>>>>
|
||||
*/
|
||||
public function toArray(Request $request): array
|
||||
{
|
||||
return $this->collection->map(function (Goal $goal) use ($request): array {
|
||||
return (new GoalResource($goal, $this->progressByGoalId[$goal->getKey()]))
|
||||
->toArray($request);
|
||||
})->all();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,15 +6,23 @@ namespace App\Http\Resources\V1\Goal;
|
||||
|
||||
use App\Http\Resources\V1\BaseResource;
|
||||
use App\Models\Goal;
|
||||
use App\Service\GoalProgressService;
|
||||
use App\Service\Dto\GoalProgressDto;
|
||||
use Illuminate\Http\Request;
|
||||
use Illuminate\Support\Carbon;
|
||||
|
||||
/**
|
||||
* @property Goal $resource
|
||||
*/
|
||||
class GoalResource extends BaseResource
|
||||
{
|
||||
private GoalProgressDto $progress;
|
||||
|
||||
public function __construct(Goal $resource, GoalProgressDto $progress)
|
||||
{
|
||||
parent::__construct($resource);
|
||||
|
||||
$this->progress = $progress;
|
||||
}
|
||||
|
||||
/**
|
||||
* Transform the resource into an array.
|
||||
*
|
||||
@@ -22,11 +30,6 @@ class GoalResource extends BaseResource
|
||||
*/
|
||||
public function toArray(Request $request): array
|
||||
{
|
||||
$progressService = app(GoalProgressService::class);
|
||||
$now = Carbon::now();
|
||||
[$periodStart, $periodEnd] = $progressService->getPeriodBounds($this->resource, $now);
|
||||
$trackedSeconds = $progressService->getProgress($this->resource, $periodStart, $periodEnd);
|
||||
|
||||
return [
|
||||
/** @var string $id ID of the goal */
|
||||
'id' => $this->resource->id,
|
||||
@@ -70,13 +73,13 @@ class GoalResource extends BaseResource
|
||||
],
|
||||
'progress' => [
|
||||
/** @var string $period_start Start of the current period (inclusive) */
|
||||
'period_start' => $this->formatDateTime($periodStart),
|
||||
'period_start' => $this->formatDateTime($this->progress->periodStart),
|
||||
/** @var string $period_end End of the current period (exclusive) */
|
||||
'period_end' => $this->formatDateTime($periodEnd),
|
||||
'period_end' => $this->formatDateTime($this->progress->periodEnd),
|
||||
/** @var int $tracked_seconds Seconds tracked in the current period that match the filters, incl. the running time entry */
|
||||
'tracked_seconds' => $trackedSeconds,
|
||||
'tracked_seconds' => $this->progress->trackedSeconds,
|
||||
/** @var string $status Status in the current period (in_progress, achieved, on_track, exceeded) */
|
||||
'status' => $progressService->getStatus($this->resource, $trackedSeconds)->value,
|
||||
'status' => $this->progress->status->value,
|
||||
],
|
||||
/** @var string $created_at Date when the goal was created */
|
||||
'created_at' => $this->formatDateTime($this->resource->created_at),
|
||||
|
||||
22
app/Service/Dto/GoalProgressDto.php
Normal file
22
app/Service/Dto/GoalProgressDto.php
Normal file
@@ -0,0 +1,22 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Service\Dto;
|
||||
|
||||
use App\Enums\GoalStatus;
|
||||
use Illuminate\Support\Carbon;
|
||||
|
||||
readonly class GoalProgressDto
|
||||
{
|
||||
/**
|
||||
* @param Carbon $periodStart Start of the period in UTC (inclusive)
|
||||
* @param Carbon $periodEnd End of the period in UTC (exclusive)
|
||||
*/
|
||||
public function __construct(
|
||||
public Carbon $periodStart,
|
||||
public Carbon $periodEnd,
|
||||
public int $trackedSeconds,
|
||||
public GoalStatus $status,
|
||||
) {}
|
||||
}
|
||||
@@ -9,10 +9,40 @@ use App\Enums\GoalPeriod;
|
||||
use App\Enums\GoalStatus;
|
||||
use App\Models\Goal;
|
||||
use App\Models\TimeEntry;
|
||||
use App\Service\Dto\GoalProgressDto;
|
||||
use Illuminate\Support\Carbon;
|
||||
use Illuminate\Support\Collection;
|
||||
|
||||
class GoalProgressService
|
||||
{
|
||||
/**
|
||||
* Progress of the goal in the period that contains $now.
|
||||
*/
|
||||
public function getCurrentProgress(Goal $goal, Carbon $now): GoalProgressDto
|
||||
{
|
||||
[$periodStart, $periodEnd] = $this->getPeriodBounds($goal, $now);
|
||||
$trackedSeconds = $this->getProgress($goal, $periodStart, $periodEnd);
|
||||
|
||||
return new GoalProgressDto($periodStart, $periodEnd, $trackedSeconds, $this->getStatus($goal, $trackedSeconds));
|
||||
}
|
||||
|
||||
/**
|
||||
* Progress of the goals in the period that contains $now, keyed by goal ID.
|
||||
* Runs one query per goal, since every goal has its own filters, timezone and period.
|
||||
*
|
||||
* @param Collection<int, Goal> $goals
|
||||
* @return array<string, GoalProgressDto>
|
||||
*/
|
||||
public function getCurrentProgressForGoals(Collection $goals, Carbon $now): array
|
||||
{
|
||||
$progress = [];
|
||||
foreach ($goals as $goal) {
|
||||
$progress[$goal->getKey()] = $this->getCurrentProgress($goal, $now);
|
||||
}
|
||||
|
||||
return $progress;
|
||||
}
|
||||
|
||||
/**
|
||||
* Bounds of the period that contains $date, in UTC. The period is calculated in the timezone of the goal
|
||||
* and respects the week start of the goal. The start is inclusive, the end exclusive.
|
||||
@@ -46,7 +76,7 @@ class GoalProgressService
|
||||
* @param Carbon $periodStart Start of the period (inclusive)
|
||||
* @param Carbon $periodEnd End of the period (exclusive)
|
||||
*/
|
||||
public function getProgress(Goal $goal, Carbon $periodStart, Carbon $periodEnd): int
|
||||
private function getProgress(Goal $goal, Carbon $periodStart, Carbon $periodEnd): int
|
||||
{
|
||||
$filters = $goal->filters;
|
||||
|
||||
|
||||
@@ -205,6 +205,45 @@ class GoalEndpointTest extends ApiEndpointTestAbstract
|
||||
);
|
||||
}
|
||||
|
||||
public function test_index_endpoint_returns_the_progress_of_each_goal(): void
|
||||
{
|
||||
// Arrange
|
||||
$this->travelTo(Carbon::create(2024, 1, 3, 12, 0, 0, 'UTC'));
|
||||
$data = $this->createUserWithPermission(['goals:view:own']);
|
||||
$project = Project::factory()->forOrganization($data->organization)->create();
|
||||
$filters = new GoalFiltersDto;
|
||||
$filters->setProjectIds([$project->getKey()]);
|
||||
$dayGoal = Goal::factory()->forMember($data->member)->period(GoalPeriod::Day)->atLeast(3600)->filters($filters)->create([
|
||||
'created_at' => Carbon::now()->subDay(),
|
||||
]);
|
||||
$weekGoal = Goal::factory()->forMember($data->member)->period(GoalPeriod::Week)->atLeast(3600)->create([
|
||||
'created_at' => Carbon::now()->subDays(2),
|
||||
]);
|
||||
TimeEntry::factory()->forMember($data->member)->forProject($project)
|
||||
->startWithDuration(Carbon::now()->subHours(3), 1200)->create();
|
||||
TimeEntry::factory()->forMember($data->member)
|
||||
->startWithDuration(Carbon::now()->subDay(), 1800)->create();
|
||||
Passport::actingAs($data->user);
|
||||
|
||||
// Act
|
||||
$response = $this->getJson(route('api.v1.goals.index', [$data->organization->getKey()]));
|
||||
|
||||
// Assert
|
||||
$response->assertOk();
|
||||
$response->assertJson(fn (AssertableJson $json) => $json
|
||||
->count('data', 2)
|
||||
->where('data.0.id', $dayGoal->getKey())
|
||||
->where('data.0.progress.period_start', '2024-01-03T00:00:00Z')
|
||||
->where('data.0.progress.period_end', '2024-01-04T00:00:00Z')
|
||||
->where('data.0.progress.tracked_seconds', 1200)
|
||||
->where('data.1.id', $weekGoal->getKey())
|
||||
->where('data.1.progress.period_start', '2024-01-01T00:00:00Z')
|
||||
->where('data.1.progress.period_end', '2024-01-08T00:00:00Z')
|
||||
->where('data.1.progress.tracked_seconds', 1200 + 1800)
|
||||
->etc()
|
||||
);
|
||||
}
|
||||
|
||||
public function test_index_endpoint_does_not_show_organization_goals(): void
|
||||
{
|
||||
// Arrange
|
||||
|
||||
@@ -43,9 +43,8 @@ class GoalProgressServiceTest extends TestCase
|
||||
private function progressAt(Goal $goal, Carbon $now): int
|
||||
{
|
||||
Carbon::setTestNow($now);
|
||||
[$periodStart, $periodEnd] = $this->service->getPeriodBounds($goal, $now);
|
||||
|
||||
return $this->service->getProgress($goal, $periodStart, $periodEnd);
|
||||
return $this->service->getCurrentProgress($goal, $now)->trackedSeconds;
|
||||
}
|
||||
|
||||
public function test_period_bounds_of_day_goal_are_calculated_in_the_timezone_of_the_goal(): void
|
||||
|
||||
Reference in New Issue
Block a user