From 68a55510069d4b68514223763d2760ff07385023 Mon Sep 17 00:00:00 2001 From: Naser Mansour Date: Sat, 27 Dec 2025 01:09:03 +0200 Subject: [PATCH] complete story 4.5 with qa tests and applied pagintation fix --- app/Http/Middleware/EnsureUserIsClient.php | 19 ++ bootstrap/app.php | 1 + docs/qa/gates/4.5-client-timeline-view.yml | 49 ++++ .../stories/story-4.5-client-timeline-view.md | 107 ++++++++ lang/ar/client.php | 17 ++ lang/en/client.php | 17 ++ .../livewire/client/timelines/index.blade.php | 114 +++++++++ .../livewire/client/timelines/show.blade.php | 66 +++++ routes/web.php | 12 +- tests/Feature/Auth/AuthorizationTest.php | 4 +- tests/Feature/Client/TimelineViewTest.php | 228 ++++++++++++++++++ 11 files changed, 629 insertions(+), 5 deletions(-) create mode 100644 app/Http/Middleware/EnsureUserIsClient.php create mode 100644 docs/qa/gates/4.5-client-timeline-view.yml create mode 100644 lang/ar/client.php create mode 100644 lang/en/client.php create mode 100644 resources/views/livewire/client/timelines/index.blade.php create mode 100644 resources/views/livewire/client/timelines/show.blade.php create mode 100644 tests/Feature/Client/TimelineViewTest.php diff --git a/app/Http/Middleware/EnsureUserIsClient.php b/app/Http/Middleware/EnsureUserIsClient.php new file mode 100644 index 0000000..991edf4 --- /dev/null +++ b/app/Http/Middleware/EnsureUserIsClient.php @@ -0,0 +1,19 @@ +user()?->isClient()) { + abort(403, __('messages.unauthorized')); + } + + return $next($request); + } +} diff --git a/bootstrap/app.php b/bootstrap/app.php index 7a11e6b..d4662a8 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -18,6 +18,7 @@ return Application::configure(basePath: dirname(__DIR__)) $middleware->alias([ 'admin' => \App\Http\Middleware\EnsureUserIsAdmin::class, 'active' => \App\Http\Middleware\EnsureUserIsActive::class, + 'client' => \App\Http\Middleware\EnsureUserIsClient::class, ]); }) ->withExceptions(function (Exceptions $exceptions): void { diff --git a/docs/qa/gates/4.5-client-timeline-view.yml b/docs/qa/gates/4.5-client-timeline-view.yml new file mode 100644 index 0000000..ca3e7be --- /dev/null +++ b/docs/qa/gates/4.5-client-timeline-view.yml @@ -0,0 +1,49 @@ +# Quality Gate: 4.5 - Client Timeline View +schema: 1 +story: "4.5" +story_title: "Client Timeline View" +gate: PASS +status_reason: "All 18 acceptance criteria met with comprehensive test coverage (15 tests). Implementation follows project patterns correctly with proper authorization, eager loading, RTL/bilingual support, and read-only enforcement." +reviewer: "Quinn (Test Architect)" +updated: "2025-12-27T00:00:00Z" + +waiver: { active: false } + +top_issues: [] + +risk_summary: + totals: { critical: 0, high: 0, medium: 0, low: 0 } + recommendations: + must_fix: [] + monitor: + - "Consider adding pagination to index view for clients with many timelines" + +quality_score: 100 +expires: "2026-01-10T00:00:00Z" + +evidence: + tests_reviewed: 15 + risks_identified: 0 + trace: + ac_covered: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18] + ac_gaps: [] + +nfr_validation: + security: + status: PASS + notes: "Route middleware + component-level authorization. XSS protected via HTMLPurifier on input." + performance: + status: PASS + notes: "Eager loading prevents N+1 queries. No pagination yet but acceptable for typical usage." + reliability: + status: PASS + notes: "Proper error handling with abort_unless for unauthorized access." + maintainability: + status: PASS + notes: "Clean code following project conventions. Class-based Volt components with Flux UI." + +recommendations: + immediate: [] + future: + - action: "Add pagination to timeline index if clients accumulate many cases" + refs: ["resources/views/livewire/client/timelines/index.blade.php"] diff --git a/docs/stories/story-4.5-client-timeline-view.md b/docs/stories/story-4.5-client-timeline-view.md index 85e3b88..52dcb1c 100644 --- a/docs/stories/story-4.5-client-timeline-view.md +++ b/docs/stories/story-4.5-client-timeline-view.md @@ -539,3 +539,110 @@ test('timeline list uses eager loading', function () { **Complexity:** Medium **Estimated Effort:** 3-4 hours + +## QA Results + +### Review Date: 2025-12-27 + +### Reviewed By: Quinn (Test Architect) + +### Code Quality Assessment + +**Overall: Excellent** - The implementation follows established project patterns consistently. Both Volt components use class-based architecture as required by coding standards. The code is clean, well-organized, and matches sibling component patterns (e.g., `client/consultations/index.blade.php`). + +**Strengths:** +- Routes correctly registered with `client` middleware in `routes/web.php:113-117` +- Authorization properly enforced via `abort_unless()` in show component at `show.blade.php:13` +- Eager loading used appropriately to prevent N+1 queries (`withCount`, `with`) +- Read-only enforcement - no edit/delete/archive methods exist in components +- RTL support with locale-aware positioning (`app()->getLocale() === 'ar'`) +- Dark mode support with proper Tailwind classes +- Bilingual translations complete in both `lang/en/client.php` and `lang/ar/client.php` +- Consistent use of Flux UI components (badges, buttons, headings, icons) + +**File Structure:** +- `resources/views/livewire/client/timelines/index.blade.php` - List component +- `resources/views/livewire/client/timelines/show.blade.php` - Detail component +- `tests/Feature/Client/TimelineViewTest.php` - 15 comprehensive tests +- `lang/en/client.php` - English translations +- `lang/ar/client.php` - Arabic translations +- `app/Http/Middleware/EnsureUserIsClient.php` - Client middleware +- `bootstrap/app.php` - Middleware alias registration + +### Refactoring Performed + +None required - implementation follows project conventions correctly. + +### Compliance Check + +- Coding Standards: ✓ Class-based Volt components, Flux UI, proper testing patterns +- Project Structure: ✓ Files at correct locations, routes properly defined +- Testing Strategy: ✓ 15 Pest tests covering authorization, display, and read-only enforcement +- All ACs Met: ✓ See detailed trace below + +### Acceptance Criteria Trace + +| AC | Description | Test Coverage | Status | +|----|-------------|---------------|--------| +| 1 | Display all client's timelines | `client can view own timelines list` | ✓ | +| 2 | Active timelines prominently displayed | `active timelines displayed separately from archived` | ✓ | +| 3 | Archived timelines clearly separated | `active timelines displayed separately from archived` | ✓ | +| 4 | Visual distinction for status | `timeline detail shows status badge`, badges used in views | ✓ | +| 5 | Show case name and reference | `timeline detail shows case name and reference` | ✓ | +| 6 | Show status indicator | `timeline detail shows status badge` | ✓ | +| 7 | Show last update date | `index.blade.php:47` shows `diffForHumans()` | ✓ | +| 8 | Show update count | `timeline list shows update count` | ✓ | +| 9 | Individual view: case name/reference | `timeline detail shows case name and reference` | ✓ | +| 10 | Individual view: status indicator | `timeline detail shows status badge` | ✓ | +| 11 | Updates in chronological order | `timeline detail shows all updates chronologically` | ✓ | +| 12 | Update shows date/time | `show.blade.php:46` with `translatedFormat()` | ✓ | +| 13 | Update shows formatted content | `show.blade.php:48-49` with prose styling | ✓ | +| 14 | Read-only (no edit/comment) | `client timeline view is read-only with no edit actions` | ✓ | +| 15 | No archive/delete ability | No such methods in components | ✓ | +| 16 | Only own timelines (403) | `client cannot view other clients timeline detail` | ✓ | +| 17 | Responsive design | Tailwind responsive classes throughout | ✓ | +| 18 | Bilingual labels/dates | Translation keys + `translatedFormat()` | ✓ | + +### Improvements Checklist + +- [x] All acceptance criteria implemented +- [x] All 15 tests passing +- [x] Pint formatting verified +- [x] Authorization via middleware and component-level checks +- [x] N+1 query prevention with eager loading +- [x] RTL/LTR support implemented +- [x] Dark mode support implemented +- [ ] Consider pagination for clients with many timelines (future enhancement) + +### Security Review + +**Authorization:** ✓ +- Route-level: `client` middleware enforces client-only access (`EnsureUserIsClient`) +- Component-level: `abort_unless($timeline->user_id === auth()->id(), 403)` in show component +- Tests verify: guest redirect, admin forbidden, other client forbidden + +**XSS Protection:** ✓ +- `{!! $update->update_text !!}` uses unescaped output, BUT: +- Input is sanitized with `clean()` helper (HTMLPurifier) on admin input side (`admin/timelines/show.blade.php:46,82`) +- This matches the established pattern in the codebase (same as email templates) + +### Performance Considerations + +**Eager Loading:** ✓ +- Index: `withCount('updates')`, `with(['updates' => fn($q) => $q->latest()->limit(1)])` +- Show: `$timeline->load(['updates' => fn($q) => $q->oldest()])` + +**Potential Future Optimization:** +- If clients accumulate many timelines, consider adding pagination to index view + +### Files Modified During Review + +None - implementation is complete and follows standards. + +### Gate Status + +Gate: **PASS** → `docs/qa/gates/4.5-client-timeline-view.yml` + +### Recommended Status + +✓ **Ready for Done** - All acceptance criteria met, all tests passing, code follows project patterns. diff --git a/lang/ar/client.php b/lang/ar/client.php new file mode 100644 index 0000000..c0ec6f3 --- /dev/null +++ b/lang/ar/client.php @@ -0,0 +1,17 @@ + 'قضاياي', + 'active_cases' => 'القضايا النشطة', + 'archived_cases' => 'القضايا المؤرشفة', + 'reference' => 'المرجع', + 'updates' => 'التحديثات', + 'last_update' => 'آخر تحديث', + 'active' => 'نشط', + 'archived' => 'مؤرشف', + 'view' => 'عرض', + 'back_to_cases' => 'العودة للقضايا', + 'no_cases_yet' => 'لا توجد لديك قضايا بعد.', + 'no_updates_yet' => 'لا توجد تحديثات بعد.', +]; diff --git a/lang/en/client.php b/lang/en/client.php new file mode 100644 index 0000000..cc836e3 --- /dev/null +++ b/lang/en/client.php @@ -0,0 +1,17 @@ + 'My Cases', + 'active_cases' => 'Active Cases', + 'archived_cases' => 'Archived Cases', + 'reference' => 'Reference', + 'updates' => 'Updates', + 'last_update' => 'Last update', + 'active' => 'Active', + 'archived' => 'Archived', + 'view' => 'View', + 'back_to_cases' => 'Back to Cases', + 'no_cases_yet' => 'You don\'t have any cases yet.', + 'no_updates_yet' => 'No updates yet.', +]; diff --git a/resources/views/livewire/client/timelines/index.blade.php b/resources/views/livewire/client/timelines/index.blade.php new file mode 100644 index 0000000..0ecf821 --- /dev/null +++ b/resources/views/livewire/client/timelines/index.blade.php @@ -0,0 +1,114 @@ + auth()->user() + ->timelines() + ->active() + ->withCount('updates') + ->with(['updates' => fn ($q) => $q->latest()->limit(1)]) + ->latest('updated_at') + ->paginate(10, pageName: 'active'), + + 'archivedTimelines' => auth()->user() + ->timelines() + ->archived() + ->withCount('updates') + ->latest('updated_at') + ->paginate(10, pageName: 'archived'), + ]; + } +}; ?> + +
+ {{ __('client.my_cases') }} + + {{-- Active Timelines --}} + @if($activeTimelines->total() > 0) +
+

{{ __('client.active_cases') }}

+
+ @foreach($activeTimelines as $timeline) +
+
+
+

{{ $timeline->case_name }}

+ @if($timeline->case_reference) +

{{ __('client.reference') }}: {{ $timeline->case_reference }}

+ @endif +

+ {{ __('client.updates') }}: {{ $timeline->updates_count }} + @if($timeline->updates->first()) + · {{ __('client.last_update') }}: {{ $timeline->updates->first()->created_at->diffForHumans() }} + @endif +

+
+
+ {{ __('client.active') }} + + {{ __('client.view') }} + +
+
+
+ @endforeach +
+ @if($activeTimelines->hasPages()) +
+ {{ $activeTimelines->links() }} +
+ @endif +
+ @endif + + {{-- Archived Timelines --}} + @if($archivedTimelines->total() > 0) +
+

{{ __('client.archived_cases') }}

+
+ @foreach($archivedTimelines as $timeline) +
+
+
+

{{ $timeline->case_name }}

+ @if($timeline->case_reference) +

{{ __('client.reference') }}: {{ $timeline->case_reference }}

+ @endif +

+ {{ __('client.updates') }}: {{ $timeline->updates_count }} +

+
+
+ {{ __('client.archived') }} + + {{ __('client.view') }} + +
+
+
+ @endforeach +
+ @if($archivedTimelines->hasPages()) +
+ {{ $archivedTimelines->links() }} +
+ @endif +
+ @endif + + {{-- Empty State --}} + @if($activeTimelines->total() === 0 && $archivedTimelines->total() === 0) +
+ +

{{ __('client.no_cases_yet') }}

+
+ @endif +
diff --git a/resources/views/livewire/client/timelines/show.blade.php b/resources/views/livewire/client/timelines/show.blade.php new file mode 100644 index 0000000..50ec392 --- /dev/null +++ b/resources/views/livewire/client/timelines/show.blade.php @@ -0,0 +1,66 @@ +user_id === auth()->id(), 403); + + $this->timeline = $timeline->load(['updates' => fn ($q) => $q->oldest()]); + } +}; ?> + +
+ {{-- Header --}} +
+
+ {{ $timeline->case_name }} + @if($timeline->case_reference) +

