diff --git a/VERSION b/VERSION index ee62e7a..6972043 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.3.40 +1.3.41 diff --git a/app/Livewire/Admin/EditCustomer.php b/app/Livewire/Admin/EditCustomer.php new file mode 100644 index 0000000..06e5b47 --- /dev/null +++ b/app/Livewire/Admin/EditCustomer.php @@ -0,0 +1,204 @@ +authorize('customers.manage'); + + $customer = Customer::query()->where('uuid', $uuid)->firstOrFail(); + + $this->uuid = $uuid; + $this->name = (string) $customer->name; + $this->contactName = (string) $customer->contact_name; + $this->email = (string) $customer->email; + $this->phone = (string) $customer->phone; + $this->customerType = (string) $customer->customer_type; + $this->vatId = (string) $customer->vat_id; + $this->billingAddress = (string) $customer->billing_address; + $this->locale = (string) $customer->locale; + $this->originalEmail = (string) $customer->email; + $this->originalVatId = (string) $customer->vat_id; + } + + public function save() + { + $this->authorize('customers.manage'); + + // Read again, never trusted from the hydrated property: everything below + // is decided against the STORED record. + $customer = Customer::query()->where('uuid', $this->uuid)->firstOrFail(); + + $data = $this->validate([ + 'name' => 'required|string|max:255', + 'contactName' => 'nullable|string|max:255', + 'email' => [ + 'required', 'email', 'max:255', + // Among customers, ignoring this one. Two customers on one + // address would make every lookup that matches by address — + // inbound mail, the portal user, the register — ambiguous. + Rule::unique('customers', 'email')->ignore($customer->id), + ], + 'phone' => 'nullable|string|max:64', + // Only one of the two real answers may be stored; an empty string + // leaves the record as it was. See below for why it is never + // cleared back to "nobody asked". + 'customerType' => ['nullable', Rule::in(Customer::TYPES)], + 'vatId' => 'nullable|string|max:64', + 'billingAddress' => 'nullable|string|max:2000', + 'locale' => ['nullable', Rule::in(self::LOCALES)], + ]); + + $email = strtolower(trim($data['email'])); + $emailChanged = $email !== strtolower($customer->email); + + // R21: an address belonging to an operator may never also belong to a + // customer. Checked here rather than left to a unique index, because the + // two identities live in different tables and no index spans them. + if ($emailChanged && Operator::query()->where('email', $email)->exists()) { + $this->addError('email', __('edit_customer.email_is_operator')); + + return null; + } + + // A `users` row can outlive or predate a matching customer — see + // Customer::emailTaken(). One belonging to somebody else must not be + // walked into. + if ($emailChanged && User::query()->where('email', $email)->whereKeyNot($customer->user_id ?? 0)->exists()) { + $this->addError('email', __('edit_customer.email_taken')); + + return null; + } + + DB::transaction(function () use ($customer, $data, $email, $emailChanged) { + $customer->update([ + 'name' => trim($data['name']), + 'contact_name' => trim((string) $data['contactName']) ?: null, + 'email' => $email, + 'phone' => trim((string) $data['phone']) ?: null, + // Never cleared back to "unrecorded". Once somebody has + // answered, the answer stands until it is replaced by the other + // one — a form saved with the field blank must not quietly undo + // a recorded consumer into an unknown, and that answer decides + // whether a withdrawal right exists. + 'customer_type' => $data['customerType'] ?: $customer->customer_type, + // The verification is bound to the VALUE (see + // Customer::hasVerifiedVatId), so a new number is unverified the + // moment it is stored. `vat_id_verified_value` is deliberately + // left alone: it is the record of what was checked, and clearing + // it would destroy the evidence rather than the claim. + 'vat_id' => trim((string) $data['vatId']) ?: null, + 'billing_address' => trim((string) $data['billingAddress']) ?: null, + 'locale' => $data['locale'] ?: null, + ]); + + if (! $emailChanged || $customer->user_id === null) { + return; + } + + // The portal login moves with it, or the customer is locked out of + // an account that no longer matches their record. And the new + // address is unconfirmed by definition — nobody has shown they can + // read it — so it goes back to unverified, which is what Laravel's + // own profile update does for the same reason. No mail is sent from + // here: the portal asks them to confirm on their next sign-in, and a + // mail an operator did not knowingly send is a surprise. + User::query()->whereKey($customer->user_id)->update([ + 'email' => $email, + 'email_verified_at' => null, + ]); + }); + + $this->dispatch('notify', message: $this->message($emailChanged, $data)); + + return $this->redirectRoute('admin.customer', ['uuid' => $this->uuid], navigate: true); + } + + /** + * One message, and the consequence if there is one. + * + * Both go through the same toast and the second would replace the first, so + * the sentence that matters has to be the only one. + * + * @param array $data + */ + private function message(bool $emailChanged, array $data): string + { + if ($emailChanged) { + return __('edit_customer.saved_email_changed'); + } + + if (trim((string) $data['vatId']) !== trim($this->originalVatId)) { + return __('edit_customer.saved_vat_unverified'); + } + + return __('edit_customer.saved'); + } + + public function render() + { + return view('livewire.admin.edit-customer', [ + 'types' => Customer::TYPES, + 'locales' => self::LOCALES, + ]); + } +} diff --git a/lang/de/customer_detail.php b/lang/de/customer_detail.php index bbdd9ec..a19ecf3 100644 --- a/lang/de/customer_detail.php +++ b/lang/de/customer_detail.php @@ -15,6 +15,7 @@ return [ // Stammdaten 'identity' => 'Kunde', + 'edit' => 'Bearbeiten', 'company' => 'Firma', 'contact' => 'Ansprechpartner', 'email' => 'E-Mail', diff --git a/lang/de/edit_customer.php b/lang/de/edit_customer.php new file mode 100644 index 0000000..3d45590 --- /dev/null +++ b/lang/de/edit_customer.php @@ -0,0 +1,38 @@ + 'Stammdaten bearbeiten', + 'subtitle' => 'Was der Kunde selbst im Kundenbereich ändern kann — und was Sie für ihn richtigstellen, wenn er anruft.', + + 'name' => 'Firma', + 'contact' => 'Ansprechpartner', + 'email' => 'E-Mail', + 'email_hint' => 'Das ist auch die Anmeldeadresse und der Schlüssel, über den eingehende E-Mails zugeordnet werden. Nach einer Änderung muss der Kunde die neue Adresse beim nächsten Anmelden bestätigen.', + 'phone' => 'Telefon', + + 'type' => 'Kundenart', + 'type_keep' => 'Unverändert lassen', + 'type_hint' => 'Entscheidet, ob ein Widerrufsrecht besteht. Eine einmal erfasste Antwort wird hier nie auf „nicht erfasst“ zurückgesetzt — nur durch die andere ersetzt.', + 'vat_id' => 'UID', + 'vat_hint' => 'Eine geänderte Nummer gilt als ungeprüft, bis sie erneut geprüft wurde. Was zuletzt geprüft wurde, bleibt als Nachweis erhalten.', + + 'address' => 'Rechnungsadresse', + 'locale' => 'Sprache', + 'locale_default' => 'Wie eingestellt', + 'locale_de' => 'Deutsch', + 'locale_en' => 'Englisch', + 'locale_hint' => 'In dieser Sprache bekommt der Kunde seine E-Mails.', + + 'save' => 'Speichern', + 'cancel' => 'Abbrechen', + + 'saved' => 'Stammdaten gespeichert.', + 'saved_email_changed' => 'Gespeichert. Die Anmeldeadresse wurde mitgeändert; der Kunde muss sie beim nächsten Anmelden bestätigen.', + 'saved_vat_unverified' => 'Gespeichert. Die geänderte UID gilt als ungeprüft, bis sie erneut geprüft wurde.', + + 'email_is_operator' => 'Diese Adresse gehört zu einem Betreiber-Zugang. Kunde und Betreiber dürfen sich keine Adresse teilen.', + 'email_taken' => 'Diese Adresse ist schon einem anderen Zugang zugeordnet.', +]; diff --git a/lang/en/customer_detail.php b/lang/en/customer_detail.php index e54d01d..26d93e8 100644 --- a/lang/en/customer_detail.php +++ b/lang/en/customer_detail.php @@ -15,6 +15,7 @@ return [ // Details 'identity' => 'Customer', + 'edit' => 'Edit', 'company' => 'Company', 'contact' => 'Contact', 'email' => 'Mail', diff --git a/lang/en/edit_customer.php b/lang/en/edit_customer.php new file mode 100644 index 0000000..9b81a10 --- /dev/null +++ b/lang/en/edit_customer.php @@ -0,0 +1,38 @@ + 'Edit customer details', + 'subtitle' => 'What the customer can change themselves in the portal — and what you put right for them when they call.', + + 'name' => 'Company', + 'contact' => 'Contact', + 'email' => 'Mail', + 'email_hint' => 'This is also the sign-in address and the key incoming mail is matched by. After a change the customer has to confirm the new address at their next sign-in.', + 'phone' => 'Phone', + + 'type' => 'Customer type', + 'type_keep' => 'Leave unchanged', + 'type_hint' => 'Decides whether a withdrawal right exists. An answer already on record is never reset to "not recorded" here — only replaced by the other one.', + 'vat_id' => 'VAT ID', + 'vat_hint' => 'A changed number counts as unverified until it has been checked again. What was last checked is kept as evidence.', + + 'address' => 'Billing address', + 'locale' => 'Language', + 'locale_default' => 'As configured', + 'locale_de' => 'German', + 'locale_en' => 'English', + 'locale_hint' => 'The language the customer receives mail in.', + + 'save' => 'Save', + 'cancel' => 'Cancel', + + 'saved' => 'Details saved.', + 'saved_email_changed' => 'Saved. The sign-in address moved with it; the customer has to confirm it at their next sign-in.', + 'saved_vat_unverified' => 'Saved. The changed VAT ID counts as unverified until it has been checked again.', + + 'email_is_operator' => 'That address belongs to an operator account. A customer and an operator must not share one.', + 'email_taken' => 'That address already belongs to another account.', +]; diff --git a/resources/views/livewire/admin/customer-detail.blade.php b/resources/views/livewire/admin/customer-detail.blade.php index d8b8dd0..6a3a9a9 100644 --- a/resources/views/livewire/admin/customer-detail.blade.php +++ b/resources/views/livewire/admin/customer-detail.blade.php @@ -52,8 +52,23 @@ {{-- ══ Stammdaten ═══════════════════════════════════════════════════════ --}} @if ($tab === 'overview')
-
-

{{ __('customer_detail.identity') }}

+ {{-- `editing` is local: fetching the modal is a round trip, and a + click with nothing to show for it reads as a click that missed. --}} +
+
+

{{ __('customer_detail.identity') }}

+ {{-- R20: editing happens in a modal, never in the row. --}} + + + + {{ __('customer_detail.edit') }} + +
@foreach ([ __('customer_detail.company') => $customer->name, diff --git a/resources/views/livewire/admin/edit-customer.blade.php b/resources/views/livewire/admin/edit-customer.blade.php new file mode 100644 index 0000000..c72f86b --- /dev/null +++ b/resources/views/livewire/admin/edit-customer.blade.php @@ -0,0 +1,75 @@ +
+

{{ __('edit_customer.title') }}

+

{{ __('edit_customer.subtitle') }}

+ +
+
+ + +
+ +
+
+ + {{-- Said before it is done, not after: the address is the portal + login and the key everything matching by address uses. --}} +

{{ __('edit_customer.email_hint') }}

+
+ +
+ +
+
+ + +

{{ __('edit_customer.type_hint') }}

+
+ +
+ +

{{ __('edit_customer.vat_hint') }}

+
+
+ +
+ + + @error('billingAddress')

{{ $message }}

@enderror +
+ +
+ + + {{-- What it actually decides: the language of the mail they get. --}} +

{{ __('edit_customer.locale_hint') }}

+
+ +
+ + {{ __('edit_customer.save') }} + + {{-- Livewire.dispatch, not Alpine's: the modal listens for a Livewire + event of that name, and the Alpine form silently does nothing. --}} + + {{ __('edit_customer.cancel') }} + +
+
+
diff --git a/tests/Feature/Admin/EditCustomerTest.php b/tests/Feature/Admin/EditCustomerTest.php new file mode 100644 index 0000000..ba29643 --- /dev/null +++ b/tests/Feature/Admin/EditCustomerTest.php @@ -0,0 +1,207 @@ +customer = Customer::factory()->create([ + 'name' => 'Beispiel GmbH', + 'contact_name' => 'Bea Berger', + 'email' => 'bea@beispiel.test', + 'phone' => '+43 1 111', + 'customer_type' => Customer::TYPE_BUSINESS, + 'vat_id' => 'ATU11111111', + 'vat_id_verified_at' => now(), + 'vat_id_verified_value' => 'ATU11111111', + 'billing_address' => "Alte Gasse 1\n1010 Wien", + ]); +}); + +/** The modal, mounted on this customer. */ +function editing(Customer $customer, string $role = 'Owner') +{ + return Livewire::actingAs(operator($role), 'operator') + ->test(EditCustomer::class, ['uuid' => $customer->uuid]); +} + +it('saves the details an operator corrected', function () { + editing($this->customer) + ->set('name', 'Beispiel GmbH & Co KG') + ->set('contactName', 'Bernd Berger') + ->set('phone', '+43 1 999') + ->set('billingAddress', "Neue Gasse 9\n1020 Wien") + ->set('locale', 'en') + ->call('save') + ->assertHasNoErrors(); + + $fresh = $this->customer->fresh(); + + expect($fresh->name)->toBe('Beispiel GmbH & Co KG') + ->and($fresh->contact_name)->toBe('Bernd Berger') + ->and($fresh->phone)->toBe('+43 1 999') + ->and($fresh->billing_address)->toContain('1020 Wien') + ->and($fresh->locale)->toBe('en'); +}); + +it('never resets a recorded customer type back to nobody asked', function () { + // That answer decides whether a withdrawal right exists. A form saved with + // the field untouched must not quietly undo it — and the field opens on + // "leave unchanged", so saving anything else would do exactly that. + editing($this->customer)->set('customerType', '')->call('save'); + + expect($this->customer->fresh()->customer_type)->toBe(Customer::TYPE_BUSINESS); +}); + +it('records the other answer when that is what was chosen', function () { + editing($this->customer)->set('customerType', Customer::TYPE_CONSUMER)->call('save'); + + expect($this->customer->fresh()->isConsumer())->toBeTrue(); +}); + +it('treats a changed VAT number as unverified, and keeps what was checked', function () { + // The verification is bound to the VALUE, so a new number is unverified the + // moment it is stored. The record of what WAS checked stays: clearing it + // would destroy the evidence rather than the claim. + editing($this->customer)->set('vatId', 'ATU22222222')->call('save'); + + $fresh = $this->customer->fresh(); + + expect($fresh->vat_id)->toBe('ATU22222222') + ->and($fresh->hasVerifiedVatId())->toBeFalse() + ->and($fresh->vat_id_verified_value)->toBe('ATU11111111'); +}); + +it('leaves a verification alone when the number was not touched', function () { + editing($this->customer)->set('phone', '+43 1 222')->call('save'); + + expect($this->customer->fresh()->hasVerifiedVatId())->toBeTrue(); +}); + +// ---- The address, which is also the login ---- + +it('moves the portal login with the address and asks for it to be confirmed', function () { + // Otherwise the customer is locked out of an account that no longer matches + // their record. And the new address is unconfirmed by definition — nobody + // has shown they can read it. + $user = User::factory()->create(['email' => 'bea@beispiel.test', 'email_verified_at' => now()]); + $this->customer->update(['user_id' => $user->id]); + + editing($this->customer->fresh())->set('email', 'neu@beispiel.test')->call('save')->assertHasNoErrors(); + + expect($this->customer->fresh()->email)->toBe('neu@beispiel.test') + ->and($user->fresh()->email)->toBe('neu@beispiel.test') + ->and($user->fresh()->email_verified_at)->toBeNull(); +}); + +it('refuses an address that belongs to an operator', function () { + // R21: a customer and an operator must never share an address. No index + // spans the two tables, so this is checked rather than left to the database. + $operator = operator('Support'); + + editing($this->customer) + ->set('email', $operator->email) + ->call('save') + ->assertHasErrors('email'); + + expect($this->customer->fresh()->email)->toBe('bea@beispiel.test'); +}); + +it('refuses an address that belongs to another customer', function () { + // Two customers on one address makes every lookup that matches by address — + // inbound mail, the portal user, the register — ambiguous. + Customer::factory()->create(['email' => 'schon@da.test']); + + editing($this->customer) + ->set('email', 'schon@da.test') + ->call('save') + ->assertHasErrors('email'); + + expect($this->customer->fresh()->email)->toBe('bea@beispiel.test'); +}); + +it('refuses an address that belongs to somebody else user account', function () { + User::factory()->create(['email' => 'fremd@example.test']); + + editing($this->customer) + ->set('email', 'fremd@example.test') + ->call('save') + ->assertHasErrors('email'); +}); + +it('lets a customer keep their own address while something else is corrected', function () { + // The unique rule has to ignore this record, or nothing else on the form can + // ever be saved. + editing($this->customer)->set('name', 'Anderer Name')->call('save')->assertHasNoErrors(); + + expect($this->customer->fresh()->name)->toBe('Anderer Name'); +}); + +it('lowercases the address, because that is how every lookup asks for it', function () { + editing($this->customer)->set('email', 'GROSS@Beispiel.test')->call('save'); + + expect($this->customer->fresh()->email)->toBe('gross@beispiel.test'); +}); + +// ---- The rules around it ---- + +it('refuses an empty company name rather than storing one', function () { + editing($this->customer)->set('name', '')->call('save')->assertHasErrors('name'); + + expect($this->customer->fresh()->name)->toBe('Beispiel GmbH'); +}); + +it('takes no language this application does not have', function () { + editing($this->customer)->set('locale', 'fr')->call('save')->assertHasErrors('locale'); +}); + +it('keeps the form to operators who may manage customers', function () { + Livewire::actingAs(operator('Read-only'), 'operator') + ->test(EditCustomer::class, ['uuid' => $this->customer->uuid]) + ->assertForbidden(); +}); + +it('is reached from the customer page through openModal, not a page method', function () { + // R20: an edit button that calls a method on the page component is the inline + // editor coming back under a new name. + $page = File::get(resource_path('views/livewire/admin/customer-detail.blade.php')); + + expect($page)->toContain("component: 'admin.edit-customer'"); + + Livewire::actingAs(operator('Owner'), 'operator') + ->test(CustomerDetail::class, ['uuid' => $this->customer->uuid]) + ->assertSee(__('customer_detail.edit')); +}); + +it('does not offer to change what an operator must not change here', function () { + // status has its own action with its own consequences for access; + // stripe_customer_id points at somebody else's payments if it is wrong; the + // brand fields are the customer's own. + // Comments stripped first. The class documents at length WHY each of these + // is absent, and reading that documentation as the offence is a trap this + // repository has fallen into three times — see the @vite, prefers-color-scheme + // and clupilot.cloud scans. + $code = implode('', array_map( + fn ($token) => is_array($token) + ? (in_array($token[0], [T_COMMENT, T_DOC_COMMENT], true) ? '' : $token[1]) + : $token, + token_get_all(File::get(app_path('Livewire/Admin/EditCustomer.php'))), + )); + + expect($code)->not->toContain("'status'") + ->not->toContain('stripe_customer_id') + ->not->toContain('brand_logo_path') + // And the fields it DOES write are the ones it is meant to. + ->toContain("'billing_address'") + ->toContain("'customer_type'"); +});