Fix Queue settings save broken by nested industry reinstall form.
Deploy Ladill Queue / deploy (push) Successful in 2m5s
Deploy Ladill Queue / deploy (push) Successful in 2m5s
Browsers abort the outer Save settings form when a Reinstall package form is nested inside it. Move reinstall outside the main form and harden update validation so package failures cannot block saving. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -64,9 +64,9 @@ class SettingsController extends Controller
|
|||||||
|
|
||||||
$validated = $request->validate([
|
$validated = $request->validate([
|
||||||
'name' => ['required', 'string', 'max:255'],
|
'name' => ['required', 'string', 'max:255'],
|
||||||
'timezone' => ['required', 'timezone'],
|
'timezone' => ['required', 'string', 'timezone:all'],
|
||||||
'appointment_mode' => ['required', 'string', 'in:'.implode(',', array_keys(config('qms.appointment_modes')))],
|
'appointment_mode' => ['required', 'string', 'in:'.implode(',', array_keys(config('qms.appointment_modes')))],
|
||||||
'industry' => ['nullable', 'string', 'in:'.implode(',', array_keys(config('industry_packages.packages', [])))],
|
'industry' => ['nullable', 'string', 'max:64'],
|
||||||
'notifications_enabled' => ['boolean'],
|
'notifications_enabled' => ['boolean'],
|
||||||
'care_integration_enabled' => ['nullable', 'boolean'],
|
'care_integration_enabled' => ['nullable', 'boolean'],
|
||||||
'frontdesk_integration_enabled' => ['nullable', 'boolean'],
|
'frontdesk_integration_enabled' => ['nullable', 'boolean'],
|
||||||
@@ -74,10 +74,17 @@ class SettingsController extends Controller
|
|||||||
'remove_logo' => ['nullable', 'boolean'],
|
'remove_logo' => ['nullable', 'boolean'],
|
||||||
]);
|
]);
|
||||||
|
|
||||||
|
$registry = app(IndustryPackageRegistry::class);
|
||||||
$settings = $organization->settings ?? [];
|
$settings = $organization->settings ?? [];
|
||||||
|
$industry = isset($validated['industry']) && $validated['industry'] !== ''
|
||||||
|
? $registry->resolveKey($validated['industry'])
|
||||||
|
: ($settings['industry'] ?? null);
|
||||||
|
|
||||||
$settings['appointment_mode'] = $validated['appointment_mode'];
|
$settings['appointment_mode'] = $validated['appointment_mode'];
|
||||||
$previousIndustry = $settings['industry'] ?? null;
|
$previousIndustry = $settings['industry'] ?? null;
|
||||||
$settings['industry'] = $validated['industry'] ?? $settings['industry'] ?? null;
|
if ($industry !== null) {
|
||||||
|
$settings['industry'] = $industry;
|
||||||
|
}
|
||||||
$settings['notifications_enabled'] = $request->boolean('notifications_enabled', true);
|
$settings['notifications_enabled'] = $request->boolean('notifications_enabled', true);
|
||||||
$integrations = $settings['integrations'] ?? [];
|
$integrations = $settings['integrations'] ?? [];
|
||||||
if ($request->has('care_integration_enabled')) {
|
if ($request->has('care_integration_enabled')) {
|
||||||
@@ -108,7 +115,8 @@ class SettingsController extends Controller
|
|||||||
|
|
||||||
AuditLogger::record($owner, 'organization.updated', $organization->id, $owner, \App\Models\Organization::class, $organization->id);
|
AuditLogger::record($owner, 'organization.updated', $organization->id, $owner, \App\Models\Organization::class, $organization->id);
|
||||||
|
|
||||||
$industryChanged = $settings['industry'] && $settings['industry'] !== $previousIndustry;
|
$industryChanged = ($settings['industry'] ?? null) && ($settings['industry'] ?? null) !== $previousIndustry;
|
||||||
|
$packageNote = '';
|
||||||
if ($industryChanged) {
|
if ($industryChanged) {
|
||||||
$branch = Branch::query()
|
$branch = Branch::query()
|
||||||
->where('organization_id', $organization->id)
|
->where('organization_id', $organization->id)
|
||||||
@@ -116,15 +124,19 @@ class SettingsController extends Controller
|
|||||||
->orderBy('id')
|
->orderBy('id')
|
||||||
->first();
|
->first();
|
||||||
if ($branch) {
|
if ($branch) {
|
||||||
app(IndustryPackageInstaller::class)->install($organization->fresh(), $branch, $settings['industry'], [
|
try {
|
||||||
'actor_ref' => $owner,
|
app(IndustryPackageInstaller::class)->install($organization->fresh(), $branch, $settings['industry'], [
|
||||||
]);
|
'actor_ref' => $owner,
|
||||||
|
]);
|
||||||
|
$packageNote = ' Industry package re-provisioned.';
|
||||||
|
} catch (\Throwable $e) {
|
||||||
|
report($e);
|
||||||
|
$packageNote = ' Settings saved, but industry package re-provision failed — try Reinstall package.';
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return back()->with('success', $industryChanged
|
return back()->with('success', 'Settings saved.'.$packageNote);
|
||||||
? 'Settings saved. Industry package re-provisioned.'
|
|
||||||
: 'Settings saved.');
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public function reinstallPackage(Request $request, IndustryPackageInstaller $installer): RedirectResponse
|
public function reinstallPackage(Request $request, IndustryPackageInstaller $installer): RedirectResponse
|
||||||
|
|||||||
@@ -24,6 +24,17 @@
|
|||||||
<form method="POST" action="{{ route('qms.settings.update') }}" enctype="multipart/form-data" class="space-y-6">
|
<form method="POST" action="{{ route('qms.settings.update') }}" enctype="multipart/form-data" class="space-y-6">
|
||||||
@csrf @method('PUT')
|
@csrf @method('PUT')
|
||||||
|
|
||||||
|
@if ($errors->any())
|
||||||
|
<div class="rounded-lg border border-red-200 bg-red-50 px-4 py-3 text-sm text-red-800">
|
||||||
|
<p class="font-medium">Could not save settings:</p>
|
||||||
|
<ul class="mt-1 list-disc pl-5">
|
||||||
|
@foreach ($errors->all() as $error)
|
||||||
|
<li>{{ $error }}</li>
|
||||||
|
@endforeach
|
||||||
|
</ul>
|
||||||
|
</div>
|
||||||
|
@endif
|
||||||
|
|
||||||
<x-settings.card title="Organization" description="Name, timezone, appointment mode, and notification preferences.">
|
<x-settings.card title="Organization" description="Name, timezone, appointment mode, and notification preferences.">
|
||||||
<div class="space-y-4">
|
<div class="space-y-4">
|
||||||
<div>
|
<div>
|
||||||
@@ -121,22 +132,6 @@
|
|||||||
</x-settings.card>
|
</x-settings.card>
|
||||||
@endif
|
@endif
|
||||||
|
|
||||||
@if ($canManage && $activePackage)
|
|
||||||
<x-settings.card title="Industry template" description="Re-sync departments, stages, service points, workflow, and announcement copy from the selected industry package. Existing tickets are kept.">
|
|
||||||
<div class="flex flex-col gap-3 sm:flex-row sm:items-center sm:justify-between">
|
|
||||||
<p class="text-sm text-slate-600">
|
|
||||||
{{ $activePackage->label() }} uses
|
|
||||||
<span class="font-medium text-slate-900">{{ count($activePackage->stages()) }}</span> workflow stages and
|
|
||||||
industry-specific terminology ({{ $activePackage->terminology()['ticket'] ?? 'Ticket' }}).
|
|
||||||
</p>
|
|
||||||
<form method="POST" action="{{ route('qms.settings.package.reinstall') }}">
|
|
||||||
@csrf
|
|
||||||
<button type="submit" class="btn-secondary shrink-0 text-sm">Reinstall package</button>
|
|
||||||
</form>
|
|
||||||
</div>
|
|
||||||
</x-settings.card>
|
|
||||||
@endif
|
|
||||||
|
|
||||||
<x-settings.card title="Integrations" description="Allow Ladill Care and Frontdesk to manage queues in-app via the Queue API. Both apps must also enable integration in their settings.">
|
<x-settings.card title="Integrations" description="Allow Ladill Care and Frontdesk to manage queues in-app via the Queue API. Both apps must also enable integration in their settings.">
|
||||||
<div class="space-y-2">
|
<div class="space-y-2">
|
||||||
<label class="flex items-center gap-2 text-sm">
|
<label class="flex items-center gap-2 text-sm">
|
||||||
@@ -156,5 +151,24 @@
|
|||||||
</div>
|
</div>
|
||||||
@endif
|
@endif
|
||||||
</form>
|
</form>
|
||||||
|
|
||||||
|
{{-- Separate form: nested forms break Save settings in browsers --}}
|
||||||
|
@if ($canManage && $activePackage)
|
||||||
|
<div class="mt-6">
|
||||||
|
<x-settings.card title="Industry template" description="Re-sync departments, stages, service points, workflow, and announcement copy from the selected industry package. Existing tickets are kept.">
|
||||||
|
<div class="flex flex-col gap-3 sm:flex-row sm:items-center sm:justify-between">
|
||||||
|
<p class="text-sm text-slate-600">
|
||||||
|
{{ $activePackage->label() }} uses
|
||||||
|
<span class="font-medium text-slate-900">{{ count($activePackage->stages()) }}</span> workflow stages and
|
||||||
|
industry-specific terminology ({{ $activePackage->terminology()['ticket'] ?? 'Ticket' }}).
|
||||||
|
</p>
|
||||||
|
<form method="POST" action="{{ route('qms.settings.package.reinstall') }}">
|
||||||
|
@csrf
|
||||||
|
<button type="submit" class="btn-secondary shrink-0 text-sm">Reinstall package</button>
|
||||||
|
</form>
|
||||||
|
</div>
|
||||||
|
</x-settings.card>
|
||||||
|
</div>
|
||||||
|
@endif
|
||||||
</x-settings.page>
|
</x-settings.page>
|
||||||
</x-app-layout>
|
</x-app-layout>
|
||||||
|
|||||||
@@ -50,6 +50,45 @@ class SettingsBranchesTeamTest extends TestCase
|
|||||||
->assertSee(route('qms.members.index', absolute: false));
|
->assertSee(route('qms.members.index', absolute: false));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function test_org_admin_can_save_settings(): void
|
||||||
|
{
|
||||||
|
$this->actingAs($this->owner)
|
||||||
|
->put(route('qms.settings.update'), [
|
||||||
|
'name' => 'Settings Org Updated',
|
||||||
|
'timezone' => 'Africa/Accra',
|
||||||
|
'appointment_mode' => 'walk_in_only',
|
||||||
|
'industry' => 'retail',
|
||||||
|
'notifications_enabled' => '1',
|
||||||
|
'care_integration_enabled' => '1',
|
||||||
|
])
|
||||||
|
->assertRedirect()
|
||||||
|
->assertSessionHas('success');
|
||||||
|
|
||||||
|
$this->organization->refresh();
|
||||||
|
$this->assertSame('Settings Org Updated', $this->organization->name);
|
||||||
|
$this->assertSame('Africa/Accra', $this->organization->timezone);
|
||||||
|
$this->assertSame('walk_in_only', data_get($this->organization->settings, 'appointment_mode'));
|
||||||
|
$this->assertTrue((bool) data_get($this->organization->settings, 'integrations.care_enabled'));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function test_settings_page_does_not_nest_reinstall_form(): void
|
||||||
|
{
|
||||||
|
$html = $this->actingAs($this->owner)
|
||||||
|
->get(route('qms.settings.edit'))
|
||||||
|
->assertOk()
|
||||||
|
->getContent();
|
||||||
|
|
||||||
|
$this->assertSame(1, substr_count($html, 'action="'.route('qms.settings.update').'"'));
|
||||||
|
$this->assertSame(1, substr_count($html, 'action="'.route('qms.settings.package.reinstall').'"'));
|
||||||
|
// Reinstall form must appear after the settings form closes.
|
||||||
|
$updatePos = strpos($html, 'action="'.route('qms.settings.update').'"');
|
||||||
|
$closePos = strpos($html, '</form>', $updatePos);
|
||||||
|
$reinstallPos = strpos($html, 'action="'.route('qms.settings.package.reinstall').'"');
|
||||||
|
$this->assertNotFalse($closePos);
|
||||||
|
$this->assertNotFalse($reinstallPos);
|
||||||
|
$this->assertGreaterThan($closePos, $reinstallPos);
|
||||||
|
}
|
||||||
|
|
||||||
public function test_free_plan_branches_shows_upgrade_screen(): void
|
public function test_free_plan_branches_shows_upgrade_screen(): void
|
||||||
{
|
{
|
||||||
$this->actingAs($this->owner)
|
$this->actingAs($this->owner)
|
||||||
|
|||||||
Reference in New Issue
Block a user