Skip to content

fix(reports): request changes form validation for compounds and organisms #804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vcnainala wants to merge 3 commits into
base: development
Choose a base branch
from
+301 −51
Open

fix(reports): request changes form validation for compounds and organisms #804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d4ced89
fix(organisms): allow linking existing organisms in report changes
vcnainala Jun 7, 2026
12638d7
fix(reports): correct select validation in request changes form
vcnainala Jun 7, 2026
3c2eb9d
fix(reports): resolve PHPStan errors in ReportResource
vcnainala Jun 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 105 additions & 46 deletions app/Filament/Dashboard/Resources/ReportResource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -546,13 +540,14 @@ 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;
}

return empty($synonyms) ? [] : $synonyms;
->options(function (Get $get, ?Report $record): array {
$molecule = static::resolveMolecule($record, $get);
$synonyms = array_merge(
$molecule === null ? [] : $molecule->synonyms,
$get('existing_synonyms') ?? [],
);

return static::valueKeyedOptions($synonyms);
})
->disabled(function (Get $get, string $operation) {
return ! $get('show_synonym_existing') && $operation == 'edit';
Expand Down Expand Up @@ -581,10 +576,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';
Expand All @@ -605,10 +598,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 === null ? [] : $molecule->cas),
$get('existing_cas') ?? [],
);

return static::valueKeyedOptions($cas);
})
->disabled(function (Get $get, string $operation) {
return ! $get('show_cas_existing') && $operation == 'edit';
Expand Down Expand Up @@ -642,10 +639,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';
Expand All @@ -657,16 +655,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)
Expand All @@ -690,10 +696,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';
Expand Down Expand Up @@ -1622,4 +1629,56 @@ public static function runSQLQueries(Report $record): void
$molecule->save();
});
}

/**
* @return array<string, string>
*/
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 ($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;
}
}
47 changes: 42 additions & 5 deletions app/Models/Organism.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace App\Models;

use Closure;
use Filament\Forms;
use Filament\Forms\Components\Actions\Action;
use Illuminate\Contracts\View\View;
Expand Down Expand Up @@ -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<int, Closure>
*/
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')
Expand Down
65 changes: 65 additions & 0 deletions tests/Unit/OrganismReportChangeFormTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

namespace Tests\Unit;

use App\Models\Organism;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Validator;
use Tests\TestCase;

class OrganismReportChangeFormTest extends TestCase
{
use RefreshDatabase;

public function test_validate_name_for_report_change_allows_unlinked_existing_organism_name(): void
{
Organism::create(['name' => '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());
}
}
Loading
Loading