{{ __('client.reference') }}: {{ $timeline->case_reference }}

+ @endif +
+ + {{ __('client.' . $timeline->status->value) }} + +
+ + {{-- Timeline Updates --}} +
+ {{-- Vertical line --}} +
+ +
+ @forelse($timeline->updates as $update) +
+ {{-- Dot --}} +
+ +
+
+ {{ $update->created_at->translatedFormat('l, d M Y - g:i A') }} +
+
+ {!! $update->update_text !!} +
+
+
+ @empty +

+ {{ __('client.no_updates_yet') }} +

+ @endforelse +
+
+ +
+ + {{ __('client.back_to_cases') }} + +
+
diff --git a/routes/web.php b/routes/web.php index 796e7eb..f27f546 100644 --- a/routes/web.php +++ b/routes/web.php @@ -94,12 +94,12 @@ Route::middleware(['auth', 'active'])->group(function () { }); // Client routes - Route::prefix('client')->group(function () { + Route::middleware('client')->prefix('client')->name('client.')->group(function () { Route::view('/dashboard', 'livewire.client.dashboard-placeholder') - ->name('client.dashboard'); + ->name('dashboard'); // Consultations - Route::prefix('consultations')->name('client.consultations.')->group(function () { + Route::prefix('consultations')->name('consultations.')->group(function () { Volt::route('/', 'client.consultations.index')->name('index'); Volt::route('/book', 'client.consultations.book')->name('book'); Route::get('/{consultation}/calendar', function (Consultation $consultation) { @@ -109,6 +109,12 @@ Route::middleware(['auth', 'active'])->group(function () { return app(CalendarService::class)->generateDownloadResponse($consultation); })->name('calendar'); }); + + // Timelines + Route::prefix('timelines')->name('timelines.')->group(function () { + Volt::route('/', 'client.timelines.index')->name('index'); + Volt::route('/{timeline}', 'client.timelines.show')->name('show'); + }); }); // Settings routes diff --git a/tests/Feature/Auth/AuthorizationTest.php b/tests/Feature/Auth/AuthorizationTest.php index 12c7d24..7a254af 100644 --- a/tests/Feature/Auth/AuthorizationTest.php +++ b/tests/Feature/Auth/AuthorizationTest.php @@ -46,12 +46,12 @@ test('client can access client routes', function () { $response->assertStatus(200); }); -test('admin can access client routes', function () { +test('admin cannot access client routes', function () { $admin = User::factory()->admin()->create(); $response = $this->actingAs($admin)->get('/client/dashboard'); - $response->assertStatus(200); + $response->assertForbidden(); }); test('deactivated user logged out on request', function () { diff --git a/tests/Feature/Client/TimelineViewTest.php b/tests/Feature/Client/TimelineViewTest.php new file mode 100644 index 0000000..74e48f7 --- /dev/null +++ b/tests/Feature/Client/TimelineViewTest.php @@ -0,0 +1,228 @@ +individual()->create(); + Timeline::factory()->count(3)->create(['user_id' => $client->id]); + + $this->actingAs($client) + ->get(route('client.timelines.index')) + ->assertOk(); +}); + +test('client cannot view other clients timelines in list', function () { + $client = User::factory()->individual()->create(); + $otherClient = User::factory()->individual()->create(); + Timeline::factory()->create([ + 'user_id' => $otherClient->id, + 'case_name' => 'Other Client Case', + ]); + + $this->actingAs($client); + + Volt::test('client.timelines.index') + ->assertDontSee('Other Client Case'); +}); + +test('client can view own timeline detail', function () { + $client = User::factory()->individual()->create(); + $timeline = Timeline::factory()->create([ + 'user_id' => $client->id, + 'case_name' => 'My Contract Case', + ]); + + $this->actingAs($client) + ->get(route('client.timelines.show', $timeline)) + ->assertOk() + ->assertSee('My Contract Case'); +}); + +test('client cannot view other clients timeline detail', function () { + $client = User::factory()->individual()->create(); + $otherClient = User::factory()->individual()->create(); + $otherTimeline = Timeline::factory()->create(['user_id' => $otherClient->id]); + + $this->actingAs($client) + ->get(route('client.timelines.show', $otherTimeline)) + ->assertForbidden(); +}); + +test('guest cannot access timelines', function () { + $this->get(route('client.timelines.index')) + ->assertRedirect(route('login')); +}); + +test('admin cannot access client timeline routes', function () { + $admin = User::factory()->admin()->create(); + + $this->actingAs($admin) + ->get(route('client.timelines.index')) + ->assertForbidden(); +}); + +// List View Tests +test('active timelines displayed separately from archived', function () { + $client = User::factory()->individual()->create(); + Timeline::factory()->create([ + 'user_id' => $client->id, + 'case_name' => 'Active Case', + 'status' => 'active', + ]); + Timeline::factory()->create([ + 'user_id' => $client->id, + 'case_name' => 'Archived Case', + 'status' => 'archived', + ]); + + $this->actingAs($client); + + Volt::test('client.timelines.index') + ->assertSee('Active Case') + ->assertSee('Archived Case') + ->assertSeeInOrder([__('client.active_cases'), 'Active Case', __('client.archived_cases'), 'Archived Case']); +}); + +test('timeline list shows update count', function () { + $client = User::factory()->individual()->create(); + $timeline = Timeline::factory()->create(['user_id' => $client->id]); + TimelineUpdate::factory()->count(5)->create(['timeline_id' => $timeline->id]); + + $this->actingAs($client); + + Volt::test('client.timelines.index') + ->assertSee('5'); +}); + +test('empty state shown when no timelines', function () { + $client = User::factory()->individual()->create(); + + $this->actingAs($client); + + Volt::test('client.timelines.index') + ->assertSee(__('client.no_cases_yet')); +}); + +// Detail View Tests +test('timeline detail shows all updates chronologically', function () { + $client = User::factory()->individual()->create(); + $timeline = Timeline::factory()->create(['user_id' => $client->id]); + + TimelineUpdate::factory()->create([ + 'timeline_id' => $timeline->id, + 'update_text' => 'First Update', + 'created_at' => now()->subDays(2), + ]); + TimelineUpdate::factory()->create([ + 'timeline_id' => $timeline->id, + 'update_text' => 'Second Update', + 'created_at' => now()->subDay(), + ]); + + $this->actingAs($client); + + Volt::test('client.timelines.show', ['timeline' => $timeline]) + ->assertSeeInOrder(['First Update', 'Second Update']); +}); + +test('timeline detail shows case name and reference', function () { + $client = User::factory()->individual()->create(); + $timeline = Timeline::factory()->create([ + 'user_id' => $client->id, + 'case_name' => 'Property Dispute', + 'case_reference' => 'REF-2024-001', + ]); + + $this->actingAs($client); + + Volt::test('client.timelines.show', ['timeline' => $timeline]) + ->assertSee('Property Dispute') + ->assertSee('REF-2024-001'); +}); + +test('timeline detail shows status badge', function () { + $client = User::factory()->individual()->create(); + $activeTimeline = Timeline::factory()->create([ + 'user_id' => $client->id, + 'status' => 'active', + ]); + + $this->actingAs($client); + + Volt::test('client.timelines.show', ['timeline' => $activeTimeline]) + ->assertSee(__('client.active')); +}); + +test('empty updates shows no updates message', function () { + $client = User::factory()->individual()->create(); + $timeline = Timeline::factory()->create(['user_id' => $client->id]); + + $this->actingAs($client); + + Volt::test('client.timelines.show', ['timeline' => $timeline]) + ->assertSee(__('client.no_updates_yet')); +}); + +// Read-Only Enforcement Tests +test('client timeline view is read-only with no edit actions', function () { + $client = User::factory()->individual()->create(); + $timeline = Timeline::factory()->create(['user_id' => $client->id]); + + $this->actingAs($client); + + // Verify no edit/delete/archive buttons or forms exist in the view + Volt::test('client.timelines.show', ['timeline' => $timeline]) + ->assertDontSee('wire:click="edit"') + ->assertDontSee('wire:click="delete"') + ->assertDontSee('wire:click="archive"') + ->assertDontSee('wire:submit'); +}); + +// Company client access test +test('company client can view own timelines', function () { + $company = User::factory()->company()->create(); + Timeline::factory()->count(2)->create(['user_id' => $company->id]); + + $this->actingAs($company) + ->get(route('client.timelines.index')) + ->assertOk(); +}); + +// Pagination Tests +test('timeline list paginates active timelines', function () { + $client = User::factory()->individual()->create(); + // Create 15 active timelines (more than page size of 10) + Timeline::factory()->count(15)->create([ + 'user_id' => $client->id, + 'status' => 'active', + ]); + + $this->actingAs($client); + + // First page should show 10 timelines + $response = $this->get(route('client.timelines.index')); + $response->assertOk(); + + // Navigate to second page + $response = $this->get(route('client.timelines.index', ['active' => 2])); + $response->assertOk(); +}); + +test('timeline list paginates archived timelines separately', function () { + $client = User::factory()->individual()->create(); + // Create 15 archived timelines + Timeline::factory()->count(15)->create([ + 'user_id' => $client->id, + 'status' => 'archived', + ]); + + $this->actingAs($client); + + // Navigate to second page of archived + $response = $this->get(route('client.timelines.index', ['archived' => 2])); + $response->assertOk(); +});