fix: eeror#83
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughCentralizes view-count updates by introducing Changes
Sequence Diagram(s)sequenceDiagram
participant View as ViewCount Component
participant API as API Route
participant DBMod as DB Module
participant DB as Database
View->>DBMod: updateViewCount(entity, id)
DBMod->>DB: UPDATE <entity_stats> SET view_count = view_count + 1, updatedAt = now() RETURNING *
DB-->>DBMod: updated row
DBMod-->>View: { view_count }
API->>DBMod: updateViewCount(entity, id)
DBMod->>DB: UPDATE <entity_stats> ...
DB-->>DBMod: updated row
DBMod-->>API: { view_count }
API-->>Client: JSON { result }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
|
🤖 Meticulous evaluated 50 user flows and took 72 visual snapshots. Meticulous has not yet run on 5c8c5e2 of the main branch and so there was nothing to compare against. If you recently setup Meticulous, this is expected. Meticulous will start reporting comparisons for new pull requests after the next commit to the main branch. If you are using Vercel, please make sure you have disabled automatic job cancellation as documented here. Last updated for commit bdf73e8. This comment will update as new commits are pushed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web-nextjs/src/components/drugInfo/view-count.tsx (1)
10-16: 🛠️ Refactor suggestion | 🟠 Major
slugis declared inViewCountPropsbut never used.Neither
ViewCountnorCountusesslug. Remove it from the type to avoid confusion.Proposed fix
type ViewCountProps = { id: number; createdAt: GetDrugBySlugReturnType["createdAt"]; updatedAt: GetDrugBySlugReturnType["updatedAt"]; entity: "agents" | "generics" | "companies" | "drugs"; - slug: string; };
🤖 Fix all issues with AI agents
In `@apps/web-nextjs/src/components/drugInfo/view-count.tsx`:
- Around line 42-49: The Count component is performing a DB mutation during
render by calling use(updateViewCount(entity, id)), which causes
double/inconsistent increments; move the updateViewCount call out of the render
path—either invoke the mutation once in the parent async server component
ViewCount (perform the update there and pass the resulting view count into
Count) or keep the previous API-route boundary and fetch the updated count from
that API inside the component; locate the use(updateViewCount(...)) call in
Count and replace it with a plain prop or a single server-side call in ViewCount
so the DB write happens exactly once.
In `@packages/db/src/queries/view-count.ts`:
- Around line 11-36: In updateViewCount, validate the incoming id (e.g., const
numericId = Number(id); if (!Number.isFinite(numericId) ||
Number.isNaN(numericId)) throw new Error(...)) before using it, then perform the
db.update as currently done (db.update(table)...where(eq(column,
numericId)).returning(...)), and after the update check the returned result
length — if result[0] is undefined/empty either create the stats row (insert
into the same table with view_count: 1 and the appropriate key column from the
tables map) or throw an explicit error/return a distinguishable value to the
caller so "no row" is not silently treated as view_count: 0; use the existing
tables map, column variable, and the db.update(...).returning call to locate
where to add the checks and the insert/upsert fallback.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web-nextjs/src/components/drugInfo/view-count.tsx (1)
11-17: 🛠️ Refactor suggestion | 🟠 Major
slugis still declared inViewCountPropsbut never used.The prop was part of the old fetch-based approach and is now dead code. Remove it from the type (and from call sites).
Proposed fix
type ViewCountProps = { id: number; createdAt: GetDrugBySlugReturnType["createdAt"]; updatedAt: GetDrugBySlugReturnType["updatedAt"]; entity: "agents" | "generics" | "companies" | "drugs"; - slug: string; };
🤖 Fix all issues with AI agents
In `@apps/web-nextjs/src/components/drugInfo/view-count.tsx`:
- Around line 43-53: The Count component is declared async but performs no
asynchronous work; remove the async keyword from the Count function declaration
(the Count component that takes newView: { view_count: number | null }) so it
returns JSX synchronously and avoids creating an unnecessary Promise/microtask
wrapper; update the Count declaration accordingly and leave the JSX return (View
count: {newView.view_count ?? 0}) unchanged.
- Around line 4-9: Remove the now-unused imports left from the prior
implementation: delete the named imports Suspense and use from "react",
ErrorBoundary from "react-error-boundary", and Skeleton from "../ui/skeleton" so
only actually used symbols remain in the import list; update the import
statements that reference these identifiers (e.g., the top-level imports
containing Suspense/use and ErrorBoundary/Skeleton) to only import what the
current component uses, and run a quick lint/TS check to ensure no unused-import
errors remain.
| import { Suspense, use } from "react"; | ||
| import { ErrorBoundary } from "react-error-boundary"; | ||
|
|
||
| import type { GetDrugBySlugReturnType } from "@/services/server/getDrugs"; | ||
|
|
||
| import { Skeleton } from "../ui/skeleton"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused imports left over from the previous implementation.
Suspense, use, ErrorBoundary, and Skeleton are no longer referenced after the refactor.
Proposed cleanup
-import { Suspense, use } from "react";
-import { ErrorBoundary } from "react-error-boundary";
import type { GetDrugBySlugReturnType } from "@/services/server/getDrugs";
-import { Skeleton } from "../ui/skeleton";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { Suspense, use } from "react"; | |
| import { ErrorBoundary } from "react-error-boundary"; | |
| import type { GetDrugBySlugReturnType } from "@/services/server/getDrugs"; | |
| import { Skeleton } from "../ui/skeleton"; | |
| import type { GetDrugBySlugReturnType } from "@/services/server/getDrugs"; |
🤖 Prompt for AI Agents
In `@apps/web-nextjs/src/components/drugInfo/view-count.tsx` around lines 4 - 9,
Remove the now-unused imports left from the prior implementation: delete the
named imports Suspense and use from "react", ErrorBoundary from
"react-error-boundary", and Skeleton from "../ui/skeleton" so only actually used
symbols remain in the import list; update the import statements that reference
these identifiers (e.g., the top-level imports containing Suspense/use and
ErrorBoundary/Skeleton) to only import what the current component uses, and run
a quick lint/TS check to ensure no unused-import errors remain.
| const Count = async ({ | ||
| newView, | ||
| }: { | ||
| newView: { view_count: number | null }; | ||
| }) => { | ||
| return ( | ||
| <p className='text-muted-foreground text-sm'> | ||
| View count: {newView.view_count} | ||
| View count: {newView.view_count ?? 0} | ||
| </p> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Count doesn't need to be async.
The component receives data via props and performs no async work. Dropping async avoids an unnecessary microtask/promise wrapper.
Proposed fix
-const Count = async ({
+const Count = ({
newView,
}: {
newView: { view_count: number | null };
}) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const Count = async ({ | |
| newView, | |
| }: { | |
| newView: { view_count: number | null }; | |
| }) => { | |
| return ( | |
| <p className='text-muted-foreground text-sm'> | |
| View count: {newView.view_count} | |
| View count: {newView.view_count ?? 0} | |
| </p> | |
| ); | |
| }; | |
| const Count = ({ | |
| newView, | |
| }: { | |
| newView: { view_count: number | null }; | |
| }) => { | |
| return ( | |
| <p className='text-muted-foreground text-sm'> | |
| View count: {newView.view_count ?? 0} | |
| </p> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In `@apps/web-nextjs/src/components/drugInfo/view-count.tsx` around lines 43 - 53,
The Count component is declared async but performs no asynchronous work; remove
the async keyword from the Count function declaration (the Count component that
takes newView: { view_count: number | null }) so it returns JSX synchronously
and avoids creating an unnecessary Promise/microtask wrapper; update the Count
declaration accordingly and leave the JSX return (View count:
{newView.view_count ?? 0}) unchanged.
1 participant
@coderabbitai
[Web] Fix View Count Error
Target: Next.js Web