Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/app/(main)/boards/[boardId]/BoardShareButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import { Share } from '@/components/icons';
import { useMessages } from '@/components/hooks';
import { DialogButton } from '@/components/input/DialogButton';
import { BoardShareDialog } from './BoardShareDialog';
import { useApp } from '@/store/app';

export function BoardShareButton({ boardId }: { boardId: string }) {
const { t, labels } = useMessages();
const dateRangeValue = useApp((state) => state.dateRangeValue);

return (
<DialogButton
Expand All @@ -13,7 +15,7 @@ export function BoardShareButton({ boardId }: { boardId: string }) {
title={null}
width="900px"
>
<BoardShareDialog boardId={boardId} />
<BoardShareDialog boardId={boardId} dateRangeValue={dateRangeValue} />
</DialogButton>
);
}
}
21 changes: 17 additions & 4 deletions src/app/(main)/boards/[boardId]/BoardShareDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,25 @@ import { useBoardSharesQuery, useMessages } from '@/components/hooks';
import { BoardShareCreateForm } from './BoardShareCreateForm';
import { BoardSharesTable } from './BoardSharesTable';

export function BoardShareDialog({ boardId }: { boardId: string }) {
export function BoardShareDialog({
boardId,
dateRangeValue,
}: {
boardId: string;
dateRangeValue: string | object;
}) {
const { data, error, isLoading } = useBoardSharesQuery({ boardId });
const shares = data?.data || [];
const hasShares = shares.length > 0;

return (
<LoadingPanel data={data} isLoading={isLoading} error={error}>
<BoardShareDialogContent boardId={boardId} hasShares={hasShares} shares={shares} />
<BoardShareDialogContent
boardId={boardId}
hasShares={hasShares}
shares={shares}
dateRangeValue={dateRangeValue}
/>
</LoadingPanel>
);
}
Expand All @@ -23,10 +34,12 @@ function BoardShareDialogContent({
boardId,
hasShares,
shares,
dateRangeValue,
}: {
boardId: string;
hasShares: boolean;
shares: any[];
dateRangeValue: string | object;
}) {
const { t, labels, messages } = useMessages();
const [isCreating, setIsCreating] = useState(false);
Expand Down Expand Up @@ -54,10 +67,10 @@ function BoardShareDialogContent({
)}
{!showCreateForm &&
(hasShares ? (
<BoardSharesTable data={shares} />
<BoardSharesTable data={shares} dateRangeValue={dateRangeValue} />
) : (
<Text color="muted">{t(messages.noDataAvailable)}</Text>
))}
</Column>
);
}
}
22 changes: 17 additions & 5 deletions src/app/(main)/boards/[boardId]/BoardSharesTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,29 @@ import { ExternalLink } from '@/components/common/ExternalLink';
import { useConfig, useMessages, useMobile } from '@/components/hooks';
import { DataColumn, DataTable, type DataTableProps, Row } from '@umami/react-zen';

