add time entry filter normalizeation

This commit is contained in:
Gregor Vostrak
2026-10-02 14:20:47 +02:00
parent 349623d537
commit ded56c0798
5 changed files with 165 additions and 11 deletions

View File

@@ -117,13 +117,13 @@ class ReportPropertiesDto implements Castable
$dto->end = $data->end !== null ? Carbon::createFromFormat('Y-m-d\TH:i:s\Z', $data->end) : null; $dto->end = $data->end !== null ? Carbon::createFromFormat('Y-m-d\TH:i:s\Z', $data->end) : null;
$dto->start = $data->start !== null ? Carbon::createFromFormat('Y-m-d\TH:i:s\Z', $data->start) : null; $dto->start = $data->start !== null ? Carbon::createFromFormat('Y-m-d\TH:i:s\Z', $data->start) : null;
$dto->active = $data->active; $dto->active = $data->active;
$dto->memberIds = $data->memberIds !== null ? ReportPropertiesDto::idArrayToCollection($data->memberIds) : null; $dto->setMemberIds($data->memberIds !== null ? (array) $data->memberIds : null);
$dto->billable = $data->billable; $dto->billable = $data->billable;
$dto->clientIds = $data->clientIds !== null ? ReportPropertiesDto::idArrayToCollection($data->clientIds) : null; $dto->setClientIds($data->clientIds !== null ? (array) $data->clientIds : null);
$dto->projectIds = $data->projectIds !== null ? ReportPropertiesDto::idArrayToCollection($data->projectIds) : null; $dto->setProjectIds($data->projectIds !== null ? (array) $data->projectIds : null);
$dto->tagIds = $data->tagIds !== null ? ReportPropertiesDto::idArrayToCollection($data->tagIds) : null; $dto->setTagIds($data->tagIds !== null ? (array) $data->tagIds : null);
$dto->tagMatchType = isset($data->tagMatchType) ? TagMatchType::from($data->tagMatchType) : null; $dto->tagMatchType = isset($data->tagMatchType) ? TagMatchType::from($data->tagMatchType) : null;
$dto->taskIds = $data->taskIds ? ReportPropertiesDto::idArrayToCollection($data->taskIds) : null; $dto->setTaskIds($data->taskIds !== null ? (array) $data->taskIds : null);
$dto->group = TimeEntryAggregationType::from($data->group); $dto->group = TimeEntryAggregationType::from($data->group);
$dto->subGroup = TimeEntryAggregationType::from($data->subGroup); $dto->subGroup = TimeEntryAggregationType::from($data->subGroup);
$dto->historyGroup = TimeEntryAggregationTypeInterval::from($data->historyGroup); $dto->historyGroup = TimeEntryAggregationTypeInterval::from($data->historyGroup);
@@ -205,7 +205,7 @@ class ReportPropertiesDto implements Castable
*/ */
public function setMemberIds(?array $memberIds): void public function setMemberIds(?array $memberIds): void
{ {
$this->memberIds = $memberIds !== null ? ReportPropertiesDto::idArrayToCollection($memberIds) : null; $this->memberIds = $memberIds !== null && count($memberIds) > 0 ? ReportPropertiesDto::idArrayToCollection($memberIds) : null;
} }
/** /**
@@ -213,7 +213,7 @@ class ReportPropertiesDto implements Castable
*/ */
public function setClientIds(?array $clientIds): void public function setClientIds(?array $clientIds): void
{ {
$this->clientIds = $clientIds !== null ? ReportPropertiesDto::idArrayToCollection($clientIds) : null; $this->clientIds = $clientIds !== null && count($clientIds) > 0 ? ReportPropertiesDto::idArrayToCollection($clientIds) : null;
} }
/** /**
@@ -221,7 +221,7 @@ class ReportPropertiesDto implements Castable
*/ */
public function setProjectIds(?array $projectIds): void public function setProjectIds(?array $projectIds): void
{ {
$this->projectIds = $projectIds !== null ? ReportPropertiesDto::idArrayToCollection($projectIds) : null; $this->projectIds = $projectIds !== null && count($projectIds) > 0 ? ReportPropertiesDto::idArrayToCollection($projectIds) : null;
} }
/** /**
@@ -229,7 +229,7 @@ class ReportPropertiesDto implements Castable
*/ */
public function setTagIds(?array $tagIds): void public function setTagIds(?array $tagIds): void
{ {
$this->tagIds = $tagIds !== null ? ReportPropertiesDto::idArrayToCollection($tagIds) : null; $this->tagIds = $tagIds !== null && count($tagIds) > 0 ? ReportPropertiesDto::idArrayToCollection($tagIds) : null;
} }
public function setTagMatchType(?TagMatchType $tagMatchType): void public function setTagMatchType(?TagMatchType $tagMatchType): void
@@ -242,6 +242,6 @@ class ReportPropertiesDto implements Castable
*/ */
public function setTaskIds(?array $taskIds): void public function setTaskIds(?array $taskIds): void
{ {
$this->taskIds = $taskIds !== null ? ReportPropertiesDto::idArrayToCollection($taskIds) : null; $this->taskIds = $taskIds !== null && count($taskIds) > 0 ? ReportPropertiesDto::idArrayToCollection($taskIds) : null;
} }
} }

