PHPORM-289 Support Laravel 12 #3283
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - php: "8.4" | ||
| laravel: "11.*" | ||
| mongodb: "7.0" | ||
| os: "ubuntu-latest" |
There was a problem hiding this comment.
This was duplicating an existing case.
| - | ||
| message: "#^Class MongoDB\\\\Laravel\\\\Query\\\\Grammar does not have a constructor and must be instantiated without any parameters\\.$#" | ||
| count: 1 | ||
| path: src/Connection.php | ||
|
|
||
| - | ||
| message: "#^Class MongoDB\\\\Laravel\\\\Schema\\\\Grammar does not have a constructor and must be instantiated without any parameters\\.$#" | ||
| count: 1 | ||
| path: src/Connection.php | ||
|
|
There was a problem hiding this comment.
It tried using the @phpstan-ignore new.noConstructor in the code, but that triggers an other error ignore.unmatchedIdentifier on Laravel 12, where the constructor has this parameter.
| // Argument added in Laravel 12 | ||
| return new Query\Grammar($this); | ||
| } | ||
|
|
||
| /** @inheritdoc */ | ||
| protected function getDefaultSchemaGrammar() | ||
| { | ||
| return new Schema\Grammar(); | ||
| // Argument added in Laravel 12 | ||
| return new Schema\Grammar($this); |
There was a problem hiding this comment.
The constructor was introduced by laravel/framework@c78fa3d#diff-13a4975d935d982e846853d4328df60c7ef014b5a4010441277b2ef4de31ed2aR27
| @@ -35,6 +35,7 @@ public function tearDown(): void | |||
| Photo::truncate(); | |||
| Label::truncate(); | |||
| Skill::truncate(); | |||
| Soft::truncate(); | |||
There was a problem hiding this comment.
This was a latent bug when the test suite was run in a particular order, MassPrunableTest::testPruneSoftDelete failed.
| @@ -137,9 +142,10 @@ public function dropAllTables() | |||
| } | |||
| } | |||
|
|
|||
| public function getTables() | |||
| /** @param string|null $schema Database name */ | |||
| public function getTables($schema = null) | |||
There was a problem hiding this comment.
Are the signature changes here also a potential BC break for Laravel 11 users, or would you not expect anyone to extend this class?
There was a problem hiding this comment.
Yes, this is a breaking change if someone extends our class... I hope nobody does. We should make more classes final.
| $collections = array_map(fn ($db, $collections) => array_map(static fn ($collection) => $db . '.' . $collection, $collections), array_keys($collections), $collections); | ||
| } | ||
|
|
||
| $collections = array_merge(...array_values($collections)); |
There was a problem hiding this comment.
If $schemaQualified is false and the database name is never prepended to yield full namespaces, would this statement potentially clobber collection names?
There was a problem hiding this comment.
Yes, but that's how the feature is designed.
|
@jmikola So what's the holdup here? 😅 |
|
this build need the update of mongodb. but it's a breaking change because it needs: pecl install mongodb |
…efined ccompatible with each Laravel version
Fix PHPORM-289
Replace #3278
Closes #3288
Needed:
Connection::getDatabase()andgetClient()#3289Breaking changes
Add types to properties inMongoDB\Laravel\Schema\BluePrint:$connectionand$collection. The property was added by [12.x] feat: configure default datetime precision on per-grammar basis laravel/framework#51821MongoDB\Laravel\Schema:getTablesandgetTableListing. This were added to support multi-databases in [12.x] Enhance multi-database support laravel/framework#54274, but we decided not implementing this feature for MongoDB as this would introduce an other breaking change if collection name uses a..