From a28a766f8b411a7ee92bc6d39743ecfd47263b85 Mon Sep 17 00:00:00 2001
From: Arunavo Ray <rayanup3@gmail.com>
Date: Sat, 24 May 2025 20:12:27 +0530
Subject: [PATCH] refactor: update cleanup and schedule config to use seconds
 for retentionDays and improve nextRun calculation

---
 src/components/config/ConfigTabs.tsx          | 34 +++++++-
 .../config/DatabaseCleanupConfigForm.tsx      | 85 +++++++++++++++----
 src/components/config/ScheduleConfigForm.tsx  | 13 +--
 src/lib/cleanup-service.ts                    | 49 ++++++++---
 src/lib/db/schema.ts                          |  2 +-
 src/pages/api/config/index.ts                 | 68 +++++++++++++--
 src/types/config.ts                           |  2 +-
 7 files changed, 212 insertions(+), 41 deletions(-)

diff --git a/src/components/config/ConfigTabs.tsx b/src/components/config/ConfigTabs.tsx
index aa57404..e6a0899 100644
--- a/src/components/config/ConfigTabs.tsx
+++ b/src/components/config/ConfigTabs.tsx
@@ -53,7 +53,7 @@ export function ConfigTabs() {
     },
     cleanupConfig: {
       enabled: false,
-      retentionDays: 7,
+      retentionDays: 604800, // 7 days in seconds
     },
   });
   const { user, refreshUser } = useAuth();
