refactor(admin): centralize date formatting, fix null-timestamp rendering, and correct table filter attribute#1746
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdmin SDK views centralize timestamp formatting, update table filter configuration, remove legacy date helpers, and use a shared short-date fallback in the client plan card. ChangesAdmin SDK display updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 |
Coverage Report for CI Build 29812224075Coverage remained the same at 46.201%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/sdk/admin/views/plans/details.tsx (1)
22-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider simplifying the IIFE to a concise expression.
The immediately-invoked function expression works but can be replaced with optional chaining and nullish coalescing for the same behavior.
♻️ Optional simplification
<Text size="mini"> - {(() => { - const date = timestampToDayjs(plan?.createdAt); - return date ? date.format("DD MMM YYYY") : "-"; - })()} + {timestampToDayjs(plan?.createdAt)?.format("DD MMM YYYY") ?? "-"} </Text>
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 82f02392-ccc4-46d9-8b24-54445eb5043f
📒 Files selected for processing (18)
web/sdk/admin/views/admins/columns.tsxweb/sdk/admin/views/invoices/columns.tsxweb/sdk/admin/views/organizations/details/apis/columns.tsxweb/sdk/admin/views/organizations/details/apis/details-dialog.tsxweb/sdk/admin/views/organizations/details/invoices/columns.tsxweb/sdk/admin/views/organizations/details/members/columns.tsxweb/sdk/admin/views/organizations/details/projects/columns.tsxweb/sdk/admin/views/organizations/details/tokens/columns.tsxweb/sdk/admin/views/organizations/list/columns.tsxweb/sdk/admin/views/plans/columns.tsxweb/sdk/admin/views/plans/details.tsxweb/sdk/admin/views/preferences/columns.tsxweb/sdk/admin/views/preferences/details.tsxweb/sdk/admin/views/products/columns.tsxweb/sdk/admin/views/products/prices/columns.tsxweb/sdk/admin/views/roles/columns.tsxweb/sdk/admin/views/users/list/columns.tsxweb/sdk/admin/views/webhooks/webhooks/columns.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/sdk/admin/views/invoices/columns.tsx (1)
27-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider using
timestampCellfor consistency across timestamp columns.This file wraps
formatTimestampin<Text>manually, while other files (e.g.,organizations/details/invoices/columns.tsx,products/columns.tsx) use the sharedtimestampCellhelper directly. Standardizing ontimestampCelleliminates duplicatedgetValue() as TimeStampcasts and wrapper boilerplate. The same pattern appears inorganizations/details/members/columns.tsxandorganizations/list/columns.tsx.If
<Text>provides styling that a plain string return would lose, consider updatingtimestampCellto wrap its output in<Text>so all consumers get consistent styling.♻️ Proposed refactor
- cell: ({ getValue }) => ( - <Text>{formatTimestamp(getValue() as TimeStamp)}</Text> - ), + cell: timestampCell,
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72dff9b0-3c07-4955-b8cb-703d3fe1b413
📒 Files selected for processing (15)
web/sdk/admin/utils/connect-timestamp.tsweb/sdk/admin/views/invoices/columns.tsxweb/sdk/admin/views/organizations/details/apis/details-dialog.tsxweb/sdk/admin/views/organizations/details/invoices/columns.tsxweb/sdk/admin/views/organizations/details/members/columns.tsxweb/sdk/admin/views/organizations/details/projects/columns.tsxweb/sdk/admin/views/organizations/details/tokens/columns.tsxweb/sdk/admin/views/organizations/list/columns.tsxweb/sdk/admin/views/plans/columns.tsxweb/sdk/admin/views/plans/details.tsxweb/sdk/admin/views/preferences/details.tsxweb/sdk/admin/views/products/columns.tsxweb/sdk/admin/views/products/prices/columns.tsxweb/sdk/admin/views/users/list/columns.tsxweb/sdk/admin/views/webhooks/webhooks/columns.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- web/sdk/admin/views/organizations/details/projects/columns.tsx
- web/sdk/admin/views/organizations/details/apis/details-dialog.tsx
- web/sdk/admin/views/preferences/details.tsx
- web/sdk/admin/views/users/list/columns.tsx
- web/sdk/admin/views/plans/columns.tsx
- web/sdk/admin/views/webhooks/webhooks/columns.tsx
- web/sdk/admin/views/organizations/details/tokens/columns.tsx
Summary
Unifies admin date formatting via a shared helper, fixes a null-timestamp rendering bug, and re-enables table column filters that were silently broken. Aligns the admin UI with Apsara.
Changes
DATE_FORMAT+formatTimestampinconnect-timestamp.tsas the single source for the format string and null guard; migrated 32 call sites (dropped 11YYYY-MM-DDcols, 6toLocaleString, 15dayjsimports).DD MMM YYYY; preferences keeps its time part.{ seconds: 0 }) previously rendered01 Jan 1970/Invalid Date; now show-.filterVariantwith Apsara'sfilterTypeon the older list tables.enableColumnFilter: trueto every column declaring afilterType— the required gate that lists a column in the filter menu (filterTypealone is inert). Intentional exclusions keepenableColumnFilter: false.organizations/list,users/list, and products/prices "creation date".~/adminalias; callformatTimestamp(getValue())inline.formatTimestampand fixaccessorKey(created_at→createdAt) to match the server sort key /fieldNameMapping.createdAt/expiresAtviagetValue()instead ofrow.original.plan-carduses the client SDK'sDEFAULT_DATE_SHORT_FORMAT; removed the deadgetFormattedDateStringhelper inapps/admin.Notes
DD MMM YYYYis Apsara's built-in default and matches the client SDK — canonical, not arbitrary.DataTablemigration and keptfilterVariant, which the new API ignores; it needs bothfilterTypeandenableColumnFilter: true.filterVariantalso threw 3 TS errors inproducts/columns.tsx.Test Plan
DD MMM YYYYacross all admin views; null/missing show-; affected columns filter again.tscclean on changed files; eslint shows only pre-existing warnings.SQL Safety (if your PR touches
*_repository.goorgoqu.*)N/A