fix(forms): gate value_indexed population on is_filterable
FormValueObserver: value_indexed is filter-driven per ARCH §4.4, not hint-driven. Populating it for every string-hint field produced dead weight in the partial index and made FilterQueryBuilder logic murkier. Behaviour after fix: hint=string, is_filterable=true → populate value_indexed hint=string, is_filterable=false → leave null hint=number/date/bool, any filterable → populate typed column (unchanged) hint=json, any filterable → leave typed columns null (unchanged) value_number / value_date / value_bool remain hint-driven — they serve display and sorting beyond filtering. Only value_indexed is gated. VerifyFormsDataIntegrity: "value_indexed set on non-filterable field" is now a FAIL (was WARN) — it means the observer didn't run correctly, which is a real integrity issue. Observer tests: split the old "string hint populates value_indexed" case into filterable/non-filterable pair. Full suite 911/911. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -259,19 +259,15 @@ final class VerifyFormsDataIntegrity extends Command
|
||||
])
|
||||
->count();
|
||||
|
||||
if ($orphanSub > 0 || $orphanField > 0 || $dup > 0 || $longIndexed > 0 || $multiValueIndexed > 0) {
|
||||
if ($orphanSub > 0 || $orphanField > 0 || $dup > 0 || $longIndexed > 0 || $multiValueIndexed > 0 || $nonFilterableIndexed > 0) {
|
||||
$this->recordFailure('Value coherence',
|
||||
"{$orphanSub} orphan submission, {$orphanField} orphan field, {$dup} duplicate pairs, {$longIndexed} over-length value_indexed, {$multiValueIndexed} multi-value rows with value_indexed set"
|
||||
"{$orphanSub} orphan submission, {$orphanField} orphan field, {$dup} duplicate pairs, {$longIndexed} over-length value_indexed, {$multiValueIndexed} multi-value rows with value_indexed set, {$nonFilterableIndexed} value_indexed set on non-filterable field",
|
||||
'observer should only populate value_indexed when field.is_filterable=true — re-save affected rows to let FormValueObserver reconcile'
|
||||
);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
// Warning only — doesn't fail the check.
|
||||
if ($nonFilterableIndexed > 0) {
|
||||
$this->warn(" [WARN] {$nonFilterableIndexed} form_values have value_indexed set for a non-filterable field");
|
||||
}
|
||||
|
||||
$this->recordPass('Value coherence', "{$total} values verified");
|
||||
}
|
||||
|
||||
|
||||
@@ -36,26 +36,25 @@ final class FormValueObserver
|
||||
$this->resetTypedColumns($value);
|
||||
|
||||
// value stores the canonical payload (always JSON). Typed columns
|
||||
// are derived from it based on the field's storage hint.
|
||||
// are derived from it per field.value_storage_hint, with one
|
||||
// exception: value_indexed is filter-driven (§4.4), not hint-driven,
|
||||
// so populate it ONLY when field.is_filterable = true. Keeps the
|
||||
// partial index lean and simplifies FilterQueryBuilder (S4).
|
||||
$raw = $value->value;
|
||||
$scalar = $this->extractScalar($raw);
|
||||
|
||||
match ($field->value_storage_hint) {
|
||||
FormValueStorageHint::STRING => $value->value_indexed = $this->truncateIndexed($scalar),
|
||||
FormValueStorageHint::STRING => null, // handled below, filter-gated
|
||||
FormValueStorageHint::NUMBER => $value->value_number = is_numeric($scalar) ? (float) $scalar : null,
|
||||
FormValueStorageHint::DATE => $value->value_date = $this->castDate($scalar),
|
||||
FormValueStorageHint::BOOL => $value->value_bool = $scalar === null ? null : (bool) $scalar,
|
||||
FormValueStorageHint::JSON => null,
|
||||
};
|
||||
|
||||
// Single-value filterable fields: ensure value_indexed is populated
|
||||
// regardless of storage hint, so filter queries hit one index.
|
||||
if ($field->is_filterable && ! $this->isMultiValueType($field)) {
|
||||
if ($value->value_indexed === null) {
|
||||
$value->value_indexed = $this->truncateIndexed($scalar);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public function saved(FormValue $value): void
|
||||
{
|
||||
|
||||
@@ -32,11 +32,12 @@ final class FormValueObserverTest extends TestCase
|
||||
]);
|
||||
}
|
||||
|
||||
public function test_string_hint_populates_value_indexed(): void
|
||||
public function test_string_hint_populates_value_indexed_when_filterable(): void
|
||||
{
|
||||
$field = FormField::factory()->for($this->schema, 'schema')->create([
|
||||
'field_type' => FormFieldType::TEXT->value,
|
||||
'value_storage_hint' => FormValueStorageHint::STRING,
|
||||
'is_filterable' => true,
|
||||
]);
|
||||
|
||||
$value = FormValue::create([
|
||||
@@ -51,6 +52,23 @@ final class FormValueObserverTest extends TestCase
|
||||
$this->assertNull($value->value_bool);
|
||||
}
|
||||
|
||||
public function test_string_hint_leaves_value_indexed_null_when_not_filterable(): void
|
||||
{
|
||||
$field = FormField::factory()->for($this->schema, 'schema')->create([
|
||||
'field_type' => FormFieldType::TEXT->value,
|
||||
'value_storage_hint' => FormValueStorageHint::STRING,
|
||||
'is_filterable' => false,
|
||||
]);
|
||||
|
||||
$value = FormValue::create([
|
||||
'form_submission_id' => $this->submission->id,
|
||||
'form_field_id' => $field->id,
|
||||
'value' => ['value' => 'Niet-filterable tekst'],
|
||||
]);
|
||||
|
||||
$this->assertNull($value->fresh()->value_indexed);
|
||||
}
|
||||
|
||||
public function test_number_hint_populates_value_number(): void
|
||||
{
|
||||
$field = FormField::factory()->for($this->schema, 'schema')->create([
|
||||
|
||||
Reference in New Issue
Block a user