Comment to 'UNA 14 Timeline Cleanup Script – Expert Review Requested'
  • Hey @Michael Newton

    Hi Michael, we’re ready to begin testing after completing our prep project to replicate the live server. I’d appreciate it if you could stay on standby in case I need your advice or input.

    There’s no need for you to engage in the full cycle — just keep an eye on things in the background. Also, anyone interested is more than welcome to join in; I truly believe that within each of us lies a hidden talent, and when we combine our strengths, we become unstoppable — the sky’s the limit.

    • Here’s a code review focused on safety + “will it really delete what you think it will delete” for UNA Timeline.

      What’s good

      • Dry-run mode + batch processing + logging + lock file are all solid operational guardrails.
      • You’re not doing raw SQL deletes; you’re attempting to go through the module layer (good idea for storage cleanup, votes, comments, etc.).
      • Your cutoff uses a UNIX timestamp, which matches how Timeline stores dates in many UNA installs (also consistent with UNA queries you’ll see in logs).

      High-risk / likely-bug items

      1) Your selection filter is probably wrong: WHERE type = 'post'

      In UNA Timeline, type and action are used together (handlers join on te.type = alert_unit and te.action = alert_action as seen in typical timeline queries). ()

      So type='post' is suspicious: in many setups, “post” is the action, not the type (type is usually the unit/module).

      Safer approach:

      • First, inspect what you actually have before deleting anything:
      SELECT `type`, `action`, COUNT(*) c
      FROM `bx_timeline_events`
      GROUP BY `type`, `action`
      ORDER BY c DESC;
      

      Then set your delete criteria accordingly.

      Also consider excluding system/pinned/promoted items (see suggestions below).

      2) $timeline->deleteEvent($aEvent) may not exist / may not be the supported API

      UNA publicly documents a service call to delete a timeline event by ID:

      • bx_srv('bx_timeline', 'delete', [$iId]); ()

      That’s the most “version-stable” way compared to calling an internal method name that may differ by release.

      3) Script is dangerously callable from the web unless you block it

      If this file is in your web root (or reachable), anyone who hits it could trigger deletions once $dryRun=false.

      Minimum safety:

      • Restrict to CLI:
      if (php_sapi_name() !== 'cli') die("CLI only.\n");
      

      Or require a secret token / IP allowlist / move outside web root.

      Improvements I strongly recommend (practical)

      A) Use the documented deletion service

      Replace the deletion block with:

      $result = bx_srv('bx_timeline', 'delete', [$eventId]); // documented Timeline service
      

      This should still trigger the Timeline delete hooks (“bx_timeline”, “delete”), etc. ()

      B) Don’t log owner_id blindly

      Some Timeline versions and/or edge cases can differ. Your logging line assumes owner_id exists and is populated. It often does, but your selection query doesn’t use it anyway.

      Safer: log owner_id only if set; also consider logging type + action:

      $owner = $aEvent['owner_id'] ?? 'n/a';
      $msg = "Would delete event $eventId (type={$aEvent['type']}, action={$aEvent['action']}, owner=$owner, date=" . date('Y-m-d', $aEvent['date']) . ")\n";
      

      C) Exclude pinned/promoted/system events (optional but usually desired)

      Common “don’t touch these” filters (adjust to your schema after inspecting):

      AND `system` = 0
      AND `sticked` = 0
      AND `promoted` = 0
      

      (Your own site may differ—verify columns exist.)

      D) Dry-run summary wording is misleading

      Right now, you increment $deleted++ in dry-run and print “Successfully deleted”. That’s fine internally, but misleading.

      Suggestion:

      • Track $wouldDelete in dry-run instead of $deleted.

      A tightened-up “safe version” (minimal edits)

      Below is a conservative patch showing the core changes (CLI-only + service delete + better logging). Keep your lock/log/batch as-is.

      <?php
      require_once __DIR__ . '/inc/header.inc.php';
      
      if (php_sapi_name() !== 'cli') {
          die("CLI only.\n");
      }
      
      $daysOld = 60;
      $dryRun  = true;
      $batch   = 200;
      
      $lockFile = __DIR__ . '/timeline_cleanup.lock';
      $logFile  = __DIR__ . '/timeline_cleanup.log';
      
      $cutoff = time() - (86400 * $daysOld);
      $db = BxDolDb::getInstance();
      
      $timeline = BxDolModule::getInstance('bx_timeline');
      if (!$timeline) die("FATAL: Timeline module not found.\n");
      
      /**
       * IMPORTANT:
       * Verify your real distribution of (type, action) BEFORE relying on this.
       * Run:
       *   SELECT type, action, COUNT(*) FROM bx_timeline_events GROUP BY type, action ORDER BY COUNT(*) DESC;
       */
      $eventIds = $db->getColumn("
          SELECT `id`
          FROM `bx_timeline_events`
          WHERE `date` < ?
            AND `system` = 0
            AND `sticked` = 0
            AND `promoted` = 0
            AND `action` = 'post'
          ORDER BY `date` ASC
          LIMIT ?
      ", [$cutoff, $batch]);
      
      foreach ($eventIds as $eventId) {
          $eventId = (int)$eventId;
          $aEvent = $db->getRow("SELECT * FROM `bx_timeline_events` WHERE `id` = ?", [$eventId]);
          if (!$aEvent) continue;
      
          $owner = $aEvent['owner_id'] ?? 'n/a';
      
          if ($dryRun) {
              echo "[DRY RUN] Would delete event {$eventId} (type={$aEvent['type']}, action={$aEvent['action']}, owner={$owner}, date=" . date('Y-m-d', $aEvent['date']) . ")\n";
              continue;
          }
      
          // Documented Timeline delete service by ID
          $ok = bx_srv('bx_timeline', 'delete', [$eventId]);
          echo $ok ? "✓ Deleted {$eventId}\n" : "✗ Failed {$eventId}\n";
      
          usleep(50000);
      }
      

      Notes:

      • I used action='post' and excluded system/sticked/promoted as a sane default, but you must confirm with the distribution query first.
      • bx_srv('bx_timeline','delete',[$id]) is the documented API. ()

      What I’d do before the first live run

      1. Run the (type, action) distribution query (above) and paste the top ~10 rows back here if you want me to tune the WHERE clause exactly.
      2. Dry-run with $batch=10, verify the IDs correspond to the posts you actually consider “old timeline clutter”.
      3. Only then flip $dryRun=false.

      If you tell me what UNA version you’re on (or paste the output of the (type, action) count query), I’ll give you the correct, precise WHERE clause for your specific Timeline data.

      I'll be around...