From 0edc6dc77f8e1c46b77a5ad364afed8a4b5d702f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oj=C4=81rs=20Kapteinis?= Date: Wed, 19 Nov 2025 01:41:22 +0200 Subject: [PATCH] Fix PR review issues: loading state naming, singleton constraint, and type-safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename 'loading' to 'isLoading' in SiteConfigContext to match consumer components - Add CHECK constraint (id = 1) for singleton pattern in site_configuration table - Replace string concatenation with typed routeConfigMap in RouteGuard for type-safety Co-authored-by: Ojārs Kapteinis Co-authored-by: Claude 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../38_add_site_configuration_table.py | 3 ++- surfsense_web/components/RouteGuard.tsx | 25 +++++++++++-------- surfsense_web/contexts/SiteConfigContext.tsx | 12 ++++----- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/surfsense_backend/alembic/versions/38_add_site_configuration_table.py b/surfsense_backend/alembic/versions/38_add_site_configuration_table.py index 5cf63c409..d839da6cb 100644 --- a/surfsense_backend/alembic/versions/38_add_site_configuration_table.py +++ b/surfsense_backend/alembic/versions/38_add_site_configuration_table.py @@ -49,7 +49,8 @@ def upgrade() -> None: # Custom text sa.Column('custom_copyright', sa.String(length=200), nullable=True, server_default='SurfSense 2025'), - sa.PrimaryKeyConstraint('id') + sa.PrimaryKeyConstraint('id'), + sa.CheckConstraint('id = 1', name='check_singleton') ) # Create index diff --git a/surfsense_web/components/RouteGuard.tsx b/surfsense_web/components/RouteGuard.tsx index f7f1ed83e..5f448f88d 100644 --- a/surfsense_web/components/RouteGuard.tsx +++ b/surfsense_web/components/RouteGuard.tsx @@ -2,13 +2,22 @@ import { useEffect } from "react"; import { useRouter } from "next/navigation"; -import { useSiteConfig } from "@/contexts/SiteConfigContext"; +import { useSiteConfig, SiteConfig } from "@/contexts/SiteConfigContext"; interface RouteGuardProps { children: React.ReactNode; routeKey: "pricing" | "docs" | "contact" | "terms" | "privacy" | "registration"; } +const routeConfigMap: Record = { + pricing: "disable_pricing_route", + docs: "disable_docs_route", + contact: "disable_contact_route", + terms: "disable_terms_route", + privacy: "disable_privacy_route", + registration: "disable_registration", +}; + export function RouteGuard({ children, routeKey }: RouteGuardProps) { const { config, isLoading } = useSiteConfig(); const router = useRouter(); @@ -16,11 +25,8 @@ export function RouteGuard({ children, routeKey }: RouteGuardProps) { useEffect(() => { if (isLoading) return; - // Special case: registration uses disable_registration instead of disable_registration_route - const disableKey = routeKey === "registration" - ? "disable_registration" - : (`disable_${routeKey}_route` as keyof typeof config); - const isDisabled = config[disableKey as keyof typeof config]; + const disableKey = routeConfigMap[routeKey]; + const isDisabled = config[disableKey]; if (isDisabled) { router.replace("/404"); @@ -37,11 +43,8 @@ export function RouteGuard({ children, routeKey }: RouteGuardProps) { } // Check if route is disabled - // Special case: registration uses disable_registration instead of disable_registration_route - const disableKey = routeKey === "registration" - ? "disable_registration" - : (`disable_${routeKey}_route` as keyof typeof config); - const isDisabled = config[disableKey as keyof typeof config]; + const disableKey = routeConfigMap[routeKey]; + const isDisabled = config[disableKey]; if (isDisabled) { return null; diff --git a/surfsense_web/contexts/SiteConfigContext.tsx b/surfsense_web/contexts/SiteConfigContext.tsx index 7d4a63a74..995a69e1c 100644 --- a/surfsense_web/contexts/SiteConfigContext.tsx +++ b/surfsense_web/contexts/SiteConfigContext.tsx @@ -53,26 +53,26 @@ const defaultConfig: SiteConfig = { interface SiteConfigContextType { config: SiteConfig; - loading: boolean; + isLoading: boolean; error: string | null; refetch: () => Promise; } const SiteConfigContext = createContext({ config: defaultConfig, - loading: true, + isLoading: true, error: null, refetch: async () => {}, }); export function SiteConfigProvider({ children }: { children: React.ReactNode }) { const [config, setConfig] = useState(defaultConfig); - const [loading, setLoading] = useState(true); + const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); const fetchConfig = async () => { try { - setLoading(true); + setIsLoading(true); setError(null); const backendUrl = @@ -91,7 +91,7 @@ export function SiteConfigProvider({ children }: { children: React.ReactNode }) console.error("Error fetching site configuration:", err); // Keep default config on error } finally { - setLoading(false); + setIsLoading(false); } }; @@ -100,7 +100,7 @@ export function SiteConfigProvider({ children }: { children: React.ReactNode }) }, []); return ( - + {children} );