Add deleted_at timestamp for version deletion tracking#1442
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Backend
participant Postgres
participant Trigger
participant RetentionJob
Client->>Backend: request delete version(s)
Backend->>Postgres: UPDATE app_versions set deleted=true, deleted_at=NOW()
Postgres->>Trigger: row UPDATED (deleted_at changed)
Trigger->>Backend: invoke deleteIt (cleanup/notification)
Note over RetentionJob,Postgres: Periodic job
RetentionJob->>Postgres: CALL delete_old_deleted_versions()
Postgres->>Postgres: DELETE rows with deleted_at < now() - interval '1 year' (excl builtin/unknown)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 155c559311
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| -- Step 2: Migrate existing deleted versions (set deleted_at = created_at for old data) | ||
| UPDATE public.app_versions | ||
| SET deleted_at = created_at | ||
| WHERE deleted = true AND deleted_at IS NULL; |
There was a problem hiding this comment.
Backfilling deleted_at to created_at means any version created more than a year ago but deleted recently will look like it was deleted years ago; with the new delete_old_deleted_versions logic that uses deleted_at, those rows can be hard-deleted on the next cron run, violating the intended 1‑year grace period. This is especially likely for long‑lived apps where users delete old versions now. Consider backfilling from the previous deletion timestamp (updated_at from the prior retention logic) or NOW() to avoid premature hard deletes.
Useful? React with 👍 / 👎.
| // check if version was soft-deleted (deleted_at was set) | ||
| if (record.deleted_at && record.deleted_at !== oldRecord.deleted_at) | ||
| return deleteIt(c, record) |
There was a problem hiding this comment.
The trigger now only calls deleteIt when deleted_at changes, but the existing delete endpoints (e.g., public/bundle/delete.ts) only set deleted: true without setting deleted_at. Those updates will skip cleanup and the row will never qualify for delete_old_deleted_versions (which also requires deleted_at), leaving storage and DB rows stranded. Either set deleted_at in those update paths or add a DB trigger to populate it when deleted flips true.
Useful? React with 👍 / 👎.
Replace unreliable updated_at tracking with dedicated deleted_at column. When update_app_versions_retention() marks versions as deleted, it now sets deleted_at = NOW() for proper deletion timestamp tracking. The delete_old_deleted_versions() function uses deleted_at (not updated_at) for the 1-year retention check, and protects builtin/unknown versions from hard-deletion. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
1. Use updated_at instead of created_at for backfill to avoid premature hard-deletion of recently-deleted old versions 2. Set deleted_at in bundle/delete.ts to ensure manual deletions trigger S3 cleanup and qualify for hard-deletion after 1 year Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
riderx
force-pushed
the
riderx/investigate-updated-at
branch
from
c4535de to
d9b6f9b
Compare
Ensures the encryption enforcement function also sets deleted_at when soft-deleting non-compliant bundles, so S3 cleanup triggers properly. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Instead of manually setting deleted_at in every code path, use a PostgreSQL BEFORE UPDATE trigger that automatically sets deleted_at when deleted changes from false to true. This is more robust and ensures deleted_at is always set correctly, regardless of how the deletion happens (retention, manual, encryption enforcement, etc). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
1 participant
Summary
Add a dedicated
deleted_attimestamp column to track when app versions are soft-deleted. This replaces the unreliable approach of usingupdated_at, which is touched by many unrelated operations. The retention and cleanup functions now explicitly set/checkdeleted_at.Test plan
deleted_atis set when retention marks versions as deletedbuiltinandunknownversions are never hard-deleted, even after 1 yearChecklist
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.