diff --git a/src/Data/VisibilityData.php b/src/Data/VisibilityData.php index 05afe061..12a98b99 100644 --- a/src/Data/VisibilityData.php +++ b/src/Data/VisibilityData.php @@ -5,9 +5,11 @@ namespace Relaticle\CustomFields\Data; use Illuminate\Database\Eloquent\Model; +use Illuminate\Support\Str; use Relaticle\CustomFields\Enums\CustomFieldsFeature; use Relaticle\CustomFields\Enums\VisibilityLogic; use Relaticle\CustomFields\Enums\VisibilityMode; +use Relaticle\CustomFields\Enums\VisibilityOperator; use Relaticle\CustomFields\FeatureSystem\FeatureManager; use Relaticle\CustomFields\Services\RelationConditionResolver; use Spatie\LaravelData\Attributes\DataCollectionOf; @@ -37,8 +39,13 @@ public function requiresConditions(): bool /** * @param array $fieldValues + * @param array $membershipFieldCodes Codes of option-backed choice fields whose + * contains/not-contains conditions must be + * evaluated as exact option membership rather + * than substring, to stay identical to the + * client (which compares option ids). */ - public function evaluate(array $fieldValues, ?Model $record = null): bool + public function evaluate(array $fieldValues, ?Model $record = null, array $membershipFieldCodes = []): bool { if (! $this->requiresConditions() || ! $this->conditions instanceof DataCollection) { return $this->mode === VisibilityMode::ALWAYS_VISIBLE; @@ -56,7 +63,7 @@ public function evaluate(array $fieldValues, ?Model $record = null): bool continue; } - $results[] = $this->evaluateCondition($condition, $fieldValues, $record); + $results[] = $this->evaluateCondition($condition, $fieldValues, $record, $membershipFieldCodes); } if ($results === []) { @@ -70,8 +77,9 @@ public function evaluate(array $fieldValues, ?Model $record = null): bool /** * @param array $fieldValues + * @param array $membershipFieldCodes */ - private function evaluateCondition(VisibilityConditionData $condition, array $fieldValues, ?Model $record = null): bool + private function evaluateCondition(VisibilityConditionData $condition, array $fieldValues, ?Model $record = null, array $membershipFieldCodes = []): bool { if ($condition->isRelationAttribute()) { if (! $record instanceof Model) { @@ -98,9 +106,58 @@ private function evaluateCondition(VisibilityConditionData $condition, array $fi $fieldValue = $fieldValues[$condition->field_code] ?? null; + if ($this->isMembershipCondition($condition, $membershipFieldCodes)) { + $isMember = $this->matchesOptionMembership($fieldValue, $condition->value); + + return $condition->operator === VisibilityOperator::CONTAINS ? $isMember : ! $isMember; + } + return $condition->operator->evaluate($fieldValue, $condition->value); } + /** + * @param array $membershipFieldCodes + */ + private function isMembershipCondition(VisibilityConditionData $condition, array $membershipFieldCodes): bool + { + return in_array($condition->operator, [VisibilityOperator::CONTAINS, VisibilityOperator::NOT_CONTAINS], true) + && in_array($condition->field_code, $membershipFieldCodes, true); + } + + /** + * Exact, case-insensitive option membership: true when the selected options include any of the + * condition's options. Mirrors the client, which resolves the condition to option ids and checks + * `conditionIds.some(id => selectedIds.includes(id))`. + */ + private function matchesOptionMembership(mixed $fieldValue, mixed $conditionValue): bool + { + $selected = $this->normalizeMembershipValues($fieldValue); + + if ($selected === []) { + return false; + } + + foreach ($this->normalizeMembershipValues($conditionValue) as $wanted) { + if (in_array($wanted, $selected, true)) { + return true; + } + } + + return false; + } + + /** + * @return array + */ + private function normalizeMembershipValues(mixed $value): array + { + return collect(is_array($value) ? $value : [$value]) + ->reject(fn (mixed $item): bool => $item === null || $item === '') + ->map(fn (mixed $item): string => Str::lower(trim((string) $item))) + ->values() + ->all(); + } + /** * @return array */ diff --git a/src/Filament/Integration/Base/AbstractFormComponent.php b/src/Filament/Integration/Base/AbstractFormComponent.php index fbdc8238..c59116ea 100644 --- a/src/Filament/Integration/Base/AbstractFormComponent.php +++ b/src/Filament/Integration/Base/AbstractFormComponent.php @@ -35,15 +35,19 @@ abstract readonly class AbstractFormComponent implements FormComponentInterface { /** - * Operators whose result is identical whether evaluated against option ids (client visibleJs) - * or option names (server). Other operators (substring/ordering/membership) can diverge for - * choice fields, so the validation gate defers to normal validation for those. + * Operators the validation gate can reproduce server-side for choice fields. equals/not-equals + * compare a single option, empty/not-empty inspect presence, and contains/not-contains are + * evaluated as exact option membership (see isVisibleForValidation) — all identical to the + * client, which compares option ids. Ordering operators (greater/less than) are never offered + * for choice fields, so they are intentionally absent. * * @var list */ private const array CHOICE_SAFE_OPERATORS = [ VisibilityOperator::EQUALS, VisibilityOperator::NOT_EQUALS, + VisibilityOperator::CONTAINS, + VisibilityOperator::NOT_CONTAINS, VisibilityOperator::IS_EMPTY, VisibilityOperator::IS_NOT_EMPTY, ]; @@ -215,8 +219,8 @@ private function hasVisibilityConditions(CustomField $customField): bool * condition guarantees the client also hides the field — so a visible field is never silently * skipped (worst case is a redundant validation, never accepting invalid data). For condition * shapes the server cannot reproduce identically to the client JS (model/relation attribute - * sources, or non-equality operators on choice fields, where the client compares option ids and - * the server compares option names) we defer to normal validation instead of guessing. + * sources) we defer to normal validation instead of guessing. Choice-field contains/not-contains + * are reproduced as exact option membership, matching the client's option-id comparison. * * @param Collection $allFields */ @@ -245,14 +249,35 @@ private function isVisibleForValidation( $normalizedValues = app(BackendVisibilityService::class) ->normalizeFieldValuesUsing($fieldValues, $allFields); - return $this->coreVisibilityLogic->evaluateVisibility($customField, $normalizedValues); + return $this->coreVisibilityLogic->evaluateVisibility( + $customField, + $normalizedValues, + membershipFieldCodes: $this->optionBackedChoiceFieldCodes($allFields), + ); + } + + /** + * Codes of choice fields that carry user-defined options (e.g. multi-select, checkbox-list). + * Their contains/not-contains conditions are evaluated as exact option membership so the server + * matches the client, which resolves the condition to option ids. Option-less choice fields + * (email, tags, …) are excluded and keep substring matching, identical on both sides. + * + * @param Collection $allFields + * @return array + */ + private function optionBackedChoiceFieldCodes(Collection $allFields): array + { + return $allFields + ->filter(fn (CustomField $field): bool => $field->isChoiceField() && $field->options->isNotEmpty()) + ->pluck('code') + ->all(); } /** * Whether the server can reproduce the client-side visibleJs result for this field's own * conditions. Returns false for conditions the gate must not act on (to avoid skipping * validation on a field the user can see): non same-record-custom-field sources, and operators - * on choice fields that the client evaluates against option ids rather than names. + * on choice fields the gate cannot reproduce (see CHOICE_SAFE_OPERATORS). * * @param Collection $allFields */ diff --git a/src/Services/Visibility/CoreVisibilityLogicService.php b/src/Services/Visibility/CoreVisibilityLogicService.php index 1c35e36f..dfdc4466 100644 --- a/src/Services/Visibility/CoreVisibilityLogicService.php +++ b/src/Services/Visibility/CoreVisibilityLogicService.php @@ -79,10 +79,13 @@ public function getDependentFields(CustomField $field): array * This is the core evaluation logic used by backend implementations. * * @param array $fieldValues + * @param array $membershipFieldCodes Option-backed choice field codes whose + * contains/not-contains conditions evaluate + * as exact option membership (see VisibilityData::evaluate). */ - public function evaluateVisibility(CustomField $field, array $fieldValues, ?Model $record = null): bool + public function evaluateVisibility(CustomField $field, array $fieldValues, ?Model $record = null, array $membershipFieldCodes = []): bool { - return $this->getVisibilityData($field)->evaluate($fieldValues, $record); + return $this->getVisibilityData($field)->evaluate($fieldValues, $record, $membershipFieldCodes); } /** diff --git a/src/Services/Visibility/FrontendVisibilityService.php b/src/Services/Visibility/FrontendVisibilityService.php index 0e468c1e..02a50cb2 100644 --- a/src/Services/Visibility/FrontendVisibilityService.php +++ b/src/Services/Visibility/FrontendVisibilityService.php @@ -543,12 +543,22 @@ private function convertOptionValue( /** * Build contains expression. + * + * For option-backed choice fields "contains" means exact option membership: the selected + * option ids include (any of) the condition's option ids. This matches the server, which + * evaluates the same condition as membership over normalized option names. Substring matching + * is kept only for free-text sources (text fields and option-less multi-value fields such as + * email/tags), where the client and server both compare raw values. */ private function buildContainsExpression( string $fieldValue, mixed $value, ?CustomField $targetField ): string { + if ($targetField instanceof CustomField && $targetField->isChoiceField() && $targetField->options->isNotEmpty()) { + return $this->buildOptionExpression($fieldValue, $value, $targetField, 'equals'); + } + $resolvedValue = $targetField instanceof CustomField ? $this->resolveOptionValue($value, $targetField) : $value; diff --git a/tests/Feature/Integration/ConditionalVisibilityValidationTest.php b/tests/Feature/Integration/ConditionalVisibilityValidationTest.php index ded4e5b8..97a5d8b5 100644 --- a/tests/Feature/Integration/ConditionalVisibilityValidationTest.php +++ b/tests/Feature/Integration/ConditionalVisibilityValidationTest.php @@ -298,23 +298,96 @@ function cvvEdit(Post $post, array $customFields) }); // =========================================================================== -// Divergence safety — gate must never skip validation on a field the user can see. -// For condition shapes the server cannot reproduce identically to the client JS, the -// gate defers to normal validation (the pre-fix behavior) rather than risk silent data loss. +// Multi-choice contains — exact option membership (matches the client's option-id comparison). +// "contains X" means the selected options include X exactly, never a substring of an option +// name. The gate reproduces this server-side so a hidden field is not required and a visible +// field is. // =========================================================================== -it('still enforces required for a choice CONTAINS condition (client compares ids, server names)', function (): void { +it('does not require a field when the multi-select does not include the trigger option', function (): void { + $services = cvvField($this, 'services', 'multi-select'); + $rent = CustomFieldOption::factory()->create(['custom_field_id' => $services->id, 'name' => 'Rent', 'sort_order' => 1]); + CustomFieldOption::factory()->create(['custom_field_id' => $services->id, 'name' => 'Other', 'sort_order' => 2]); + + cvvField($this, 'other_details', 'text', ['required' => true], cvvShowWhen('services', VisibilityOperator::CONTAINS, ['Other'])); + + cvvCreate(['services' => [$rent->id], 'other_details' => null]) + ->assertHasNoFormErrors(['custom_fields.other_details']) + ->assertRedirect(); +}); + +it('requires a field when the multi-select includes the trigger option', function (): void { + $services = cvvField($this, 'services', 'multi-select'); + CustomFieldOption::factory()->create(['custom_field_id' => $services->id, 'name' => 'Rent', 'sort_order' => 1]); + $other = CustomFieldOption::factory()->create(['custom_field_id' => $services->id, 'name' => 'Other', 'sort_order' => 2]); + + cvvField($this, 'other_details', 'text', ['required' => true], cvvShowWhen('services', VisibilityOperator::CONTAINS, ['Other'])); + + cvvCreate(['services' => [$other->id], 'other_details' => null]) + ->assertHasFormErrors(['custom_fields.other_details' => 'required']); +}); + +it('treats contains as exact membership, not substring, for overlapping option names', function (): void { $pets = cvvField($this, 'pets', 'multi-select'); $dog = CustomFieldOption::factory()->create(['custom_field_id' => $pets->id, 'name' => 'Dog', 'sort_order' => 1]); CustomFieldOption::factory()->create(['custom_field_id' => $pets->id, 'name' => 'Do', 'sort_order' => 2]); - cvvField($this, 'detail', 'text', ['required' => true], cvvShowWhen('pets', VisibilityOperator::NOT_CONTAINS, 'Do')); + // "Do" is a substring of "Dog". Selecting only "Dog" must NOT satisfy contains "Do": + // the field stays hidden (matching the client, which compares option ids) and is not required. + cvvField($this, 'detail', 'text', ['required' => true], cvvShowWhen('pets', VisibilityOperator::CONTAINS, ['Do'])); - // Selecting only "Dog" shows "detail" on the client (NOT_CONTAINS "Do"); it must stay required. cvvCreate(['pets' => [$dog->id], 'detail' => null]) - ->assertHasFormErrors(['custom_fields.detail' => 'required']); + ->assertHasNoFormErrors(['custom_fields.detail']) + ->assertRedirect(); }); +it('applies exact membership to checkbox-list contains conditions', function (): void { + $colors = cvvField($this, 'colors', 'checkbox-list'); + $red = CustomFieldOption::factory()->create(['custom_field_id' => $colors->id, 'name' => 'Red', 'sort_order' => 1]); + $blue = CustomFieldOption::factory()->create(['custom_field_id' => $colors->id, 'name' => 'Blue', 'sort_order' => 2]); + + cvvField($this, 'shade', 'text', ['required' => true], cvvShowWhen('colors', VisibilityOperator::CONTAINS, ['Blue'])); + + cvvCreate(['colors' => [$red->id], 'shade' => null]) + ->assertHasNoFormErrors(['custom_fields.shade']) + ->assertRedirect(); + + cvvField($this, 'shade_when_blue', 'text', ['required' => true], cvvShowWhen('colors', VisibilityOperator::CONTAINS, ['Blue'])); + + cvvCreate(['colors' => [$blue->id], 'shade_when_blue' => null]) + ->assertHasFormErrors(['custom_fields.shade_when_blue' => 'required']); +}); + +it('hides a not-contains field when the excluded option is selected', function (): void { + $services = cvvField($this, 'services', 'multi-select'); + CustomFieldOption::factory()->create(['custom_field_id' => $services->id, 'name' => 'Rent', 'sort_order' => 1]); + $other = CustomFieldOption::factory()->create(['custom_field_id' => $services->id, 'name' => 'Other', 'sort_order' => 2]); + + cvvField($this, 'note', 'text', ['required' => true], cvvShowWhen('services', VisibilityOperator::NOT_CONTAINS, ['Other'])); + + cvvCreate(['services' => [$other->id], 'note' => null]) + ->assertHasNoFormErrors(['custom_fields.note']) + ->assertRedirect(); +}); + +it('requires a not-contains field when the excluded option is not selected', function (): void { + $services = cvvField($this, 'services', 'multi-select'); + $rent = CustomFieldOption::factory()->create(['custom_field_id' => $services->id, 'name' => 'Rent', 'sort_order' => 1]); + CustomFieldOption::factory()->create(['custom_field_id' => $services->id, 'name' => 'Other', 'sort_order' => 2]); + + cvvField($this, 'note', 'text', ['required' => true], cvvShowWhen('services', VisibilityOperator::NOT_CONTAINS, ['Other'])); + + cvvCreate(['services' => [$rent->id], 'note' => null]) + ->assertHasFormErrors(['custom_fields.note' => 'required']); +}); + +// =========================================================================== +// Divergence safety — gate must never skip validation on a field the user can see. +// For condition shapes the server cannot reproduce identically to the client JS (model and +// relation attribute sources), the gate defers to normal validation rather than risk silent +// data loss. +// =========================================================================== + it('still enforces required for a model-attribute condition on create', function (): void { cvvField($this, 'why_high', 'text', ['required' => true], [ 'mode' => VisibilityMode::SHOW_WHEN,