Commit 32e52a04 authored by Mahmoud Aglan's avatar Mahmoud Aglan

fix(expenses): let a phone photo of a receipt actually attach and display

Three things stood between the desk and an attached receipt:

- The picker on the create form listed extensions (.jpg,.png,…), which on
  iOS greys out the camera roll; and both components validated
  mimes:jpg,jpeg,png,pdf,webp, so a HEIC photo — the iPhone default —
  was refused after the picker had accepted it. Both now accept image/*
  plus heic/heif, and validate the same set.
- The attachment stream built Content-Disposition by hand with
  addslashes(). Receipt names here are Arabic, and an Arabic filename is
  not a legal header value: the inline preview came back broken and the
  download came back mangled. Symfony builds the header now, via
  Storage::response() for inline and download() for the save.
- HEIC is an image no browser but Safari paints, so it falls back to the
  file card instead of a broken <img> (attachmentIsViewableImage()).

Also: the create form advertised a 5MB ceiling while the real one is
config('uploads.max_kb') (150MB), and its uploaded-file chip called
getClientOriginalName() on whatever sat in the property.
Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent 54df3e88
...@@ -109,6 +109,17 @@ public function attachmentIsImage(): bool ...@@ -109,6 +109,17 @@ public function attachmentIsImage(): bool
return $this->hasAttachment() && str_starts_with((string) $this->attachment_mime, 'image/'); return $this->hasAttachment() && str_starts_with((string) $this->attachment_mime, 'image/');
} }
/**
* An image an <img> tag can actually paint. HEIC/HEIF is an image and is
* accepted on upload, but only Safari renders it — everywhere else the
* page showed a broken picture, so those fall back to the file card.
*/
public function attachmentIsViewableImage(): bool
{
return $this->attachmentIsImage()
&& ! in_array($this->attachment_mime, ['image/heic', 'image/heif', 'image/heic-sequence'], true);
}
public function attachmentIsPdf(): bool public function attachmentIsPdf(): bool
{ {
return $this->hasAttachment() && $this->attachment_mime === 'application/pdf'; return $this->hasAttachment() && $this->attachment_mime === 'application/pdf';
......
...@@ -24,6 +24,14 @@ public function download(Expense $expense): StreamedResponse ...@@ -24,6 +24,14 @@ public function download(Expense $expense): StreamedResponse
return $this->stream($expense, 'attachment'); return $this->stream($expense, 'attachment');
} }
/**
* The disposition header is built by Symfony, never by hand: receipt names
* here are Arabic more often than not, and `filename="فاتورة.jpg"` is not a
* legal header value — the browser got a mangled name, or nothing at all,
* and the inline preview on the expense page rendered a broken image.
* makeDisposition() writes the RFC 6266 `filename*` form with an ASCII
* fallback, which is what both cases need.
*/
private function stream(Expense $expense, string $disposition): StreamedResponse private function stream(Expense $expense, string $disposition): StreamedResponse
{ {
Gate::authorize('expenses.create'); Gate::authorize('expenses.create');
...@@ -37,10 +45,11 @@ private function stream(Expense $expense, string $disposition): StreamedResponse ...@@ -37,10 +45,11 @@ private function stream(Expense $expense, string $disposition): StreamedResponse
$filename = $expense->attachment_name ?: basename($expense->attachment_path); $filename = $expense->attachment_name ?: basename($expense->attachment_path);
return $disk->download($expense->attachment_path, $filename, [ $headers = ['Content-Type' => $expense->attachment_mime ?: 'application/octet-stream'];
'Content-Type' => $expense->attachment_mime ?: 'application/octet-stream',
'Content-Disposition' => $disposition . '; filename="' . addslashes($filename) . '"', return $disposition === 'attachment'
]); ? $disk->download($expense->attachment_path, $filename, $headers)
: $disk->response($expense->attachment_path, $filename, $headers, 'inline');
} }
/** /**
......
...@@ -45,7 +45,9 @@ public function rules(): array ...@@ -45,7 +45,9 @@ public function rules(): array
'receipt_reference' => 'nullable|string|max:100', 'receipt_reference' => 'nullable|string|max:100',
'expense_date' => 'required|date', 'expense_date' => 'required|date',
'notes' => 'nullable|string', 'notes' => 'nullable|string',
'attachment' => 'nullable|file|max:' . config('uploads.max_kb') . '|mimes:jpg,jpeg,png,pdf,webp', // heic/heif: an iPhone photo is HEIC by default, and refusing it
// after the picker accepted it reads as a broken upload.
'attachment' => 'nullable|file|max:' . config('uploads.max_kb') . '|mimes:jpg,jpeg,png,pdf,webp,heic,heif,gif,bmp',
]; ];
} }
...@@ -60,7 +62,7 @@ public function messages(): array ...@@ -60,7 +62,7 @@ public function messages(): array
'payment_method.required' => 'اختر طريقة الدفع', 'payment_method.required' => 'اختر طريقة الدفع',
'expense_date.required' => 'تاريخ المصروف مطلوب', 'expense_date.required' => 'تاريخ المصروف مطلوب',
'attachment.max' => __('حجم الملف لا يتجاوز :mb ميجابايت', ['mb' => intdiv((int) config('uploads.max_kb'), 1024)]), 'attachment.max' => __('حجم الملف لا يتجاوز :mb ميجابايت', ['mb' => intdiv((int) config('uploads.max_kb'), 1024)]),
'attachment.mimes' => 'الملف يجب أن يكون صورة (jpg, png, webp) أو PDF', 'attachment.mimes' => 'الملف يجب أن يكون صورة (jpg, png, webp, heic) أو PDF',
]; ];
} }
......
...@@ -59,7 +59,10 @@ private function expense(): Expense ...@@ -59,7 +59,10 @@ private function expense(): Expense
public function rules(): array public function rules(): array
{ {
return [ return [
'receipt' => 'required|file|max:' . config('uploads.max_kb') . '|mimes:jpg,jpeg,png,pdf,webp', // heic/heif are here because that is what an iPhone's camera roll
// hands over: the picker accepted the photo and validation then
// refused it, which read to the desk as "the upload is broken".
'receipt' => 'required|file|max:' . config('uploads.max_kb') . '|mimes:jpg,jpeg,png,pdf,webp,heic,heif,gif,bmp',
]; ];
} }
...@@ -68,7 +71,7 @@ public function messages(): array ...@@ -68,7 +71,7 @@ public function messages(): array
return [ return [
'receipt.required' => 'اختر ملف الإيصال أولاً', 'receipt.required' => 'اختر ملف الإيصال أولاً',
'receipt.max' => __('حجم الملف لا يتجاوز :mb ميجابايت', ['mb' => intdiv((int) config('uploads.max_kb'), 1024)]), 'receipt.max' => __('حجم الملف لا يتجاوز :mb ميجابايت', ['mb' => intdiv((int) config('uploads.max_kb'), 1024)]),
'receipt.mimes' => 'الملف يجب أن يكون صورة (jpg, png, webp) أو PDF', 'receipt.mimes' => 'الملف يجب أن يكون صورة (jpg, png, webp, heic) أو PDF',
]; ];
} }
......
...@@ -131,7 +131,7 @@ class="w-full rounded-lg border-gray-300 text-sm py-2.5"></textarea> ...@@ -131,7 +131,7 @@ class="w-full rounded-lg border-gray-300 text-sm py-2.5"></textarea>
{{-- Attachment --}} {{-- Attachment --}}
<div class="md:col-span-2"> <div class="md:col-span-2">
<label class="block text-sm text-gray-600 mb-1">{{ __('مرفق (إيصال / إثبات)') }}</label> <label class="block text-sm text-gray-600 mb-1">{{ __('مرفق (إيصال / إثبات)') }}</label>
@if($attachment) @if($attachment && method_exists($attachment, 'getClientOriginalName'))
<div class="flex items-center gap-3 p-3 bg-green-50 border border-green-200 rounded-lg mb-2"> <div class="flex items-center gap-3 p-3 bg-green-50 border border-green-200 rounded-lg mb-2">
<svg class="w-5 h-5 text-green-600 shrink-0" fill="none" stroke="currentColor" viewBox="0 0 24 24"> <svg class="w-5 h-5 text-green-600 shrink-0" fill="none" stroke="currentColor" viewBox="0 0 24 24">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z"/> <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M9 12l2 2 4-4m6 2a9 9 0 11-18 0 9 9 0 0118 0z"/>
...@@ -147,9 +147,11 @@ class="w-full rounded-lg border-gray-300 text-sm py-2.5"></textarea> ...@@ -147,9 +147,11 @@ class="w-full rounded-lg border-gray-300 text-sm py-2.5"></textarea>
<svg class="w-8 h-8 text-gray-400 mb-1" fill="none" stroke="currentColor" viewBox="0 0 24 24"> <svg class="w-8 h-8 text-gray-400 mb-1" fill="none" stroke="currentColor" viewBox="0 0 24 24">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="1.5" d="M4 16l4.586-4.586a2 2 0 012.828 0L16 16m-2-2l1.586-1.586a2 2 0 012.828 0L20 14m-6-6h.01M6 20h12a2 2 0 002-2V6a2 2 0 00-2-2H6a2 2 0 00-2 2v12a2 2 0 002 2z"/> <path stroke-linecap="round" stroke-linejoin="round" stroke-width="1.5" d="M4 16l4.586-4.586a2 2 0 012.828 0L16 16m-2-2l1.586-1.586a2 2 0 012.828 0L20 14m-6-6h.01M6 20h12a2 2 0 002-2V6a2 2 0 00-2-2H6a2 2 0 00-2 2v12a2 2 0 002 2z"/>
</svg> </svg>
<span class="text-xs text-gray-500">{{ __('اضغط لرفع صورة أو ملف PDF (حد أقصى 5MB)') }}</span> <span class="text-xs text-gray-500">{{ __('اضغط لرفع صورة أو ملف PDF (حد أقصى :mb ميجابايت)', ['mb' => intdiv((int) config('uploads.max_kb'), 1024)]) }}</span>
</div> </div>
<input type="file" wire:model="attachment" class="hidden" accept=".jpg,.jpeg,.png,.webp,.pdf"> {{-- image/* rather than an extension list: an extension list
greys out the camera roll on iOS and hides HEIC photos. --}}
<input type="file" wire:model="attachment" class="hidden" accept="image/*,.heic,.heif,application/pdf">
</label> </label>
<div wire:loading wire:target="attachment" class="mt-1 text-xs text-amber-600">{{ __('جارٍ رفع الملف...') }}</div> <div wire:loading wire:target="attachment" class="mt-1 text-xs text-amber-600">{{ __('جارٍ رفع الملف...') }}</div>
@error('attachment') <p class="text-red-500 text-xs mt-1">{{ $message }}</p> @enderror @error('attachment') <p class="text-red-500 text-xs mt-1">{{ $message }}</p> @enderror
......
...@@ -170,7 +170,7 @@ class="inline-flex items-center gap-2 px-4 py-2 border border-red-200 text-red-7 ...@@ -170,7 +170,7 @@ class="inline-flex items-center gap-2 px-4 py-2 border border-red-200 text-red-7
<div class="p-5"> <div class="p-5">
@if($expense->hasAttachment()) @if($expense->hasAttachment())
<div class="rounded-lg border border-gray-200 overflow-hidden bg-gray-50 mb-3"> <div class="rounded-lg border border-gray-200 overflow-hidden bg-gray-50 mb-3">
@if($expense->attachmentIsImage()) @if($expense->attachmentIsViewableImage())
<a href="{{ route('expenses.attachment', $expense) }}" target="_blank" rel="noopener"> <a href="{{ route('expenses.attachment', $expense) }}" target="_blank" rel="noopener">
<img src="{{ route('expenses.attachment', $expense) }}" <img src="{{ route('expenses.attachment', $expense) }}"
alt="{{ $expense->attachment_name }}" alt="{{ $expense->attachment_name }}"
...@@ -228,7 +228,7 @@ class="inline-flex items-center gap-1.5 px-3 py-1.5 text-xs text-red-600 rounded ...@@ -228,7 +228,7 @@ class="inline-flex items-center gap-1.5 px-3 py-1.5 text-xs text-red-600 rounded
<label class="block text-sm font-medium text-gray-700 mb-2"> <label class="block text-sm font-medium text-gray-700 mb-2">
{{ $expense->hasAttachment() ? __('استبدال المرفق') : __('إرفاق مستند') }} {{ $expense->hasAttachment() ? __('استبدال المرفق') : __('إرفاق مستند') }}
</label> </label>
<input type="file" wire:model="receipt" accept="image/*,application/pdf" <input type="file" wire:model="receipt" accept="image/*,.heic,.heif,application/pdf"
class="block w-full text-sm text-gray-600 file:me-3 file:py-2 file:px-4 file:rounded-lg file:border-0 file:text-sm file:bg-gray-100 file:text-gray-700 hover:file:bg-gray-200"> class="block w-full text-sm text-gray-600 file:me-3 file:py-2 file:px-4 file:rounded-lg file:border-0 file:text-sm file:bg-gray-100 file:text-gray-700 hover:file:bg-gray-200">
<div wire:loading wire:target="receipt" class="text-xs text-gray-400 mt-1">{{ __('جارٍ الرفع...') }}</div> <div wire:loading wire:target="receipt" class="text-xs text-gray-400 mt-1">{{ __('جارٍ الرفع...') }}</div>
@error('receipt') @error('receipt')
......
...@@ -93,6 +93,33 @@ public function test_an_expense_without_a_receipt_does_not_claim_one_was_uploade ...@@ -93,6 +93,33 @@ public function test_an_expense_without_a_receipt_does_not_claim_one_was_uploade
$this->assertNull($expense->attachment_uploaded_at); $this->assertNull($expense->attachment_uploaded_at);
} }
public function test_an_iphone_heic_receipt_is_stored_but_not_rendered_as_an_img(): void
{
$expense = $this->service()->recordExpense($this->expenseData([
'attachment_path' => 'expenses/receipts/photo.heic',
'attachment_name' => 'فاتورة الكهرباء.heic',
'attachment_disk' => 'local',
'attachment_mime' => 'image/heic',
'attachment_size' => 2048,
]), $this->actor);
$this->assertTrue($expense->attachmentIsImage());
$this->assertFalse($expense->attachmentIsViewableImage());
}
public function test_a_jpeg_receipt_is_rendered_inline(): void
{
$expense = $this->service()->recordExpense($this->expenseData([
'attachment_path' => 'expenses/receipts/photo.jpg',
'attachment_name' => 'فاتورة الكهرباء.jpg',
'attachment_disk' => 'local',
'attachment_mime' => 'image/jpeg',
'attachment_size' => 2048,
]), $this->actor);
$this->assertTrue($expense->attachmentIsViewableImage());
}
// ---- attaching to an expense already in the ledger -------------------- // ---- attaching to an expense already in the ledger --------------------
public function test_a_receipt_can_be_attached_to_an_expense_recorded_without_one(): void public function test_a_receipt_can_be_attached_to_an_expense_recorded_without_one(): void
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment