Collection view redesign: replace table with record-based UI#23
Conversation
- Replace wide table with record list + detail drawer - Add scalable components for large schemas - Update backend logic and caching to support new data flow
|
@KushagraJaiswar02 is attempting to deploy a commit to the Yash Pouranik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughMoved dotenv initialization to startup; enhanced Redis client with retryStrategy, maxRetriesPerRequest, and an error handler; removed OTP console logs; added guards to Redis caching helpers to no-op when Redis isn't ready; added dnd-kit deps and five new frontend components, and refactored Database page to use them (default list view). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@backend/config/redis.js`:
- Line 6: Remove or guard the debug log that prints environment variable keys:
the console.log("DEBUG: ENV KEYS:", Object.keys(process.env)) line should not
run unconditionally; either delete it or wrap it behind a strict debug condition
(e.g., only when a specific debug flag is set or when NODE_ENV !== 'production')
so env keys are not exposed in normal or production logs.
In `@backend/controllers/auth.controller.js`:
- Around line 118-120: Remove plaintext OTP logging: delete the console.log
statements that print "GENERATED OTP FOR " + email + ":" + otp and the later
logs that print expected/received OTP; instead, either remove them entirely or
replace with a gated debug path (e.g., only log via a logger.debug when a secure
DEBUG flag is enabled) and always redact secrets (showing only masked values
like otp.slice(0,2) + '***'). Update the code paths around the OTP
generation/verification (references to the variables email, otp, expectedOtp,
receivedOtp and the console.log calls) to use the gated/redacted logging or no
logging at all.
In `@frontend/src/components/AddRecordDrawer.jsx`:
- Around line 68-104: handleSubmit currently normalizes Number and Boolean but
leaves Date fields as raw datetime-local strings; update the loop that iterates
over fields in handleSubmit to detect field.type === "Date" and, when value is
present, convert formattedData[field.key] to an ISO timestamp (e.g., new
Date(val).toISOString()) before submission so backend receives consistent ISO
datetimes; keep existing validation flow (set newErrors and call
setErrors/onSubmit) and ensure you reference formattedData, formData, fields,
setErrors, and onSubmit when making the change.
- Around line 44-51: The empty conditional in handleChange checking if (type ===
"Number") is dead code; remove the entire if block (including the comment) or
replace it with the intended behavior: convert non-empty numeric input to a
Number while leaving empty/partial inputs as strings before validation. Update
the handleChange function to either drop the type === "Number" branch entirely
or implement the conversion logic (preserve "-" and "." by only converting when
the value is a valid numeric string) and ensure finalValue is used consistently
afterward.
In `@frontend/src/components/CollectionTable.jsx`:
- Around line 78-161: The columns useMemo in CollectionTable captures onView
inside a cell renderer but the dependency array only lists [activeCollection,
onDelete]; update the useMemo dependency array to include onView (e.g.,
[activeCollection, onDelete, onView]) so the columns (and the cell that calls
onView) are recreated when onView changes; ensure you modify the dependency
array next to the columns declaration in CollectionTable.
In `@frontend/src/pages/Database.jsx`:
- Around line 311-317: The RecordList prop onDelete is passed as
onDelete={fetchShowModal} but RecordList does not use it; either remove the
unused prop from the parent call (remove onDelete={fetchShowModal} where
RecordList is rendered) or implement deletion handling inside RecordList by
wiring the onDelete prop to a delete UI (e.g., add a delete button in
RecordList's item renderer that calls props.onDelete(recordId) and ensure
fetchShowModal accepts that id), and update RecordList's prop signature to
reflect usage if implemented.
- Around line 163-174: The ConfirmationModal is conditionally rendered inside
TableView but its state (showModal, selectedId, setShowModal, handleDelete)
lives in the Database component, so move the <ConfirmationModal ... /> JSX out
of TableView and place it at the Database component level (alongside where
viewMode is used) so it always renders regardless of which child view is shown;
keep the same props (open={showModal}, title, message, onConfirm={() => {
handleDelete(selectedId); setShowModal(false); }}, onCancel={() =>
setShowModal(false)}) and remove the ConfirmationModal instance from TableView
to avoid duplicate/missing modals when other views like RecordList invoke
fetchShowModal/onDelete.
🧹 Nitpick comments (7)
frontend/src/components/RecordList.jsx (2)
1-2: Remove unused imports.
List,MoreHorizontal, andCalendarare imported but never used in this component. Additionally, the explicitReactimport is not required for JSX in React 19.Suggested fix
-import React from "react"; -import { List, MoreHorizontal, Calendar, ArrowRight } from "lucide-react"; +import { ArrowRight } from "lucide-react";
4-14: UnusedonDeleteprop and misleading comment.
The
onDeleteprop is accepted but never used. If delete functionality isn't needed in list view, consider removing it from the signature to avoid confusion.The comment on line 5 says "skip _id and system fields" but the implementation doesn't filter them out—it simply takes the first 3 fields from the model.
Suggested fix if system fields should be skipped
export default function RecordList({ data, activeCollection, onView, onDelete }) { - // Helper to get important fields (skip _id and system fields) + // Helper to get preview fields (first 3 from model) const getPreviewFields = (record) => { if (!activeCollection?.model) return []; - // Take first 3 fields from model - return activeCollection.model.slice(0, 3).map(field => ({ + // Filter out system fields and take first 3 + const userFields = activeCollection.model.filter( + field => !['_id', 'createdAt', 'updatedAt'].includes(field.key) + ); + return userFields.slice(0, 3).map(field => ({ key: field.key, value: record[field.key], type: field.type })); };frontend/src/components/RowDetailDrawer.jsx (2)
1-2: Minor: Remove unnecessary React import.The explicit
Reactimport is not required for JSX in React 19.Suggested fix
-import React from "react"; -import { X, Calendar, Type, Hash, ToggleLeft, FileText, Link as LinkIcon, AlertCircle } from "lucide-react"; +import { X, Calendar, Type, Hash, ToggleLeft, FileText, Link as LinkIcon, AlertCircle } from "lucide-react";
30-32: Consider handling very large objects gracefully.
JSON.stringify(value, null, 2)on deeply nested or large objects could produce very long output. While the CSS limits the preview to 200px height with scrolling, consider adding a size check or truncation for extremely large values to prevent potential performance issues.frontend/src/components/DatabaseSidebar.jsx (1)
56-69: Consider adding keyboard accessibility to collection items.The collection items are clickable
<div>elements but lack keyboard navigation support. For better accessibility, consider addingtabIndex="0",role="button", and anonKeyDownhandler for Enter/Space key activation.Suggested fix
collections.map((c) => ( <div key={c._id} onClick={() => setActiveCollection(c)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setActiveCollection(c); + } + }} + tabIndex={0} + role="button" className={`collection-item ${activeCollection?._id === c._id ? "active" : "" }`} >frontend/src/components/AddRecordDrawer.jsx (1)
25-31: Potential unexpected form reset iffieldsreference changes.The effect depends on both
isOpenandfields. If the parent component creates a newfieldsarray reference on each render (even with identical content), this will reset the form while the user is typing.Consider removing
fieldsfrom the dependency array or using a ref to track if this is the initial open:Suggested fix
// Reset form when drawer opens/closes or fields change useEffect(() => { if (isOpen) { setFormData({}); setErrors({}); } - }, [isOpen, fields]); + }, [isOpen]);frontend/src/pages/Database.jsx (1)
155-195: Consider extracting inner components or converting to render functions.
TableView,JsonView, andSkeletonLoaderare recreated on every render since they're defined insideDatabase. This can cause subtle issues with reconciliation and prevents memoization.Since these components rely on parent scope variables, consider converting them to inline JSX or extracting with explicit props:
Option 1: Inline JSX in the render
- ) : viewMode === "table" ? ( - <TableView /> + ) : viewMode === "table" ? ( + <div className="table-container-wrapper" style={{ height: "100%", overflow: "hidden" }}> + <CollectionTable + data={data} + activeCollection={activeCollection} + onDelete={fetchShowModal} + onView={setSelectedRecord} + /> + </div>Option 2: Extract to separate file with explicit props
Extract
TableViewto a separate component file that accepts all needed props explicitly, enabling React to properly optimize re-renders.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
yash-pouranik
left a comment
There was a problem hiding this comment.
@KushagraJaiswar02 resolve the issues pointed by code rabbit that are essential and
please ensure there are no linting error by running this command npm run lint in /frontend dir and test backend by running npm test in backend directory
for more info read Contribution.md
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/pages/Database.jsx (1)
248-272: Add accessible labels/state to icon-only view toggles.Line 249–271 are icon-only buttons; screen readers need
aria-label, andaria-pressedimproves toggle semantics.♿ Suggested accessibility tweaks
<button className={`toggle-btn ${viewMode === "list" ? "active" : "" }`} onClick={() => setViewMode("list")} title="List View" + aria-label="List view" + aria-pressed={viewMode === "list"} > <ListIcon size={16} /> </button> <button className={`toggle-btn ${viewMode === "table" ? "active" : "" }`} onClick={() => setViewMode("table")} title="Table View (Advanced)" + aria-label="Table view" + aria-pressed={viewMode === "table"} > <TableIcon size={16} /> </button> <button className={`toggle-btn ${viewMode === "json" ? "active" : "" }`} onClick={() => setViewMode("json")} title="JSON View" + aria-label="JSON view" + aria-pressed={viewMode === "json"} > <Code size={16} /> </button>
♻️ Duplicate comments (1)
frontend/src/pages/Database.jsx (1)
204-222: Close the detail drawer when the selected record is deleted.With Line 205 tying the drawer to
selectedRecord, deleting that same record leaves the drawer open with stale data. Consider clearingselectedRecordinhandleDeletewhen the deleted_idmatches.🛠️ Proposed fix
const handleDelete = async (id) => { try { await axios.delete( `${API_URL}/api/projects/${projectId}/collections/${activeCollection.name}/data/${id}`, { headers: { Authorization: `Bearer ${token}` }, } ); setData((prev) => prev.filter((item) => item._id !== id)); + setSelectedRecord((prev) => (prev?._id === id ? null : prev)); toast.success("Document deleted"); } catch (err) { console.error(err); toast.error("Failed to delete document"); } };
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@frontend/src/components/AddRecordDrawer.jsx`:
- Around line 74-91: The Boolean and Date coercions in AddRecordDrawer.jsx
currently force-empty booleans to false and call toISOString() without
validation; update the conversion logic so for field.type === "Boolean" you only
set formattedData[field.key] when val !== "" && val !== undefined (preserve
undefined for optional/unset booleans), and for field.type === "Date" validate
the date by constructing new Date(val) and checking !isNaN(date.getTime())
before calling date.toISOString(); if invalid, leave formattedData[field.key]
undefined or handle as an error instead, and confirm whether converting the
local datetime to UTC via toISOString() is the intended behavior.
In `@frontend/src/pages/Database.jsx`:
- Around line 201-206: The RowDetailDrawer stays open with stale data after
deleting a record because handleDelete removes the item from the data array but
doesn't clear selectedRecord; update the handleDelete function to check if the
deleted record's id equals selectedRecord?.id (or selectedRecord depending on id
shape) and call setSelectedRecord(null) when they match so the drawer closes;
refer to handleDelete, selectedRecord, setSelectedRecord and RowDetailDrawer to
locate and implement this conditional clearing.
- Around line 96-102: When activeCollection changes in the useEffect (the effect
that currently calls setSearchParams, setSelectedRecord, fetchData, and
setIsSidebarOpen), also reset the delete-confirmation state by closing the modal
and clearing the pending ID: call the state setters that control showModal and
selectedId (or their equivalents) to set showModal false and selectedId
null/undefined so any in-flight delete confirmation is cleared before fetching
the new collection and preventing deletes on the wrong collection.
This PR overhauls the collection page to remove the wide-table layout.
Key changes:
This fixes column overflow issues and improves usability for large schemas.
Summary by CodeRabbit
New Features
Chores
Reliability
Privacy
✏️ Tip: You can customize this high-level summary in your review settings.