diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e5fb17a..92754cc 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -73,3 +73,81 @@ jobs: - name: Test Suite (Windows) if: runner.os == 'Windows' run: vendor/bin/pest + + databases: + runs-on: ubuntu-latest + + name: ${{ matrix.database }} + + strategy: + fail-fast: false + matrix: + include: + - database: MySQL 8.4 + driver: mysql + port: 3306 + username: root + password: '' + - database: PostgreSQL 17 + driver: pgsql + port: 5432 + username: postgres + password: secret + + services: + mysql: + image: mysql:8.4 + env: + MYSQL_ALLOW_EMPTY_PASSWORD: yes + MYSQL_DATABASE: custom_fields + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping --silent" + --health-interval=10s + --health-timeout=5s + --health-retries=10 + + postgres: + image: postgres:17 + env: + POSTGRES_PASSWORD: secret + POSTGRES_DB: custom_fields + ports: + - 5432:5432 + options: >- + --health-cmd=pg_isready + --health-interval=10s + --health-timeout=5s + --health-retries=10 + + steps: + - name: Checkout code + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Setup PHP + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2 + with: + php-version: 8.4 + extensions: dom, curl, libxml, mbstring, zip, fileinfo, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql, bcmath, intl + ini-values: error_reporting=E_ALL + tools: composer:v2 + coverage: none + + - name: Install dependencies + run: | + composer update --prefer-stable --prefer-dist --no-interaction --no-progress --no-scripts + composer run prepare + + # Serial on purpose. composer test:unit runs Pest in parallel, and the + # processes would create and drop the same tables in the one database the + # service container provides. + - name: Test Suite + env: + DB_DRIVER: ${{ matrix.driver }} + DB_HOST: 127.0.0.1 + DB_PORT: ${{ matrix.port }} + DB_DATABASE: custom_fields + DB_USERNAME: ${{ matrix.username }} + DB_PASSWORD: ${{ matrix.password }} + run: vendor/bin/pest diff --git a/CHANGELOG.md b/CHANGELOG.md index d1147a7..2e138ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- The test suite runs against MySQL 8.4 and PostgreSQL 17 on every build, alongside the + SQLite matrix. `DB_DRIVER` points it at a server locally. + +### Fixed + +- `getCustomFields()` and the generated filters and sorts come back ordered by + `sort_order` and then by slug. The order was whatever the database returned, so the same + code gave a different order on SQLite and on a server, and the `sort_order` column was + never read. +- A `decimal` value reads back the same on every driver. MySQL and PostgreSQL return the + full scale of the column, so a stored `12.5` came back as `12.500000` while SQLite gave + `12.5`. + ## 0.1.0 - 2026-09-07 First public release. diff --git a/README.md b/README.md index 5460480..01b5fb9 100644 --- a/README.md +++ b/README.md @@ -776,6 +776,19 @@ on purpose, so a refactoring it proposes is read before it is taken: vendor/bin/rector process ``` +The suite runs against an in memory SQLite database by default. Point it at a server with +`DB_DRIVER`, which is how the workflow exercises MySQL and PostgreSQL on every build: + +```bash +DB_DRIVER=mysql DB_PORT=3306 DB_USERNAME=root vendor/bin/pest +DB_DRIVER=pgsql DB_PORT=5432 DB_USERNAME=postgres DB_PASSWORD=secret vendor/bin/pest +``` + +`DB_HOST` and `DB_DATABASE` are read the same way and default to `127.0.0.1` and +`custom_fields`. Run Pest directly rather than through `composer test:unit` when you point +it at a server: that script runs in parallel, and the processes would create and drop the +same tables in the same database. + ## Contributing Thank you for considering contributing to Laravel Custom Fields! Please review our [contributing guide](.github/CONTRIBUTING.md) to get started. diff --git a/src/Concerns/HasCustomFields.php b/src/Concerns/HasCustomFields.php index c486c6c..6fa6f49 100644 --- a/src/Concerns/HasCustomFields.php +++ b/src/Concerns/HasCustomFields.php @@ -198,7 +198,7 @@ private function customFieldDefinitions(bool $includeInactive = false): array $query->where('is_active', true); } - return $query->get() + return $query->orderBy('sort_order')->orderBy('slug')->get() ->keyBy(static fn (CustomField $field): string => (string) $field->getAttribute('slug')) ->all(); } diff --git a/src/CustomFields.php b/src/CustomFields.php index c6a6f23..98e8d45 100644 --- a/src/CustomFields.php +++ b/src/CustomFields.php @@ -235,6 +235,8 @@ private function queryableFields(Model|string $model): Collection $fields = $fieldModel::query() ->where('entity_type', $this->entityKey($model)) ->where('is_active', true) + ->orderBy('sort_order') + ->orderBy('slug') ->get(); return $fields; diff --git a/src/Types/DecimalType.php b/src/Types/DecimalType.php index a85f7a6..1bbfaf1 100644 --- a/src/Types/DecimalType.php +++ b/src/Types/DecimalType.php @@ -23,9 +23,20 @@ public function serialize(mixed $value, Model $field): mixed return $value === null ? null : (string) $value; } + /** + * MySQL and PostgreSQL hand back the full scale of the decimal column while + * SQLite returns what was written, so the fraction is trimmed and the same + * stored value reads the same on every driver. + */ public function deserialize(mixed $value, Model $field): mixed { - return $value === null ? null : (string) $value; + if ($value === null) { + return null; + } + + $value = (string) $value; + + return str_contains($value, '.') ? rtrim(rtrim($value, '0'), '.') : $value; } public function rules(Model $field): array diff --git a/src/Validation/ValueValidator.php b/src/Validation/ValueValidator.php index 96c9187..6dcc89c 100644 --- a/src/Validation/ValueValidator.php +++ b/src/Validation/ValueValidator.php @@ -24,6 +24,8 @@ public function definitions(Model|string $model): array /** @var array $definitions */ $definitions = $fieldModel::query() ->where('entity_type', CustomFields::entityKey($model)) + ->orderBy('sort_order') + ->orderBy('slug') ->get() ->keyBy(static fn (Model $field): string => (string) $field->getAttribute('slug')) ->all(); diff --git a/tests/Feature/CustomFieldValueTest.php b/tests/Feature/CustomFieldValueTest.php index 0c74c99..3185112 100644 --- a/tests/Feature/CustomFieldValueTest.php +++ b/tests/Feature/CustomFieldValueTest.php @@ -44,8 +44,8 @@ function field(array $attributes): CustomField ], complete: true); expect($article->getCustomFields())->toBe([ - 'rank' => 10, 'email' => 'person@example.com', + 'rank' => 10, ]); }); diff --git a/tests/Feature/KeyTypeTest.php b/tests/Feature/KeyTypeTest.php index 142a4f4..66e36a8 100644 --- a/tests/Feature/KeyTypeTest.php +++ b/tests/Feature/KeyTypeTest.php @@ -19,14 +19,30 @@ function columnType(string $table, string $column): string return is_array($found) ? (string) $found['type_name'] : ''; } +/** + * Every driver names its column types differently, so a kind lists what SQLite, + * MySQL and PostgreSQL each report for it. Asserting the kind keeps the test + * about the key shape rather than about the vocabulary of one database. + * + * @return array + */ +function columnTypesOf(string $kind): array +{ + return match ($kind) { + 'integer' => ['integer', 'bigint', 'int8'], + 'uuid' => ['varchar', 'char', 'uuid'], + 'ulid' => ['varchar', 'char', 'bpchar'], + }; +} + it('keeps integer keys on both tables by default', function (): void { CustomFields::registerEntity(Article::class, 'article'); CustomField::create(['entity_type' => 'article', 'name' => 'Rank', 'type' => 'number']); $article = Article::create(['title' => 'A']); $article->setCustomField('rank', 5); - expect(columnType('custom_fields', 'id'))->toBe('integer') - ->and(columnType('custom_field_values', 'valuable_id'))->toBe('integer') + expect(columnType('custom_fields', 'id'))->toBeIn(columnTypesOf('integer')) + ->and(columnType('custom_field_values', 'valuable_id'))->toBeIn(columnTypesOf('integer')) ->and($article->customFieldValues()->first()?->getKey())->toBeInt() ->and((new CustomField)->getKeyType())->toBe('int') ->and((new CustomField)->getIncrementing())->toBeTrue(); @@ -37,6 +53,7 @@ function columnType(string $table, string $column): string config()->set('laravel-custom-fields.morph_key_type', 'uuid'); $this->rebuildPackageTables(); + Schema::dropIfExists('uuid_documents'); Schema::create('uuid_documents', function (Blueprint $table): void { $table->uuid('id')->primary(); $table->string('title'); @@ -49,8 +66,8 @@ function columnType(string $table, string $column): string $document->setCustomField('rank', 7); $row = $document->customFieldValues()->first(); - expect(columnType('custom_fields', 'id'))->toBe('varchar') - ->and(columnType('custom_field_values', 'valuable_id'))->toBe('varchar') + expect(columnType('custom_fields', 'id'))->toBeIn(columnTypesOf('uuid')) + ->and(columnType('custom_field_values', 'valuable_id'))->toBeIn(columnTypesOf('uuid')) ->and(Str::isUuid((string) $field->getKey()))->toBeTrue() ->and(Str::isUuid((string) $row?->getKey()))->toBeTrue() ->and($row?->getAttribute('valuable_id'))->toBe($document->getKey()) @@ -62,6 +79,7 @@ function columnType(string $table, string $column): string config()->set('laravel-custom-fields.morph_key_type', 'ulid'); $this->rebuildPackageTables(); + Schema::dropIfExists('ulid_documents'); Schema::create('ulid_documents', function (Blueprint $table): void { $table->ulid('id')->primary(); $table->string('title'); @@ -74,8 +92,8 @@ function columnType(string $table, string $column): string $document->setCustomField('rank', 7); $row = $document->customFieldValues()->first(); - expect(columnType('custom_fields', 'id'))->toBe('varchar') - ->and(columnType('custom_field_values', 'valuable_id'))->toBe('varchar') + expect(columnType('custom_fields', 'id'))->toBeIn(columnTypesOf('ulid')) + ->and(columnType('custom_field_values', 'valuable_id'))->toBeIn(columnTypesOf('ulid')) ->and(Str::isUlid((string) $field->getKey()))->toBeTrue() ->and(Str::isUlid((string) $row?->getKey()))->toBeTrue() ->and($row?->getAttribute('valuable_id'))->toBe($document->getKey()) diff --git a/tests/TestCase.php b/tests/TestCase.php index 5627d4b..dd63961 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -24,26 +24,73 @@ protected function getPackageProviders($app): array } /** - * The suite always runs against an in memory database. - * - * Without this, a database/database.sqlite left behind by composer build - * makes Testbench prefer the file, and the package migrations it published - * into the skeleton then collide with the ones loaded below. + * The suite runs against an in memory database unless DB_DRIVER asks for + * one of the servers, which is how the workflow exercises MySQL and + * PostgreSQL. Pinning the connection also keeps a database/database.sqlite + * left behind by composer build from making Testbench prefer the file, + * whose published migrations would then collide with the ones loaded below. */ protected function defineEnvironment($app): void { $app['config']->set('database.default', 'testing'); - $app['config']->set('database.connections.testing', [ - 'driver' => 'sqlite', - 'database' => ':memory:', - 'prefix' => '', - 'foreign_key_constraints' => true, - ]); + $app['config']->set('database.connections.testing', $this->connectionConfiguration()); + } + + /** @return array */ + protected function connectionConfiguration(): array + { + return match ($this->fromEnvironment('DB_DRIVER', 'sqlite')) { + 'mysql' => [ + 'driver' => 'mysql', + 'host' => $this->fromEnvironment('DB_HOST', '127.0.0.1'), + 'port' => $this->fromEnvironment('DB_PORT', '3306'), + 'database' => $this->fromEnvironment('DB_DATABASE', 'custom_fields'), + 'username' => $this->fromEnvironment('DB_USERNAME', 'root'), + 'password' => $this->fromEnvironment('DB_PASSWORD', ''), + 'charset' => 'utf8mb4', + 'collation' => 'utf8mb4_unicode_ci', + 'prefix' => '', + ], + 'pgsql' => [ + 'driver' => 'pgsql', + 'host' => $this->fromEnvironment('DB_HOST', '127.0.0.1'), + 'port' => $this->fromEnvironment('DB_PORT', '5432'), + 'database' => $this->fromEnvironment('DB_DATABASE', 'custom_fields'), + 'username' => $this->fromEnvironment('DB_USERNAME', 'postgres'), + 'password' => $this->fromEnvironment('DB_PASSWORD', ''), + 'charset' => 'utf8', + 'prefix' => '', + 'search_path' => 'public', + ], + default => [ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + 'foreign_key_constraints' => true, + ], + }; } + /** Reads a workflow variable without the env() helper the arch test forbids. */ + protected function fromEnvironment(string $key, string $default): string + { + $value = getenv($key); + + return $value === false || $value === '' ? $default : $value; + } + + /** + * The host tables belong to the fixtures rather than to the package. + * + * An in memory database starts empty for every test, while a MySQL or a + * PostgreSQL server keeps what the previous test left, so they are dropped + * on both ends of the test rather than only created. + */ protected function defineDatabaseMigrations(): void { + $this->dropLeftoverTables(); $this->loadMigrationsFrom(__DIR__.'/../database/migrations'); + $this->dropHostTables(); Schema::create('authors', function (Blueprint $table): void { $table->id(); @@ -62,6 +109,42 @@ protected function defineDatabaseMigrations(): void $table->string('title'); $table->timestamps(); }); + + $this->beforeApplicationDestroyed(fn () => $this->dropHostTables()); + } + + protected function dropHostTables(): void + { + foreach (['articles', 'projects', 'authors'] as $table) { + Schema::dropIfExists($table); + } + } + + /** + * Clears whatever a previous run left on a server. + * + * An in memory database has nothing to clear. A MySQL or a PostgreSQL + * server keeps the tables of a run that was interrupted, and the migrator + * would then try to create them again, so every later test fails on a + * schema it never made. + */ + protected function dropLeftoverTables(): void + { + if ($this->fromEnvironment('DB_DRIVER', 'sqlite') === 'sqlite') { + return; + } + + foreach ([ + 'custom_field_values', + 'custom_fields', + 'uuid_documents', + 'ulid_documents', + 'migrations', + ] as $table) { + Schema::dropIfExists($table); + } + + $this->dropHostTables(); } /** diff --git a/tests/Unit/FieldTypeTest.php b/tests/Unit/FieldTypeTest.php index 6bdeb91..5978a93 100644 --- a/tests/Unit/FieldTypeTest.php +++ b/tests/Unit/FieldTypeTest.php @@ -108,9 +108,11 @@ function typedField(string $type, ?array $options = null): CustomField typedField('multiselect', [['key' => 'alpha', 'label' => 'Alpha', 'is_active' => true]]); $article = Article::create(['title' => 'A']); + // Definitions come back ordered by sort_order and then by slug, so the + // order is the same on every driver rather than the order rows went in. expect($article->getCustomFields())->toBe([ - 'number-field' => null, 'multiselect-field' => [], + 'number-field' => null, ]); });