feat:updates for year in review #1952
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a "2025 Recap" sidebar entry and a Yearly Recap feature: banner, modal (multi-slide with sharing/download), a 3D globe component, backend Tinybird pipeline and pipe wrapper, updated year-in-review logic and API route, dashboard integration, CSS animation, localhost geo update, and html2canvas dependency. Changes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (12)
components/sidebar/app-sidebar.tsx (1)
217-222: Consider making the year dynamic for future reuse.The implementation is correct. However, the hardcoded "2025" will require manual updates annually if this becomes a recurring feature.
+const currentYear = new Date().getFullYear(); + const data = { navMain: [ // ... other items { - title: "2025 Recap", + title: `${currentYear} Recap`, url: "/dashboard?openRecap=true", icon: SparklesIcon, current: false, }, ], };styles/globals.css (1)
5-18: Consider consolidating utility layers.The animation is well-defined. However, there's another
@layer utilitiesblock at line 119. Consider moving this animation into the existing utilities layer for better organization.-@layer utilities { - @keyframes fade-in { - from { - opacity: 0; - } - to { - opacity: 1; - } - } - - .animate-fade-in { - animation: fade-in 0.5s ease-in-out; - } -} - @layer base { /* ... existing base styles ... */ } @layer utilities { .touch-action-manipulation { touch-action: manipulation; } /* ... existing utilities ... */ + + @keyframes fade-in { + from { opacity: 0; } + to { opacity: 1; } + } + + .animate-fade-in { + animation: fade-in 0.5s ease-in-out; + } }components/sidebar/nav-main.tsx (1)
63-63: Consider using a flag property instead of string comparison.Hardcoding the title check creates coupling and will break if the title changes. Add a
highlightorfeaturedproperty to the NavItem interface instead.In
NavIteminterface (line 29), add:featured?: boolean;Then update the styling logic:
-item.title === "2025 Recap" && "text-orange-500 dark:text-orange-400 hover:text-orange-700 dark:hover:text-orange-300", +item.featured && "text-orange-500 dark:text-orange-400 hover:text-orange-700 dark:hover:text-orange-300",And in
app-sidebar.tsx, addfeatured: trueto the recap nav item.components/yearly-recap/globe.tsx (1)
24-28: Add resize handling for responsive canvas.The canvas dimensions are set once on mount. If the container resizes (e.g., window resize, sidebar toggle), the globe will appear distorted or cropped.
useEffect(() => { const canvas = canvasRef.current; if (!canvas) return; const ctx = canvas.getContext("2d"); if (!ctx) return; - // Set canvas size - const width = canvas.clientWidth; - const height = canvas.clientHeight; - canvas.width = width; - canvas.height = height; + // Set canvas size with resize handler + const updateCanvasSize = () => { + canvas.width = canvas.clientWidth; + canvas.height = canvas.clientHeight; + }; + updateCanvasSize(); + + window.addEventListener("resize", updateCanvasSize); // ... rest of the code ... return () => { cancelAnimationFrame(animationId); + window.removeEventListener("resize", updateCanvasSize); }; }, []);components/yearly-recap/yearly-recap-banner.tsx (2)
24-29: Remove unusedPeachPaperSVGcomponent.This SVG component is defined but only referenced in commented-out code (lines 100, 106). Consider removing it to reduce dead code.
96-131: Clean up commented-out code before merging.Lines 100 and 106 contain commented-out JSX. If these decorative elements are not needed, remove them entirely rather than leaving commented code.
components/yearly-recap/yearly-recap-modal.tsx (4)
10-35: Remove unused icon imports.Several icons appear to be imported but not used in the component:
Files,Folder,Paperclip,StickyNote,Grid3x3,Circle. Removing unused imports keeps the bundle size smaller.
486-489: Arbitrary fallback calculation for cities count.The fallback
Math.round(countriesCount * 1.5)for cities seems arbitrary and could mislead users if actual city data isn't available. Consider showing "N/A" or hiding the cities card when data is unavailable.function GlobalFootprintSlide({ stats }: { stats: YearlyRecapStats }) { const countriesCount = stats.uniqueCountries.length; - const citiesCount = stats.uniqueCities?.length || Math.round(countriesCount * 1.5); + const citiesCount = stats.uniqueCities?.length; const distanceTraveled = Math.round(countriesCount * 2500);Then conditionally render the cities card only when
citiesCountis available.
307-310: Remove commented-out code blocks.Multiple sections have commented-out JSX (lines 307-310, 390-392, 648-658). Clean these up before merging to improve code readability.
Also applies to: 390-392, 648-658
164-185: Image capture may fail silently iftoBlobreturns null.
canvas.toBlob()can returnnullif the canvas is tainted or if there's an encoding error. The promise resolves withnullwithout any error indication.return new Promise((resolve) => { canvas.toBlob((blob) => { - resolve(blob); + if (blob) { + resolve(blob); + } else { + resolve(null); + } }, "image/png"); }); } catch (error) { console.error("Error capturing image:", error); toast.error("Failed to capture image"); return null; }The current code handles this, but consider adding a timeout or explicit null check with user feedback in
handleDownloadwhenblobis null (line 200 already checks this).pages/api/teams/[teamId]/yearly-recap.ts (1)
14-16: Add radix parameter toparseInt.Always specify the radix to avoid unexpected parsing behavior with leading zeros.
const year = req.query.year - ? parseInt(req.query.year as string) + ? parseInt(req.query.year as string, 10) : undefined;lib/year-in-review/get-stats.ts (1)
9-11: Consider explicit Date constructor for consistency.The string-based date parsing (
new Date("2025-01-01")) is parsed as UTC midnight, which is generally fine here. However, usingnew Date(currentYear, 0, 1)would be more explicit and avoids potential browser/Node inconsistencies in date string parsing.const currentYear = year || new Date().getFullYear(); -const yearStart = new Date(`${currentYear}-01-01`); -const yearEnd = new Date(`${currentYear + 1}-01-01`); +const yearStart = new Date(Date.UTC(currentYear, 0, 1)); +const yearEnd = new Date(Date.UTC(currentYear + 1, 0, 1));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
components/sidebar/app-sidebar.tsx(2 hunks)components/sidebar/nav-main.tsx(1 hunks)components/yearly-recap/globe.tsx(1 hunks)components/yearly-recap/index.ts(1 hunks)components/yearly-recap/yearly-recap-banner.tsx(1 hunks)components/yearly-recap/yearly-recap-modal.tsx(1 hunks)lib/tinybird/endpoints/get_total_team_duration.pipe(1 hunks)lib/tinybird/pipes.ts(1 hunks)lib/utils/geo.ts(1 hunks)lib/year-in-review/get-stats.ts(4 hunks)package.json(1 hunks)pages/api/teams/[teamId]/yearly-recap.ts(1 hunks)pages/dashboard.tsx(2 hunks)styles/globals.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
pages/dashboard.tsx (1)
components/yearly-recap/yearly-recap-banner.tsx (1)
YearlyRecapBanner(49-134)
components/yearly-recap/yearly-recap-banner.tsx (3)
context/team-context.tsx (1)
useTeam(87-87)lib/utils.ts (1)
fetcher(48-62)components/yearly-recap/yearly-recap-modal.tsx (1)
YearlyRecapModal(124-460)
components/yearly-recap/yearly-recap-modal.tsx (5)
components/yearly-recap/index.ts (1)
YearlyRecapModal(2-2)lib/utils.ts (2)
fetcher(48-62)cn(18-20)components/ui/dialog.tsx (2)
Dialog(149-149)DialogContent(151-151)components/ui/button.tsx (1)
Button(71-71)components/ui/calendar.tsx (1)
Calendar(70-70)
🔇 Additional comments (13)
components/yearly-recap/index.ts (1)
1-3: LGTM - clean barrel exports.Standard pattern for module exports. The
Globecomponent fromglobe.tsxappears to be intentionally internal to this module.lib/utils/geo.ts (1)
25-32: Confirm this change is intentional.This changes the localhost fallback geo data from Munich to San Francisco. While this only affects local development, verify this is the intended default for the team's testing environment.
lib/tinybird/endpoints/get_total_team_duration.pipe (1)
1-34: LGTM - well-structured Tinybird pipeline.The pipeline correctly:
- Uses parameterized inputs avoiding injection risks
- Applies inclusive/exclusive time bounds consistently
- Filters out invalid country values
- Handles empty documentIds gracefully (returns zero/empty)
components/yearly-recap/globe.tsx (1)
80-128: LGTM - clean animation loop with proper cleanup.The animation implementation is well-structured:
- Rotation and projection math is correct
- Depth sorting provides proper occlusion
cancelAnimationFramecleanup prevents memory leaks- Performance is acceptable for 2500 points
package.json (1)
99-99: LGTM - appropriate dependency for the share/download feature.html2canvas is the standard library for DOM-to-canvas capture. Version 1.4.1 is the latest release, and the caret range allows automatic patch updates for security fixes.
pages/dashboard.tsx (1)
26-27: LGTM!The
YearlyRecapBannerintegration is clean and well-positioned above the dashboard header. The banner handles its own state and data fetching, keeping the dashboard component simple.Also applies to: 127-127
lib/tinybird/pipes.ts (1)
201-213: LGTM!The new
getTotalTeamDurationpipe follows the established patterns in the file. The Zod schema properly types the parameters and return data, and the comma-separated documentIds pattern is consistent with other pipes likeexcludedLinkIds.lib/year-in-review/get-stats.ts (6)
22-25: Documents query fetches all team documents regardless of year.This query fetches all document IDs for the team without a year filter. While this is intentional since Tinybird filters by
since/untilparameters, consider if filtering documents bycreatedAtwithin the year would be more efficient for teams with many historical documents. The current approach is acceptable if most teams have reasonable document counts.
123-153: Verify privacy implications of exposing viewer email.The
mostActiveViewerquery returns the viewer's email address, which will be exposed in the API response and potentially displayed in the UI. Ensure this aligns with your privacy policy and doesn't create GDPR/CCPA compliance concerns. Consider whether this data should be anonymized (e.g., masked email likej***@example.com) or require explicit user consent before display.
64-65: Good handling of PostgreSQL bigint conversion.The raw SQL queries correctly type
viewCountand convert results usingNumber()(lines 197, 205, 212). PostgreSQLCOUNTreturnsbigint, and this conversion handles it properly.Also applies to: 92-92, 124-129
180-185: Good defensive handling for country data.The optional chaining and fallback to empty array properly handle cases where Tinybird data may be missing or incomplete.
156-170: Consistent return shape for early exit.The early return correctly includes all new fields (
year,totalDatarooms,mostActiveViewer) with appropriate default values, maintaining a consistent API contract.
187-217: Well-structured return with proper null handling.The return statement correctly aggregates all statistics with appropriate null checks and type conversions. The structure is clean and consistent.
| // Check if URL has openRecap parameter - open modal regardless of stats | ||
| useEffect(() => { | ||
| if (router.isReady && router.query.openRecap === "true" && teamInfo?.currentTeam?.id) { | ||
| setShowModal(true); | ||
| // Remove the query parameter | ||
| const { openRecap, ...rest } = router.query; | ||
| router.replace({ pathname: router.pathname, query: rest }, undefined, { | ||
| shallow: true, | ||
| }); | ||
| } | ||
| }, [router.isReady, router.query.openRecap, teamInfo?.currentTeam?.id, router]); |
There was a problem hiding this comment.
Potential infinite loop due to router in dependency array.
Including the entire router object in the dependency array can cause the effect to re-run on every render since router is a new object reference each time. This may lead to an infinite loop or unnecessary re-renders.
useEffect(() => {
if (router.isReady && router.query.openRecap === "true" && teamInfo?.currentTeam?.id) {
setShowModal(true);
// Remove the query parameter
const { openRecap, ...rest } = router.query;
router.replace({ pathname: router.pathname, query: rest }, undefined, {
shallow: true,
});
}
- }, [router.isReady, router.query.openRecap, teamInfo?.currentTeam?.id, router]);
+ }, [router.isReady, router.query.openRecap, teamInfo?.currentTeam?.id]);| mostActiveViewer: { | ||
| email: string; | ||
| name: string | null; | ||
| viewCount: number; | ||
| } | null; |
There was a problem hiding this comment.
Privacy concern: Displaying viewer email address.
The mostActiveViewer.email is displayed directly to users in the UI (line 622). This could be a privacy concern depending on your data handling policies, as it exposes PII of viewers to document owners.
Consider whether displaying the full email is necessary, or if you should mask it (e.g., j***@example.com) or use only the name when available.
| const handleShareLinkedIn = async () => { | ||
| const text = getShareText(); | ||
| window.open( | ||
| `https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent("https://www.papermark.com/")}&summary=${encodeURIComponent(text)}`, | ||
| "_blank" | ||
| ); | ||
| }; |
There was a problem hiding this comment.
LinkedIn share URL parameter may not work as intended.
The LinkedIn share-offsite endpoint doesn't support the summary parameter. LinkedIn will only use the url parameter and fetch Open Graph metadata from that URL.
const handleShareLinkedIn = async () => {
- const text = getShareText();
window.open(
- `https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent("https://www.papermark.com/")}&summary=${encodeURIComponent(text)}`,
+ `https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent("https://www.papermark.com/")}`,
"_blank"
);
};If you need custom text sharing on LinkedIn, consider using their Post API with OAuth instead.
📝 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 handleShareLinkedIn = async () => { | |
| const text = getShareText(); | |
| window.open( | |
| `https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent("https://www.papermark.com/")}&summary=${encodeURIComponent(text)}`, | |
| "_blank" | |
| ); | |
| }; | |
| const handleShareLinkedIn = async () => { | |
| window.open( | |
| `https://www.linkedin.com/sharing/share-offsite/?url=${encodeURIComponent("https://www.papermark.com/")}`, | |
| "_blank" | |
| ); | |
| }; |
🤖 Prompt for AI Agents
components/yearly-recap/yearly-recap-modal.tsx around lines 234 to 240: the
LinkedIn share URL is including a non-supported summary parameter which LinkedIn
ignores; update handleShareLinkedIn to open only the supported url parameter
(encodeURIComponent the target URL) so LinkedIn will fetch Open Graph metadata
from that page, and if you need custom text included in the share, implement the
LinkedIn Post API flow with OAuth instead of relying on the share-offsite
endpoint.
| <div className="animate-in zoom-in-50 duration-1000"> | ||
| <span className="text-8xl font-bold text-orange-500 mb-2"> | ||
|
|
||
| <h2 | ||
| className="text-8xl font-semibold text-black mb-2" | ||
|
|
||
| > | ||
| $50 | ||
| </h2> | ||
| </span> |
There was a problem hiding this comment.
Fix invalid nested heading structure.
There's a <span> wrapping an <h2> element, which is semantically incorrect (inline element containing block element). Also, text-orange-500 class on the outer span has no effect since the inner h2 is styled black.
<div className="animate-in zoom-in-50 duration-1000">
- <span className="text-8xl font-bold text-orange-500 mb-2">
-
- <h2
- className="text-8xl font-semibold text-black mb-2"
-
- >
- $50
- </h2>
- </span>
+ <span className="text-8xl font-bold text-orange-500 mb-2 block">
+ $50
+ </span>
</div>📝 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.
| <div className="animate-in zoom-in-50 duration-1000"> | |
| <span className="text-8xl font-bold text-orange-500 mb-2"> | |
| <h2 | |
| className="text-8xl font-semibold text-black mb-2" | |
| > | |
| $50 | |
| </h2> | |
| </span> | |
| <div className="animate-in zoom-in-50 duration-1000"> | |
| <span className="text-8xl font-bold text-orange-500 mb-2 block"> | |
| $50 | |
| </span> | |
| </div> |
🤖 Prompt for AI Agents
In components/yearly-recap/yearly-recap-modal.tsx around lines 670 to 679, there
is an inline <span> wrapping a block-level <h2> which is semantically invalid
and the outer span's text-orange-500 has no effect because the inner h2 is
styled black; fix by replacing the span with a block-level wrapper (e.g., a div)
that keeps the animation classes, remove the redundant span, and move/merge the
color and font classes onto the h2 (or adjust h2 class to use text-orange-500 if
intended) so the heading stands alone as the correct block-level element with
the desired styling.
| export default async function handler( | ||
| req: NextApiRequest, | ||
| res: NextApiResponse, | ||
| ) { | ||
| if (req.method !== "GET") { | ||
| return res.status(405).json({ message: "Method not allowed" }); | ||
| } | ||
|
|
||
| const { teamId } = req.query as { teamId: string }; | ||
| const year = req.query.year | ||
| ? parseInt(req.query.year as string) | ||
| : undefined; | ||
|
|
||
| if (!teamId) { | ||
| return res.status(400).json({ message: "Team ID is required" }); | ||
| } | ||
|
|
||
| try { | ||
| const stats = await getYearInReviewStats(teamId, year); | ||
| return res.status(200).json(stats); | ||
| } catch (error) { | ||
| console.error("Error fetching yearly recap stats:", error); | ||
| return res.status(500).json({ | ||
| message: "Internal server error", | ||
| error: error instanceof Error ? error.message : "Unknown error", | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for middleware handling auth for API routes
find . -type f -name "*middleware*" | head -20Repository: mfts/papermark
Length of output: 73
🏁 Script executed:
# Look at the actual file in question
cat -n pages/api/teams/[teamId]/yearly-recap.tsRepository: mfts/papermark
Length of output: 1196
🏁 Script executed:
# Check other team API endpoints for auth patterns
fd -e ts "pages/api/teams" | head -10Repository: mfts/papermark
Length of output: 423
🏁 Script executed:
# Search for getServerSession usage in API routes
rg "getServerSession" pages/api/ -A 3 | head -50Repository: mfts/papermark
Length of output: 2804
🏁 Script executed:
cat -n middleware.tsRepository: mfts/papermark
Length of output: 3181
🏁 Script executed:
cat -n pages/api/teams/[teamId]/branding.ts | head -40Repository: mfts/papermark
Length of output: 1430
🏁 Script executed:
rg -A 10 "getYearInReviewStats" lib/year-in-review/get-stats.tsRepository: mfts/papermark
Length of output: 434
Add authentication and authorization checks.
This endpoint lacks authentication and authorization while all sibling team endpoints (branding.ts, global-block-list.ts) implement them. The getYearInReviewStats function performs no access control, allowing any user to fetch any team's statistics.
Add the standard pattern used throughout the codebase:
+import { getServerSession } from "next-auth";
+
+import { errorhandler } from "@/lib/errorHandler";
+import prisma from "@/lib/prisma";
+import { CustomUser } from "@/lib/types";
+
+import { authOptions } from "../../auth/[...nextauth]";
import { getYearInReviewStats } from "@/lib/year-in-review/get-stats";
export default async function handler(
req: NextApiRequest,
res: NextApiResponse,
) {
+ const session = await getServerSession(req, res, authOptions);
+ if (!session) {
+ return res.status(401).end("Unauthorized");
+ }
+
if (req.method !== "GET") {
return res.status(405).json({ message: "Method not allowed" });
}
const { teamId } = req.query as { teamId: string };
+
+ // Verify user is a member of the team
+ const team = await prisma.team.findUnique({
+ where: { id: teamId },
+ select: { users: { select: { userId: true } } },
+ });
+
+ const isUserPartOfTeam = team?.users?.some(
+ (user) => user.userId === (session.user as CustomUser).id,
+ );
+ if (!isUserPartOfTeam) {
+ return res.status(403).json({ message: "Forbidden" });
+ }Committable suggestion skipped: line range outside the PR's diff.
| } catch (error) { | ||
| console.error("Error fetching yearly recap stats:", error); | ||
| return res.status(500).json({ | ||
| message: "Internal server error", | ||
| error: error instanceof Error ? error.message : "Unknown error", | ||
| }); |
There was a problem hiding this comment.
Avoid exposing internal error details in API response.
Returning error.message in the response could leak internal implementation details or sensitive information to clients.
} catch (error) {
console.error("Error fetching yearly recap stats:", error);
return res.status(500).json({
message: "Internal server error",
- error: error instanceof Error ? error.message : "Unknown error",
});
}🤖 Prompt for AI Agents
In pages/api/teams/[teamId]/yearly-recap.ts around lines 25 to 30, the catch
block currently includes error.message in the HTTP response which can leak
internal details; instead keep logging the full error server-side (e.g.,
console.error(error, { stack: error.stack })) but return a generic JSON response
with no internal error text (e.g., message: "Internal server error" and
optionally a safe errorCode or correlationId), removing error.message from the
response body.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
components/yearly-recap/yearly-recap-banner.tsx (1)
65-75: Already flagged: Potential infinite loop due torouterin dependency array.This issue was already identified in a previous review. Including the entire
routerobject in the dependency array can cause unnecessary re-renders sincerouteris a new object reference on each render.components/yearly-recap/yearly-recap-modal.tsx (3)
56-60: Already flagged: Privacy concern displaying viewer email address.The previous review identified that displaying
mostActiveViewer.emaildirectly in the UI (line 616) could be a privacy concern, as it exposes PII of viewers to document owners. Consider masking the email or displaying only the name when available.
234-240: Already flagged: LinkedIn share URL parameter may not work as intended.The previous review identified that LinkedIn's
share-offsiteendpoint doesn't support thesummaryparameter and will only use theurlparameter to fetch Open Graph metadata.
675-684: Already flagged: Fix invalid nested heading structure.The previous review identified that there's a
<span>wrapping an<h2>element, which is semantically incorrect (inline element containing block element). Additionally, the outer span'stext-orange-500class has no effect since the inner h2 is styled withtext-black.
🧹 Nitpick comments (1)
components/yearly-recap/yearly-recap-banner.tsx (1)
102-102: Consider removing commented-out code.The commented
PeachPaperSVGcomponents (lines 102 and 108) can be removed if they are no longer needed, improving code cleanliness.Also applies to: 108-108
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/yearly-recap/yearly-recap-banner.tsx(1 hunks)components/yearly-recap/yearly-recap-modal.tsx(1 hunks)lib/year-in-review/get-stats.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
lib/year-in-review/get-stats.ts (2)
lib/tinybird/pipes.ts (1)
getTotalTeamDuration(202-213)lib/constants.ts (1)
COUNTRIES(207-458)
components/yearly-recap/yearly-recap-banner.tsx (3)
context/team-context.tsx (1)
useTeam(87-87)lib/utils.ts (1)
fetcher(48-62)components/yearly-recap/yearly-recap-modal.tsx (1)
YearlyRecapModal(124-465)
🔇 Additional comments (5)
lib/year-in-review/get-stats.ts (5)
14-21: Good use of Promise.all for parallel queries.The expanded parallel execution efficiently fetches all required data simultaneously, which is good for performance.
55-55: Good addition of isArchived filter for data quality.Consistently excluding archived views across all queries ensures accurate statistics.
Also applies to: 79-79, 103-103, 142-142
112-121: Dataroom count query looks correct.The query structure properly filters by team and date range.
173-186: Excellent null-safety and error handling.The use of optional chaining for country mapping and the fallback to the country code when not found in the COUNTRIES map demonstrates good defensive coding.
187-217: Return object correctly includes all new fields.The expanded return object properly includes the new
year,totalDatarooms, andmostActiveViewerfields with appropriate null safety and type conversions for BigInt values.
| export async function getYearInReviewStats(teamId: string, year?: number) { | ||
| const currentYear = year || new Date().getFullYear(); | ||
| const yearStart = new Date(`${currentYear}-01-01`); | ||
| const yearEnd = new Date(`${currentYear + 1}-01-01`); | ||
|
|
There was a problem hiding this comment.
Use UTC dates to avoid timezone-dependent boundaries.
The Date constructor with ISO-like strings creates dates in the local timezone, which can cause misalignment with database timestamps (typically stored in UTC). For example, new Date("2025-01-01") in PST becomes 2025-01-01T08:00:00Z, potentially excluding data from the first 8 hours of January 1st UTC or including data from December 31st.
Apply this diff to use UTC dates:
- const yearStart = new Date(`${currentYear}-01-01`);
- const yearEnd = new Date(`${currentYear + 1}-01-01`);
+ const yearStart = new Date(Date.UTC(currentYear, 0, 1));
+ const yearEnd = new Date(Date.UTC(currentYear + 1, 0, 1));📝 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.
| export async function getYearInReviewStats(teamId: string, year?: number) { | |
| const currentYear = year || new Date().getFullYear(); | |
| const yearStart = new Date(`${currentYear}-01-01`); | |
| const yearEnd = new Date(`${currentYear + 1}-01-01`); | |
| export async function getYearInReviewStats(teamId: string, year?: number) { | |
| const currentYear = year || new Date().getFullYear(); | |
| const yearStart = new Date(Date.UTC(currentYear, 0, 1)); | |
| const yearEnd = new Date(Date.UTC(currentYear + 1, 0, 1)); |
🤖 Prompt for AI Agents
In lib/year-in-review/get-stats.ts around lines 8 to 12, the code constructs
yearStart and yearEnd with ISO-like strings which produce local-time dates and
can shift UTC boundaries; replace those with explicit UTC midnight boundaries
(e.g. yearStart = new Date(Date.UTC(currentYear, 0, 1)) and yearEnd = new
Date(Date.UTC(currentYear + 1, 0, 1))) so the range uses 00:00:00 UTC on Jan 1
of each year and avoids timezone-dependent inclusion/exclusion of records.
| // 5. Most active viewer (person who viewed most) | ||
| prisma.$queryRaw< | ||
| Array<{ | ||
| viewerEmail: string; | ||
| viewerName: string | null; | ||
| viewCount: number; | ||
| }> | ||
| >(Prisma.sql` | ||
| WITH RankedViewers AS ( | ||
| SELECT | ||
| v."viewerEmail", | ||
| v."viewerName", | ||
| COUNT(*) as "viewCount", | ||
| ROW_NUMBER() OVER (ORDER BY COUNT(*) DESC) as rn | ||
| FROM "View" v | ||
| WHERE | ||
| v."teamId" = ${teamId} | ||
| AND v."viewedAt" >= ${yearStart} | ||
| AND v."viewedAt" < ${yearEnd} | ||
| AND v."isArchived" = false | ||
| AND v."viewerEmail" IS NOT NULL | ||
| GROUP BY v."viewerEmail", v."viewerName" | ||
| ) | ||
| SELECT | ||
| "viewerEmail", | ||
| "viewerName", | ||
| "viewCount" | ||
| FROM RankedViewers | ||
| WHERE rn = 1 | ||
| LIMIT 1 | ||
| `), |
There was a problem hiding this comment.
Consider grouping by viewerEmail only to handle name inconsistencies.
The query groups by both viewerEmail and viewerName. If the same email appears with different names (e.g., due to data entry variations or updates), the view count will be split across multiple rows, potentially misidentifying the most active viewer.
Consider this alternative:
WITH RankedViewers AS (
SELECT
v."viewerEmail",
- v."viewerName",
+ MAX(v."viewerName") as "viewerName",
COUNT(*) as "viewCount",
ROW_NUMBER() OVER (ORDER BY COUNT(*) DESC) as rn
FROM "View" v
WHERE
v."teamId" = ${teamId}
AND v."viewedAt" >= ${yearStart}
AND v."viewedAt" < ${yearEnd}
AND v."isArchived" = false
AND v."viewerEmail" IS NOT NULL
- GROUP BY v."viewerEmail", v."viewerName"
+ GROUP BY v."viewerEmail"
)📝 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.
| // 5. Most active viewer (person who viewed most) | |
| prisma.$queryRaw< | |
| Array<{ | |
| viewerEmail: string; | |
| viewerName: string | null; | |
| viewCount: number; | |
| }> | |
| >(Prisma.sql` | |
| WITH RankedViewers AS ( | |
| SELECT | |
| v."viewerEmail", | |
| v."viewerName", | |
| COUNT(*) as "viewCount", | |
| ROW_NUMBER() OVER (ORDER BY COUNT(*) DESC) as rn | |
| FROM "View" v | |
| WHERE | |
| v."teamId" = ${teamId} | |
| AND v."viewedAt" >= ${yearStart} | |
| AND v."viewedAt" < ${yearEnd} | |
| AND v."isArchived" = false | |
| AND v."viewerEmail" IS NOT NULL | |
| GROUP BY v."viewerEmail", v."viewerName" | |
| ) | |
| SELECT | |
| "viewerEmail", | |
| "viewerName", | |
| "viewCount" | |
| FROM RankedViewers | |
| WHERE rn = 1 | |
| LIMIT 1 | |
| `), | |
| // 5. Most active viewer (person who viewed most) | |
| prisma.$queryRaw< | |
| Array<{ | |
| viewerEmail: string; | |
| viewerName: string | null; | |
| viewCount: number; | |
| }> | |
| >(Prisma.sql` | |
| WITH RankedViewers AS ( | |
| SELECT | |
| v."viewerEmail", | |
| MAX(v."viewerName") as "viewerName", | |
| COUNT(*) as "viewCount", | |
| ROW_NUMBER() OVER (ORDER BY COUNT(*) DESC) as rn | |
| FROM "View" v | |
| WHERE | |
| v."teamId" = ${teamId} | |
| AND v."viewedAt" >= ${yearStart} | |
| AND v."viewedAt" < ${yearEnd} | |
| AND v."isArchived" = false | |
| AND v."viewerEmail" IS NOT NULL | |
| GROUP BY v."viewerEmail" | |
| ) | |
| SELECT | |
| "viewerEmail", | |
| "viewerName", | |
| "viewCount" | |
| FROM RankedViewers | |
| WHERE rn = 1 | |
| LIMIT 1 | |
| `), |
🤖 Prompt for AI Agents
In lib/year-in-review/get-stats.ts around lines 123 to 153, the SQL groups by
both viewerEmail and viewerName which can split counts when the same email has
multiple name variants; change the query to GROUP BY viewerEmail only and
aggregate the name (e.g., use MAX(viewerName) or another deterministic
aggregation) in the SELECT so all views for a single email are counted together,
keep the ORDER BY COUNT(*) DESC and row-numbering logic the same to pick the top
viewer, and ensure NULL names are handled consistently by the chosen
aggregation.
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.