Compare commits

..

2 Commits

Author SHA1 Message Date
Naser Mansour 6ae226f5f2 Fix AdminNotificationService crashing on string throttle_minutes config
env('ERROR_NOTIFICATION_THROTTLE_MINUTES', 15) returns the literal string
"15" from production's .env (env() doesn't cast numeric strings), which
Carbon 3.11's addMinutes() rejects with a TypeError. That crash happened
before the throttle-dedup cache write, so it silently masked the *original*
exception on every notify-worthy error, indefinitely (discovered while
investigating a missing workshop-registration email — the real SMTP error
was hidden behind this crash). Cast to (int).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-28 00:45:27 +03:00
Naser Mansour ac22a0b982 Add missing street name to footer address
Client reported the site's address was missing "Jaffa Street" — added to
both Arabic and English footer strings (lang/ar/footer.php, lang/en/footer.php).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-28 00:34:07 +03:00
8 changed files with 191 additions and 3 deletions
+1 -1
View File
@@ -23,7 +23,7 @@ class AdminNotificationService
} }
$errorKey = $this->getErrorKey($exception); $errorKey = $this->getErrorKey($exception);
$throttleMinutes = config('libra.notifications.throttle_minutes', 15); $throttleMinutes = (int) config('libra.notifications.throttle_minutes', 15);
if (Cache::has($errorKey)) { if (Cache::has($errorKey)) {
return; return;
+61
View File
@@ -0,0 +1,61 @@
---
id: 22
title: 22 Fix Footer Address - Add Missing Street Name
status: Done
date: 2026-09-28
parent: null
---
## Description
Client reported the site's footer address is missing the street name. Naser confirmed the correct
full address (his own words, pasted): "شارع يافا - بجانب الشني / العمارة الأمريكية - الطابق
الرابع" (Jaffa Street - next to Al-Shini / American Building - 4th Floor). Pure content/copy
change, no logic.
**File to modify:** `lang/ar/footer.php`
Current:
```php
'address' => 'العمارة الأمريكية - بجانب الشني - الطابق الرابع',
```
Change to:
```php
'address' => 'شارع يافا - بجانب الشني - العمارة الأمريكية - الطابق الرابع',
```
**File to modify:** `lang/en/footer.php`
Current:
```php
'address' => 'American Building - Next to Al-Shini - 4th Floor',
```
Change to:
```php
'address' => 'Jaffa Street - Next to Al-Shini - American Building - 4th Floor',
```
No other key in either file changes. This string is rendered in `resources/views/components/footer.blade.php`
line 24 via `{{ __('footer.address') }}` — no Blade file needs editing, only the two language
files above.
## Acceptance Criteria
- [x] `lang/ar/footer.php`'s `address` value equals exactly
`شارع يافا - بجانب الشني - العمارة الأمريكية - الطابق الرابع`
- [x] `lang/en/footer.php`'s `address` value equals exactly
`Jaffa Street - Next to Al-Shini - American Building - 4th Floor`
- [x] No other key in either file changed (verify: `git diff lang/ar/footer.php lang/en/footer.php`
shows exactly one changed line per file)
- [x] Visiting the home page (`/`) still returns HTTP 200 and its rendered HTML contains the new
Arabic address string (verify with a feature test or an `assertSee` against the existing
home-page test suite if one covers the footer; if none does, a simple new assertion is fine
— this is a content check, not new behavior, so TDD's RED/GREEN cycle doesn't apply the
normal way: writing an assertion that currently fails because the string doesn't match yet
counts as the "RED" step here)
## Sub-tasks
None.
+93
View File
@@ -0,0 +1,93 @@
---
id: 23
title: 23 Fix AdminNotificationService Carbon TypeError Masking Exceptions
status: Done
date: 2026-09-28
parent: null
---
## Description
Investigating a client report of a missing workshop-registration email led to discovering a real
bug: `App\Services\AdminNotificationService::notifyError()` throws its own `TypeError` whenever
it runs, which **masks the original exception** it was trying to report and prevents the admin
notification from ever being sent. Root cause: `config('libra.notifications.throttle_minutes', 15)`
resolves to `env('ERROR_NOTIFICATION_THROTTLE_MINUTES', 15)` (see `config/libra.php` line 14).
Production's `.env` has `ERROR_NOTIFICATION_THROTTLE_MINUTES=15` set — Laravel's `env()` helper
does not cast a numeric-looking string to an int, so this returns the **string** `"15"`, not the
int `15`. That string is then passed to `now()->addMinutes($throttleMinutes)` in
`AdminNotificationService::notifyError()` (line 32), and Carbon 3.11 (this project's installed
version, per `composer.lock`) requires `addMinutes()`'s argument to be `int|float` — passing a
string throws `TypeError: Carbon\Carbon::rawAddUnit(): Argument #3 ($value) must be of type
int|float, string given`.
This is a live production bug, confirmed via `storage/logs/laravel.log` on h1
(`/home/libra/public_html/storage/logs/laravel.log`, entries timestamped `2026-09-27 21:35:26`
and `21:40:31`) — every exception that reaches `Handler->report()` and is judged
notify-worthy by `AdminNotificationService::shouldNotifyAdmin()` (e.g. any
`Symfony\Component\Mailer\Exception\TransportExceptionInterface`, or a critical
`QueryException`) currently crashes this method instead of sending the notification, and the
crash happens **before** `Cache::put()` on line 32 ever executes, so the throttle-dedup cache key
is never written — meaning this crash also can't self-heal or throttle, it fires on every single
qualifying exception, indefinitely, until fixed.
**File to modify:** `app/Services/AdminNotificationService.php`
Current (lines 25-32):
```php
$errorKey = $this->getErrorKey($exception);
$throttleMinutes = config('libra.notifications.throttle_minutes', 15);
if (Cache::has($errorKey)) {
return;
}
Cache::put($errorKey, true, now()->addMinutes($throttleMinutes));
```
Change to cast the config value to an int before use:
```php
$errorKey = $this->getErrorKey($exception);
$throttleMinutes = (int) config('libra.notifications.throttle_minutes', 15);
if (Cache::has($errorKey)) {
return;
}
Cache::put($errorKey, true, now()->addMinutes($throttleMinutes));
```
This is the only line that changes. No other method, no other file.
## Acceptance Criteria
- [x] `app/Services/AdminNotificationService.php`'s `$throttleMinutes` assignment casts the config
value with `(int)`
- [x] A test constructs `AdminNotificationService` and calls `notifyError()` with a throwable that
makes `shouldNotifyAdmin()` return true (e.g. a mock/stub implementing
`Symfony\Component\Mailer\Exception\TransportExceptionInterface`, or simply an instance of a
class implementing `App\Contracts\ShouldNotifyAdmin`), with
`config(['libra.notifications.throttle_minutes' => '15'])` explicitly set as a **string** in
the test (reproducing production's actual condition — `env()` never returns a string this
way in tests, so it must be set explicitly to catch this class of bug) — the call completes
without throwing, and a mail notification is sent (assert via `Notification::fake()` /
`Mail::fake()` as appropriate for however `SystemErrorNotification` delivers)
- [x] The same test, run against the **old** code (temporarily, to confirm the test actually
catches the bug) throws a `TypeError` — state in the completion report that this was
confirmed, per this task's RED step
## QA Verification (2026-09-28)
All 3 acceptance criteria verified directly:
1. Confirmed `(int)` cast present at `app/Services/AdminNotificationService.php:26`.
2. Ran `tests/Unit/Services/AdminNotificationServiceTest.php`'s new test
(`does not throw and still sends notification when throttle_minutes config is a string`) —
passed.
3. Temporarily removed the `(int)` cast, re-ran the same test — reproduced the exact TypeError
(`Carbon\Carbon::rawAddUnit(): Argument #3 ($value) must be of type int|float, string given`),
confirming RED. Restored the `(int)` cast. Full suite re-run: 17/17 passed. `git diff` confirms
only the single intended line differs from the pre-fix version.
## Sub-tasks
None.
+2
View File
@@ -133,3 +133,5 @@
| 19 | 19 Nablus Workshop Form - Remove Logo, Shrink Flyer Further | Done | | 19 | 19 Nablus Workshop Form - Remove Logo, Shrink Flyer Further | Done |
| 20 | 20 Nablus Workshop Form - Match Site Background and Card Styling | Done | | 20 | 20 Nablus Workshop Form - Match Site Background and Card Styling | Done |
| 21 | 21 Nablus Workshop Form - Flyer Size Adjustment | Done | | 21 | 21 Nablus Workshop Form - Flyer Size Adjustment | Done |
| 22 | 22 Fix Footer Address - Add Missing Street Name | Done |
| 23 | 23 Fix AdminNotificationService Carbon TypeError Masking Exceptions | Done |
+1 -1
View File
@@ -3,7 +3,7 @@
return [ return [
'description' => 'خدمات قانونية مهنية بنزاهة وتميز.', 'description' => 'خدمات قانونية مهنية بنزاهة وتميز.',
'contact' => 'تواصل معنا', 'contact' => 'تواصل معنا',
'address' => 'العمارة الأمريكية - بجانب الشني - الطابق الرابع', 'address' => 'شارع يافا - بجانب الشني - العمارة الأمريكية - الطابق الرابع',
'phone' => '+970 599 353 502', 'phone' => '+970 599 353 502',
'email' => 'info@libra.adv.ps', 'email' => 'info@libra.adv.ps',
'legal' => 'قانوني', 'legal' => 'قانوني',
+1 -1
View File
@@ -3,7 +3,7 @@
return [ return [
'description' => 'Professional legal services with integrity and excellence.', 'description' => 'Professional legal services with integrity and excellence.',
'contact' => 'Contact Us', 'contact' => 'Contact Us',
'address' => 'American Building - Next to Al-Shini - 4th Floor', 'address' => 'Jaffa Street - Next to Al-Shini - American Building - 4th Floor',
'phone' => '+970 599 353 502', 'phone' => '+970 599 353 502',
'email' => 'info@libra.adv.ps', 'email' => 'info@libra.adv.ps',
'legal' => 'Legal', 'legal' => 'Legal',
+9
View File
@@ -556,3 +556,12 @@ test('home page displays post publication date', function () {
->assertOk() ->assertOk()
->assertSee('15 January 2026'); ->assertSee('15 January 2026');
}); });
// Footer Section Tests
test('home page displays footer address with street name in Arabic', function () {
$this->withSession(['locale' => 'ar'])
->get('/')
->assertOk()
->assertSee('شارع يافا - بجانب الشني - العمارة الأمريكية - الطابق الرابع');
});
@@ -184,4 +184,27 @@ describe('notifyError', function () {
Notification::assertSentOnDemandTimes(SystemErrorNotification::class, 2); Notification::assertSentOnDemandTimes(SystemErrorNotification::class, 2);
}); });
it('does not throw and still sends notification when throttle_minutes config is a string', function () {
// Reproduces a live production bug: env('ERROR_NOTIFICATION_THROTTLE_MINUTES', 15) returns
// the string "15" (env() never casts numeric-looking strings to int), which used to be
// passed straight into now()->addMinutes(), and Carbon 3.11 throws a TypeError for a
// string argument there. That TypeError masked the original exception being reported and
// notifyError() never actually sent the admin notification.
Notification::fake();
Cache::flush();
config(['libra.notifications.throttle_minutes' => '15']);
$service = new AdminNotificationService;
$exception = new TransportException('SMTP error');
$service->notifyError($exception);
Notification::assertSentOnDemand(
SystemErrorNotification::class,
function ($notification, $channels, $notifiable) {
return $notifiable->routes['mail'] === 'admin@test.com';
}
);
});
}); });