From ec34647f77dc4b8ef7f1ca150eaec37a65adda3a Mon Sep 17 00:00:00 2001 From: William Allen Date: Fri, 11 Sep 2026 11:26:26 -0400 Subject: [PATCH] Use null instead of magic 1980 constant in buildgroups Active buildgroups have a magic 1980 timestamp in the `endtime` column, which adds additional logic any time build groups are manipulated. This PR switches from the magic 1980 timestamp to null as the active build group marker. --- app/Jobs/PerformLegacyDailyUpdates.php | 8 ++++---- app/Models/BuildGroup.php | 12 +++++++++++- app/Services/ProjectService.php | 4 ++-- app/cdash/app/Controller/Api/Index.php | 3 +-- app/cdash/app/Model/BuildGroup.php | 10 +++++----- .../CDash/MultipleSubprojectsEmailTest.php | 5 ++++- app/cdash/tests/test_buildmodel.php | 2 ++ app/cdash/tests/test_projectindb.php | 6 +++--- database/factories/BuildGroupFactory.php | 2 +- .../2026_09_10_194315_buildgroup_times.php | 19 +++++++++++++++++++ phpstan-baseline.neon | 9 +++++++++ tests/Feature/Services/ProjectServiceTest.php | 8 ++++++++ 12 files changed, 69 insertions(+), 19 deletions(-) create mode 100644 database/migrations/2026_09_10_194315_buildgroup_times.php diff --git a/app/Jobs/PerformLegacyDailyUpdates.php b/app/Jobs/PerformLegacyDailyUpdates.php index c24da5901e..e7b90721e3 100644 --- a/app/Jobs/PerformLegacyDailyUpdates.php +++ b/app/Jobs/PerformLegacyDailyUpdates.php @@ -81,7 +81,7 @@ private function sendEmailExpectedBuilds($projectid, $currentstarttime): void AND bg.starttime? - OR bg.endtime='1980-01-01 00:00:00' + OR bg.endtime IS NULL ) AND site.id=t1.siteid ", [ @@ -203,10 +203,10 @@ private function addDailyChanges(int $projectid): void BuildGroupRule::DeleteExpiredRulesForProject($project->Id, $cutoff_date); $stmt = $db->prepare( - "SELECT id FROM buildgroup + 'SELECT id FROM buildgroup WHERE projectid = :projectid AND - endtime != '1980-01-01 00:00:00' AND - endtime < :endtime"); + endtime IS NOT NULL AND + endtime < :endtime'); $query_params = [ ':projectid' => $project->Id, ':endtime' => $cutoff_date, diff --git a/app/Models/BuildGroup.php b/app/Models/BuildGroup.php index 83021e3d69..adaf225817 100644 --- a/app/Models/BuildGroup.php +++ b/app/Models/BuildGroup.php @@ -3,6 +3,7 @@ namespace App\Models; use Database\Factories\BuildGroupFactory; +use Illuminate\Database\Eloquent\Attributes\Scope; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; @@ -16,7 +17,7 @@ * @property string $name * @property int $projectid * @property Carbon $starttime - * @property Carbon $endtime + * @property ?Carbon $endtime * @property int $autoremovetimeframe * @property ?string $description * @property int $summaryemail @@ -93,4 +94,13 @@ public function rules(): HasMany { return $this->hasMany(BuildGroupRule::class, 'groupid'); } + + /** + * @param Builder<$this> $query + */ + #[Scope] + protected function active(Builder $query): void + { + $query->whereNull('endtime'); + } } diff --git a/app/Services/ProjectService.php b/app/Services/ProjectService.php index 8d66015ce6..b308eb27cf 100644 --- a/app/Services/ProjectService.php +++ b/app/Services/ProjectService.php @@ -59,7 +59,7 @@ public static function initializeBuildGroups(Project $project): void { $common_defaults = [ 'starttime' => Carbon::create(1980), - 'endtime' => Carbon::create(1980), + 'endtime' => null, 'type' => 'Daily', 'includesubprojectotal' => 1, 'emailcommitters' => 0, @@ -121,7 +121,7 @@ public static function getBuildGroups(int $projectid): array { $eloquent_buildgroups = Project::findOrFail($projectid) ->buildgroups() - ->where('endtime', Carbon::create(1980)) + ->whereNull('endtime') ->get(); $buildgroups = []; diff --git a/app/cdash/app/Controller/Api/Index.php b/app/cdash/app/Controller/Api/Index.php index b70f94fd90..1b5cfa4dee 100644 --- a/app/cdash/app/Controller/Api/Index.php +++ b/app/cdash/app/Controller/Api/Index.php @@ -194,7 +194,7 @@ public function getDynamicBuilds(): array LEFT JOIN buildgroupposition AS gp ON (gp.buildgroupid = bg.id) WHERE bg.projectid = ? - AND bg.endtime = ? + AND bg.endtime IS NULL AND bg.type != 'Daily' AND b2gr.starttime < ? AND ( @@ -203,7 +203,6 @@ public function getDynamicBuilds(): array ) ", [ (int) $this->project->Id, - self::BEGIN_EPOCH, $this->endDate, self::BEGIN_EPOCH, $this->endDate, diff --git a/app/cdash/app/Model/BuildGroup.php b/app/cdash/app/Model/BuildGroup.php index f31cf05882..63e66f2382 100644 --- a/app/cdash/app/Model/BuildGroup.php +++ b/app/cdash/app/Model/BuildGroup.php @@ -39,7 +39,7 @@ public function __construct() 'projectid' => 0, 'name' => '', 'starttime' => Carbon::create(1980), - 'endtime' => Carbon::create(1980), + 'endtime' => null, 'description' => '', 'summaryemail' => 0, 'type' => 'Daily', @@ -335,7 +335,7 @@ public function Save(): bool $this->eloquent_model->positions()->create([ 'position' => $position, 'starttime' => $this->eloquent_model->starttime, - 'endtime' => $this->eloquent_model->endtime, + 'endtime' => Carbon::create(1980), ]); } return true; @@ -463,14 +463,14 @@ public static function GetBuildGroups($projectid, $begin): array { $buildgroups = []; - $stmt = DB::select(" + $stmt = DB::select(' SELECT bg.id, bg.name, bgp.position FROM buildgroup AS bg LEFT JOIN buildgroupposition AS bgp ON (bgp.buildgroupid = bg.id) WHERE bg.projectid = ? AND bg.starttime < ? AND - (bg.endtime > ? OR bg.endtime='1980-01-01 00:00:00') - ", [$projectid, $begin, $begin]); + (bg.endtime > ? OR bg.endtime IS NULL) + ', [$projectid, $begin, $begin]); foreach ($stmt as $row) { $buildgroup = new self(); diff --git a/app/cdash/tests/case/CDash/MultipleSubprojectsEmailTest.php b/app/cdash/tests/case/CDash/MultipleSubprojectsEmailTest.php index d046b16fa3..2a08f2f0a2 100644 --- a/app/cdash/tests/case/CDash/MultipleSubprojectsEmailTest.php +++ b/app/cdash/tests/case/CDash/MultipleSubprojectsEmailTest.php @@ -16,6 +16,7 @@ */ use App\Http\Submission\Handlers\ActionableBuildInterface; +use App\Models\BuildGroup; use CDash\Collection\SubscriberCollection; use CDash\Database; use CDash\Messaging\Notification\Email\EmailBuilder; @@ -28,6 +29,7 @@ use CDash\Model\Subscriber; use CDash\Test\CDashUseCaseTestCase; use CDash\Test\UseCase\UseCase; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\URL; use Illuminate\Support\Str; @@ -125,9 +127,10 @@ public function setUp(): void } // Do the same for build groups - DB::table('buildgroup')->insertOrIgnore([ + BuildGroup::insertOrIgnore([ 'id' => 0, 'projectid' => self::$projectid, + 'starttime' => Carbon::now(), 'description' => 'MultipleSubprojectsEmailTest-' . Str::uuid()->toString(), ]); } diff --git a/app/cdash/tests/test_buildmodel.php b/app/cdash/tests/test_buildmodel.php index 9a0bb98f04..6ab5e7f86a 100644 --- a/app/cdash/tests/test_buildmodel.php +++ b/app/cdash/tests/test_buildmodel.php @@ -10,6 +10,7 @@ use App\Utils\DatabaseCleanupUtils; use CDash\Model\Build; use CDash\Model\BuildError; +use Illuminate\Support\Carbon; use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; @@ -38,6 +39,7 @@ public function __construct() DB::table('buildgroup')->insertOrIgnore([ 'id' => 0, 'projectid' => $project->id, + 'starttime' => Carbon::now(), 'description' => 'MultipleSubprojectsEmailTest-' . Str::uuid()->toString(), ]); diff --git a/app/cdash/tests/test_projectindb.php b/app/cdash/tests/test_projectindb.php index 5281855386..d82b1090ce 100644 --- a/app/cdash/tests/test_projectindb.php +++ b/app/cdash/tests/test_projectindb.php @@ -39,15 +39,15 @@ public function testProjectInBuildGroup(): void $result = $this->db->query($query); $expected = ['0' => ['name' => 'Nightly', 'starttime' => '1980-01-01 00:00:00', - 'endtime' => '1980-01-01 00:00:00', + 'endtime' => null, 'description' => 'Nightly builds'], '1' => ['name' => 'Experimental', 'starttime' => '1980-01-01 00:00:00', - 'endtime' => '1980-01-01 00:00:00', + 'endtime' => null, 'description' => 'Experimental builds'], '2' => ['name' => 'Continuous', 'starttime' => '1980-01-01 00:00:00', - 'endtime' => '1980-01-01 00:00:00', + 'endtime' => null, 'description' => 'Continuous builds']]; $this->assertEqual($result, $expected); } diff --git a/database/factories/BuildGroupFactory.php b/database/factories/BuildGroupFactory.php index 6880767cd1..0f5342eba1 100644 --- a/database/factories/BuildGroupFactory.php +++ b/database/factories/BuildGroupFactory.php @@ -22,7 +22,7 @@ public function definition(): array return [ 'name' => Str::uuid()->toString(), 'starttime' => Carbon::create(1980), - 'endtime' => Carbon::create(1980), + 'endtime' => null, 'autoremovetimeframe' => 0, 'description' => Str::uuid()->toString(), 'summaryemail' => 0, diff --git a/database/migrations/2026_09_10_194315_buildgroup_times.php b/database/migrations/2026_09_10_194315_buildgroup_times.php new file mode 100644 index 0000000000..8619ff3e4f --- /dev/null +++ b/database/migrations/2026_09_10_194315_buildgroup_times.php @@ -0,0 +1,19 @@ +buildgroups()->pluck('name')->toArray(), ); + + foreach ($project->buildgroups as $buildgroup) { + self::assertNotNull($buildgroup->starttime); + self::assertNull($buildgroup->endtime); + } + + $active_groups = ProjectService::getBuildGroups($project->id); + self::assertCount(3, $active_groups); } }