From 19c449ee20f727cf1cce11b71e5e6be292071eb2 Mon Sep 17 00:00:00 2001
From: denkfabrik-li <274324701+denkfabrik-li@users.noreply.github.com>
Date: Fri, 28 Aug 2026 03:25:32 +0200
Subject: [PATCH] Stop an editable-once checkbox locking before anybody ticks
 it

save() writes '0' for an unticked checkbox, and filled('0') is true in
Laravel -- so isLocked(), which asks whether anything is stored, locked
the field the first time the client saved the page it sits on, whatever
they had chosen. A box they never ticked could then never be ticked, and
the one edit the setting promises was spent on a decision they had not
made.

A text field left empty stores null and stays open. That asymmetry is the
bug: '0' is the absence of a decision, which is what null means for every
other type.

So a checkbox locks on a stored '1' and nothing else. Everything else is
unchanged, including the existing case of a client ticking the box and
then being unable to untick it.

Two tests: an unrelated save leaves the box open and the tick that follows
still lands and locks it; and an editable-once text field behaves exactly
as before. Without the fix the first goes red.
---
 .../Clients/ClientPortalCustomFields.php      | 20 ++++++-
 .../Clients/ClientPortalCustomFieldsTest.php  | 60 +++++++++++++++++++
 2 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/app/Modules/Clients/ClientPortalCustomFields.php b/app/Modules/Clients/ClientPortalCustomFields.php
index 4fc3af01..aab417c3 100644
--- a/app/Modules/Clients/ClientPortalCustomFields.php
+++ b/app/Modules/Clients/ClientPortalCustomFields.php
@@ -158,7 +158,23 @@ class ClientPortalCustomFields
      */
     private function isLocked(ClientCustomField $field, BaseCollection $values): bool
     {
-        return $field->client_editability === ClientFieldEditability::EditableOnce
-            && filled($values->get($field->id));
+        if ($field->client_editability !== ClientFieldEditability::EditableOnce) {
+            return false;
+        }
+
+        $stored = $values->get($field->id);
+
+        // A checkbox has a stored value from the first save onwards: an
+        // unticked box is written as '0', and filled('0') is true. Asking
+        // "is anything stored" therefore locked the field on the first save
+        // of the form it sits on, whatever the client had chosen — and a
+        // box they never ticked can then never be ticked. '0' is the
+        // absence of a decision, which is the state the other types express
+        // as null, so it is what an unlocked checkbox looks like.
+        if ($field->type === ClientCustomFieldType::Checkbox) {
+            return $stored === '1';
+        }
+
+        return filled($stored);
     }
 }
diff --git a/tests/Feature/Clients/ClientPortalCustomFieldsTest.php b/tests/Feature/Clients/ClientPortalCustomFieldsTest.php
index 871c6a5c..a995fd42 100644
--- a/tests/Feature/Clients/ClientPortalCustomFieldsTest.php
+++ b/tests/Feature/Clients/ClientPortalCustomFieldsTest.php
@@ -104,6 +104,66 @@ test('an editable_once field locks after the client sets it once', function () {
         ->toBe('1');
 });
 
+test('an editable_once checkbox is not locked by never having been ticked', function () {
+    // Saving the form writes '0' for an unticked box, and filled('0') is
+    // true — so the field locked itself on the first save of the page it
+    // sits on, before the client had decided anything. A text field left
+    // empty stores null and stays open, which is the behaviour this now
+    // matches.
+    $client = User::factory()->client()->create();
+
+    $field = ClientCustomField::query()->create([
+        'name' => 'newsletter', 'label' => 'Send me the newsletter', 'type' => 'checkbox',
+        'required' => false, 'client_editability' => 'editable_once',
+        'client_contexts' => ['account_edit'],
+    ]);
+
+    // A save that has nothing to do with the checkbox.
+    $this->actingAs($client)->patch('/settings/profile', [
+        'name' => 'Renamed', 'email' => $client->email,
+    ])->assertSessionDoesntHaveErrors();
+
+    $this->actingAs($client)->get('/settings/profile')->assertInertia(
+        fn (AssertableInertia $page) => $page->where('custom_fields.0.locked', false),
+    );
+
+    // And the one decision they are entitled to still lands.
+    $this->actingAs($client)->patch('/settings/profile', [
+        'name' => 'Renamed', 'email' => $client->email,
+        'custom_field_values' => [$field->id => '1'],
+    ])->assertSessionDoesntHaveErrors();
+
+    expect(ClientCustomFieldValue::query()
+        ->where('client_custom_field_id', $field->id)->where('user_id', $client->id)->value('value'))->toBe('1');
+
+    $this->actingAs($client)->get('/settings/profile')->assertInertia(
+        fn (AssertableInertia $page) => $page->where('custom_fields.0.locked', true),
+    );
+});
+
+test('an editable_once text field is unchanged by all this', function () {
+    $client = User::factory()->client()->create();
+
+    $field = ClientCustomField::query()->create([
+        'name' => 'vat', 'label' => 'VAT number', 'type' => 'text',
+        'required' => false, 'client_editability' => 'editable_once',
+        'client_contexts' => ['account_edit'],
+    ]);
+
+    $this->actingAs($client)->patch('/settings/profile', [
+        'name' => $client->name, 'email' => $client->email,
+        'custom_field_values' => [$field->id => 'ATU12345678'],
+    ])->assertSessionDoesntHaveErrors();
+
+    $this->actingAs($client)->patch('/settings/profile', [
+        'name' => $client->name, 'email' => $client->email,
+        'custom_field_values' => [$field->id => 'changed'],
+    ])->assertSessionDoesntHaveErrors();
+
+    expect(ClientCustomFieldValue::query()
+        ->where('client_custom_field_id', $field->id)->where('user_id', $client->id)->value('value'))->toBe('ATU12345678');
+});
+
 test('a required checkbox in a client context must actually be checked', function () {
     $client = User::factory()->client()->create();