Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
How to use the Graphite Merge QueueAdd the label Main to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 54 files out of 161 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Suggestion:
Hardcoded database credentials have been committed to source code, which is a critical security vulnerability. The database URL should use the DATABASE_URL environment variable instead.
View Details
📝 Patch Details
diff --git a/packages/db/src/client.ts b/packages/db/src/client.ts
index ab849d54..c9fcdc40 100644
--- a/packages/db/src/client.ts
+++ b/packages/db/src/client.ts
@@ -6,7 +6,7 @@ import * as schema from "./drizzle/schema";
const fullSchema = { ...schema, ...relations };
-const databaseUrl = "postgres://databuddy:databuddy_dev_password@localhost:5432/databuddy";
+const databaseUrl = process.env.DATABASE_URL as string;
if (!databaseUrl) {
throw new Error("DATABASE_URL is not set");
Analysis
Hardcoded database credentials expose plaintext password in source code
What fails: packages/db/src/client.ts line 9 contains a hardcoded database URL with plaintext credentials (postgres://databuddy:databuddy_dev_password@localhost:5432/databuddy), compromising security of the database connection.
How to reproduce:
# Check the current state of the file
cat packages/db/src/client.ts | grep -A1 "const databaseUrl"
# Will show: const databaseUrl = "postgres://databuddy:databuddy_dev_password@localhost:5432/databuddy";
# Verify credentials are in Git history
git log -p packages/db/src/client.ts | grep -A2 "const databaseUrl"What happened vs expected behavior:
- Current (broken): Credentials hardcoded in source code -
const databaseUrl = "postgres://databuddy:databuddy_dev_password@localhost:5432/databuddy"; - Expected: Load from environment variable -
const databaseUrl = process.env.DATABASE_URL as string;
The environment variable validation check on lines 11-13 (if (!databaseUrl) throw new Error(...)) becomes ineffective since the hardcoded string is always truthy, preventing proper error detection if DATABASE_URL is not set.
Security impact:
- Credentials visible in repository and Git history to anyone with code access
- Development credentials hardcoded instead of using environment-specific configuration
- Database vulnerable to unauthorized access from code visibility
The environment variable pattern is correctly used elsewhere in the codebase (e.g., drizzle.config.ts uses process.env.DATABASE_URL) and was the previous working implementation per commit 12b191bf.
| {!isAssistantPage && ( | ||
| <div | ||
| className="fixed top-12 right-0 left-0 z-50 shrink-0 space-y-0 bg-background md:top-0 md:left-76 lg:left-84" | ||
| className="sticky top-0 right-0 left-0 z-50 shrink-0 space-y-0 bg-background md:top-0 md:left-84" |
There was a problem hiding this comment.
The analytics toolbar positioning was changed from fixed to sticky, which breaks toolbar visibility when scrolling page content because the sticky element is not inside the scrolling container.
View Details
📝 Patch Details
diff --git a/apps/dashboard/app/(main)/websites/[id]/layout.tsx b/apps/dashboard/app/(main)/websites/[id]/layout.tsx
index b10d471e..857fce07 100644
--- a/apps/dashboard/app/(main)/websites/[id]/layout.tsx
+++ b/apps/dashboard/app/(main)/websites/[id]/layout.tsx
@@ -3,7 +3,6 @@
import { useQueryClient } from "@tanstack/react-query";
import { useAtom } from "jotai";
import { useParams, usePathname } from "next/navigation";
-import { useLayoutEffect, useRef, useState } from "react";
import { toast } from "sonner";
import NotFound from "@/app/not-found";
import { useTrackingSetup } from "@/hooks/use-tracking-setup";
@@ -25,8 +24,6 @@ export default function WebsiteLayout({ children }: WebsiteLayoutProps) {
);
const { isLoading: isWebsiteLoading } = useWebsite(id as string);
const [isRefreshing, setIsRefreshing] = useAtom(isAnalyticsRefreshingAtom);
- const toolbarRef = useRef<HTMLDivElement>(null);
- const [toolbarHeight, setToolbarHeight] = useState(88);
const isAssistantPage =
pathname.includes("/assistant") ||
@@ -36,28 +33,6 @@ export default function WebsiteLayout({ children }: WebsiteLayoutProps) {
pathname.includes("/settings") ||
pathname.includes("/users");
- useLayoutEffect(() => {
- const element = toolbarRef.current;
- if (!element || isAssistantPage) {
- setToolbarHeight(0);
- return;
- }
-
- const updateHeight = () => {
- const height = element.getBoundingClientRect().height;
- setToolbarHeight(height);
- };
-
- updateHeight();
-
- const resizeObserver = new ResizeObserver(updateHeight);
- resizeObserver.observe(element);
-
- return () => {
- resizeObserver.disconnect();
- };
- }, [isAssistantPage]);
-
if (!id) {
return <NotFound />;
}
@@ -91,10 +66,7 @@ export default function WebsiteLayout({ children }: WebsiteLayoutProps) {
return (
<div className="flex h-full flex-col overflow-hidden">
{!isAssistantPage && (
- <div
- className="sticky top-0 right-0 left-0 z-50 shrink-0 space-y-0 bg-background md:top-0 md:left-84"
- ref={toolbarRef}
- >
+ <div className="fixed top-12 right-0 left-0 z-50 shrink-0 space-y-0 bg-background md:top-0 md:left-84">
<AnalyticsToolbar
isDisabled={isToolbarDisabled}
isLoading={isToolbarLoading}
@@ -107,7 +79,7 @@ export default function WebsiteLayout({ children }: WebsiteLayoutProps) {
)}
<div
- className={`${isAssistantPage ? "min-h-0 flex-1" : "min-h-0 flex-1 overflow-y-auto"}`}
+ className={`${isAssistantPage ? "min-h-0 flex-1" : "min-h-0 flex-1 overflow-y-auto pt-[88px]"}`}
>
{children}
</div>
Analysis
Analytics toolbar disappears when page scrolls due to incorrect sticky positioning
What fails: WebsiteLayout component in apps/dashboard/app/(main)/websites/[id]/layout.tsx line 68. Toolbar with position: sticky; top-0 scrolls off-screen when page content exceeds viewport height.
How to reproduce:
- Navigate to an analytics page (e.g.,
/websites/[id]) with sufficient content to cause page scrolling - Scroll the page vertically downward
- Observe: toolbar scrolls away instead of remaining fixed to the top
Result: Toolbar becomes invisible, users lose access to refresh button and analytics filters.
Expected: Toolbar remains fixed at the top of the viewport while page scrolls.
Root cause: Sticky positioning anchors to the nearest ancestor with a scrolling mechanism. Per MDN, overflow: hidden creates a scrolling context. The parent WebsiteLayout has overflow-hidden, so sticky elements anchor there instead of the viewport. When the outer MainLayout container scrolls (which happens due to h-screen overflow-y-auto pt-16 causing content to exceed viewport), the WebsiteLayout moves with it, making the sticky toolbar disappear.
Additionally, toolbarHeight state was computed via ResizeObserver but never used, indicating incomplete refactoring from the previous fixed implementation.
Fix:
- Changed toolbar from
position: stickyback toposition: fixedwithtop-12(mobile) /top-0 md:left-84(desktop) - Added
pt-[88px]padding to content div to prevent content from rendering under the fixed toolbar - Removed unused
toolbarHeightstate, ResizeObserver, and related imports (useLayoutEffect,useRef,useState)
2 participants
This PR: Introduces visual fixes, visual improvements, color improvements, and more to the dashboard. Landing page now has a bento box section.