export function BoardSharesTable(props: DataTableProps) {
interface BoardSharesTableProps extends DataTableProps {
dateRangeValue?: string | object;
}

export function BoardSharesTable({ dateRangeValue, ...props }: BoardSharesTableProps) {
const { t, labels } = useMessages();
const { cloudMode } = useConfig();
const { isMobile } = useMobile();

const getUrl = (slug: string) => {
if (cloudMode) {
return `${process.env.cloudUrl}/share/${slug}`;
let baseUrl = cloudMode
? `${process.env.cloudUrl}/share/${slug}`
: `${window?.location.origin}${process.env.basePath || ''}/share/${slug}`;

if (dateRangeValue) {
const rangeString = typeof dateRangeValue === 'object'
? JSON.stringify(dateRangeValue)
: dateRangeValue;

return `${baseUrl}?range=${encodeURIComponent(rangeString)}`;
}

return `${window?.location.origin}${process.env.basePath || ''}/share/${slug}`;
return baseUrl;
};

return (
Expand Down Expand Up @@ -45,4 +57,4 @@ export function BoardSharesTable(props: DataTableProps) {
</DataColumn>
</DataTable>
);
}
}
35 changes: 33 additions & 2 deletions src/app/(main)/websites/page.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,41 @@
import type { Metadata } from 'next';
import { useEffect } from 'react';
import { useSearchParams } from 'next/navigation';
import { setDateRangeValue, setBoardDateRangeValue } from '@/store/app';
import { getItem } from '@/lib/storage';
import { DATE_RANGE_CONFIG } from '@/lib/constants';
import { WebsitesPage } from './WebsitesPage';

export default function () {
export default function Page({ params }: { params: { websiteId?: string } }) {
const websiteId = params?.websiteId;
const searchParams = useSearchParams();

useEffect(() => {
if (!websiteId) return;

const urlRange = searchParams.get('range');
const savedRange = getItem(`${DATE_RANGE_CONFIG}:${websiteId}`);

if (urlRange) {
try {
const parsedRange = urlRange.startsWith('{') ? JSON.parse(urlRange) : urlRange;
setBoardDateRangeValue(parsedRange, websiteId);
} catch {
setBoardDateRangeValue(urlRange, websiteId);
}
} else if (savedRange) {
try {
const parsedRange = savedRange.startsWith('{') ? JSON.parse(savedRange) : savedRange;
setDateRangeValue(parsedRange);
} catch {
setDateRangeValue(savedRange);
}
}
Comment on lines +26 to +33

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 getItem returns a parsed value — calling .startsWith on an object throws at runtime

getItem in src/lib/storage.ts always calls JSON.parse internally before returning. When a custom date range object (e.g., { startDate, endDate }) was stored, getItem returns a plain JavaScript object, not a JSON string. Calling .startsWith('{') on that object throws TypeError: savedRange.startsWith is not a function at runtime. The guard only works correctly when the stored value is a primitive string like "30d".

}, [websiteId, searchParams]);

return <WebsitesPage />;
}

export const metadata: Metadata = {
title: 'Websites',
};
};
Comment on lines 1 to +41

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Server Component using client-only hooks with conflicting metadata export

This file is src/app/(main)/websites/page.tsx — the websites listing page at route /websites. In Next.js App Router, pages that export metadata are Server Components and cannot use hooks. This component now imports and calls useEffect and useSearchParams, which are client-only hooks. Adding "use client" would fix the hook errors but would then break the metadata export, since client components cannot export metadata. These two requirements are mutually exclusive in a single file.

Additionally, the route /websites has no [websiteId] path segment, so params?.websiteId will always be undefined, the guard if (!websiteId) return triggers on every render, and the entire hydration effect is dead code. Based on the PR description, this logic was intended for src/app/(main)/websites/[websiteId]/page.tsx (or the boards equivalent), not this file.

12 changes: 10 additions & 2 deletions src/store/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
TIMEZONE_CONFIG,
} from '@/lib/constants';
import { getTimezone } from '@/lib/date';
import { getItem } from '@/lib/storage';
import { getItem, setItem } from '@/lib/storage';

const initialState = {
locale: getItem(LOCALE_CONFIG) || process.env.defaultLocale || DEFAULT_LOCALE,
Expand Down Expand Up @@ -51,4 +51,12 @@ export function setDateRangeValue(dateRangeValue: string) {
store.setState({ dateRangeValue });
}

export const useApp = store;
// Added scoped board setter to handle unique board filter persistence
export function setBoardDateRangeValue(dateRangeValue: string, boardId: string) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The parameter type is declared as string but callers pass the already-parsed result of JSON.parse(urlRange), which can be a plain object. The type should be widened to match actual usage.

Suggested change
export function setBoardDateRangeValue(dateRangeValue: string, boardId: string) {
export function setBoardDateRangeValue(dateRangeValue: string | object, boardId: string) {

store.setState({ dateRangeValue });
if (boardId) {
setItem(`${DATE_RANGE_CONFIG}:${boardId}`, dateRangeValue);
}
Comment on lines +55 to +59

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Global state mutation breaks per-board isolation

setBoardDateRangeValue calls store.setState({ dateRangeValue }), which overwrites the single global dateRangeValue in the Zustand store. This directly contradicts the stated goal of per-board isolation. If a user visits Board A (triggering a range hydration of 7d), then navigates to Board B (triggering 30d), then returns to Board A, the global state will still read 30d — the last board to mount wins. All boards and all other date-range-aware components share this one value, so any board that loads after another will silently overwrite the previous board's intended range.

}

export const useApp = store;