@@ -181,6 +181,22 @@ export function ConfigTabs() {
           // Removed refreshUser() call to prevent page reload
           // Invalidate config cache so other components get fresh data
           invalidateConfigCache();
+
+          // Fetch updated config to get the recalculated nextRun time
+          try {
+            const updatedResponse = await apiRequest<ConfigApiResponse>(
+              `/config?userId=${user.id}`,
+              { method: 'GET' },
+            );
+            if (updatedResponse && !updatedResponse.error) {
+              setConfig(prev => ({
+                ...prev,
+                scheduleConfig: updatedResponse.scheduleConfig || prev.scheduleConfig,
+              }));
+            }
+          } catch (fetchError) {
+            console.warn('Failed to fetch updated config after auto-save:', fetchError);
+          }
         } else {
           toast.error(
             `Auto-save failed: ${result.message || 'Unknown error'}`,
@@ -233,6 +249,22 @@ export function ConfigTabs() {
           // Silent success - no toast for auto-save
           // Invalidate config cache so other components get fresh data
           invalidateConfigCache();
+
+          // Fetch updated config to get the recalculated nextRun time
+          try {
+            const updatedResponse = await apiRequest<ConfigApiResponse>(
+              `/config?userId=${user.id}`,
+              { method: 'GET' },
+            );
+            if (updatedResponse && !updatedResponse.error) {
+              setConfig(prev => ({
+                ...prev,
+                cleanupConfig: updatedResponse.cleanupConfig || prev.cleanupConfig,
+              }));
+            }
+          } catch (fetchError) {
+            console.warn('Failed to fetch updated config after auto-save:', fetchError);
+          }
         } else {
           toast.error(
             `Auto-save failed: ${result.message || 'Unknown error'}`,
diff --git a/src/components/config/DatabaseCleanupConfigForm.tsx b/src/components/config/DatabaseCleanupConfigForm.tsx
index a4c7d78..c77382f 100644
--- a/src/components/config/DatabaseCleanupConfigForm.tsx
+++ b/src/components/config/DatabaseCleanupConfigForm.tsx
@@ -18,38 +18,79 @@ interface DatabaseCleanupConfigFormProps {
   isAutoSaving?: boolean;
 }
 
+
+// Helper to calculate cleanup interval in hours (should match backend logic)
+function calculateCleanupInterval(retentionSeconds: number): number {
+  const retentionDays = retentionSeconds / (24 * 60 * 60);
+  if (retentionDays <= 1) {
+    return 6;
+  } else if (retentionDays <= 3) {
+    return 12;
+  } else if (retentionDays <= 7) {
+    return 24;
+  } else if (retentionDays <= 30) {
+    return 48;
+  } else {
+    return 168;
+  }
+}
+
 export function DatabaseCleanupConfigForm({
   config,
   setConfig,
   onAutoSave,
   isAutoSaving = false,
 }: DatabaseCleanupConfigFormProps) {
+  // Optimistically update nextRun when enabled or retention changes
   const handleChange = (
     e: React.ChangeEvent<HTMLInputElement | HTMLSelectElement>
   ) => {
     const { name, value, type } = e.target;
-    const newConfig = {
+    let newConfig = {
       ...config,
-      [name]:
-        type === "checkbox" ? (e.target as HTMLInputElement).checked : value,
+      [name]: type === "checkbox" ? (e.target as HTMLInputElement).checked : value,
     };
-    setConfig(newConfig);
 
-    // Trigger auto-save for cleanup config changes
+    // If enabling or changing retention, recalculate nextRun
+    if (
+      (name === "enabled" && (e.target as HTMLInputElement).checked) ||
+      (name === "retentionDays" && config.enabled)
+    ) {
+      const now = new Date();
+      const retentionSeconds =
+        name === "retentionDays"
+          ? Number(value)
+          : Number(newConfig.retentionDays);
+      const intervalHours = calculateCleanupInterval(retentionSeconds);
+      const nextRun = new Date(now.getTime() + intervalHours * 60 * 60 * 1000);
+      newConfig = {
+        ...newConfig,
+        nextRun,
+      };
+    }
+    // If disabling, clear nextRun
+    if (name === "enabled" && !(e.target as HTMLInputElement).checked) {
+      newConfig = {
+        ...newConfig,
+        nextRun: undefined,
+      };
+    }
+
+    setConfig(newConfig);
     if (onAutoSave) {
       onAutoSave(newConfig);
     }
   };
 
-  // Predefined retention periods
+  // Predefined retention periods (in seconds, like schedule intervals)
   const retentionOptions: { value: number; label: string }[] = [
-    { value: 1, label: "1 day" },
-    { value: 3, label: "3 days" },
-    { value: 7, label: "7 days" },
-    { value: 14, label: "14 days" },
-    { value: 30, label: "30 days" },
-    { value: 60, label: "60 days" },
-    { value: 90, label: "90 days" },
+    { value: 86400, label: "1 day" },        // 24 * 60 * 60
+    { value: 259200, label: "3 days" },      // 3 * 24 * 60 * 60
+    { value: 604800, label: "7 days" },      // 7 * 24 * 60 * 60
+    { value: 1209600, label: "14 days" },    // 14 * 24 * 60 * 60
+    { value: 2592000, label: "30 days" },    // 30 * 24 * 60 * 60
+    { value: 5184000, label: "60 days" },    // 60 * 24 * 60 * 60
+    { value: 7776000, label: "90 days" },    // 90 * 24 * 60 * 60
   ];
 
   return (
@@ -92,7 +133,7 @@ export function DatabaseCleanupConfigForm({
           {config.enabled && (
             <div>
               <label className="block text-sm font-medium mb-2">
-                Retention Period
+                Data Retention Period
               </label>
 
               <Select
@@ -123,12 +164,18 @@ export function DatabaseCleanupConfigForm({
               <p className="text-xs text-muted-foreground mt-1">
                 Activities and events older than this period will be automatically deleted.
               </p>
+              <div className="mt-2 p-2 bg-muted/50 rounded-md">
+                <p className="text-xs text-muted-foreground">
+                  <strong>Cleanup Frequency:</strong> The cleanup process runs automatically at optimal intervals:
+                  shorter retention periods trigger more frequent cleanups, longer periods trigger less frequent cleanups.
+                </p>
+              </div>
             </div>
           )}
 
           <div className="flex gap-x-4">
             <div className="flex-1">
-              <label className="block text-sm font-medium mb-1">Last Run</label>
+              <label className="block text-sm font-medium mb-1">Last Cleanup</label>
               <div className="text-sm">
                 {config.lastRun ? formatDate(config.lastRun) : "Never"}
               </div>
@@ -136,9 +183,13 @@ export function DatabaseCleanupConfigForm({
 
             {config.enabled && (
               <div className="flex-1">
-                <label className="block text-sm font-medium mb-1">Next Run</label>
+                <label className="block text-sm font-medium mb-1">Next Cleanup</label>
                 <div className="text-sm">
-                  {config.nextRun ? formatDate(config.nextRun) : "Never"}
+                  {config.nextRun
+                    ? formatDate(config.nextRun)
+                    : config.enabled
+                      ? "Calculating..."
+                      : "Never"}
                 </div>
               </div>
             )}
diff --git a/src/components/config/ScheduleConfigForm.tsx b/src/components/config/ScheduleConfigForm.tsx
index 1f2dd5a..dd8da0e 100644
--- a/src/components/config/ScheduleConfigForm.tsx
+++ b/src/components/config/ScheduleConfigForm.tsx
@@ -43,9 +43,6 @@ export function ScheduleConfigForm({
 
   // Predefined intervals
   const intervals: { value: number; label: string }[] = [
-    // { value: 120, label: "2 minutes" }, //for testing
-    { value: 900, label: "15 minutes" },
-    { value: 1800, label: "30 minutes" },
     { value: 3600, label: "1 hour" },
     { value: 7200, label: "2 hours" },
     { value: 14400, label: "4 hours" },
@@ -127,12 +124,18 @@ export function ScheduleConfigForm({
               <p className="text-xs text-muted-foreground mt-1">
                 How often the mirroring process should run.
               </p>
+              <div className="mt-2 p-2 bg-muted/50 rounded-md">
+                <p className="text-xs text-muted-foreground">
+                  <strong>Sync Schedule:</strong> Repositories will be synchronized at the specified interval.
+                  Choose shorter intervals for frequently updated repositories, longer intervals for stable ones.
+                </p>
+              </div>
             </div>
           )}
 
           <div className="flex gap-x-4">
             <div className="flex-1">
-              <label className="block text-sm font-medium mb-1">Last Run</label>
+              <label className="block text-sm font-medium mb-1">Last Sync</label>
               <div className="text-sm">
                 {config.lastRun ? formatDate(config.lastRun) : "Never"}
               </div>
@@ -140,7 +143,7 @@ export function ScheduleConfigForm({
 
             {config.enabled && (
               <div className="flex-1">
-                <label className="block text-sm font-medium mb-1">Next Run</label>
+                <label className="block text-sm font-medium mb-1">Next Sync</label>
                 <div className="text-sm">
                   {config.nextRun ? formatDate(config.nextRun) : "Never"}
                 </div>
diff --git a/src/lib/cleanup-service.ts b/src/lib/cleanup-service.ts
index de75070..2e61fc6 100644
--- a/src/lib/cleanup-service.ts
+++ b/src/lib/cleanup-service.ts
@@ -15,15 +15,39 @@ interface CleanupResult {
 }
 
 /**
- * Clean up old events and mirror jobs for a specific user
+ * Calculate cleanup interval in hours based on retention period
+ * For shorter retention periods, run more frequently
+ * For longer retention periods, run less frequently
+ * @param retentionSeconds - Retention period in seconds
  */
-async function cleanupForUser(userId: string, retentionDays: number): Promise<CleanupResult> {
-  try {
-    console.log(`Running cleanup for user ${userId} with ${retentionDays} days retention`);
+export function calculateCleanupInterval(retentionSeconds: number): number {
+  const retentionDays = retentionSeconds / (24 * 60 * 60); // Convert seconds to days
 
-    // Calculate cutoff date
+  if (retentionDays <= 1) {
+    return 6; // Every 6 hours for 1 day retention
+  } else if (retentionDays <= 3) {
+    return 12; // Every 12 hours for 1-3 days retention
+  } else if (retentionDays <= 7) {
+    return 24; // Daily for 4-7 days retention
+  } else if (retentionDays <= 30) {
+    return 48; // Every 2 days for 8-30 days retention
+  } else {
+    return 168; // Weekly for 30+ days retention
+  }
+}
+
+/**
+ * Clean up old events and mirror jobs for a specific user
+ * @param retentionSeconds - Retention period in seconds
+ */
+async function cleanupForUser(userId: string, retentionSeconds: number): Promise<CleanupResult> {
+  try {
+    const retentionDays = retentionSeconds / (24 * 60 * 60); // Convert to days for logging
+    console.log(`Running cleanup for user ${userId} with ${retentionDays} days retention (${retentionSeconds} seconds)`);
+
+    // Calculate cutoff date using seconds
     const cutoffDate = new Date();
-    cutoffDate.setDate(cutoffDate.getDate() - retentionDays);
+    cutoffDate.setTime(cutoffDate.getTime() - retentionSeconds * 1000);
 
     let eventsDeleted = 0;
     let mirrorJobsDeleted = 0;
@@ -75,7 +99,9 @@ async function cleanupForUser(userId: string, retentionDays: number): Promise<Cl
 async function updateCleanupConfig(userId: string, cleanupConfig: any) {
   try {
     const now = new Date();
-    const nextRun = new Date(now.getTime() + 24 * 60 * 60 * 1000); // Next day
+    const retentionSeconds = cleanupConfig.retentionDays || 604800; // Default 7 days in seconds
+    const cleanupIntervalHours = calculateCleanupInterval(retentionSeconds);
+    const nextRun = new Date(now.getTime() + cleanupIntervalHours * 60 * 60 * 1000);
 
     const updatedConfig = {
       ...cleanupConfig,
@@ -91,7 +117,8 @@ async function updateCleanupConfig(userId: string, cleanupConfig: any) {
       })
       .where(eq(configs.userId, userId));
 
-    console.log(`Updated cleanup config for user ${userId}, next run: ${nextRun.toISOString()}`);
+    const retentionDays = retentionSeconds / (24 * 60 * 60);
+    console.log(`Updated cleanup config for user ${userId}, next run: ${nextRun.toISOString()} (${cleanupIntervalHours}h interval for ${retentionDays}d retention)`);
   } catch (error) {
     console.error(`Error updating cleanup config for user ${userId}:`, error);
   }
@@ -116,7 +143,7 @@ export async function runAutomaticCleanup(): Promise<CleanupResult[]> {
     for (const config of userConfigs) {
       try {
         const cleanupConfig = config.cleanupConfig;
-        
+
         // Skip if cleanup is not enabled
         if (!cleanupConfig?.enabled) {
           continue;
@@ -124,10 +151,10 @@ export async function runAutomaticCleanup(): Promise<CleanupResult[]> {
 
         // Check if it's time to run cleanup
         const nextRun = cleanupConfig.nextRun ? new Date(cleanupConfig.nextRun) : null;
-        
+
         // If nextRun is null or in the past, run cleanup
         if (!nextRun || now >= nextRun) {
-          const result = await cleanupForUser(config.userId, cleanupConfig.retentionDays || 7);
+          const result = await cleanupForUser(config.userId, cleanupConfig.retentionDays || 604800);
           results.push(result);
 
           // Update the cleanup config with new run times
diff --git a/src/lib/db/schema.ts b/src/lib/db/schema.ts
index 8759046..cfc60e8 100644
--- a/src/lib/db/schema.ts
+++ b/src/lib/db/schema.ts
@@ -54,7 +54,7 @@ export const configSchema = z.object({
   }),
   cleanupConfig: z.object({
     enabled: z.boolean().default(false),
-    retentionDays: z.number().min(1).default(7), // in days
+    retentionDays: z.number().min(1).default(604800), // in seconds (default: 7 days)
     lastRun: z.date().optional(),
     nextRun: z.date().optional(),
   }),
diff --git a/src/pages/api/config/index.ts b/src/pages/api/config/index.ts
index 3b245fd..cb519ff 100644
--- a/src/pages/api/config/index.ts
+++ b/src/pages/api/config/index.ts
@@ -2,6 +2,7 @@ import type { APIRoute } from "astro";
 import { db, configs, users } from "@/lib/db";
 import { v4 as uuidv4 } from "uuid";
 import { eq } from "drizzle-orm";
+import { calculateCleanupInterval } from "@/lib/cleanup-service";
 
 export const POST: APIRoute = async ({ request }) => {
   try {
@@ -56,6 +57,63 @@ export const POST: APIRoute = async ({ request }) => {
       }
     }
 
+    // Process schedule config - set/update nextRun if enabled, clear if disabled
+    const processedScheduleConfig = { ...scheduleConfig };
+    if (scheduleConfig.enabled) {
+      const now = new Date();
+      const interval = scheduleConfig.interval || 3600; // Default to 1 hour
+
+      // Check if we need to recalculate nextRun
+      // Recalculate if: no nextRun exists, or interval changed from existing config
+      let shouldRecalculate = !scheduleConfig.nextRun;
+
+      if (existingConfig && existingConfig.scheduleConfig) {
+        const existingScheduleConfig = existingConfig.scheduleConfig;
+        const existingInterval = existingScheduleConfig.interval || 3600;
+
+        // If interval changed, recalculate nextRun
+        if (interval !== existingInterval) {
+          shouldRecalculate = true;
+        }
+      }
+
+      if (shouldRecalculate) {
+        processedScheduleConfig.nextRun = new Date(now.getTime() + interval * 1000);
+      }
+    } else {
+      // Clear nextRun when disabled
+      processedScheduleConfig.nextRun = null;
+    }
+
+    // Process cleanup config - set/update nextRun if enabled, clear if disabled
+    const processedCleanupConfig = { ...cleanupConfig };
+    if (cleanupConfig.enabled) {
+      const now = new Date();
+      const retentionSeconds = cleanupConfig.retentionDays || 604800; // Default 7 days in seconds
+      const cleanupIntervalHours = calculateCleanupInterval(retentionSeconds);
+
+      // Check if we need to recalculate nextRun
+      // Recalculate if: no nextRun exists, or retention period changed from existing config
+      let shouldRecalculate = !cleanupConfig.nextRun;
+
+      if (existingConfig && existingConfig.cleanupConfig) {
+        const existingCleanupConfig = existingConfig.cleanupConfig;
+        const existingRetentionSeconds = existingCleanupConfig.retentionDays || 604800;
+
+        // If retention period changed, recalculate nextRun
+        if (retentionSeconds !== existingRetentionSeconds) {
+          shouldRecalculate = true;
+        }
+      }
+
+      if (shouldRecalculate) {
+        processedCleanupConfig.nextRun = new Date(now.getTime() + cleanupIntervalHours * 60 * 60 * 1000);
+      }
+    } else {
+      // Clear nextRun when disabled
+      processedCleanupConfig.nextRun = null;
+    }
+
     if (existingConfig) {
       // Update path
       await db
@@ -63,8 +121,8 @@ export const POST: APIRoute = async ({ request }) => {
         .set({
           githubConfig,
           giteaConfig,
-          scheduleConfig,
-          cleanupConfig,
+          scheduleConfig: processedScheduleConfig,
+          cleanupConfig: processedCleanupConfig,
           updatedAt: new Date(),
         })
         .where(eq(configs.id, existingConfig.id));
@@ -113,8 +171,8 @@ export const POST: APIRoute = async ({ request }) => {
       giteaConfig,
       include: [],
       exclude: [],
-      scheduleConfig,
-      cleanupConfig,
+      scheduleConfig: processedScheduleConfig,
+      cleanupConfig: processedCleanupConfig,
       createdAt: new Date(),
       updatedAt: new Date(),
     });
@@ -201,7 +259,7 @@ export const GET: APIRoute = async ({ request }) => {
           },
           cleanupConfig: {
             enabled: false,
-            retentionDays: 7,
+            retentionDays: 604800, // 7 days in seconds
             lastRun: null,
             nextRun: null,
           },
diff --git a/src/types/config.ts b/src/types/config.ts
index 4bca4da..65c5835 100644
--- a/src/types/config.ts
+++ b/src/types/config.ts
@@ -20,7 +20,7 @@ export interface ScheduleConfig {
 
 export interface DatabaseCleanupConfig {
   enabled: boolean;
-  retentionDays: number;
+  retentionDays: number; // Actually stores seconds, but keeping the name for compatibility
   lastRun?: Date;
   nextRun?: Date;
 }