mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-25 04:42:01 +00:00
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.
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user