View File

@@ -111,7 +111,8 @@ class TimeEntryFilter
*/ */
public function addMemberIdsFilter(?array $memberIds): self public function addMemberIdsFilter(?array $memberIds): self
{ {
if ($memberIds === null) { // An empty selection is no constraint, the same as null
if ($memberIds === null || count($memberIds) === 0) {
return $this; return $this;
} }
$this->builder->whereIn('member_id', $memberIds); $this->builder->whereIn('member_id', $memberIds);
@@ -181,6 +182,10 @@ class TimeEntryFilter
} }
$includeNone = in_array(self::NONE_VALUE, $clientIds, true); $includeNone = in_array(self::NONE_VALUE, $clientIds, true);
$clientIds = array_values(array_filter($clientIds, fn (string $id): bool => $id !== self::NONE_VALUE)); $clientIds = array_values(array_filter($clientIds, fn (string $id): bool => $id !== self::NONE_VALUE));
// An empty selection (no client IDs and not filtering for "none") is no constraint, so apply nothing
if (count($clientIds) === 0 && ! $includeNone) {
return $this;
}
$this->builder->where(function (Builder $builder) use ($clientIds, $includeNone): void { $this->builder->where(function (Builder $builder) use ($clientIds, $includeNone): void {
if (count($clientIds) > 0) { if (count($clientIds) > 0) {
@@ -204,6 +209,10 @@ class TimeEntryFilter
} }
$includeNone = in_array(self::NONE_VALUE, $projectIds, true); $includeNone = in_array(self::NONE_VALUE, $projectIds, true);
$projectIds = array_values(array_filter($projectIds, fn (string $id): bool => $id !== self::NONE_VALUE)); $projectIds = array_values(array_filter($projectIds, fn (string $id): bool => $id !== self::NONE_VALUE));
// An empty selection (no project IDs and not filtering for "none") is no constraint, so apply nothing
if (count($projectIds) === 0 && ! $includeNone) {
return $this;
}
$this->builder->where(function (Builder $builder) use ($projectIds, $includeNone): void { $this->builder->where(function (Builder $builder) use ($projectIds, $includeNone): void {
if (count($projectIds) > 0) { if (count($projectIds) > 0) {
@@ -269,6 +278,10 @@ class TimeEntryFilter
} }
$includeNone = in_array(self::NONE_VALUE, $taskIds, true); $includeNone = in_array(self::NONE_VALUE, $taskIds, true);
$taskIds = array_values(array_filter($taskIds, fn (string $id): bool => $id !== self::NONE_VALUE)); $taskIds = array_values(array_filter($taskIds, fn (string $id): bool => $id !== self::NONE_VALUE));
// An empty selection (no task IDs and not filtering for "none") is no constraint, so apply nothing
if (count($taskIds) === 0 && ! $includeNone) {
return $this;
}
$this->builder->where(function (Builder $builder) use ($taskIds, $includeNone): void { $this->builder->where(function (Builder $builder) use ($taskIds, $includeNone): void {
if (count($taskIds) > 0) { if (count($taskIds) > 0) {

View File

@@ -606,6 +606,51 @@ class ReportEndpointTest extends ApiEndpointTestAbstract
); );
} }
public function test_store_endpoint_stores_empty_id_filters_as_null(): void
{
// Arrange
$data = $this->createUserWithPermission([
'reports:create',
]);
Passport::actingAs($data->user);
// Act
$response = $this->postJson(route('api.v1.reports.store', [$data->organization->getKey()]), [
'name' => 'Test Report with empty Filters',
'is_public' => false,
'properties' => [
'start' => Carbon::now()->subDays(30)->toIso8601ZuluString(),
'end' => Carbon::now()->toIso8601ZuluString(),
'group' => TimeEntryAggregationType::Project->value,
'sub_group' => TimeEntryAggregationType::Task->value,
'history_group' => TimeEntryAggregationType::Day->value,
'member_ids' => [],
'project_ids' => [],
'client_ids' => [],
'tag_ids' => [],
'task_ids' => [],
],
]);
// Assert
$response->assertStatus(201);
$response->assertJson(fn (AssertableJson $json) => $json
->where('data.properties.member_ids', null)
->where('data.properties.project_ids', null)
->where('data.properties.client_ids', null)
->where('data.properties.tag_ids', null)
->where('data.properties.task_ids', null)
->etc()
);
/** @var Report $report */
$report = Report::query()->findOrFail($response->json('data.id'));
$this->assertNull($report->properties->memberIds);
$this->assertNull($report->properties->projectIds);
$this->assertNull($report->properties->clientIds);
$this->assertNull($report->properties->tagIds);
$this->assertNull($report->properties->taskIds);
}
public function test_store_endpoint_creates_report_with_none_filter_values(): void public function test_store_endpoint_creates_report_with_none_filter_values(): void
{ {
// Arrange // Arrange

View File

@@ -88,4 +88,25 @@ class ReportPropertiesDtoTest extends TestCase
$this->assertSame(TimeEntryType::Work, $dtoWork->timeEntryType); $this->assertSame(TimeEntryType::Work, $dtoWork->timeEntryType);
$this->assertSame(TimeEntryType::Break, $dtoBreak->timeEntryType); $this->assertSame(TimeEntryType::Break, $dtoBreak->timeEntryType);
} }
public function test_empty_id_filters_of_a_persisted_report_are_read_as_null(): void
{
// Arrange
$properties = $this->getBaseProperties();
$properties['memberIds'] = [];
$properties['clientIds'] = [];
$properties['projectIds'] = [];
$properties['tagIds'] = [];
$properties['taskIds'] = [];
// Act
$dto = $this->castFromJson($properties);
// Assert
$this->assertNull($dto->memberIds);
$this->assertNull($dto->clientIds);
$this->assertNull($dto->projectIds);
$this->assertNull($dto->tagIds);
$this->assertNull($dto->taskIds);
}
} }

View File

@@ -416,6 +416,81 @@ class TimeEntryFilterTest extends TestCaseWithDatabase
$this->assertCount(3, $builderContains->get()); $this->assertCount(3, $builderContains->get());
} }
public function test_add_member_ids_filter_with_empty_array_applies_no_filter(): void
{
// Arrange
TimeEntry::factory()->createMany(2);
$builder = TimeEntry::query();
$filter = new TimeEntryFilter($builder);
// Act
$filter->addMemberIdsFilter([]);
// Assert
$this->assertCount(2, $builder->get());
}
public function test_add_project_ids_filter_with_empty_array_applies_no_filter(): void
{
// Arrange
$project = Project::factory()->create();
TimeEntry::factory()->create([
'project_id' => $project->getKey(),
'organization_id' => $project->organization_id,
]);
TimeEntry::factory()->create(['project_id' => null]);
$builder = TimeEntry::query();
$filter = new TimeEntryFilter($builder);
// Act
$filter->addProjectIdsFilter([]);
// Assert
$this->assertCount(2, $builder->get());
}
public function test_add_task_ids_filter_with_empty_array_applies_no_filter(): void
{
// Arrange
$task = Task::factory()->create();
TimeEntry::factory()->create([
'task_id' => $task->getKey(),
'organization_id' => $task->organization_id,
]);
TimeEntry::factory()->create(['task_id' => null]);
$builder = TimeEntry::query();
$filter = new TimeEntryFilter($builder);
// Act
$filter->addTaskIdsFilter([]);
// Assert
$this->assertCount(2, $builder->get());
}
public function test_add_client_ids_filter_with_empty_array_applies_no_filter(): void
{
// Arrange
$client = Client::factory()->create();
TimeEntry::factory()->create([
'client_id' => $client->getKey(),
'organization_id' => $client->organization_id,
]);
TimeEntry::factory()->create(['client_id' => null]);
$builder = TimeEntry::query();
$filter = new TimeEntryFilter($builder);
// Act
$filter->addClientIdsFilter([]);
// Assert
$this->assertCount(2, $builder->get());
}
public function test_add_tag_ids_filter_with_null_match_type_defaults_to_contains(): void public function test_add_tag_ids_filter_with_null_match_type_defaults_to_contains(): void
{ {
// Arrange // Arrange