Fix/first release hardening - #2
Merged
Merged
Conversation
filtersFor() built one equality filter per active field without consulting queryOperations(), so a multiselect stored in value_json received a scalar comparison and returned zero rows with no error. It now registers one filter per declared operation and refuses any operation the field type cannot serve. Every operator the types advertised is implemented: equals, in, contains, greater_than, less_than, between, contains_any and contains_all, plus is_null and is_not_null on all twelve types. contains uses LIKE with an explicit escape clause so % and _ are matched literally, SQLite included. Filter and sort names now read key_prefix from the config instead of a hardcoded cf_ in two places. Non equality filters are suffixed, cf_slug:between. queryOptionsFor() returns filters and sorts from a single load of the definitions. The sorter normalises its direction through the Direction helper of plin-code/laravel-eloquent-sorts, which is the only part of that package able to serve an EAV sort, and orders on a presence subquery so hosts without a value sort last in both directions on every driver. Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
Complete validation looked only at the submitted array, so patching one field failed on any required field that was already stored. It now validates the merged state of stored values plus the changes. An explicit null still counts as clearing, so a required field set to null fails. Every lookup went through one method filtering is_active, which made a deactivated field unreadable and unremovable. Reads take an explicit includeInactive argument, clearing needs no flag, and writes still refuse an inactive field. Writing to an unsaved model hit a NOT NULL constraint and surfaced a QueryException. It now throws ModelNotPersistedException before any query runs. Unknown or inactive slugs raise ValidationException before the transaction opens, so an API answers 422 instead of 500, while direct accessors throw UnknownCustomFieldException, a subclass of InvalidArgumentException so existing catches still match. A null value now deletes the stored row inside the batch transaction instead of writing a row with every value column null. An empty array on a multiselect keeps the row, so an unanswered field stays distinct from a cleared selection. Definitions and stored rows are each loaded once per call and shared between validation and writing. A three slug write went from thirteen queries to five, getCustomFields from two plus one per field to two. Validation messages now resolve through the laravel-custom-fields translation namespace in English and Italian, with the field name as the attribute, rather than a hardcoded English string. Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
The provider hand rolled every publish call while spatie/laravel-package-tools sat in require and the sibling package plin-code/laravel-eloquent-sorts already extended PackageServiceProvider. All six publish tags, the key validation, the singleton and the twelve built in type registrations were verified by booting a Testbench application. The migration published into consumer applications was named create_laravel_custom_fields_placeholder_table. Both migrations are renamed to create_custom_fields_table and create_custom_field_values_table. morph_key_type now defaults to id instead of uuid. The old default produced a char(36) valuable_id against a default Laravel install with bigint keys and only survived because SQLite does not enforce types. Every config option is documented, including that key_type and morph_key_type have to be chosen before migrating. phpstan no longer excludes src/Concerns/HasCustomFields.php, the whole model facing API, and analyses workbench so the trait has a using class. gitattributes stops shipping the planning documents and the art directory. composer.json gains the dev dependencies and the rector script the sibling package carries. Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
The suite held 21 tests, two placeholder files and a case that registered one model twice under two keys while asserting nothing about it. It is replaced by 115 tests over six files. Covered: the four publish tags and their contents, the migration names, the twelve built in types, an unsupported key type rejected at register, the en and it translations, a reflection check that the facade annotations match the manager, every type round tripping through serialize and deserialize, a consumer defined type with its own rules and storage column, the uuid and ulid key configurations against real tables, partial against complete validation, the null clear and the empty array distinction, inactive reads and clears, the query counts asserted with the query log, the key prefix being honoured, every filter operation, and a custom field sort composed with a sorter from plin-code/laravel-eloquent-sorts. The case that fails without the validator fix is included: a type whose deserialized shape would not pass its own input rules. TestCase pins an in memory database, so composer build followed by composer test no longer collides with the migrations workbench:build publishes into the Testbench skeleton. Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
Complete validation merged the stored values into the validator payload and ran each field type's input rules over them. The stored values arrive deserialized, and a type is free to read back a shape it would not accept as input, so every complete write failed on a field the caller never touched. The shipped example type demonstrates it: serialize gives an int, deserialize gives "3 stars", and the rules ask for an integer. Presence is now decided directly against the value the record ends up with, submitted or stored, so a required definition is satisfied without the stored value passing its own input rules a second time. The rules array still injects required when asked, since a consumer building a form request depends on it. The values migration also fell back to uuid for morph_key_type while the config and the fields migration fall back to id, so a missing key built a table matching neither default. Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
Rector promotes the five events to final readonly classes with promoted constructor properties. The storage column list gains its const type, which is what phpstan reported once the trait stopped being excluded from analysis. Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
Every sample was executed against a booted Testbench application before being written down. The querying section opens on a worked example with three records, the requests a client sends and the rows that actually come back. Added: where to call registerEntity with the full provider, a table of the twelve types with their storage column and query operations, a table of every config option, the events, the whereCustomField scope, the exceptions and what to answer with, the form metadata, the composition with plin-code/laravel-eloquent-sorts, and a testing section. Corrected: one fence style throughout instead of three, one example domain instead of two, the dependency descriptions, and the claims about null and about inactive definitions, which no longer match what the code did before this branch. The changelog moves to Keep a Changelog sections so later releases have a shape to follow. Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
The skeleton user model declared its mass assignable and hidden attributes through Fillable and Hidden, which the framework only ships from Laravel 13. The package supports Laravel 12 as well, and this branch added workbench to the analysed paths, so PHPStan failed on every Laravel 12 row of the matrix with "Attribute class Illuminate\Database\Eloquent\Attributes\Fillable does not exist". The properties say the same thing on both versions. Verified by installing laravel/framework 12.69.1 with orchestra/testbench 10 and running the whole chain. Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
The four publish tag tests failed on every Windows row of the matrix.
Package tools joins its base path to a directory that already begins
with a separator, so a published path arrives as
D:\...\laravel-custom-fields\src\/../config/laravel-custom-fields.php
and neither the repository prefix nor the src/../ segment matched. The
helper now folds backslashes into forward slashes and collapses the
doubled separator before comparing, and the destination assertion goes
through the same helper.
Claude-Session: https://claude.ai/code/session_0131mg78D5c5ao3zWZ5c8sDX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.