From f242ce48b5e5a7a030e85f585927bc9a885c790d Mon Sep 17 00:00:00 2001 From: Gregor Vostrak Date: Wed, 14 Jan 2026 18:25:48 +0100 Subject: [PATCH] change rounding up on boundaries so it does not round up but keeps the value, fixes #994 --- app/Service/TimeEntryService.php | 11 ++- .../TimeEntryAggregationServiceTest.php | 97 +++++++++++++++++++ 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/app/Service/TimeEntryService.php b/app/Service/TimeEntryService.php index 6cd97aa5..633f97d7 100644 --- a/app/Service/TimeEntryService.php +++ b/app/Service/TimeEntryService.php @@ -31,12 +31,17 @@ class TimeEntryService throw new LogicException('Rounding minutes must be greater than 0'); } $end = 'coalesce("end", \''.Carbon::now()->toDateTimeString().'\')'; + $start = $this->getStartSelectRawForRounding($roundingType, $roundingMinutes); if ($roundingType === TimeEntryRoundingType::Down) { - return 'date_bin(\''.$roundingMinutes.' minutes\', '.$end.', '.$this->getStartSelectRawForRounding($roundingType, $roundingMinutes).')'; + return 'date_bin(\''.$roundingMinutes.' minutes\', '.$end.', '.$start.')'; } elseif ($roundingType === TimeEntryRoundingType::Up) { - return 'date_bin(\''.$roundingMinutes.' minutes\', '.$end.' + interval \''.$roundingMinutes.' minutes\', '.$this->getStartSelectRawForRounding($roundingType, $roundingMinutes).')'; + // If end is already on a boundary, keep it; otherwise round up to next boundary + return 'CASE WHEN '.$end.' = date_bin(\''.$roundingMinutes.' minutes\', '.$end.', '.$start.') '. + 'THEN '.$end.' '. + 'ELSE date_bin(\''.$roundingMinutes.' minutes\', '.$end.' + interval \''.$roundingMinutes.' minutes\', '.$start.') '. + 'END'; } elseif ($roundingType === TimeEntryRoundingType::Nearest) { - return 'date_bin(\''.$roundingMinutes.' minutes\', '.$end.' + interval \''.($roundingMinutes / 2).' minutes\', '.$this->getStartSelectRawForRounding($roundingType, $roundingMinutes).')'; + return 'date_bin(\''.$roundingMinutes.' minutes\', '.$end.' + interval \''.($roundingMinutes / 2).' minutes\', '.$start.')'; } } } diff --git a/tests/Unit/Service/TimeEntryAggregationServiceTest.php b/tests/Unit/Service/TimeEntryAggregationServiceTest.php index 4988ef06..83bb6e3d 100644 --- a/tests/Unit/Service/TimeEntryAggregationServiceTest.php +++ b/tests/Unit/Service/TimeEntryAggregationServiceTest.php @@ -1205,4 +1205,101 @@ class TimeEntryAggregationServiceTest extends TestCaseWithDatabase ]; $this->assertEqualsCanonicalizing($expected, $result); } + + /** + * Test that rounding up does NOT add extra time when the entry is already on a 15-minute boundary. + * f.e. 13:00 - 14:30 (90 minutes) should stay at 90 minutes when rounding up with 15-minute interval. + */ + public function test_aggregate_time_round_up_does_not_add_time_when_already_on_boundary(): void + { + // Arrange + // Create a time entry with duration exactly on a 15-minute boundary (90 minutes = 5400 seconds) + // This simulates 13:00 - 14:30 (or any 90-minute entry) + $project = Project::factory()->create(); + TimeEntry::factory()->startWithDuration( + Carbon::createFromFormat('Y-m-d H:i:s', '2020-01-01 13:00:00'), + 5400 // 90 minutes = 1 hour 30 minutes, exactly on 15-minute boundary + )->forProject($project)->create(); + $query = TimeEntry::query(); + + // Act + $result = $this->service->getAggregatedTimeEntries( + $query, + TimeEntryAggregationType::Project, + null, + 'Europe/Vienna', + Weekday::Monday, + false, + null, + null, + true, + TimeEntryRoundingType::Up, + 15 + ); + + // Assert + // The entry is already on a 15-minute boundary (90 minutes), so it should stay at 90 minutes (5400 seconds) + $this->assertEqualsCanonicalizing([ + 'seconds' => 5400, // 90 minutes - should NOT be rounded to 105 minutes (6300 seconds) + 'cost' => 0, + 'grouped_type' => 'project', + 'grouped_data' => [ + [ + 'key' => $project->getKey(), + 'seconds' => 5400, // 90 minutes + 'cost' => 0, + 'grouped_type' => null, + 'grouped_data' => null, + ], + ], + ], $result); + } + + /** + * Test that rounding up works correctly for entries NOT on a boundary. + * Example: 13:00 - 13:48 (48 minutes) should round up to 13:00 - 14:00 (60 minutes). + */ + public function test_aggregate_time_round_up_works_when_not_on_boundary(): void + { + // Arrange + // Create a time entry with duration NOT on a 15-minute boundary (48 minutes = 2880 seconds) + $project = Project::factory()->create(); + TimeEntry::factory()->startWithDuration( + Carbon::createFromFormat('Y-m-d H:i:s', '2020-01-01 13:00:00'), + 2880 // 48 minutes, not on 15-minute boundary + )->forProject($project)->create(); + $query = TimeEntry::query(); + + // Act + $result = $this->service->getAggregatedTimeEntries( + $query, + TimeEntryAggregationType::Project, + null, + 'Europe/Vienna', + Weekday::Monday, + false, + null, + null, + true, + TimeEntryRoundingType::Up, + 15 + ); + + // Assert + // 48 minutes rounded up to 15-minute interval = 60 minutes (3600 seconds) + $this->assertEqualsCanonicalizing([ + 'seconds' => 3600, // 60 minutes + 'cost' => 0, + 'grouped_type' => 'project', + 'grouped_data' => [ + [ + 'key' => $project->getKey(), + 'seconds' => 3600, // 60 minutes + 'cost' => 0, + 'grouped_type' => null, + 'grouped_data' => null, + ], + ], + ], $result); + } }