[3.x] L10 compatibility

[saade]

Hi!

This PR adds L10 compatibility!

NOTE: needs stancl/virtualcolumn AND stancl/jobpipeline to support 10.x first. PRs opened.

archtechx/virtualcolumn#12 archtechx/virtualcolumn#11

archtechx/jobpipeline#12

[StefanoMantero]

[jetwes]

Please add this

[stancl]

The code should be fully updated now. L9 tests are passing, but L10 tests are failing with this message:

Package laravel/framework at version ^10.0 has a PHP requirement incompatible with your PHP version, PHP extensions and Composer version

The job seems to use PHP 8.1 correctly, as well as composer v2. So maybe some PHP extensions were added that we don't install in our Dockerfile? But looking at the 9.x & 10.x composer.json, the required PHP extensions are identical.

The tests seem to work for me locally though, so the issue only exists on GH. I might try just updating the Dockerfile to what we have in v4.

[StefanoMantero]

The tests seem to work for me locally though, so the issue only exists on GH. I might try just updating the Dockerfile to what we have in v4.

Sounds like a plan!

[stancl]

Actually the issue might be the composer version. For some reason the docker image installs this:

Composer (version 2.0.3) successfully installed to: /usr/local/bin/composer

While Laravel 10 requires composer 2.2. No idea why the installer would go for an older composer version but a new Docker image should fix it anyway.

[jetwes]

I think you have to push PHP version in composer.json to 8.1 instead of 8.0

[jetwes]

in composer.json "php": "^8.0|^8.1",

[stancl]

^8.0 supports 8.0, 8.1,, 8.2, ...

[jetwes]

^8.0 supports 8.0, 8.1,, 8.2, ...

sure - you're right

[saade]

@stancl do you know ACT? With it you can run your GH actions locally and its compatible with all actions. I've used in the past and worked pretty well.

[stancl]

Since we dockerize our tests, CI and local tests are very similar already. The issue here is (or was, I may have just fixed it) something GH Actions-specific. I haven't used ACT but I'm not sure if it'd be helpful here when locally all docker-related things work properly for me but they don't on GH.

If the tests finally pass (or at least run — there are still some minor things to fix in code for L10 support), then the two issues were:

1. Microsoft's unixodbc-dev being broken. This was fixed by specifying an older stable version
2. Specifying PHP 8.0 as 8.0 instead of "8.0" in the CI matrix, which apparently converted it to a float and then an int, resulting in the PHP_VERSION variable in docker being 8 instead of 8.0

[stancl]

The tests did run, here are the final things to fix in code. I'd appreciate any help with these.

(fixed) TypeError: PDO::prepare(): Argument #1 ($query) must be of type string, Illuminate\Database\Query\Expression given

1) Stancl\Tenancy\Tests\DatabaseUsersTest::users_are_created_when_permission_controlled_mysql_manager_is_used
TypeError: PDO::prepare(): Argument #1 ($query) must be of type string, Illuminate\Database\Query\Expression given

$version = $this->database()->select($this->database()->raw('select version()'))[0]->{'version()'};

Here select() doesn't accept an Expression (returned by raw()). So we need to run the raw select query in a different way.

---

(fixed) Error: Call to undefined method Stancl\Tenancy\Tests\EventListenerTest::hasFailed()

4) Stancl\Tenancy\Tests\EventListenerTest::database_is_not_migrated_if_creation_is_disabled
Error: Call to undefined method Stancl\Tenancy\Tests\EventListenerTest::hasFailed()

We use $this->assertFalse($this->hasFailed()); and the hasFailed() method has been removed.

---

(fixed) Error: Call to undefined method Doctrine\DBAL\Connection::createSchemaManager()

7) Stancl\Tenancy\Tests\TenantModelTest::autoincrement_ids_are_supported
Error: Call to undefined method Doctrine\DBAL\Connection::createSchemaManager()

No idea what this comes from. DBAL is installed as a dependency, but the createSchemaManager() is called from Laravel. Stack trace:

/var/www/html/vendor/laravel/framework/src/Illuminate/Database/Connection.php:1112
/var/www/html/vendor/laravel/framework/src/Illuminate/Database/Schema/Grammars/ChangeColumn.php:36
/var/www/html/vendor/laravel/framework/src/Illuminate/Database/Schema/Grammars/Grammar.php:93
/var/www/html/vendor/laravel/framework/src/Illuminate/Database/Schema/Grammars/MySqlGrammar.php:243
/var/www/html/vendor/laravel/framework/src/Illuminate/Database/Schema/Blueprint.php:135
/var/www/html/vendor/laravel/framework/src/Illuminate/Database/Schema/Blueprint.php:108
/var/www/html/vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php:439
/var/www/html/vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php:269
/var/www/html/vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php:339
/var/www/html/tests/TenantModelTest.php:72

TenantModelTest.php:72 seems to be just a change() call in a Schema::table() block.

[saade]

Here select() doesn't accept an Expression (returned by raw()). So we need to run the raw select query in a different way.

Its just a matter of adding ->getValue() after ->raw(). Raw now returns a expression instead of a string, so we need to cast to string using ->getValue()

[saade]

I can take a look at the other issues after lunch.

[stancl]

I tried that, but getValue() also expects a Grammar argument. I think there should be some more straightforward way to do raw selects in L10.

Edit: We went with getValue($grammr), seems to be good enough.

[saade]

Good work guys, sorry for not being able to help with this. I'm not having some spare time.

[stancl]

No worries, thanks a lot for all your work on these PRs!