Comment to 'UNA 14 Timeline Cleanup Script – Expert Review Requested'
  • 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...