bugfix: deleted document still shown until refresh #1705
Conversation
|
@AshishViradiya153 is attempting to deploy a commit to the mftsio Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe updates refine cache mutation and optimistic UI update logic for document deletion and movement, ensuring correct handling of different data shapes and query parameters. Breadcrumb key assignments were improved for React fragments and dropdown items. No exported function signatures were changed, and no control flow or error handling logic was altered. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Cache
participant Server
User->>UI: Delete Document/Folder
UI->>Server: Send delete request
Server-->>UI: Confirm deletion
UI->>Cache: Mutate cache
alt Viewing folder
Cache->>Cache: Mutate /documents/folder/[folderPathName]
else General documents view
Cache->>Cache: Mutate /documents with query params (search, sort, page, limit)
end
UI->>User: Update UI to reflect deletion
sequenceDiagram
participant User
participant UI
participant Cache
participant Server
User->>UI: Move Document(s)
UI->>Server: Send move request
Server-->>UI: Confirm move
UI->>Cache: Mutate cache
alt Cache data is { documents: [...] }
Cache->>Cache: Filter moved docs from documents array
else Cache data is array
Cache->>Cache: Filter moved docs from array
end
UI->>User: Update UI to reflect move
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
components/layouts/breadcrumb.tsx (1)
296-315: Remove redundant keys inside the Fragment for cleaner markup
React.Fragmentalready owns the key for every iteration, so the nestedBreadcrumbItem key={"item-${index}"}duplicates information and can trigger “key prop on non-list element” lint warnings.- <BreadcrumbItem key={`item-${index}`}> + <BreadcrumbItem>This keeps a single source of truth for reconciliation keys.
components/documents/actions/delete-documents-modal.tsx (1)
142-167: Query-string construction logic duplicated across componentsThe same “build query string from
search|sort|page|limit” block now exists here and indocument-card.tsx. Extracting it to a tiny helper (e.g.lib/utils/buildDocumentQuery.ts) removes duplication and ensures both callers stay in sync with future param additions.No functional bug – purely a maintainability gain.
components/documents/document-card.tsx (2)
134-145: Query-string assembly duplicated & a minor edge-caseThis block is identical to the logic added in the delete-modal. Moving the code to a shared util (see previous comment) avoids divergence.
Edge-case: when neither
searchnorsortis present but the user is on page > 1, pagination params are silently dropped, so cache keys for pages > 1 won’t be mutated. Consider always appendingpage/limitwhen they differ from defaults.
155-178: Safe-guard whencurrentData.documentsis missing
else if (currentData.documents)assumes truthiness; if the field exists but isundefined, the clause is skipped and the function returns the unfiltered data (good) but still evaluates the finalreturn currentDataafter running an unnecessary branch. Compact the logic and add anArray.isArrayguard to prevent surprises.-} else if (currentData.documents) { +} else if (Array.isArray(currentData?.documents)) {Minor, yet avoids a potential runtime error if
documentsis not an array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/documents/actions/delete-documents-modal.tsx(2 hunks)components/documents/document-card.tsx(2 hunks)components/layouts/breadcrumb.tsx(8 hunks)lib/documents/move-documents.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
components/layouts/breadcrumb.tsx (1)
components/ui/dropdown-menu.tsx (1)
DropdownMenuItem(192-192)
components/documents/document-card.tsx (1)
lib/types.ts (1)
DocumentWithLinksAndLinkCountAndViewCount(26-39)
🪛 Biome (1.9.4)
lib/documents/move-documents.ts
[error] 29-29: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
components/layouts/breadcrumb.tsx (1)
54-62: Composite key is a solid improvement
Switching from the bareindexto a composite${folder.path}-${index}key avoids subtle re-ordering bugs when folders with identical indexes are rendered in different lists.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
2 participants
Summary by CodeRabbit