Fix cleanup_unsubmitted_forms date logic and safe entity cleanup

[BilenAssres]

Summary by CodeRabbit

• Refactor
  • Improved unsubmitted forms cleanup process with enhanced status tracking (completed vs. failed states).
  • Optimized query logic for more efficient data retrieval and removal operations.
  • Enhanced error handling with better edge case management for incomplete data scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Refactored token cleanup logic in the unsubmitted forms cleanup utility. Replaced 7-day date calculations with a simpler cutoffDate approach, split bulk transaction handling into conditional logic based on entity ID presence, added guard clauses for missing entity IDs, and updated error handling with string literal status values.

Changes

Cohort / File(s)	Summary
Token cleanup refactoring tests/unsubmitted_forms/cleanup_unsubmitted_forms.ts	Simplified date calculations using cutoffDate; refactored deletion logic to conditionally handle missing entityId with guard clause; split single transaction into conditional branches for tokens with/without entities; updated status updates and error messages to use string literals

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring extra attention:
  • The refactored transaction logic and conditional branching based on token.entityId presence—verify all deletion paths (token-only vs. full entity cleanup) are correct and complete
  • Edge cases where entityId is missing or null—confirm the guard clause and warning log are sufficient
  • Database consistency after splitting the previous atomic operation into conditional transactions—ensure no orphaned records remain across different deletion paths

Poem

🐰 Seven days dissolve to one clean cutoff date,
Form tokens vanish, entities meet their fate,
Guard clauses whisper of missing IDs found,
Transactions spring forth from logic's solid ground,
The warren of data grows tidy and bright!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title 'add basic logging for expired tokens' describes only a minor aspect of the changes. The primary modifications involve significant control flow restructuring, transaction logic changes, and entity deletion patterns, which are not reflected in the title.	Revise the title to reflect the main changes, such as 'refactor(cleanup): restructure token cleanup logic with conditional entity deletion' to accurately represent the substantial control flow and transaction modifications.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/unsubmitted_forms/cleanup_unsubmitted_forms.ts (2)

42-42: Good addition for observability.

Consider also logging a summary at the end (e.g., how many tokens were successfully cleaned up vs. failed) to aid debugging in production.

---

44-86: Consider per-token error handling for resilience.

If any single token transaction fails, the job marks as "failed" but tokens processed before the failure are already deleted (each transaction commits independently). This can leave the cleanup in an inconsistent state with no way to identify which tokens were processed.

Consider wrapping individual token processing in try-catch, collecting failures, and continuing with remaining tokens:

🔎 Proposed approach for resilient processing

+    const failures: string[] = [];
+
     for (const token of expiredTokens) {
+      try {
         // Guard against missing entityId; delete only the token in that case
         if (!token.entityId) {
           // ... existing guard logic
         }
 
         await prisma.$transaction([
           // ... existing transaction
         ]);
+      } catch (tokenError) {
+        console.error(`Failed to cleanup token ${token.token}:`, tokenError);
+        failures.push(token.token);
+      }
     }
 
+    if (failures.length > 0) {
+      console.error(`cleanup_unsubmitted_forms: ${failures.length} tokens failed to cleanup`);
+      // Optionally: still mark as 'completed' with warnings, or use 'partial_failure' status
+    }
+
     await update_job_status(job.id, 'completed');

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 1dd21f7 and f848af6.

📒 Files selected for processing (1)

• tests/unsubmitted_forms/cleanup_unsubmitted_forms.ts (1 hunks)

🔇 Additional comments (3)

tests/unsubmitted_forms/cleanup_unsubmitted_forms.ts (3)

32-40: LGTM!

The cutoff date calculation and query logic are correct. Using Date.now() - 7 * 24 * 60 * 60 * 1000 correctly computes 7 days ago, and the lt: cutoffDate filter properly finds tokens older than that threshold.

---

44-54: Good defensive programming.

The guard clause correctly handles the edge case where entityId is missing, preventing potential errors downstream. The warning log helps with visibility into data quality issues.

---

56-79: Potential foreign key constraint violation when deleting entity.

The transaction deletes only relationships with status: "new" and matching product_id, but then unconditionally deletes the entity. If the entity has relationships with other statuses or from other products, the entity.delete will fail due to foreign key constraints. This could cause the cleanup job to fail mid-transaction, leaving some tokens partially deleted.

Either:

1. Delete all relationships for the entity (remove the product_id and status filters), or
2. Add error handling to skip entity deletion if other relationships exist, or
3. Verify that entities only ever have relationships for a single product with "new" status.