From d4ced89448d0fe2e3ded727e2652b967fcbfca63 Mon Sep 17 00:00:00 2001 From: Venkata Nainala Date: Sun, 7 Jun 2026 17:15:14 +0200 Subject: [PATCH 1/3] fix(organisms): allow linking existing organisms in report changes Relax unique-name validation in the request-changes organism repeater so users can link organisms that already exist in the database. Reject names already attached to the target molecule instead. Fixes #772 --- app/Models/Organism.php | 47 +++++++++++++-- tests/Unit/OrganismReportChangeFormTest.php | 65 +++++++++++++++++++++ 2 files changed, 107 insertions(+), 5 deletions(-) create mode 100644 tests/Unit/OrganismReportChangeFormTest.php diff --git a/app/Models/Organism.php b/app/Models/Organism.php index 85ac84a7..64ff4ac6 100644 --- a/app/Models/Organism.php +++ b/app/Models/Organism.php @@ -2,6 +2,7 @@ namespace App\Models; +use Closure; use Filament\Forms; use Filament\Forms\Components\Actions\Action; use Illuminate\Contracts\View\View; @@ -79,13 +80,49 @@ public function transformAudit(array $data): array return changeAudit($data); } - public static function getForm(): array + public static function validateNameForReportChange(string $name, array $linkedNames): ?string { + if (in_array($name, $linkedNames, true)) { + return 'This organism is already linked to this molecule.'; + } + + return null; + } + + /** + * @return array + */ + public static function reportChangeNameRules(array $linkedNames = []): array + { + if ($linkedNames === []) { + return []; + } + + return [ + function (string $attribute, mixed $value, Closure $fail) use ($linkedNames): void { + $message = static::validateNameForReportChange((string) $value, $linkedNames); + + if ($message !== null) { + $fail($message); + } + }, + ]; + } + + public static function getForm(bool $requireUniqueName = true, array $excludedNames = []): array + { + $nameField = Forms\Components\TextInput::make('name') + ->required() + ->maxLength(255); + + if ($requireUniqueName) { + $nameField->unique(Organism::class, 'name'); + } else { + $nameField->rules(static::reportChangeNameRules($excludedNames)); + } + return [ - Forms\Components\TextInput::make('name') - ->required() - ->unique(Organism::class, 'name') - ->maxLength(255) + $nameField // ->suffixAction( // Action::make('infoFromSources') // ->icon('heroicon-m-clipboard') diff --git a/tests/Unit/OrganismReportChangeFormTest.php b/tests/Unit/OrganismReportChangeFormTest.php new file mode 100644 index 00000000..9278ebf6 --- /dev/null +++ b/tests/Unit/OrganismReportChangeFormTest.php @@ -0,0 +1,65 @@ + 'Escherichia coli', 'slug' => 'escherichia-coli']); + + $this->assertNull( + Organism::validateNameForReportChange('Escherichia coli', ['Homo sapiens']) + ); + } + + public function test_validate_name_for_report_change_rejects_already_linked_organism_name(): void + { + $message = Organism::validateNameForReportChange('Homo sapiens', ['Homo sapiens']); + + $this->assertSame('This organism is already linked to this molecule.', $message); + } + + public function test_report_change_name_rules_allow_existing_database_organism_name(): void + { + Organism::create(['name' => 'Escherichia coli', 'slug' => 'escherichia-coli']); + + $rules = ['name' => Organism::reportChangeNameRules(['Homo sapiens'])]; + + $validator = Validator::make(['name' => 'Escherichia coli'], $rules); + + $this->assertFalse($validator->fails()); + } + + public function test_report_change_name_rules_reject_already_linked_organism_name(): void + { + $rules = ['name' => Organism::reportChangeNameRules(['Homo sapiens'])]; + + $validator = Validator::make(['name' => 'Homo sapiens'], $rules); + + $this->assertTrue($validator->fails()); + $this->assertSame( + 'This organism is already linked to this molecule.', + $validator->errors()->first('name') + ); + } + + public function test_default_get_form_still_requires_unique_organism_name(): void + { + Organism::create(['name' => 'Escherichia coli', 'slug' => 'escherichia-coli']); + + $validator = Validator::make( + ['name' => 'Escherichia coli'], + ['name' => ['required', 'unique:organisms,name']] + ); + + $this->assertTrue($validator->fails()); + } +} From 12638d7d07b4028f9e67a200cf4b3170ffcd2370 Mon Sep 17 00:00:00 2001 From: Venkata Nainala Date: Sun, 7 Jun 2026 17:15:17 +0200 Subject: [PATCH 2/3] fix(reports): correct select validation in request changes form Align Filament v4 Select option keys with stored state for existing synonyms, CAS, geo locations, organisms, and citations. Resolve the target molecule from form state and report records so options remain valid on Livewire submit requests. Fixes #775 --- .../Dashboard/Resources/ReportResource.php | 156 +++++++++++++----- .../ReportResourceMoleculeOptionsTest.php | 89 ++++++++++ 2 files changed, 200 insertions(+), 45 deletions(-) create mode 100644 tests/Unit/ReportResourceMoleculeOptionsTest.php diff --git a/app/Filament/Dashboard/Resources/ReportResource.php b/app/Filament/Dashboard/Resources/ReportResource.php index cfb13560..21e120d1 100644 --- a/app/Filament/Dashboard/Resources/ReportResource.php +++ b/app/Filament/Dashboard/Resources/ReportResource.php @@ -29,6 +29,7 @@ use Filament\Actions\BulkActionGroup; use Filament\Actions\DeleteBulkAction; use Filament\Forms\Components\Checkbox; +use Filament\Forms\Components\Hidden; use Filament\Forms\Components\Radio; use Filament\Forms\Components\Repeater; use Filament\Forms\Components\Select; @@ -65,21 +66,16 @@ class ReportResource extends Resource protected static string|\BackedEnum|null $navigationIcon = 'heroicon-o-rectangle-stack'; - protected static $molecule = null; - protected static $approved_changes = null; protected static $overall_changes = null; - public function __construct() - { - self::$molecule = request()->has('compound_id') ? Molecule::where('identifier', request()->compound_id)->first() : null; - } - public static function form(Schema $schema): Schema { return $schema ->components([ + Hidden::make('compound_id'), + Hidden::make('type'), Grid::make() ->schema([ Select::make('report_category') @@ -509,13 +505,11 @@ public static function form(Schema $schema): Schema Select::make('existing_geo_locations') ->label('Existing') ->multiple() - ->options(function (): array { - $geo_locations = []; - if (self::$molecule) { - $geo_locations = self::$molecule->geo_locations->pluck('name', 'id')->toArray(); - } + ->options(function (Get $get, ?Report $record): array { + $molecule = static::resolveMolecule($record, $get); + $names = $molecule?->geo_locations->pluck('name')->toArray() ?? []; - return $geo_locations; + return static::valueKeyedOptions(array_merge($names, $get('existing_geo_locations') ?? [])); }) ->disabled(function (Get $get, string $operation) { return ! $get('show_geo_location_existing') && $operation == 'edit'; @@ -546,13 +540,11 @@ public static function form(Schema $schema): Schema Select::make('existing_synonyms') ->label('Existing') ->multiple() - ->options(function (): array { - $synonyms = []; - if (self::$molecule) { - $synonyms = self::$molecule->synonyms; - } + ->options(function (Get $get, ?Report $record): array { + $molecule = static::resolveMolecule($record, $get); + $synonyms = array_merge($molecule?->synonyms ?? [], $get('existing_synonyms') ?? []); - return empty($synonyms) ? [] : $synonyms; + return static::valueKeyedOptions($synonyms); }) ->disabled(function (Get $get, string $operation) { return ! $get('show_synonym_existing') && $operation == 'edit'; @@ -581,10 +573,8 @@ public static function form(Schema $schema): Schema }) ->columnSpan(1), Textarea::make('name') - ->default(function () { - if (self::$molecule) { - return self::$molecule->name; - } + ->default(function (Get $get, ?Report $record) { + return static::resolveMolecule($record, $get)?->name; }) ->disabled(function (Get $get, string $operation) { return ! $get('show_name_change') && $operation == 'edit'; @@ -605,10 +595,14 @@ public static function form(Schema $schema): Schema Select::make('existing_cas') ->label('Existing') ->multiple() - ->options(function () { - if (self::$molecule) { - return self::$molecule->cas; - } + ->options(function (Get $get, ?Report $record): array { + $molecule = static::resolveMolecule($record, $get); + $cas = array_merge( + array_values($molecule?->cas ?? []), + $get('existing_cas') ?? [], + ); + + return static::valueKeyedOptions($cas); }) ->disabled(function (Get $get, string $operation) { return ! $get('show_cas_existing') && $operation == 'edit'; @@ -642,10 +636,11 @@ public static function form(Schema $schema): Schema Select::make('existing_organisms') ->label('Existing') ->multiple() - ->options(function () { - if (self::$molecule) { - return self::$molecule->organisms->pluck('name', 'id')->toArray(); - } + ->options(function (Get $get, ?Report $record): array { + $molecule = static::resolveMolecule($record, $get); + $names = $molecule?->organisms->pluck('name')->toArray() ?? []; + + return static::valueKeyedOptions(array_merge($names, $get('existing_organisms') ?? [])); }) ->disabled(function (Get $get, string $operation) { return ! $get('show_organism_existing') && $operation == 'edit'; @@ -657,16 +652,24 @@ public static function form(Schema $schema): Schema Repeater::make('new_organisms') ->label('') - ->schema([ - Checkbox::make('approve_new_organism') - ->label('Approve') - ->hidden(function (string $operation) { - return ! auth()->user()->isCurator() || $operation == 'create'; - }) - ->columnSpanFull(), - Grid::make() - ->schema(Organism::getForm()), - ]) + ->schema(function (Get $get, ?Report $record): array { + $molecule = static::resolveMolecule($record, $get); + $linkedNames = $molecule?->organisms->pluck('name')->toArray() ?? []; + + return [ + Checkbox::make('approve_new_organism') + ->label('Approve') + ->hidden(function (string $operation) { + return ! auth()->user()->isCurator() || $operation == 'create'; + }) + ->columnSpanFull(), + Grid::make() + ->schema(Organism::getForm( + requireUniqueName: false, + excludedNames: $linkedNames, + )), + ]; + }) ->reorderable(false) ->addActionLabel('Add New Organism') ->defaultItems(0) @@ -690,10 +693,11 @@ public static function form(Schema $schema): Schema Select::make('existing_citations') ->label('Existing') ->multiple() - ->options(function () { - if (self::$molecule) { - return self::$molecule->citations->where('title', '!=', null)->pluck('title', 'id')->toArray(); - } + ->options(function (Get $get, ?Report $record): array { + $molecule = static::resolveMolecule($record, $get); + $titles = $molecule?->citations->where('title', '!=', null)->pluck('title')->toArray() ?? []; + + return static::valueKeyedOptions(array_merge($titles, $get('existing_citations') ?? [])); }) ->disabled(function (Get $get, string $operation) { return ! $get('show_citation_existing') && $operation == 'edit'; @@ -1622,4 +1626,66 @@ public static function runSQLQueries(Report $record): void $molecule->save(); }); } + + /** + * @return array + */ + protected static function valueKeyedOptions(array $values): array + { + $values = array_values(array_unique(array_filter($values, fn ($value) => filled($value)))); + + if ($values === []) { + return []; + } + + return array_combine($values, $values); + } + + protected static function resolveMolecule(?Report $record = null, ?Get $get = null): ?Molecule + { + $identifier = static::resolveMoleculeIdentifier($record, $get); + + return $identifier ? Molecule::where('identifier', $identifier)->first() : null; + } + + protected static function resolveMoleculeIdentifier(?Report $record = null, ?Get $get = null): ?string + { + if ($record) { + $molIds = $record->mol_ids; + + if (is_array($molIds) && count($molIds) > 0) { + return $molIds[0]; + } + + if (is_string($molIds)) { + $decoded = json_decode($molIds, true); + + if (json_last_error() === JSON_ERROR_NONE && is_array($decoded)) { + return $decoded[0] ?? null; + } + + return $molIds; + } + } + + if ($get) { + $molIds = $get('mol_ids'); + + if (is_array($molIds) && count($molIds) > 0) { + return $molIds[0]; + } + + $compoundId = $get('compound_id'); + + if (filled($compoundId)) { + return $compoundId; + } + } + + if (request()->has('compound_id')) { + return request()->compound_id; + } + + return null; + } } diff --git a/tests/Unit/ReportResourceMoleculeOptionsTest.php b/tests/Unit/ReportResourceMoleculeOptionsTest.php new file mode 100644 index 00000000..7433c44d --- /dev/null +++ b/tests/Unit/ReportResourceMoleculeOptionsTest.php @@ -0,0 +1,89 @@ +setAccessible(true); + + return $reflection->invoke(null, ...$args); + } + + public function test_value_keyed_options_uses_value_as_key_and_label(): void + { + $options = $this->callStatic('valueKeyedOptions', ['methane', 'CH4']); + + $this->assertSame(['methane' => 'methane', 'CH4' => 'CH4'], $options); + } + + public function test_value_keyed_options_filters_empty_values(): void + { + $options = $this->callStatic('valueKeyedOptions', ['methane', '', null]); + + $this->assertSame(['methane' => 'methane'], $options); + } + + public function test_value_keyed_options_deduplicates_values(): void + { + $options = $this->callStatic('valueKeyedOptions', ['methane', 'methane']); + + $this->assertSame(['methane' => 'methane'], $options); + } + + public function test_prefilled_select_state_matches_option_keys(): void + { + $synonyms = ['methane', 'CH4']; + $cas = ['74-82-8']; + + $synonymOptions = $this->callStatic('valueKeyedOptions', $synonyms); + $casOptions = $this->callStatic('valueKeyedOptions', $cas); + + $this->assertEmpty(array_diff($synonyms, array_keys($synonymOptions))); + $this->assertEmpty(array_diff($cas, array_keys($casOptions))); + } + + public function test_resolve_molecule_from_report_mol_ids(): void + { + $molecule = Molecule::create([ + 'identifier' => 'CNP0520383.0', + 'name' => 'old name', + 'synonyms' => ['CH4'], + 'cas' => ['74-82-8'], + ]); + + $report = new Report([ + 'mol_ids' => ['CNP0520383.0'], + 'title' => 'test', + ]); + + $resolved = $this->callStatic('resolveMolecule', $report, null); + + $this->assertTrue($resolved->is($molecule)); + } + + public function test_resolve_molecule_from_request_compound_id(): void + { + $molecule = Molecule::create([ + 'identifier' => 'CNP0520383.0', + 'name' => 'methane', + ]); + + request()->merge(['compound_id' => 'CNP0520383.0']); + + $resolved = $this->callStatic('resolveMolecule', null, null); + + $this->assertTrue($resolved->is($molecule)); + } +} From 3c2eb9dc65612fe773f1e349d0b6d72b87c421c7 Mon Sep 17 00:00:00 2001 From: Venkata Nainala Date: Sun, 7 Jun 2026 21:43:49 +0200 Subject: [PATCH 3/3] fix(reports): resolve PHPStan errors in ReportResource Use explicit null checks for molecule synonyms/CAS options and remove unreachable is_string handling for mol_ids array cast. --- .../Dashboard/Resources/ReportResource.php | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/app/Filament/Dashboard/Resources/ReportResource.php b/app/Filament/Dashboard/Resources/ReportResource.php index 21e120d1..56fc80c9 100644 --- a/app/Filament/Dashboard/Resources/ReportResource.php +++ b/app/Filament/Dashboard/Resources/ReportResource.php @@ -542,7 +542,10 @@ public static function form(Schema $schema): Schema ->multiple() ->options(function (Get $get, ?Report $record): array { $molecule = static::resolveMolecule($record, $get); - $synonyms = array_merge($molecule?->synonyms ?? [], $get('existing_synonyms') ?? []); + $synonyms = array_merge( + $molecule === null ? [] : $molecule->synonyms, + $get('existing_synonyms') ?? [], + ); return static::valueKeyedOptions($synonyms); }) @@ -598,7 +601,7 @@ public static function form(Schema $schema): Schema ->options(function (Get $get, ?Report $record): array { $molecule = static::resolveMolecule($record, $get); $cas = array_merge( - array_values($molecule?->cas ?? []), + array_values($molecule === null ? [] : $molecule->cas), $get('existing_cas') ?? [], ); @@ -1656,16 +1659,6 @@ protected static function resolveMoleculeIdentifier(?Report $record = null, ?Get if (is_array($molIds) && count($molIds) > 0) { return $molIds[0]; } - - if (is_string($molIds)) { - $decoded = json_decode($molIds, true); - - if (json_last_error() === JSON_ERROR_NONE && is_array($decoded)) { - return $decoded[0] ?? null; - } - - return $molIds; - } } if ($get) {