Promote suggestion misses through the bulk-add modal, then delete them
Suggestion Misses' "Promote Selected" now opens the same bulk-add modal shape as Suggestions' "Create Many" (editable phrases textarea + lang/intent), prefilled with the selected misses' own text/language, and submits via batchCreateSuggestions() so edited/added lines are honored. Once that succeeds, the selected misses are deleted via the new batchDeleteSuggestionMisses() (DELETE /admin/suggestion-misses/batch) so they don't linger in the list now that they're suggestions. This is best-effort: a failed cleanup doesn't undo the promotion, just surfaces a separate warning notification.
This commit is contained in:
@@ -16,3 +16,9 @@ For custom-data (`->records()`-backed, non-Eloquent) tables: use `->callTableAct
|
|||||||
On a `Table::records()`-backed (non-Eloquent) page, `BulkAction::make(...)->fetchSelectedRecords(false)` does NOT skip hydration the way it does for an Eloquent table — the `Collection $records` passed to `->action()` still contains full row arrays (keyed by the record key), not bare ids. Use `$records->keys()->all()` to get just the selected ids; `$records->all()`/`$records->values()` gives you full row data instead. See `ManageSuggestions::deleteSelectedBulkAction()` / `ManageSuggestionMisses::promoteBulkAction()`.
|
On a `Table::records()`-backed (non-Eloquent) page, `BulkAction::make(...)->fetchSelectedRecords(false)` does NOT skip hydration the way it does for an Eloquent table — the `Collection $records` passed to `->action()` still contains full row arrays (keyed by the record key), not bare ids. Use `$records->keys()->all()` to get just the selected ids; `$records->all()`/`$records->values()` gives you full row data instead. See `ManageSuggestions::deleteSelectedBulkAction()` / `ManageSuggestionMisses::promoteBulkAction()`.
|
||||||
|
|
||||||
Also: `BnfexpressAdminClient`'s non-2xx handling (`errorMessage()`) must handle `detail` being a list of `{msg, ...}` objects, not just a string — FastAPI's own request-validation failures (422s) return `detail` in that shape, and casting it straight to `(string)` silently produces the literal "Array".
|
Also: `BnfexpressAdminClient`'s non-2xx handling (`errorMessage()`) must handle `detail` being a list of `{msg, ...}` objects, not just a string — FastAPI's own request-validation failures (422s) return `detail` in that shape, and casting it straight to `(string)` silently produces the literal "Array".
|
||||||
|
|
||||||
|
## "Promote" on ManageSuggestionMisses now reuses the Create Many bulk-add modal
|
||||||
|
`ManageSuggestionMisses::promoteBulkAction()` no longer calls `promoteSuggestionMisses()` (miss-id based). It opens the same modal shape as `ManageSuggestions::createManyAction()` — info text + editable `items_raw` textarea + lang/intent — prefilled via `fillForm(fn (Collection $records) => ...)` (auto-injected selected records, works in `fillForm` the same way it does in a bulk `->action()`) from the selected misses' own `text_norm`/`lang`. Submits through `batchCreateSuggestions()`, so edited/added lines in the textarea are honored — the created rows no longer map 1:1 to the original miss ids, and the original miss rows are NOT auto-dismissed by this path (unlike the old promote endpoint).
|
||||||
|
|
||||||
|
## `assertNotified()` pulls (and clears) ALL queued notifications on its first call
|
||||||
|
Filament's `Notification::assertNotified()` reads via `session()->pull('filament.notifications')`, which empties the session key. Chaining `->assertNotified('A')->assertNotified('B')` after one action fails on the second call even if both were actually sent — the first call already drained the queue. To check two notifications from one action, assert only one (whichever isn't covered by another test) rather than chaining. See `ManageSuggestionMisses`'s promote-cleanup-failure test.
|
||||||
|
|||||||
@@ -5,9 +5,11 @@ namespace Modules\AiAgent\Filament\Pages;
|
|||||||
use BackedEnum;
|
use BackedEnum;
|
||||||
use Filament\Actions\Action;
|
use Filament\Actions\Action;
|
||||||
use Filament\Actions\BulkAction;
|
use Filament\Actions\BulkAction;
|
||||||
|
use Filament\Forms\Components\Textarea;
|
||||||
use Filament\Forms\Components\TextInput;
|
use Filament\Forms\Components\TextInput;
|
||||||
use Filament\Notifications\Notification;
|
use Filament\Notifications\Notification;
|
||||||
use Filament\Pages\Page;
|
use Filament\Pages\Page;
|
||||||
|
use Filament\Schemas\Components\Text;
|
||||||
use Filament\Support\Icons\Heroicon;
|
use Filament\Support\Icons\Heroicon;
|
||||||
use Filament\Tables\Columns\IconColumn;
|
use Filament\Tables\Columns\IconColumn;
|
||||||
use Filament\Tables\Columns\TextColumn;
|
use Filament\Tables\Columns\TextColumn;
|
||||||
@@ -20,6 +22,7 @@ use Illuminate\Support\Collection;
|
|||||||
use Modules\AiAgent\Filament\Concerns\HandlesBnfexpressErrors;
|
use Modules\AiAgent\Filament\Concerns\HandlesBnfexpressErrors;
|
||||||
use Modules\AiAgent\Filament\Concerns\PaginatesBnfexpressLists;
|
use Modules\AiAgent\Filament\Concerns\PaginatesBnfexpressLists;
|
||||||
use Modules\Shared\Bnfexpress\BnfexpressAdminClient;
|
use Modules\Shared\Bnfexpress\BnfexpressAdminClient;
|
||||||
|
use Modules\Shared\Bnfexpress\Exceptions\BnfexpressApiException;
|
||||||
use UnitEnum;
|
use UnitEnum;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -112,35 +115,74 @@ class ManageSuggestionMisses extends Page implements HasTable
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Opens the same "bulk add" modal shape as ManageSuggestions'
|
||||||
|
* createManyAction() (info text + editable items textarea + lang/intent),
|
||||||
|
* prefilled with the selected misses' own text — one per line — so an
|
||||||
|
* admin can review/tweak the batch before it's created. Submits via
|
||||||
|
* batchCreateSuggestions() (same endpoint createManyAction uses, and per
|
||||||
|
* its own comment, bnfexpress's thin wrapper around the misses-promotion
|
||||||
|
* promote()), not promoteSuggestionMisses() — editing the textarea means
|
||||||
|
* the created rows no longer map 1:1 to the original miss ids. Once that
|
||||||
|
* succeeds, the selected misses are deleted via batchDeleteSuggestionMisses()
|
||||||
|
* so they don't linger in this list now that they're suggestions — best
|
||||||
|
* effort, since editing the textarea already broke the id mapping and the
|
||||||
|
* promotion itself is done by that point either way.
|
||||||
|
*/
|
||||||
protected function promoteBulkAction(): BulkAction
|
protected function promoteBulkAction(): BulkAction
|
||||||
{
|
{
|
||||||
return BulkAction::make('promote')
|
return BulkAction::make('promote')
|
||||||
->label('Promote Selected')
|
->label('Promote Selected')
|
||||||
->icon(Heroicon::OutlinedArrowUp)
|
->icon(Heroicon::OutlinedArrowUp)
|
||||||
->fetchSelectedRecords(false)
|
->fillForm(fn (Collection $records): array => [
|
||||||
|
'items_raw' => $records->pluck('text_norm')->implode("\n"),
|
||||||
|
'lang' => $records->pluck('lang')->filter()->unique()->count() === 1
|
||||||
|
? $records->pluck('lang')->filter()->first()
|
||||||
|
: null,
|
||||||
|
])
|
||||||
->schema([
|
->schema([
|
||||||
|
Text::make('Created rows are tagged source: mined by bnfexpress.')
|
||||||
|
->color('gray'),
|
||||||
|
Textarea::make('items_raw')
|
||||||
|
->label('Phrases (one per line)')
|
||||||
|
->required()
|
||||||
|
->rows(8),
|
||||||
TextInput::make('lang')
|
TextInput::make('lang')
|
||||||
->label('Language override')
|
->required()
|
||||||
->helperText("Applied to every selected miss; leave blank to keep each one's own language.")
|
|
||||||
->maxLength(10),
|
->maxLength(10),
|
||||||
TextInput::make('intent'),
|
TextInput::make('intent'),
|
||||||
])
|
])
|
||||||
->deselectRecordsAfterCompletion()
|
->deselectRecordsAfterCompletion()
|
||||||
->action(function (array $data, Collection $records): void {
|
->action(function (array $data, Collection $records): void {
|
||||||
// Same caveat as ManageSuggestions' deleteSelected — the collection
|
$items = collect(preg_split('/\r\n|\r|\n/', (string) $data['items_raw']))
|
||||||
// holds full row arrays, not just keys, for a custom-data table.
|
->map(fn (string $line): string => trim($line))
|
||||||
|
->filter()
|
||||||
|
->map(fn (string $text): array => array_filter([
|
||||||
|
'text' => $text,
|
||||||
|
'lang' => $data['lang'],
|
||||||
|
'intent' => $data['intent'] ?: null,
|
||||||
|
], fn (mixed $value): bool => $value !== null))
|
||||||
|
->values()
|
||||||
|
->all();
|
||||||
|
|
||||||
$this->callBnfexpressForResult(
|
$this->callBnfexpressForResult(
|
||||||
fn () => app(BnfexpressAdminClient::class)->promoteSuggestionMisses(
|
fn () => app(BnfexpressAdminClient::class)->batchCreateSuggestions($items),
|
||||||
$records->keys()->all(),
|
function (array $result) use ($records): void {
|
||||||
$data['lang'] ?: null,
|
|
||||||
$data['intent'] ?: null,
|
|
||||||
),
|
|
||||||
function (array $result): void {
|
|
||||||
Notification::make()
|
Notification::make()
|
||||||
->title("{$result['created']} promoted, {$result['skipped']} skipped")
|
->title("{$result['created']} promoted, {$result['skipped']} skipped")
|
||||||
->success()
|
->success()
|
||||||
->send();
|
->send();
|
||||||
|
|
||||||
|
try {
|
||||||
|
app(BnfexpressAdminClient::class)->batchDeleteSuggestionMisses($records->keys()->all());
|
||||||
|
} catch (BnfexpressApiException $exception) {
|
||||||
|
Notification::make()
|
||||||
|
->title('Promoted, but failed to remove the original misses')
|
||||||
|
->body($exception->getMessage())
|
||||||
|
->warning()
|
||||||
|
->send();
|
||||||
|
}
|
||||||
|
|
||||||
$this->resetTable();
|
$this->resetTable();
|
||||||
},
|
},
|
||||||
failureTitle: 'Failed to promote suggestion misses',
|
failureTitle: 'Failed to promote suggestion misses',
|
||||||
|
|||||||
@@ -69,9 +69,25 @@ test('a failed dismiss surfaces the gateway detail message', function () {
|
|||||||
->assertNotified('Failed to dismiss miss');
|
->assertNotified('Failed to dismiss miss');
|
||||||
});
|
});
|
||||||
|
|
||||||
test('promote bulk action calls promoteSuggestionMisses with the selected ids and shows the result', function () {
|
test('promote bulk action prefills the bulk-add modal with the selected misses\' text and language', function () {
|
||||||
|
Http::fake(['bnfexpress.test/*' => Http::response([
|
||||||
|
['id' => 1, 'text_norm' => 'ev charging cost', 'lang' => 'my', 'syllables' => 3, 'was_used' => false, 'created_at' => now()->toIso8601String()],
|
||||||
|
['id' => 2, 'text_norm' => 'nearest charger', 'lang' => 'my', 'syllables' => 2, 'was_used' => false, 'created_at' => now()->toIso8601String()],
|
||||||
|
])]);
|
||||||
|
|
||||||
|
Livewire::test(ManageSuggestionMisses::class)
|
||||||
|
->loadTable()
|
||||||
|
->mountTableBulkAction('promote', [1, 2])
|
||||||
|
->assertTableBulkActionDataSet([
|
||||||
|
'items_raw' => "ev charging cost\nnearest charger",
|
||||||
|
'lang' => 'my',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('promote bulk action calls batchCreateSuggestions with the (possibly edited) phrases and shows the result', function () {
|
||||||
Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) {
|
Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) {
|
||||||
str_contains($request->url(), '/admin/suggestion-misses/promote') => Http::response(['created' => 2, 'skipped' => 0, 'trie_rebuilt' => true]),
|
str_contains($request->url(), '/admin/suggestions/batch') => Http::response(['created' => 2, 'skipped' => 0, 'trie_rebuilt' => true]),
|
||||||
|
str_contains($request->url(), '/admin/suggestion-misses/batch') => Http::response(['deleted' => 2, 'skipped' => 0]),
|
||||||
default => Http::response([
|
default => Http::response([
|
||||||
['id' => 1, 'text_norm' => 'a', 'lang' => 'en', 'syllables' => 1, 'was_used' => false, 'created_at' => now()->toIso8601String()],
|
['id' => 1, 'text_norm' => 'a', 'lang' => 'en', 'syllables' => 1, 'was_used' => false, 'created_at' => now()->toIso8601String()],
|
||||||
['id' => 2, 'text_norm' => 'b', 'lang' => 'en', 'syllables' => 1, 'was_used' => false, 'created_at' => now()->toIso8601String()],
|
['id' => 2, 'text_norm' => 'b', 'lang' => 'en', 'syllables' => 1, 'was_used' => false, 'created_at' => now()->toIso8601String()],
|
||||||
@@ -80,10 +96,54 @@ test('promote bulk action calls promoteSuggestionMisses with the selected ids an
|
|||||||
|
|
||||||
Livewire::test(ManageSuggestionMisses::class)
|
Livewire::test(ManageSuggestionMisses::class)
|
||||||
->loadTable()
|
->loadTable()
|
||||||
->callTableBulkAction('promote', [1, 2], data: ['lang' => 'my', 'intent' => null])
|
->callTableBulkAction('promote', [1, 2], data: ['items_raw' => "a\nb\nc", 'lang' => 'my', 'intent' => null])
|
||||||
->assertNotified('2 promoted, 0 skipped');
|
->assertNotified('2 promoted, 0 skipped');
|
||||||
|
|
||||||
Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestion-misses/promote')
|
Http::assertSent(fn (Request $request) => str_contains($request->url(), '/admin/suggestions/batch')
|
||||||
&& $request['miss_ids'] === [1, 2]
|
&& $request['items'] === [
|
||||||
&& $request['lang'] === 'my');
|
['text' => 'a', 'lang' => 'my'],
|
||||||
|
['text' => 'b', 'lang' => 'my'],
|
||||||
|
['text' => 'c', 'lang' => 'my'],
|
||||||
|
]);
|
||||||
|
|
||||||
|
Http::assertSent(fn (Request $request) => $request->method() === 'DELETE'
|
||||||
|
&& str_contains($request->url(), '/admin/suggestion-misses/batch')
|
||||||
|
&& $request['miss_ids'] === [1, 2]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('promoting deletes the original misses after a successful promote', function () {
|
||||||
|
Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) {
|
||||||
|
str_contains($request->url(), '/admin/suggestions/batch') => Http::response(['created' => 1, 'skipped' => 0, 'trie_rebuilt' => true]),
|
||||||
|
str_contains($request->url(), '/admin/suggestion-misses/batch') => Http::response(['deleted' => 1, 'skipped' => 0]),
|
||||||
|
default => Http::response([
|
||||||
|
['id' => 1, 'text_norm' => 'ev charging cost', 'lang' => 'en', 'syllables' => 3, 'was_used' => false, 'created_at' => now()->toIso8601String()],
|
||||||
|
]),
|
||||||
|
}]);
|
||||||
|
|
||||||
|
Livewire::test(ManageSuggestionMisses::class)
|
||||||
|
->loadTable()
|
||||||
|
->callTableBulkAction('promote', [1], data: ['items_raw' => 'ev charging cost', 'lang' => 'en', 'intent' => null])
|
||||||
|
->assertNotified('1 promoted, 0 skipped');
|
||||||
|
|
||||||
|
Http::assertSent(fn (Request $request) => $request->method() === 'DELETE'
|
||||||
|
&& str_contains($request->url(), '/admin/suggestion-misses/batch')
|
||||||
|
&& $request['miss_ids'] === [1]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a failed cleanup delete after a successful promote surfaces a separate warning notification', function () {
|
||||||
|
Http::fake(['bnfexpress.test/*' => fn (Request $request) => match (true) {
|
||||||
|
str_contains($request->url(), '/admin/suggestions/batch') => Http::response(['created' => 1, 'skipped' => 0, 'trie_rebuilt' => true]),
|
||||||
|
str_contains($request->url(), '/admin/suggestion-misses/batch') => Http::response(['detail' => 'boom'], 500),
|
||||||
|
default => Http::response([
|
||||||
|
['id' => 1, 'text_norm' => 'ev charging cost', 'lang' => 'en', 'syllables' => 3, 'was_used' => false, 'created_at' => now()->toIso8601String()],
|
||||||
|
]),
|
||||||
|
}]);
|
||||||
|
|
||||||
|
// assertNotified() pulls (and clears) every queued notification on its
|
||||||
|
// first call, so only one chained call can see anything — check the
|
||||||
|
// warning here; the success title is covered by the other promote tests.
|
||||||
|
Livewire::test(ManageSuggestionMisses::class)
|
||||||
|
->loadTable()
|
||||||
|
->callTableBulkAction('promote', [1], data: ['items_raw' => 'ev charging cost', 'lang' => 'en', 'intent' => null])
|
||||||
|
->assertNotified('Promoted, but failed to remove the original misses');
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -275,6 +275,15 @@ class BnfexpressAdminClient
|
|||||||
], fn (mixed $value): bool => $value !== null));
|
], fn (mixed $value): bool => $value !== null));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @param list<int|string> $missIds
|
||||||
|
* @return array{deleted: int, skipped: int}
|
||||||
|
*/
|
||||||
|
public function batchDeleteSuggestionMisses(array $missIds): array
|
||||||
|
{
|
||||||
|
return $this->request('DELETE', '/admin/suggestion-misses/batch', body: ['miss_ids' => $missIds]);
|
||||||
|
}
|
||||||
|
|
||||||
// --- Suggestion sync/embeddings ------------------------------------------
|
// --- Suggestion sync/embeddings ------------------------------------------
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user