refactor(web): fixed async ordering and UI state issues in project flows - #2381
refactor(web): fixed async ordering and UI state issues in project flows#2381mkumbobeaty wants to merge 4 commits into
Conversation
|
🚀 Cloud Run Preview Deployed |
There was a problem hiding this comment.
Pull request overview
Refactors and hardens the Dashboard “Projects” feature by fixing several async/correctness edge cases (create/remove/import flows), reorganizing modal components, and adding targeted test coverage. This fits into the web app’s Dashboard Projects UI/UX stability work by preventing stale UI state, duplicate submissions, and incorrect mutation ordering.
Changes:
- Refactored Projects-related modals into a
ProjectModals/folder and updated imports accordingly. - Fixed async flow correctness and UI state handling in project create/remove/import (await ordering, “in-flight” guards, stale polling/validation handling, cleanup of object URLs).
- Added comprehensive unit tests for the fixed behaviors, plus an AI review log entry and documentation pointer.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| web/src/app/features/ProjectSettings/innerPages/GeneralSettings/index.tsx | Updates ProjectRemoveModal import path after refactor. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/useProjectImport.ts | Prevents status regression on ambiguous poll responses; cleans up download object URL + DOM node. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/useProjectImport.test.ts | Adds tests for polling behavior and download cleanup. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/ProjectModals/ProjectRemoveModal.tsx | Adds data-testid passthrough and supports disabling actions while removing. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/ProjectModals/ProjectRemoveModal.test.tsx | Adds coverage for rendering and button behaviors (cancel/remove/disabled). |
| web/src/app/features/Dashboard/ContentsContainer/Projects/ProjectModals/ProjectImportErrorModal.tsx | Fixes invalid HTML structure by moving <ul> out of typography wrapper. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/ProjectModals/ProjectImportErrorModal.test.tsx | Adds coverage for modal content and callbacks. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/ProjectModals/ProjectCreatorModal.tsx | Prevents stale alias validation overwrite; awaits create result; prevents double submit. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/ProjectModals/ProjectCreatorModal.test.tsx | Adds coverage for validation, stale responses, submit success/failure, and feature/policy toggles. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/Project/types.ts | Updates onProjectRemove contract to return Promise<boolean>. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/Project/ProjectListViewItem.tsx | Stops input click propagation; disables remove while removing; updates modal import. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/Project/ProjectGridViewItem.tsx | Disables remove while removing; updates modal import. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/Project/hooks.tsx | Fixes rename blanking; fixes selection clearing logic; enforces remove mutation ordering + in-flight guard. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/Project/hooks.test.tsx | Adds tests ensuring unpublish happens before archive and modal-close behavior depends on success. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/index.tsx | Updates modal import paths after refactor. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/hooks.ts | Makes create/remove return success booleans; prevents cache eviction on failed archive/unpublish; minor cleanup. |
| web/src/app/features/Dashboard/ContentsContainer/Projects/hooks.test.ts | Adds tests covering success/failure behavior for archive + cache eviction (and related guards). |
| docs/ai-review-log.md | Adds the AI review log and a dated entry describing findings/fixes + tests added. |
| CLAUDE.md | Links to the AI review log documentation. |
Suppressed comments (3)
web/src/app/features/Dashboard/ContentsContainer/Projects/hooks.test.ts:366
- Same issue here:
act(...)doesn’t return the callback’s value. Store thehandleProjectRemoveresult in a local variable inside theactcallback.
const removed = await act(async () =>
result.current.handleProjectRemove({ ...baseProject, status: "published" })
);
web/src/app/features/Dashboard/ContentsContainer/Projects/hooks.test.ts:381
- Same issue here:
act(...)doesn’t return the callback’s value. Store thehandleProjectRemoveresult in a local variable inside theactcallback.
const removed = await act(async () =>
result.current.handleProjectRemove(baseProject)
);
web/src/app/features/Dashboard/ContentsContainer/Projects/ProjectModals/ProjectCreatorModal.tsx:217
- This component passes
data-testidtoModal, butModal’s API uses thedataTestidprop to setdata-testidon the rendered dialog. As-is, the test id won’t be applied to the modal root.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const removed = await act(async () => | ||
| result.current.handleProjectRemove(baseProject) | ||
| ); |
Overview
This Pr improved the existing project create/remove flows, focusing on async ordering and UI state consistency. Create and remove mutations are now properly awaited before closing modals or updating the Apollo cache, preventing failed operations from leaving the UI in an incorrect state. Also improved alias validation to avoid stale results, added protection against duplicate remove submissions, handled blank project names correctly, and cleaned up several smaller correctness
What I've done
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Checklist