feat: history and analytics - #88
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds an authenticated request-history page backed by Supabase persistence and retrieval, with localized empty/error states, a paginated analytics table, alert styling, and component and utility tests. ChangesRequest history
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant HistoryPage
participant getAuthenticatedUser
participant getRequestHistory
participant Supabase
participant HistoryList
HistoryPage->>getAuthenticatedUser: authenticate route request
HistoryPage->>getRequestHistory: load request history
getRequestHistory->>Supabase: query ordered request history
Supabase-->>getRequestHistory: rows and query status
getRequestHistory-->>HistoryPage: entries and hasError
HistoryPage->>HistoryList: render entries when available
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/components/history-list/history-table-row.tsx (1)
21-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
getMethodColortracked via TODO.Self-acknowledged duplication with the upcoming
feature/4-swagger-viewerimplementation; no action needed beyond the existing TODO before that branch merges.As per path instructions, "Flag duplicate implementations of the same logic in different files."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/history-list/history-table-row.tsx` around lines 21 - 35, Remove the local METHOD_COLOR mapping and getMethodColor function from this component, and import and reuse getMethodColor from `@/utils/swagger-editor/get-method-color`. Remove the obsolete TODO once the shared implementation is used.Source: Path instructions
src/utils/history/get-request-history.ts (1)
22-25: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftNo limit on the query while the client already assumes full-list pagination.
The query fetches the entire
request_historytable for the user on every page load (no.limit()), andHistoryListthen slices client-side. As history grows this bloats the SSR payload and the DB query cost.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/history/get-request-history.ts` around lines 22 - 25, Update the request-history query in getRequestHistory to apply a server-side limit and return only the page-sized results expected by HistoryList, rather than fetching the full table for client-side slicing. Preserve the existing user filtering, descending created_at ordering, and pagination behavior while passing the appropriate page parameters to the Supabase query.src/app/[locale]/history/page.test.tsx (1)
45-52: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert that a failed load does not render
HistoryList.This test only checks for the error alert. Add a negative assertion for
history-list-stub; otherwise the current error-plus-empty-table behavior can pass the suite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`[locale]/history/page.test.tsx around lines 45 - 52, Update the “shows an error alert when the history fails to load” test around HistoryPage to also assert that the history-list-stub element is not present after rendering. Keep the existing error-alert assertion unchanged and verify the failed load renders no HistoryList.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`[locale]/history/page.tsx:
- Around line 20-26: The history page should render mutually exclusive error,
empty-success, and non-empty-success states. Update the rendering around the
hasError condition in src/app/[locale]/history/page.tsx:20-26 so HistoryList is
omitted when loading fails; then update the error-case test in
src/app/[locale]/history/page.test.tsx:45-52 to assert that history-list-stub is
absent.
- Around line 12-14: Update the history page’s getRequestHistory flow to catch
failures from both createClient and the query, ensuring any Supabase client or
cookies error returns the localized history error state with empty entries and
hasError true instead of rejecting page rendering.
- Line 5: Replace the static HistoryList import in the history page with a
next/dynamic lazy import, preserving the component’s existing usage and enabling
SSR so initial rows can remain server-rendered.
In `@src/components/history-empty-state/history-empty-state.tsx`:
- Around line 14-18: The HistoryEmptyState component currently links only to the
Editor; add a second localized AppLinkButton for the Viewer using the existing
Viewer route and translation keys, and add the corresponding Viewer translation
in both locale files while preserving the existing Editor link.
---
Nitpick comments:
In `@src/app/`[locale]/history/page.test.tsx:
- Around line 45-52: Update the “shows an error alert when the history fails to
load” test around HistoryPage to also assert that the history-list-stub element
is not present after rendering. Keep the existing error-alert assertion
unchanged and verify the failed load renders no HistoryList.
In `@src/components/history-list/history-table-row.tsx`:
- Around line 21-35: Remove the local METHOD_COLOR mapping and getMethodColor
function from this component, and import and reuse getMethodColor from
`@/utils/swagger-editor/get-method-color`. Remove the obsolete TODO once the
shared implementation is used.
In `@src/utils/history/get-request-history.ts`:
- Around line 22-25: Update the request-history query in getRequestHistory to
apply a server-side limit and return only the page-sized results expected by
HistoryList, rather than fetching the full table for client-side slicing.
Preserve the existing user filtering, descending created_at ordering, and
pagination behavior while passing the appropriate page parameters to the
Supabase query.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aad29674-7c03-41c5-9c3a-3d4ef3832de4
📒 Files selected for processing (13)
src/app/[locale]/history/page.test.tsxsrc/app/[locale]/history/page.tsxsrc/components/history-empty-state/history-empty-state.test.tsxsrc/components/history-empty-state/history-empty-state.tsxsrc/components/history-list/history-list.test.tsxsrc/components/history-list/history-list.tsxsrc/components/history-list/history-table-header.tsxsrc/components/history-list/history-table-row.tsxsrc/i18n/messages/en.jsonsrc/i18n/messages/ru.jsonsrc/theme/components.tssrc/utils/history/get-request-history.test.tssrc/utils/history/get-request-history.ts
| import { getTranslations } from 'next-intl/server'; | ||
|
|
||
| import { HistoryEmptyState } from '@/components/history-empty-state/history-empty-state'; | ||
| import { HistoryList } from '@/components/history-list/history-list'; |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## File map\n'
git ls-files 'src/app/[locale]/history/page.tsx' 'src/components/history-list/history-list.*' 'src/app/**/page.tsx' | sed 's#^`#-` #'
printf '\n## Outline: history page\n'
ast-grep outline src/app/[locale]/history/page.tsx --view expanded || true
printf '\n## Outline: history list component\n'
ast-grep outline src/components/history-list/history-list.tsx --view expanded || true
printf '\n## Relevant route imports/usages\n'
rg -n --glob 'src/app/**/page.tsx' "HistoryList|dynamic\\(|next/dynamic|history-list" src/app || true
printf '\n## History page contents (numbered)\n'
wc -l src/app/[locale]/history/page.tsx
cat -n src/app/[locale]/history/page.tsx | sed -n '1,220p'
printf '\n## History list contents (numbered, first 260 lines)\n'
wc -l src/components/history-list/history-list.tsx
cat -n src/components/history-list/history-list.tsx | sed -n '1,260p'Repository: AlyaEngineer/swagger-editor-app
Length of output: 4494
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Helper outlines\n'
ast-grep outline src/utils/auth/get-authenticated-user.ts --view expanded || true
ast-grep outline src/utils/history/get-request-history.ts --view expanded || true
printf '\n## Auth helper\n'
wc -l src/utils/auth/get-authenticated-user.ts
cat -n src/utils/auth/get-authenticated-user.ts | sed -n '1,220p'
printf '\n## History helper\n'
wc -l src/utils/history/get-request-history.ts
cat -n src/utils/history/get-request-history.ts | sed -n '1,260p'Repository: AlyaEngineer/swagger-editor-app
Length of output: 2671
Lazy-load HistoryList on the history route.
src/app/[locale]/history/page.tsx still statically imports the table component, which violates the History & Analytics lazy-loading requirement. Switch this to next/dynamic here; keep SSR on if you still want the initial rows in the server-rendered HTML.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/`[locale]/history/page.tsx at line 5, Replace the static HistoryList
import in the history page with a next/dynamic lazy import, preserving the
component’s existing usage and enabling SSR so initial rows can remain
server-rendered.
Source: Path instructions
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/utils/history/get-request-history.ts (1)
6-9: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftBound the history query for pagination.
select('*')retrieves every history row on each SSR request. As a user’s history grows, database, network, and rendering costs grow without bound; pass page/limit parameters and use Supabase range pagination (or enforce retention).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/history/get-request-history.ts` around lines 6 - 9, Update the request-history query in getRequestHistory to accept page and limit parameters and apply Supabase range pagination with .range() after ordering by created_at. Ensure the calculated range returns only the requested page and retains the existing data/error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/history/save-request-history.ts`:
- Around line 5-29: Wrap the Supabase operations in both saveRequestHistory in
src/utils/history/save-request-history.ts (lines 5-29) and the corresponding
getRequestHistory path in src/utils/history/get-request-history.ts (lines 4-14)
with try/catch handling. Preserve the existing unauthenticated skip and { error
} checks, while catching rejected auth or query-builder calls, logging or
returning the established history error state as appropriate, and preventing
failures from propagating into the request path.
---
Nitpick comments:
In `@src/utils/history/get-request-history.ts`:
- Around line 6-9: Update the request-history query in getRequestHistory to
accept page and limit parameters and apply Supabase range pagination with
.range() after ordering by created_at. Ensure the calculated range returns only
the requested page and retains the existing data/error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 039233fb-d990-4330-85dc-b3a2f53b4b0a
📒 Files selected for processing (7)
src/components/history-list/history-list.test.tsxsrc/components/history-list/history-list.tsxsrc/components/history-list/history-table-row.tsxsrc/utils/history/get-request-history.tssrc/utils/history/history-types.tssrc/utils/history/save-request-history.test.tssrc/utils/history/save-request-history.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/history-list/history-table-row.tsx
- src/components/history-list/history-list.test.tsx
- src/components/history-list/history-list.tsx
Summary
Implemented the History & Analytics feature for authenticated users. Added a server-side rendered history page that displays executed API requests with analytics, including an empty state and error handling.
Changes Made
TODO
Replace the local
METHOD_COLORconstant andgetMethodColor()function insrc/components/history-list/history-table-row.tsxwith imports from@/utils/swagger-editor/after merging thefeature/4-swagger-viewer branch.Related issue
Closes #33
Feature
Feature 5: History and Analytics
Acceptance criteria
Feature 1: App Header
Feature 2: Sign In / Sign Up
Feature 3: Swagger Editor
Feature 4: Swagger Viewer
Feature 5: History and Analytics
Feature 6: About Page
Feature 7: General Requirements
Feature 8: YouTube Video
Screenshot
Type of change
Checklist
any/@ts-ignoreSummary by CodeRabbit