Implemented the first version of review output resolve actions. Included a ReviewOutputResolveActionMapper, commands to seed browser fixtures, updated CustomerReviewWorkspace, EnvironmentReviewResource, UI enforcement, and related views. Also added extensive unit, feature, and browser tests, and updated the design coverage matrix. Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de> Reviewed-on: #422
118 lines
13 KiB
Markdown
118 lines
13 KiB
Markdown
# Tasks: Spec 351 - Review Output Resolve Actions v1
|
|
|
|
**Input**: `specs/351-review-output-resolve-actions-v1/spec.md`, `plan.md`, `repo-truth-map.md`, `contracts/review-output-resolve-action-map.md`, and `checklists/requirements.md`
|
|
|
|
**Tests**: Required. This is a review-output action-selection and operator-trust change over existing Filament pages, detail surfaces, and source-owned lifecycle actions.
|
|
|
|
## Test Governance Checklist
|
|
|
|
- [x] Lane assignment is explicit and narrow: Unit for deterministic action selection, Feature for workspace/detail integration, Browser for first-screen CTA trust proof.
|
|
- [x] New or changed tests stay in the smallest honest family, and the browser addition is explicit.
|
|
- [x] Shared helpers, factories, seeds, fixtures, and context defaults stay cheap by default.
|
|
- [x] Planned validation commands cover the change without pulling unrelated lane cost.
|
|
- [x] The declared surface profiles (`global-context-shell` and `shared-detail-family`) are explicit.
|
|
- [x] Any new action metadata remains derived-only and does not create hidden persistence or a workflow engine.
|
|
|
|
## Phase 1: Preparation And Repo Truth
|
|
|
|
**Purpose**: Keep the implementation bounded to the repo-real review-output and lifecycle surfaces.
|
|
|
|
- [x] T001 Re-read `spec.md`, `plan.md`, `repo-truth-map.md`, `contracts/review-output-resolve-action-map.md`, and `checklists/requirements.md` before runtime changes.
|
|
- [x] T002 Re-read Specs 347, 349, and 350 as historical context only. Do not modify their artifacts.
|
|
- [x] T003 Re-verify the current runtime truth in `apps/platform/app/Support/ReviewPacks/ReviewPackOutputResolutionGuidance.php`, `apps/platform/app/Support/ResolutionGuidance/Adapters/ReviewPackOutputResolutionAdapter.php`, `apps/platform/app/Support/ResolutionGuidance/ResolutionAction.php`, and `apps/platform/app/Support/ResolutionGuidance/ResolutionCase.php`.
|
|
- [x] T004 Re-verify the current runtime truth in `apps/platform/app/Filament/Pages/Reviews/CustomerReviewWorkspace.php`, `apps/platform/app/Filament/Resources/EnvironmentReviewResource.php`, `apps/platform/app/Filament/Resources/EnvironmentReviewResource/Pages/ViewEnvironmentReview.php`, `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewService.php`, `apps/platform/app/Services/EnvironmentReviews/EnvironmentReviewLifecycleService.php`, and `apps/platform/app/Filament/Resources/EvidenceSnapshotResource/Pages/ViewEvidenceSnapshot.php`.
|
|
- [x] T005 Keep `specs/351-review-output-resolve-actions-v1/repo-truth-map.md` and `specs/351-review-output-resolve-actions-v1/contracts/review-output-resolve-action-map.md` current if runtime inspection reveals a narrower or broader safe action map.
|
|
- [x] T006 Confirm no migration, package, env var, queue family, scheduler change, storage-topology change, panel/provider change, or global-search change is required, and confirm Filament v5 / Livewire v4.0+ plus `apps/platform/bootstrap/providers.php` remain unchanged.
|
|
- [x] T007 Decide and document whether `create_next_review` is confirmation-hardened in-scope; if it is not, keep it as truthful navigation/disclosure fallback and do not surface it as an executable dominant CTA.
|
|
|
|
## Phase 2: Tests First
|
|
|
|
**Purpose**: Lock the deterministic mapper behavior and the workspace/detail safety rules before runtime changes.
|
|
|
|
- [x] T008 Add `apps/platform/tests/Unit/ResolutionGuidance/Spec351ReviewOutputResolveActionMapperTest.php`.
|
|
- [x] T009 Add mapper assertions for published blocked review + known successor review -> open successor review only when the target is repo-real.
|
|
- [x] T010 Add mapper assertions for published blocked review + no known successor + safe next-cycle execution -> `create_next_review`.
|
|
- [x] T011 Add mapper assertions for published blocked review + no safe execution path -> truthful fallback such as review limitations or open review.
|
|
- [x] T012 Add mapper assertions for mutable review + stale/missing evidence -> `refresh_review` when executable, otherwise evidence fallback.
|
|
- [x] T013 Add mapper assertions for ready mutable review -> `publish_review`.
|
|
- [x] T014 Add mapper assertions for internal-only / PII / export-not-ready / no supported action states.
|
|
- [x] T015 Add `apps/platform/tests/Feature/Filament/Spec351CustomerReviewWorkspaceResolveActionTest.php`.
|
|
- [x] T016 Add `apps/platform/tests/Feature/EnvironmentReview/Spec351EnvironmentReviewResolveActionTest.php`.
|
|
- [x] T017 Extend existing lifecycle or action semantics tests if `create_next_review` confirmation/visibility changes.
|
|
- [x] T018 Add `apps/platform/tests/Browser/Spec351ReviewOutputResolveActionsSmokeTest.php` covering at least one blocked path and one ready-state path.
|
|
- [x] T019 Reuse or extend Spec 347/349/350 regressions instead of duplicating their existing readiness/output contract coverage wholesale.
|
|
|
|
## Phase 3: Mapper And Contract Extension
|
|
|
|
**Purpose**: Introduce the narrowest review-output-only mapper over the existing contract.
|
|
|
|
- [x] T020 Create `apps/platform/app/Support/ResolutionGuidance/ReviewOutputResolveActionMapper.php`.
|
|
- [x] T021 Accept the current `EnvironmentReview`, current guidance/readiness truth, known target URLs, surface context, and capability/execution-envelope inputs.
|
|
- [x] T022 Rank actions deterministically in this order: known successor review, next-cycle creation, review refresh, review publish, evidence/proof fallback, disclosure fallback.
|
|
- [x] T023 Extend `apps/platform/app/Support/ResolutionGuidance/ResolutionAction.php` only if necessary to carry missing source-owned execution-routing metadata such as `action_name` and `execution_surface`, while reusing existing `disabled_reason`, capability, confirmation, audit, and `OperationRun` fields.
|
|
- [x] T024 Keep the mapper canonical for `resolution_case.primary_action` and `secondary_actions` on workspace/detail surfaces, derived-only and request-scoped; do not add persistence or request-crossing cache behavior.
|
|
- [x] T025 Ensure unsupported or unsafe executable actions degrade to truthful navigation, disclosure, or `none`.
|
|
- [x] T026 Reuse existing capability keys, audit verbs, and service-owned `OperationRun` behavior instead of duplicating lifecycle semantics inside the mapper.
|
|
- [x] T027 Only return "Open successor review" when a concrete repo-verified successor review target is actually known, such as `superseded_by_review_id` or another deterministic lookup proven during implementation; do not invent draft discovery.
|
|
|
|
## Phase 4: Workspace Execution Bridge
|
|
|
|
**Purpose**: Let the workspace surface real repo-backed actions without creating a second action runtime.
|
|
|
|
- [x] T028 Add or reuse source-owned Filament page actions on `apps/platform/app/Filament/Pages/Reviews/CustomerReviewWorkspace.php` for `refresh_review`, `publish_review`, and `create_next_review`.
|
|
- [x] T029 Reuse `EnvironmentReviewService`, `EnvironmentReviewLifecycleService`, `GovernanceActionCatalog` where already repo-real, `UiEnforcement`, existing notifications, and current policy/capability behavior inside those page actions; if `create_next_review` is surfaced as executable, harden the source-owned `ViewEnvironmentReview::createNextReviewAction()` with confirmation before reuse.
|
|
- [x] T030 Render the dominant workspace CTA through `mountAction()` and existing Filament modals when the action is executable on this surface, while keeping all other mapped actions in a clearly secondary supporting-action group.
|
|
- [x] T031 When the dominant action is not executable for the current actor or surface, downgrade to the strongest truthful fallback action instead of showing a fake enabled button.
|
|
- [x] T032 Preserve current findings and accepted-risk follow-up overrides above the base review-output resolve-action mapping.
|
|
- [x] T033 If copy changes are adopted (`Create next review draft`, `Refresh review inputs`), apply them through shared localization and keep the source-owned action vocabulary consistent across button label, modal title, notification, and audit prose.
|
|
|
|
## Phase 5: Adapter Integration And Detail Alignment
|
|
|
|
**Purpose**: Use the same action semantics on the detail surface without duplicate CTA rails.
|
|
|
|
- [x] T034 Update `apps/platform/app/Support/ResolutionGuidance/Adapters/ReviewPackOutputResolutionAdapter.php` so primary and secondary action selection comes from the new mapper.
|
|
- [x] T035 Update `apps/platform/app/Filament/Pages/Reviews/CustomerReviewWorkspace.php` to consume the mapped resolve action while preserving existing output guidance fields, follow-up overrides, and a semantically split review-pack state model.
|
|
- [x] T036 Update `apps/platform/resources/views/filament/pages/reviews/customer-review-workspace.blade.php` so the decision card renders exactly one primary CTA, demotes other mapped actions into supporting actions, and keeps the review-consumption flow as a subordinate reference surface.
|
|
- [x] T037 Update `apps/platform/app/Filament/Resources/EnvironmentReviewResource.php` so `outputGuidanceState()` aligns the next-step semantics and `resolution_case` to the existing dominant header lifecycle action in normal detail mode.
|
|
- [x] T038 Preserve `customer_workspace` detail-mode suppression and the current context note in `apps/platform/resources/views/filament/infolists/entries/review-pack-output-guidance.blade.php`.
|
|
- [x] T039 In normal detail mode, keep exactly one dominant lifecycle action surface. Do not move lifecycle CTA ownership out of `ViewEnvironmentReview`, and if the header action already represents the same resolve action, do not duplicate it inside the guidance card.
|
|
- [x] T040 Only show successor-review-open navigation on detail when the actual target review is known; otherwise keep create/fallback semantics.
|
|
|
|
## Phase 6: Copy, Audit, And Browser Proof
|
|
|
|
**Purpose**: Align visible copy and coverage artifacts with the new action-first workflow.
|
|
|
|
- [x] T041 Update only the required resolve-action, acknowledgement-semantics, and supporting-surface keys in `apps/platform/lang/en/localization.php`.
|
|
- [x] T042 Update matching keys in `apps/platform/lang/de/localization.php`.
|
|
- [x] T043 Update `docs/ui-ux-enterprise-audit/page-reports/ui-006-customer-review-workspace.md` for resolve-action-first behavior, one-primary-CTA hierarchy, and honest fallbacks.
|
|
- [x] T044 Update `docs/ui-ux-enterprise-audit/page-reports/ui-040-environment-review-detail.md` for aligned next-step semantics and CTA suppression behavior.
|
|
- [x] T045 Capture screenshots under `specs/351-review-output-resolve-actions-v1/artifacts/screenshots/` for published-blocked, mutable-blocked, ready-draft, and fallback states.
|
|
- [x] T056 Add a local/testing-only ready-path fixture command in `apps/platform/app/Console/Commands/SeedReviewOutputBrowserFixture.php` plus `tenantpilot.review_output.browser_smoke_fixture` config so manual browser proof can seed a published predecessor with a linked ready successor without widening product scope.
|
|
- [x] T057 Add `apps/platform/tests/Feature/Console/TenantpilotSeedReviewOutputBrowserFixtureCommandTest.php` to verify the fixture command seeds the workspace/detail publish path honestly.
|
|
|
|
## Phase 7: Validation
|
|
|
|
**Purpose**: Prove the new mapper stays bounded and preserves current trust/safety rules.
|
|
|
|
- [x] T046 Run `cd apps/platform && ./vendor/bin/sail artisan test tests/Unit/ResolutionGuidance/Spec351ReviewOutputResolveActionMapperTest.php --compact`.
|
|
- [x] T047 Run `cd apps/platform && ./vendor/bin/sail artisan test tests/Feature/Filament/Spec351CustomerReviewWorkspaceResolveActionTest.php tests/Feature/EnvironmentReview/Spec351EnvironmentReviewResolveActionTest.php --compact`.
|
|
- [x] T048 Run `cd apps/platform && ./vendor/bin/sail php vendor/bin/pest tests/Browser/Spec351ReviewOutputResolveActionsSmokeTest.php --compact`.
|
|
- [x] T049 Run `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec347`.
|
|
- [x] T050 Run `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec349`.
|
|
- [x] T051 Run `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=Spec350`.
|
|
- [x] T052 Run `cd apps/platform && ./vendor/bin/sail artisan test --compact --filter=CustomerReviewWorkspace`.
|
|
- [x] T053 Run `cd apps/platform && ./vendor/bin/sail pint --dirty`.
|
|
- [x] T054 Run `git diff --check`.
|
|
- [x] T055 Report any unrelated broader-suite failures honestly if they remain out of scope.
|
|
- [x] T058 Run `cd apps/platform && ./vendor/bin/sail artisan test tests/Feature/Console/TenantpilotSeedReviewOutputBrowserFixtureCommandTest.php --compact`.
|
|
- [x] T059 Run `cd apps/platform && ./vendor/bin/sail artisan tenantpilot:review-output:seed-browser-fixture --no-interaction` before manual browser verification when a persistent ready-path demo is required.
|
|
|
|
## Non-Goals Checklist
|
|
|
|
- [x] NT001 Do not create a new persisted resolution entity, table, or workflow queue.
|
|
- [x] NT002 Do not roll the mapper out to provider readiness, governance inbox, or environment dashboard in this slice.
|
|
- [x] NT003 Do not invent a generic "open draft review" workflow when no deterministic target review exists.
|
|
- [x] NT004 Do not call lifecycle services directly from Blade or bypass existing source-owned Filament actions.
|
|
- [x] NT005 Do not weaken workspace/environment scope, confirmation, authorization, audit, or signed-download safety.
|
|
- [x] NT006 Do not add portal, PDF/HTML renderer, PSA, or AI follow-up work.
|