diff --git a/database/factories/SponsorFactory.php b/database/factories/SponsorFactory.php index 56010fd..17ea1eb 100644 --- a/database/factories/SponsorFactory.php +++ b/database/factories/SponsorFactory.php @@ -33,6 +33,26 @@ public function definition(): array ]; } + /** + * Create a sponsor without a website. + */ + public function withoutWebsite(): static + { + return $this->state(fn (array $attributes) => [ + 'website' => null, + ]); + } + + /** + * Create a sponsor with an HTTP logo URL (avoids S3 dependency in tests). + */ + public function withTestLogo(): static + { + return $this->state(fn (array $attributes) => [ + 'logo' => 'https://example.com/logos/test-logo.png', + ]); + } + /** * Create PHP Architect sponsor with specific data. */ diff --git a/database/migrations/2026_03_31_004613_make_sponsor_website_nullable.php b/database/migrations/2026_03_31_004613_make_sponsor_website_nullable.php new file mode 100644 index 0000000..316960d --- /dev/null +++ b/database/migrations/2026_03_31_004613_make_sponsor_website_nullable.php @@ -0,0 +1,18 @@ +string('website')->nullable()->change(); + }); + } +}; diff --git a/docs/superpowers/plans/2026-04-01-pr54-review-fixes.md b/docs/superpowers/plans/2026-04-01-pr54-review-fixes.md new file mode 100644 index 0000000..12cd294 --- /dev/null +++ b/docs/superpowers/plans/2026-04-01-pr54-review-fixes.md @@ -0,0 +1,365 @@ +# PR #54 Review Fixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix issues identified in PR #54 code review comments and add tests for identified coverage gaps. + +**Architecture:** Fix the SponsorControllerTest to avoid S3 dependency by using HTTP logo URLs in the factory, clean up redundant code in attach() calls and RefreshDatabase import, and add new tests for the Sponsor `logoUrl` accessor and model relationships. + +**Tech Stack:** Laravel 12, Pest PHP, SQLite (test), PHP 8.4 + +--- + +## Review Comment Triage + +### Will Fix +1. **S3 dependency in tests (CodeRabbit - Major)** — Factory generates relative logo paths triggering `Storage::disk('s3')->url()`. Fix by adding a `withTestLogo()` factory state that uses an HTTP URL. +2. **Redundant `uses(RefreshDatabase::class)` (Copilot)** — Already applied globally in `tests/Pest.php:15`. Remove. +3. **Redundant pivot attributes in `attach()` (Copilot + CodeRabbit)** — Remove `sponsor_uuid` and `conference_uuid` from attach calls; the relationship handles them. + +### Will NOT Fix (with justification) +1. **Migration missing `down()` method** — Project convention in CLAUDE.md: "do not write down methods in migrations, only up methods." +2. **Migration `->change()` without doctrine/dbal** — False positive. Laravel 11+ handles `->change()` natively without doctrine/dbal. +3. **Dynamic property `$this->conference` PHP 8.4 deprecation** — Standard Pest PHP pattern. Pest's test case handles dynamic properties via magic methods. This is idiomatic Pest code. +4. **File path doesn't match behavior tested** — Low-priority cosmetic change. Would rename file but not change behavior. Deferring to avoid unnecessary churn. +5. **fwallen's `sponsor.websiteEl` comment** — The code at lines 162-171 already uses a local `websiteEl` variable correctly. The code is `websiteEl.href = sponsor.website` which is correct. Eric confirmed in the thread. + +## File Structure + +| File | Action | Responsibility | +|------|--------|----------------| +| `tests/Feature/Http/Controllers/SponsorControllerTest.php` | Modify | Fix S3 dep, remove redundant RefreshDatabase, clean attach() calls | +| `database/factories/SponsorFactory.php` | Modify | Add `withTestLogo()` state for tests | +| `tests/Feature/SponsorModelTest.php` | Create | Test `logoUrl` accessor edge cases | +| `tests/Feature/RelationshipTest.php` | Create | Test Conference-Sponsor and Conference-User relationships | + +--- + +### Task 1: Add `withTestLogo()` Factory State + +**Files:** +- Modify: `database/factories/SponsorFactory.php:46` + +- [ ] **Step 1: Add the factory state method** + +Add after the `withoutWebsite()` method: + +```php +/** + * Create a sponsor with an HTTP logo URL (avoids S3 dependency in tests). + */ +public function withTestLogo(): static +{ + return $this->state(fn (array $attributes) => [ + 'logo' => 'https://example.com/logos/test-logo.png', + ]); +} +``` + +- [ ] **Step 2: Verify factory still works** + +Run: `php artisan tinker --execute="echo App\Models\Sponsor::factory()->withTestLogo()->make()->logo_url;"` +Expected: `https://example.com/logos/test-logo.png` + +- [ ] **Step 3: Commit** + +```bash +git add database/factories/SponsorFactory.php +git commit -m "feat: add withTestLogo factory state to avoid S3 dependency in tests" +``` + +--- + +### Task 2: Fix SponsorControllerTest + +**Files:** +- Modify: `tests/Feature/Http/Controllers/SponsorControllerTest.php` + +- [ ] **Step 1: Remove redundant RefreshDatabase import and usage** + +Remove line 5 (`use Illuminate\Foundation\Testing\RefreshDatabase;`) and line 7 (`uses(RefreshDatabase::class);`). This is already applied globally in `tests/Pest.php:14-16`. + +- [ ] **Step 2: Update all `Sponsor::factory()` calls to chain `->withTestLogo()`** + +Every `Sponsor::factory()` call must include `->withTestLogo()` to avoid S3: + +```php +// Line 25 - sponsor with website +$sponsor = Sponsor::factory()->withTestLogo()->create([ + 'website' => 'https://example.com', +]); + +// Line 43 - sponsor without website +$sponsor = Sponsor::factory()->withTestLogo()->withoutWebsite()->create(); + +// Line 58 - sponsor with site +$sponsorWithSite = Sponsor::factory()->withTestLogo()->create([ + 'website' => 'https://has-website.com', +]); + +// Line 62 - sponsor without site +$sponsorWithoutSite = Sponsor::factory()->withTestLogo()->withoutWebsite()->create(); +``` + +- [ ] **Step 3: Clean up all `attach()` calls to remove redundant pivot keys** + +Replace each attach call. Only pass `sponsorship_level` — Laravel fills the foreign keys automatically. + +Before: +```php +$this->conference->sponsors()->attach($sponsor->uuid, [ + 'sponsor_uuid' => $sponsor->uuid, + 'conference_uuid' => $this->conference->uuid, + 'sponsorship_level' => 'gold', +]); +``` + +After: +```php +$this->conference->sponsors()->attach($sponsor->uuid, [ + 'sponsorship_level' => 'gold', +]); +``` + +Apply to all 4 attach calls (lines 29, 45, 64, 70). + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `php artisan test --filter=SponsorControllerTest` +Expected: 3 tests pass, 0 failures + +- [ ] **Step 5: Commit** + +```bash +git add tests/Feature/Http/Controllers/SponsorControllerTest.php +git commit -m "fix: remove S3 dependency, redundant RefreshDatabase, and redundant pivot keys in sponsor tests" +``` + +--- + +### Task 3: Add Sponsor `logoUrl` Accessor Tests + +**Files:** +- Create: `tests/Feature/SponsorModelTest.php` + +- [ ] **Step 1: Create the test file** + +Run: `php artisan make:test SponsorModelTest --pest` + +- [ ] **Step 2: Write tests for all logoUrl branches** + +The `logoUrl` accessor in `app/Models/Sponsor.php:62-87` has 4 branches: +1. No logo → returns null +2. HTTP URL logo → returns as-is +3. Local file exists → returns asset URL +4. Relative path (S3 fallback) → calls Storage::disk('s3') + +```php +make(['logo' => null]); + + expect($sponsor->logo_url)->toBeNull(); +}); + +test('logoUrl returns http url as-is', function () { + $sponsor = Sponsor::factory()->make([ + 'logo' => 'https://example.com/logo.png', + ]); + + expect($sponsor->logo_url)->toBe('https://example.com/logo.png'); +}); + +test('logoUrl returns asset url for existing public file', function () { + $sponsor = Sponsor::factory()->make([ + 'logo' => 'favicon.ico', + ]); + + expect($sponsor->logo_url)->toContain('favicon.ico'); +}); + +test('logoUrl falls back to s3 for non-public relative paths', function () { + Storage::fake('s3'); + + $sponsor = Sponsor::factory()->make([ + 'logo' => 'vendor_logos/some-logo.png', + ]); + + $url = $sponsor->logo_url; + + expect($url)->toContain('vendor_logos/some-logo.png'); +}); + +test('logoUrl prepends vendor_logos prefix for bare paths on s3', function () { + Storage::fake('s3'); + + $sponsor = Sponsor::factory()->make([ + 'logo' => 'some-logo.png', + ]); + + $url = $sponsor->logo_url; + + expect($url)->toContain('vendor_logos/some-logo.png'); +}); +``` + +- [ ] **Step 3: Run the tests** + +Run: `php artisan test tests/Feature/SponsorModelTest.php` +Expected: 5 tests pass + +- [ ] **Step 4: Commit** + +```bash +git add tests/Feature/SponsorModelTest.php +git commit -m "test: add comprehensive tests for Sponsor logoUrl accessor" +``` + +--- + +### Task 4: Add Model Relationship Tests + +**Files:** +- Create: `tests/Feature/RelationshipTest.php` + +- [ ] **Step 1: Create the test file** + +Run: `php artisan make:test RelationshipTest --pest` + +- [ ] **Step 2: Write relationship tests** + +```php + 'rel-test-conf-uuid', + 'name' => 'Relationship Test Conference', + 'venue_name' => 'Test Venue', + 'start_date' => '2026-05-19', + 'end_date' => '2026-05-21', + ]); + + $sponsor = Sponsor::factory()->withTestLogo()->create(); + + $conference->sponsors()->attach($sponsor->uuid, [ + 'sponsorship_level' => 'gold', + ]); + + $conference->refresh(); + + expect($conference->sponsors)->toHaveCount(1); + expect($conference->sponsors->first()->name)->toBe($sponsor->name); + expect($conference->sponsors->first()->pivot->sponsorship_level)->toBe('gold'); +}); + +test('sponsor belongs to many conferences through pivot', function () { + $conference = Conference::create([ + 'uuid' => 'rel-test-conf-uuid-2', + 'name' => 'Reverse Relationship Conference', + 'venue_name' => 'Test Venue', + 'start_date' => '2026-05-19', + 'end_date' => '2026-05-21', + ]); + + $sponsor = Sponsor::factory()->withTestLogo()->create(); + + $conference->sponsors()->attach($sponsor->uuid, [ + 'sponsorship_level' => 'silver', + ]); + + $sponsor->refresh(); + + expect($sponsor->conferences)->toHaveCount(1); + expect($sponsor->conferences->first()->name)->toBe($conference->name); + expect($sponsor->conferences->first()->pivot->sponsorship_level)->toBe('silver'); +}); + +test('conference has many users through pivot', function () { + $conference = Conference::create([ + 'uuid' => 'rel-test-conf-uuid-3', + 'name' => 'User Relationship Conference', + 'venue_name' => 'Test Venue', + 'start_date' => '2026-05-19', + 'end_date' => '2026-05-21', + ]); + + $user = User::factory()->create(); + + $conference->users()->attach($user->uuid); + + $conference->refresh(); + + expect($conference->users)->toHaveCount(1); + expect($conference->users->first()->email)->toBe($user->email); +}); + +test('conference can have multiple sponsors at different levels', function () { + $conference = Conference::create([ + 'uuid' => 'rel-test-conf-uuid-4', + 'name' => 'Multi Sponsor Conference', + 'venue_name' => 'Test Venue', + 'start_date' => '2026-05-19', + 'end_date' => '2026-05-21', + ]); + + $goldSponsor = Sponsor::factory()->withTestLogo()->create(); + $silverSponsor = Sponsor::factory()->withTestLogo()->create(); + + $conference->sponsors()->attach($goldSponsor->uuid, [ + 'sponsorship_level' => 'gold', + ]); + + $conference->sponsors()->attach($silverSponsor->uuid, [ + 'sponsorship_level' => 'silver', + ]); + + $conference->refresh(); + + expect($conference->sponsors)->toHaveCount(2); + + $levels = $conference->sponsors->pluck('pivot.sponsorship_level')->toArray(); + expect($levels)->toContain('gold'); + expect($levels)->toContain('silver'); +}); +``` + +- [ ] **Step 3: Run the tests** + +Run: `php artisan test tests/Feature/RelationshipTest.php` +Expected: 4 tests pass + +- [ ] **Step 4: Commit** + +```bash +git add tests/Feature/RelationshipTest.php +git commit -m "test: add model relationship tests for Conference, Sponsor, and User" +``` + +--- + +### Task 5: Run Full Test Suite + +- [ ] **Step 1: Run all tests** + +Run: `php artisan test` +Expected: All tests pass (existing + new) + +- [ ] **Step 2: Run Pint for formatting** + +Run: `vendor/bin/pint --dirty` + +- [ ] **Step 3: Final commit if Pint made changes** + +```bash +git add -A +git commit -m "style: apply Pint formatting" +``` diff --git a/resources/js/app.js b/resources/js/app.js index 8647582..593a08a 100644 --- a/resources/js/app.js +++ b/resources/js/app.js @@ -2,4 +2,37 @@ import Alpine from 'alpinejs' window.Alpine = Alpine -Alpine.start() \ No newline at end of file +Alpine.data('countdown', (targetIso) => ({ + days: 0, + hours: 0, + minutes: 0, + seconds: 0, + started: false, + timer: null, + init() { + this.update(targetIso) + this.timer = setInterval(() => this.update(targetIso), 1000) + }, + destroy() { + if (this.timer) { + clearInterval(this.timer) + } + }, + update(targetIso) { + const diff = new Date(targetIso).getTime() - Date.now() + + if (diff <= 0) { + this.started = true + this.days = this.hours = this.minutes = this.seconds = 0 + this.destroy() + return + } + + this.days = Math.floor(diff / 86400000) + this.hours = Math.floor((diff % 86400000) / 3600000) + this.minutes = Math.floor((diff % 3600000) / 60000) + this.seconds = Math.floor((diff % 60000) / 1000) + }, +})) + +Alpine.start() diff --git a/resources/views/components/hero-section.blade.php b/resources/views/components/hero-section.blade.php index 3338e19..bc2930f 100644 --- a/resources/views/components/hero-section.blade.php +++ b/resources/views/components/hero-section.blade.php @@ -20,6 +20,27 @@ class="inline-block py-1 px-3 rounded-full bg-tek-blue-100 dark:bg-tek-blue-800/ networking opportunities with the PHP community.
+ @if($conference && $conference->getStartDate()) ++ Conference starts in + The conference is here! +
+