Skip to content

FIX Check all classes and don't rely on connection_attempted() #11807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
emteknetnz merged 1 commit into from
Jul 29, 2025
Merged

FIX Check all classes and don't rely on connection_attempted() #11807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/ORM/DataObjectSchema.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ public function tablesAreReadyForClass(string $class): bool
}

// Bail if there's no active database connection yet
if (!DB::connection_attempted() || !DB::is_active()) {
if (!DB::is_active()) {
return false;
}

Expand All @@ -870,7 +870,7 @@ public function tablesAreReadyForClass(string $class): bool
}

// Don't check again if we already know the db is ready for this class.
if (!empty($this->tableReadyClasses[$class])) {
if (!empty($this->tableReadyClasses[$required])) {
Copy link
Copy Markdown
Member Author

@GuySartorelli GuySartorelli Jul 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! This was only ever checking the same class over and over instead of the one in the loop 😅
The unit tests were incorrect so it looked like things were working as expected.

continue;
}

Expand Down
5 changes: 3 additions & 2 deletions tests/php/ORM/DataObjectSchemaTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,12 +516,13 @@ public function testTablesAreReadyForClass(): void
$this->assertTrue($schema->tablesAreReadyForClass(Folder::class), 'Folder table should be ready at start');

// Reset cache and drop a column from the subclass. See AdditionalFieldsExtension
// Note that the superclass also reports as not ready because File::get() will return images as well.
$schema->clearTableReadyForClass();
DB::query(sprintf(
'ALTER TABLE "%s" DROP COLUMN "SomeField";',
DataObject::getSchema()->tableForField(Folder::class, 'SomeField')
));
$this->assertTrue($schema->tablesAreReadyForClass(File::class), 'File table should be ready after drop column');
$this->assertFalse($schema->tablesAreReadyForClass(File::class), 'File table should NOT be ready after drop column');
$this->assertTrue($schema->tablesAreReadyForClass(Image::class), 'Image table should be ready after drop column');
$this->assertFalse($schema->tablesAreReadyForClass(Folder::class), 'Folder table should NOT be ready after drop column');

Expand All @@ -542,7 +543,7 @@ public function testTablesAreReadyForClass(): void
$schema->reset();
// Add an extension that adds new columns
Image::add_extension(AdditionalFieldsExtension::class);
$this->assertTrue($schema->tablesAreReadyForClass(File::class), 'File table should be ready after add extension');
$this->assertFalse($schema->tablesAreReadyForClass(File::class), 'File table should NOT be ready after add extension');
$this->assertFalse($schema->tablesAreReadyForClass(Image::class), 'Image table should NOT be ready after add extension');
$this->assertTrue($schema->tablesAreReadyForClass(Folder::class), 'Folder table should be ready after add extension');

Expand Down