release/v0.1.7-phase7-target-format-step#103
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…uard CONV-084: Add format step guard
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rget-formats CONV-085: Load available target formats
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mat-card-view-model CONV-086: Create target format card view model
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…mat-cards CONV-087: Render target format cards
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…arget-marker CONV-088: Add recommended target marker
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ource-format-empty-state CONV-089: Add unsupported source format empty state
Add Livewire tests for supported/unsupported target selection. Verified red against the absent selectTargetFormat() handler, then marked skip pending CONV-091 to keep the suite green for merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t-selection CONV-090: Test target format selection
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…format-selection CONV-091: Implement target format selection
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…target-format-in-component-state CONV-092: Persist selected target format in component state
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion-from-format-step CONV-093: Add back navigation from format step
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t-step-loading-and-error-states CONV-094: Add target format step loading and error states
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t-step-smoke-tests CONV-095: Add target format step smoke tests
📝 WalkthroughWalkthroughThe PR introduces target-format selection to the DashboardConverter component. It adds state tracking for selected format and errors, registry-driven step navigation with format validation, view-model factories to render format cards, UI sections for selection with error/loading/empty states, and comprehensive feature and smoke tests covering selection logic, navigation, and edge cases. ChangesTarget Format Selection & Navigation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/Unit/ViewModels/TargetFormatCardViewModelTest.php (1)
8-10: ⚡ Quick winAdd an explicit non-null assertion before
fromConverter()Guard
$converterwith an assertion so failures clearly indicate missing registry fixture instead of a downstream type error.Suggested patch
it('creates target format card data from converter metadata', function () { $converter = app(ConverterRegistry::class)->find('png', 'jpg'); + expect($converter)->not->toBeNull(); $card = TargetFormatCardViewModel::fromConverter($converter);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Unit/ViewModels/TargetFormatCardViewModelTest.php` around lines 8 - 10, The test calls ConverterRegistry::find('png', 'jpg') and passes the result into TargetFormatCardViewModel::fromConverter without asserting it is non-null; add an explicit non-null assertion (e.g. PHPUnit's assertNotNull or an explicit check) on $converter immediately after the find call so a missing registry fixture fails with a clear assertion message rather than causing a downstream type error inside TargetFormatCardViewModel::fromConverter.app/Livewire/Dashboard/DashboardConverter.php (1)
80-86:ensureValidStep()isn’t wired into the Livewire runtime
app/Livewire/Dashboard/DashboardConverter.php(lines 80-86) definesensureValidStep(), but it’s never called by the component (no lifecycle hook calls it) or byresources/views/livewire/dashboard/dashboard-converter.blade.php(nowire:init/wire:*call). The only invocation is intests/Feature/Livewire/DashboardConverterTest.phpvia->call('ensureValidStep'), so the format-step guard is effectively dead unless you explicitly hook it in (or remove it if redundant with the existing$step+$this->currentFilechecks/step transition methods).public function ensureValidStep(): void { if ($this->step === 'format' && $this->currentFile === null) { $this->currentFileId = null; $this->step = 'upload'; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Livewire/Dashboard/DashboardConverter.php` around lines 80 - 86, ensureValidStep() is never invoked at runtime so its guard never runs; either remove it if redundant or call it from the component lifecycle (e.g., invoke $this->ensureValidStep() inside the Livewire component's mount() and updated($name, $value) methods) and/or add a wire:init="ensureValidStep" to resources/views/livewire/dashboard/dashboard-converter.blade.php so the check runs on initialization; update DashboardConverter::mount and/or DashboardConverter::updated to call ensureValidStep (or add the wire:init hook in the blade) to ensure the format-step guard actually executes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/Feature/Livewire/DashboardConverterTargetFormatSmokeTest.php`:
- Line 45: The assertion in DashboardConverterTargetFormatSmokeTest uses
FileRecord::query()->count() which counts globally; change it to scope to the
test user by filtering the FileRecord query with the test user's identifier
(e.g., use the test user variable in the test such as $user or $this->user and
add a where('user_id', $user->id) or equivalent relation before calling count())
so the expect compares only records belonging to the test user.
- Line 67: The test's FileRecord existence assertion lacks user scoping; update
the expectation to include the test user's id so it mirrors the component's
scoping (see getCurrentFileProperty). Locate the assertion using
FileRecord::query()->where('original_name', 'roundtrip.png')->exists() and add a
where('user_id', $user->id) (or the test's user reference, e.g. $this->user or
$userFromSetup) before ->exists() so the check only looks for files owned by the
test user.
---
Nitpick comments:
In `@app/Livewire/Dashboard/DashboardConverter.php`:
- Around line 80-86: ensureValidStep() is never invoked at runtime so its guard
never runs; either remove it if redundant or call it from the component
lifecycle (e.g., invoke $this->ensureValidStep() inside the Livewire component's
mount() and updated($name, $value) methods) and/or add a
wire:init="ensureValidStep" to
resources/views/livewire/dashboard/dashboard-converter.blade.php so the check
runs on initialization; update DashboardConverter::mount and/or
DashboardConverter::updated to call ensureValidStep (or add the wire:init hook
in the blade) to ensure the format-step guard actually executes.
In `@tests/Unit/ViewModels/TargetFormatCardViewModelTest.php`:
- Around line 8-10: The test calls ConverterRegistry::find('png', 'jpg') and
passes the result into TargetFormatCardViewModel::fromConverter without
asserting it is non-null; add an explicit non-null assertion (e.g. PHPUnit's
assertNotNull or an explicit check) on $converter immediately after the find
call so a missing registry fixture fails with a clear assertion message rather
than causing a downstream type error inside
TargetFormatCardViewModel::fromConverter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 750bd439-4583-478b-a123-7508f2def0bd
📒 Files selected for processing (9)
app/Livewire/Dashboard/DashboardConverter.phpapp/ViewModels/TargetFormatCardViewModel.phpresources/views/livewire/dashboard/dashboard-converter.blade.phptests/Feature/Livewire/DashboardConverterNavigationTest.phptests/Feature/Livewire/DashboardConverterTargetFormatSmokeTest.phptests/Feature/Livewire/DashboardConverterTargetFormatTest.phptests/Feature/Livewire/DashboardConverterTest.phptests/Feature/Livewire/DashboardConverterUploadTest.phptests/Unit/ViewModels/TargetFormatCardViewModelTest.php
… fixture, wire ensureValidStep - Smoke test: scope FileRecord count/exists assertions to the test user - TargetFormatCardViewModel test: assert converter is non-null before fromConverter - Run ensureValidStep guard on component init via wire:init Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Release v0.1.7 — Phase 7: Target Format Step
Promotes Phase 7 (CONV-084 → CONV-095) from
developtomain.What's in this release
Phase 7 turns the Phase 6 format-step placeholder into a real target-format selection step:
ConverterRegistryfor the currentFileRecord.TargetFormatCardViewModel(fromConverter/fromTarget).ConversionJobis created.Out of scope (per plan)
Dynamic settings form, options schema rendering/validation, conversion jobs/execution, downloads, billing — all deferred to Phase 8+.
Quality gates
composer test— 201 passed (529 assertions)composer lint— passednpm run build— passedphp artisan migrate:fresh --seed— cleanTag (after merge)
v0.1.7-phase7-target-format-step🤖 Generated with Claude Code
Summary by cubic
Adds a real target-format selection step to the dashboard converter so users can pick supported outputs, see a recommended option, and navigate without losing their file. Also wires an init guard to auto-return to upload if state is stale.
ConverterRegistryfor the currentFileRecord.TargetFormatCardViewModelwith labels, descriptions, and a "Recommended" badge.wire:init; missing file routes to upload; upload summary with Replace/Remove and "Choose format".ConversionJob.TargetFormatCardViewModelunit test; scopedFileRecordassertions to the user.Written for commit 52ac719. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Tests