diff --git a/app/Http/Controllers/Qms/SettingsController.php b/app/Http/Controllers/Qms/SettingsController.php index a1158eb..6dbe493 100644 --- a/app/Http/Controllers/Qms/SettingsController.php +++ b/app/Http/Controllers/Qms/SettingsController.php @@ -64,9 +64,9 @@ class SettingsController extends Controller $validated = $request->validate([ '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')))], - 'industry' => ['nullable', 'string', 'in:'.implode(',', array_keys(config('industry_packages.packages', [])))], + 'industry' => ['nullable', 'string', 'max:64'], 'notifications_enabled' => ['boolean'], 'care_integration_enabled' => ['nullable', 'boolean'], 'frontdesk_integration_enabled' => ['nullable', 'boolean'], @@ -74,10 +74,17 @@ class SettingsController extends Controller 'remove_logo' => ['nullable', 'boolean'], ]); + $registry = app(IndustryPackageRegistry::class); $settings = $organization->settings ?? []; + $industry = isset($validated['industry']) && $validated['industry'] !== '' + ? $registry->resolveKey($validated['industry']) + : ($settings['industry'] ?? null); + $settings['appointment_mode'] = $validated['appointment_mode']; $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); $integrations = $settings['integrations'] ?? []; 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); - $industryChanged = $settings['industry'] && $settings['industry'] !== $previousIndustry; + $industryChanged = ($settings['industry'] ?? null) && ($settings['industry'] ?? null) !== $previousIndustry; + $packageNote = ''; if ($industryChanged) { $branch = Branch::query() ->where('organization_id', $organization->id) @@ -116,15 +124,19 @@ class SettingsController extends Controller ->orderBy('id') ->first(); if ($branch) { - app(IndustryPackageInstaller::class)->install($organization->fresh(), $branch, $settings['industry'], [ - 'actor_ref' => $owner, - ]); + try { + 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 - ? 'Settings saved. Industry package re-provisioned.' - : 'Settings saved.'); + return back()->with('success', 'Settings saved.'.$packageNote); } public function reinstallPackage(Request $request, IndustryPackageInstaller $installer): RedirectResponse diff --git a/resources/views/qms/settings/edit.blade.php b/resources/views/qms/settings/edit.blade.php index e4e732f..d9c77d0 100644 --- a/resources/views/qms/settings/edit.blade.php +++ b/resources/views/qms/settings/edit.blade.php @@ -24,6 +24,17 @@
@csrf @method('PUT') + @if ($errors->any()) +
+

Could not save settings:

+ +
+ @endif +
@@ -121,22 +132,6 @@ @endif - @if ($canManage && $activePackage) - -
-

- {{ $activePackage->label() }} uses - {{ count($activePackage->stages()) }} workflow stages and - industry-specific terminology ({{ $activePackage->terminology()['ticket'] ?? 'Ticket' }}). -

- - @csrf - - -
-
- @endif -
@endif + + {{-- Separate form: nested forms break Save settings in browsers --}} + @if ($canManage && $activePackage) +
+ +
+

+ {{ $activePackage->label() }} uses + {{ count($activePackage->stages()) }} workflow stages and + industry-specific terminology ({{ $activePackage->terminology()['ticket'] ?? 'Ticket' }}). +

+
+ @csrf + +
+
+
+
+ @endif diff --git a/tests/Feature/SettingsBranchesTeamTest.php b/tests/Feature/SettingsBranchesTeamTest.php index 807923d..56a1db0 100644 --- a/tests/Feature/SettingsBranchesTeamTest.php +++ b/tests/Feature/SettingsBranchesTeamTest.php @@ -50,6 +50,45 @@ class SettingsBranchesTeamTest extends TestCase ->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, '', $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 { $this->actingAs($this->owner)