Skip to content

feat: implement soft-delete recovery#203

Open
MdTowfikomer wants to merge 7 commits into
geturbackend:mainfrom
MdTowfikomer:feat/soft-delete-recovery
Open

feat: implement soft-delete recovery#203
MdTowfikomer wants to merge 7 commits into
geturbackend:mainfrom
MdTowfikomer:feat/soft-delete-recovery

Conversation

@MdTowfikomer
Copy link
Copy Markdown
Contributor

@MdTowfikomer MdTowfikomer commented May 24, 2026

🚀 Pull Request Description

Fixes #193
This PR implements the recovery (undelete) functionality for soft-deleted documents, completing the lifecycle initiated in PR #185. Users can now restore documents from the "trash" state back to "active" directly from the dashboard or via the public API.

Key Changes:

  • Backend (Dashboard & Public API):
    • Added PATCH /recover endpoints to both APIs.
    • Implemented logic to flip isDeleted to false and nullify deletedAt.
    • Integrated Row Level Security (RLS) to ensure only authorized users can recover specific records.
    • Added conflict handling (409) for unique constraint violations during restoration.
  • Frontend (Web Dashboard):
    • Added a Restore Icon (RotateCcw) in CollectionTable and RecordList that appears only for deleted items.
    • Implemented handleRecoverRecord in Database.jsx with Optimistic UI updates for a seamless user experience.
    • Updated RowDetailDrawer to display deletedAt metadata and disable editing for deleted records until restored.

🛠️ Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🎨 UI/UX improvement (Frontend only)

🧪 Testing & Validation

Backend Verification:

  • Verified PATCH /:projectId/collections/:collectionName/data/:id/recover
    via Thunder Client.
  • Confirmed that RLS correctly blocks unauthorized recovery attempts.
  • Verified that unique field conflicts (Error 11000) return a proper 409
    status code.
  • Confirmed restored docs are no longer picked up by the 30-day cleanup
    worker.

Frontend Verification:

  • Verified the Restore button correctly triggers the recovery and updates
    the UI state without a full page reload.
  • Checked responsiveness of the new action buttons on mobile (RecordList
    view).
  • Verified "Deleted" badges and tooltips update correctly in real-time.

📸 Screenshots

  • Table View: Shows the new RotateCcw icon in the "Actions" column for records marked as deleted.
image Success Toast: "Document restored successfully" appears after clicking restore. image

✅ Checklist

  • My code follows the code style of this project.
  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings or errors.

Built with ❤️ for urBackend.

Summary by CodeRabbit

  • New Features

    • Restore soft-deleted records via new recover endpoints (public & dashboard) and UI recover actions
    • Webhook support for recover events and automatic cleanup rescheduling for trash jobs
  • UI

    • Recovery buttons in lists/tables, “DELETED” badges with deletion countdowns, visual styling and small spinner
  • Tests

    • Added/expanded tests covering recovery success, not-found, conflict, and invalid-id cases
  • Documentation

    • JSDoc updated to document delete/recover semantics

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026

Warning

Review limit reached

@MdTowfikomer, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 13 minutes. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 02defdc4-1509-4f2f-a2ff-14d90bfd57a9

📥 Commits

Reviewing files that changed from the base of the PR and between 12c2611 and 18dddac.

📒 Files selected for processing (5)
  • apps/dashboard-api/src/controllers/project.controller.js
  • apps/public-api/src/__tests__/softDelete.test.js
  • apps/web-dashboard/src/components/CollectionTable.jsx
  • apps/web-dashboard/src/components/RecordList.jsx
  • apps/web-dashboard/src/pages/Webhooks.jsx
📝 Walkthrough

Walkthrough

Adds recover endpoints, tests, common webhook/cleanup utilities, and web-dashboard UI to restore soft-deleted documents by unsetting isDeleted/deletedAt; maps 400/404/409/500 errors via AppError and synchronizes cleanup jobs and webhook dispatch.

Changes

Document Recovery Feature

Layer / File(s) Summary
Controllers, common exports, webhook & cleanup utils
apps/dashboard-api/src/controllers/project.controller.js, apps/public-api/src/controllers/data.controller.js, packages/common/src/*
Adds recoverRow and recoverSingleDoc handlers, re-exports dispatchWebhooks and syncCollectionCleanup, implements dispatchWebhooks and syncCollectionCleanup, and adds recover flag to webhook schema.
Routes and middleware wiring
apps/dashboard-api/src/routes/projects.js, apps/public-api/src/routes/data.js
New PATCH endpoints /:projectId/.../data/:id/recover and /:collectionName/:id/recover wired with existing auth/verification/usage/write-authorization middleware.
Backend tests (dashboard & public)
apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js, apps/dashboard-api/src/__tests__/routes.projects.storage.test.js, apps/public-api/src/__tests__/softDelete.test.js, apps/public-api/src/__tests__/storage.controller.test.js
Adds/updates tests for recover success, 404 (not found / recovery window expired), 409 (duplicate-key), and 400 (invalid id) and extends mocks in @urbackend/common.
Web dashboard UI: display and metadata
apps/web-dashboard/src/components/CollectionTable.jsx, apps/web-dashboard/src/components/RowDetailDrawer.jsx, apps/web-dashboard/src/components/RecordList.jsx, apps/web-dashboard/src/index.css
Show DELETED badge and metadata rows, exclude deletion fields from dynamic render, format deletion timestamps and countdowns, and add spinner CSS.
Web dashboard recovery actions & wiring
apps/web-dashboard/src/components/CollectionTable.jsx, apps/web-dashboard/src/components/RecordList.jsx, apps/web-dashboard/src/pages/Database.jsx
Render Recover button (RotateCcw) with disabled/loading state via recoveringIds; add handleRecoverRecord to call PATCH recover endpoint, perform optimistic update and rollback on error, wire onRecover into list/table, and disable edit for deleted records.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant WebDashboard
  participant PublicAPI
  participant DashboardAPI
  participant MongoDB
  participant WebhookService
  User->>WebDashboard: Click Recover on deleted record
  WebDashboard->>PublicAPI: PATCH /:collectionName/:id/recover
  PublicAPI->>MongoDB: findOneAndUpdate {_id,isDeleted:true,deletedAt>=threshold,rlsFilter?} -> {$set isDeleted:false,deletedAt:null}
  MongoDB-->>PublicAPI: recovered document / null
  PublicAPI->>WebhookService: dispatchWebhooks(action: "recover", documentId, options.bypassLimit?)
  WebDashboard->>WebDashboard: update local state, clear isDeleted/deletedAt
  WebDashboard-->>User: show success toast
  alt dashboard scoped recovery
    WebDashboard->>DashboardAPI: PATCH /:projectId/.../data/:id/recover
    DashboardAPI->>MongoDB: findOne (project owner) + findOneAndUpdate restore filter
    MongoDB-->>DashboardAPI: recovered document / null
    DashboardAPI->>WebhookService: dispatchWebhooks(action: "recover", documentId, options.bypassLimit)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

feature, backend, type:feature

Suggested reviewers

  • yash-pouranik

Poem

🐰 A hop, a patch, a gentle beep,

The deleted ones no longer sleep.
DELETED badge fades, the spinner spins,
Thirty days' grace, recovery wins.
Hop—restored! Back to their bins.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: implement soft-delete recovery' accurately summarizes the primary change: adding recovery/restore functionality for soft-deleted documents across backend and frontend.
Linked Issues check ✅ Passed The PR comprehensively implements all key coding requirements from issue #193: PATCH recovery endpoints in both dashboard-api and public-api with proper error handling (404, 409, 400), RLS enforcement, webhook dispatch for recovery events, and BullMQ cleanup synchronization.
Out of Scope Changes check ✅ Passed All changes directly support soft-delete recovery: controller/route additions for recovery endpoints, webhook schema updates for recovery events, cleanup queue synchronization, moved webhook dispatcher to common package for reuse, and frontend UI for recovery actions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js (1)

108-172: ⚡ Quick win

Add a recoverRow duplicate-key conflict test (409 path).

The new recover tests cover success and not-found, but not the unique-conflict branch (err.code === 11000). That path is part of this feature’s critical behavior and should be locked with a test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js`
around lines 108 - 172, Add a new test for recoverRow that simulates a
duplicate-key conflict by having mockFindOneAndUpdate return a Promise rejection
with an error object whose code === 11000; call recoverRow(req, res) with the
same project/request setup used in the existing tests (use makeReq/makeRes,
mockFindOne returning the project), and assert that res.status was called with
409 and res.json was called with the appropriate error message for duplicate key
conflicts. Target the recoverRow function and the mockFindOneAndUpdate mock to
trigger the err.code === 11000 branch.
apps/public-api/src/__tests__/softDelete.test.js (1)

100-138: ⚡ Quick win

Expand recoverSingleDoc tests for conflict and invalid-id branches.

Please add coverage for:

  • duplicate-key conflict (err.code === 11000 → 409), and
  • invalid ID input handling (expected 400 once validation is added).

This will close the most likely regression gaps for the new endpoint.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/public-api/src/__tests__/softDelete.test.js` around lines 100 - 138, Add
two tests for recoverSingleDoc: one that simulates a duplicate-key conflict by
having mockFindOneAndUpdate return a rejected promise with an error object
containing code: 11000 and assert the handler responds with status 409 and an
appropriate error JSON; and another that simulates invalid ID input by setting
req.params.id to an invalid value and having mockFindOneAndUpdate reject with a
validation/CastError (e.g., { name: 'CastError' } or a validation error) and
assert the handler responds with status 400 and the expected error JSON; use the
existing helper functions makeReq, makeRes and the same mockFindOneAndUpdate to
throw/reject to trigger the conflict and invalid-id branches in
recoverSingleDoc.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/dashboard-api/src/controllers/project.controller.js`:
- Around line 993-1042: The recoverRow handler must validate the incoming id and
normalize all responses to the API envelope and use AppError for errors;
specifically, in recoverRow validate req.params.id (e.g., with
mongoose.Types.ObjectId.isValid or a helper) and return res.status(400).json({
success:false, data:{}, message: "Invalid id" }) instead of letting a cast error
occur, replace all direct error JSON returns (Project not found, Collection not
found, Document not found or not in trash) with the standard envelope
(res.status(...).json({ success:false, data:{}, message: "..."})), and in the
catch block wrap errors with AppError (map err.code === 11000 to a 409 AppError
with a safe conflict message) and never return raw err.message to the
client—return a generic message via the envelope and appropriate status code;
locate fixes around recoverRow, the Model.findOneAndUpdate call, and the catch
block to implement these changes.

In `@apps/public-api/src/controllers/data.controller.js`:
- Around line 627-681: recoverSingleDoc currently uses the raw id in
Model.findOneAndUpdate (can throw on invalid ObjectId) and returns
raw/non-enveloped errors; validate the id up-front (e.g. use
mongoose.Types.ObjectId.isValid or your isValidId helper) before calling
getConnection/Model and return a 400 AppError if invalid; replace direct
res.status(...).json(...) error responses (the 404s and the 500 and the
duplicate-key 409 branch) with AppError instances (or call next(new
AppError(..., statusCode))) so all error responses follow the { success, data,
message } envelope and never expose raw err.message from Mongo; keep the
duplicate-key handling but map it to an AppError with status 409 and a safe user
message and preserve the successful response envelope for the recovered
document.

In `@apps/web-dashboard/src/components/RecordList.jsx`:
- Around line 92-107: The icon-only buttons in RecordList.jsx lack stable
accessible names; update both button elements (the recover button inside the
record.isDeleted branch that calls onRecover(record._id) and the fallback
ArrowRight button) to include an explicit aria-label (keep the existing title
attribute), e.g., use a descriptive aria-label like "Recover record" for the
RotateCcw button (optionally append record id or date if needed for uniqueness)
and "Open record" for the ArrowRight button so assistive tech can identify the
actions; ensure these aria-label strings are meaningful and match the intent
provided by getDeletionTooltip/onRecover and the ArrowRight action.

In `@apps/web-dashboard/src/pages/Database.jsx`:
- Around line 170-171: The toast currently concatenates "Failed to restore
document: " with errMsg which can be undefined; update the restore error
handling (the errMsg calculation and the toast.error call in the restore
routine) to ensure a definite fallback string (e.g., "Unknown error" or an empty
string and omit the colon) before concatenation so the toast never shows
"undefined" — set errMsg = err.response?.data?.message ||
err.response?.data?.error || err.message || "Unknown error" (or conditionally
build the message) and then call toast.error with the safe message.

---

Nitpick comments:
In `@apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js`:
- Around line 108-172: Add a new test for recoverRow that simulates a
duplicate-key conflict by having mockFindOneAndUpdate return a Promise rejection
with an error object whose code === 11000; call recoverRow(req, res) with the
same project/request setup used in the existing tests (use makeReq/makeRes,
mockFindOne returning the project), and assert that res.status was called with
409 and res.json was called with the appropriate error message for duplicate key
conflicts. Target the recoverRow function and the mockFindOneAndUpdate mock to
trigger the err.code === 11000 branch.

In `@apps/public-api/src/__tests__/softDelete.test.js`:
- Around line 100-138: Add two tests for recoverSingleDoc: one that simulates a
duplicate-key conflict by having mockFindOneAndUpdate return a rejected promise
with an error object containing code: 11000 and assert the handler responds with
status 409 and an appropriate error JSON; and another that simulates invalid ID
input by setting req.params.id to an invalid value and having
mockFindOneAndUpdate reject with a validation/CastError (e.g., { name:
'CastError' } or a validation error) and assert the handler responds with status
400 and the expected error JSON; use the existing helper functions makeReq,
makeRes and the same mockFindOneAndUpdate to throw/reject to trigger the
conflict and invalid-id branches in recoverSingleDoc.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b24f947c-a807-4a00-8edc-57bdb92951ce

📥 Commits

Reviewing files that changed from the base of the PR and between efe95a8 and 4f47fb6.

📒 Files selected for processing (11)
  • apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js
  • apps/dashboard-api/src/__tests__/routes.projects.storage.test.js
  • apps/dashboard-api/src/controllers/project.controller.js
  • apps/dashboard-api/src/routes/projects.js
  • apps/public-api/src/__tests__/softDelete.test.js
  • apps/public-api/src/controllers/data.controller.js
  • apps/public-api/src/routes/data.js
  • apps/web-dashboard/src/components/CollectionTable.jsx
  • apps/web-dashboard/src/components/RecordList.jsx
  • apps/web-dashboard/src/components/RowDetailDrawer.jsx
  • apps/web-dashboard/src/pages/Database.jsx

Comment thread apps/dashboard-api/src/controllers/project.controller.js Outdated
Comment thread apps/public-api/src/controllers/data.controller.js Outdated
Comment thread apps/web-dashboard/src/components/RecordList.jsx Outdated
Comment thread apps/web-dashboard/src/pages/Database.jsx Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
apps/dashboard-api/src/controllers/project.controller.js (1)

1007-1013: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

recoverRow still mixes direct error responses with AppError flow

Line 1007, Line 1019, Line 1030, and Line 1057 return errors via res.status(...).json(...) instead of next(new AppError(...)). Please route these branches through AppError too, to keep controller error handling consistent.

Suggested patch
-    if (!mongoose.isValidObjectId(id)) {
-      return res.status(400).json({
-        success: false,
-        data: {},
-        message: "Invalid id"
-      });
-    }
+    if (!mongoose.isValidObjectId(id)) {
+      return next(new AppError(400, "Invalid id"));
+    }
@@
-    if (!project) {
-      return res.status(404).json({
-        success: false,
-        data: {},
-        message: "Project not found."
-      });
-    }
+    if (!project) {
+      return next(new AppError(404, "Project not found."));
+    }
@@
-    if (!collectionConfig) {
-      return res.status(404).json({
-        success: false,
-        data: {},
-        message: "Collection not found."
-      });
-    }
+    if (!collectionConfig) {
+      return next(new AppError(404, "Collection not found."));
+    }
@@
-    if (!result) {
-      return res.status(404).json({
-        success: false,
-        data: {},
-        message: "Document not found or not in trash."
-      });
-    }
+    if (!result) {
+      return next(new AppError(404, "Document not found or not in trash."));
+    }

As per coding guidelines, "**/src/controllers/**/*.{js,ts}: All API endpoints return: { success: bool, data: {}, message: "" }. Use AppError class for errors — never raw throw, never expose MongoDB errors to client."

Also applies to: 1019-1025, 1030-1036, 1057-1063

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/dashboard-api/src/controllers/project.controller.js` around lines 1007 -
1013, The recoverRow controller currently returns errors directly with
res.status(...).json(...); update each error branch in recoverRow (the invalid
ObjectId check using mongoose.isValidObjectId(id), the "project not found" path,
the "already deleted"/path checks, and the permission/unauthorized path) to call
next(new AppError(message, statusCode)) instead of sending JSON directly,
preserving the `{ success, data, message }` shape in the AppError message and
status code; ensure AppError is imported/available in this module and replace
the four direct-responses at the checks you see surrounding id validation, the
not-found lookup, the deleted-state check, and the ownership/permission check
with next(new AppError(...)).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@apps/dashboard-api/src/controllers/project.controller.js`:
- Around line 1007-1013: The recoverRow controller currently returns errors
directly with res.status(...).json(...); update each error branch in recoverRow
(the invalid ObjectId check using mongoose.isValidObjectId(id), the "project not
found" path, the "already deleted"/path checks, and the permission/unauthorized
path) to call next(new AppError(message, statusCode)) instead of sending JSON
directly, preserving the `{ success, data, message }` shape in the AppError
message and status code; ensure AppError is imported/available in this module
and replace the four direct-responses at the checks you see surrounding id
validation, the not-found lookup, the deleted-state check, and the
ownership/permission check with next(new AppError(...)).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ac31648-a280-4760-836b-9d8c12a42e05

📥 Commits

Reviewing files that changed from the base of the PR and between 4f47fb6 and 258e3bb.

📒 Files selected for processing (6)
  • apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js
  • apps/dashboard-api/src/controllers/project.controller.js
  • apps/public-api/src/__tests__/softDelete.test.js
  • apps/public-api/src/controllers/data.controller.js
  • apps/web-dashboard/src/components/RecordList.jsx
  • apps/web-dashboard/src/pages/Database.jsx

@MdTowfikomer
Copy link
Copy Markdown
Contributor Author

Hey @yash-pouranik , could you please review the changes and on the frontend, I’ve updated the dashboard with recovery actions let me know if you need any adjustments.!

@yash-pouranik
Copy link
Copy Markdown
Collaborator

Please show sequence diagram
So that i can understand flow
@coderabbitai

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

Tip

For best results, initiate chat on the files or code changes.

The PR description Image 1 shows the "posts" collection table with several records marked "DELETED" (rows 1, 2, 4, 5, 6) and a tooltip on row 4 reading "Deleted on: 19/05/2026, 12:01:53 am (24 days until permanent deletion)". Each deleted row has a restore (↺) icon in the ACTIONS column.

The PR description Image 2 shows the same table after restoration, with a green success banner "Document restored successfully". Records 2, 4, and 7 (among others) no longer show the DELETED badge, while records 1, 5, and 6 still remain deleted.


@yash-pouranik Here is the full sequence diagram for the soft-delete recovery flow covering both the Dashboard API path and the Public API path:

sequenceDiagram
    autonumber

    actor User
    participant UI as Web Dashboard<br/>(CollectionTable / RecordList)
    participant DB_Page as Database.jsx<br/>(handleRecoverRecord)

    box "`#1E64B4`" "Dashboard API"
        participant Auth as authMiddleware
        participant DCtrl as recoverRow<br/>Controller
    end

    box "`#1EA050`" "Public API"
        participant PAuth as apiKey + RLS +<br/>writeAuth Middleware
        participant PCtrl as recoverSingleDoc<br/>Controller
    end

    participant Mongoose as Mongoose Model
    participant MongoDB as MongoDB
    participant Webhook as Webhook Dispatcher

    Note over User,MongoDB: ── Dashboard API path ──

    User->>UI: Clicks Restore button on deleted row
    UI->>DB_Page: onRecover(record._id)
    DB_Page->>Auth: PATCH /api/:projectId/collections/:collectionName/data/:id/recover
    Auth->>Auth: Verify JWT & session
    Auth-->>DCtrl: Attach req.user

    DCtrl->>DCtrl: Validate ObjectId format<br/>(400 if invalid)
    DCtrl->>MongoDB: Project.findOne({ _id: projectId, owner: userId })
    MongoDB-->>DCtrl: project doc (or null → 404)

    DCtrl->>DCtrl: Locate collectionName in project schema<br/>(404 if missing)
    DCtrl->>Mongoose: getConnection() + getCompiledModel()
    DCtrl->>MongoDB: findOneAndUpdate<br/>({ _id: id, isDeleted: true },<br/>{ $set: { isDeleted: false, deletedAt: null } },<br/>{ new: true })
    MongoDB-->>DCtrl: restored doc (null → 404)<br/>or duplicate key error → 409

    DCtrl-->>DB_Page: 200 { success: true, data: restoredDoc }

    DB_Page->>DB_Page: Optimistic UI update:<br/>clear isDeleted & deletedAt in local state
    DB_Page-->>UI: Re-render row (badge removed, icons changed)
    DB_Page-->>User: Toast "Document restored successfully"

    Note over User,MongoDB: ── Public API path ──

    User->>PAuth: PATCH /public/:collectionName/:id/recover<br/>(API key in header)
    PAuth->>PAuth: Verify API key
    PAuth->>PAuth: Check collection not blocked
    PAuth->>PAuth: Enforce usage limit
    PAuth->>PAuth: Resolve public auth context (RLS filter)
    PAuth->>PAuth: Check write authorization
    PAuth-->>PCtrl: Attach req.rlsFilter

    PCtrl->>PCtrl: Validate ObjectId format<br/>(400 if invalid)
    PCtrl->>PCtrl: Verify collection exists<br/>(404 if missing)
    PCtrl->>Mongoose: getCompiledModel()
    PCtrl->>MongoDB: findOneAndUpdate<br/>({ _id: id, isDeleted: true, ...rlsFilter },<br/>{ $set: { isDeleted: false, deletedAt: null } },<br/>{ new: true })
    MongoDB-->>PCtrl: restored doc (null → 404)<br/>or duplicate key error (code 11000) → 409

    PCtrl->>Webhook: dispatch "update" event with restored doc
    PCtrl-->>User: 200 { success: true, data: restoredDoc,<br/>message: "Document recovered from trash" }
Loading

Key points from the diagram

Step Detail
ObjectId validation Both controllers reject malformed IDs with 400 before any DB call
Owner / RLS check Dashboard API checks Project.owner === req.user; Public API injects req.rlsFilter into the query
DB operation findOneAndUpdate filters on isDeleted: true — already-active docs are silently 404'd
Conflict (409) If restoring a doc would violate a unique index (e.g. duplicate slug), Mongo error code 11000 is caught and surfaced as 409
Optimistic UI handleRecoverRecord clears isDeleted/deletedAt in local React state immediately, so the DELETED badge disappears without a full reload
Webhook Only the Public API dispatches an update webhook after a successful recovery

@yash-pouranik
Copy link
Copy Markdown
Collaborator

reallly nice design
is there anything to change?
@coderabbitai

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 25, 2026

Tip

For best results, initiate chat on the files or code changes.

@yash-pouranik The PR description images show:

  • Image 1: The "posts" collection with records 1, 2, 4, 5, 6 all bearing a "DELETED" badge, a tooltip on row 4 reading "Deleted on: 19/05/2026, 12:01:53 am (24 days until permanent deletion)", and a restore (↺) icon in the ACTIONS column for each deleted row.
  • Image 2: After restoration, a green "Document restored successfully" banner appears. Records 2 and 4 no longer show the DELETED badge, while 1, 5, and 6 remain deleted.

The design is clean! Here are a few things worth considering before merging:


1. Webhook inconsistency between the two APIs

recoverSingleDoc (Public API) dispatches an update webhook after recovery, but recoverRow (Dashboard API) does not. If consumers subscribe to webhooks, they'll miss recovery events triggered from the dashboard.

Suggestion: Add a webhook dispatch call in recoverRow after the successful findOneAndUpdate, matching what recoverSingleDoc already does.


2. BullMQ cleanup job cancellation (from issue #193)

Issue #193 explicitly mentioned optionally cancelling/removing pending BullMQ cleanup jobs when a document is recovered. This PR skips that step. The worker already filters by deletedAt < 30 days, so it won't accidentally delete a restored doc — but the orphaned job will linger unnecessarily.

Suggestion: Either implement job cancellation or open a follow-up issue to track it.


3. Optimistic UI rollback on failure

handleRecoverRecord in Database.jsx updates local state (clearing isDeleted/deletedAt) and then shows a toast on error. If the state is cleared before the API call resolves, a failed recovery would leave the UI in an incorrect "restored" state until a full reload.

Suggestion: Confirm the state update only runs inside the success branch (after await api.patch(...) resolves successfully), or implement a proper rollback in the catch block.


4. Static "days until deletion" countdown

Both CollectionTable and RecordList capture now in component state once on mount. The tooltip text (e.g., "24 days until permanent deletion") is therefore static for the lifetime of the component — it won't tick down. This is a minor UX nit, but worth noting.

Suggestion: This is cosmetic and can be left as a follow-up if real-time countdown isn't a priority.


5. Missing aria-label on the restore button in CollectionTable

RecordList's recover button includes an aria-label, but the equivalent button in CollectionTable may not (based on the summary). Screen-reader users would see an unlabelled icon button.

Suggestion: Add aria-label="Restore record" (or similar) to the restore button in CollectionTable.


None of these are showstoppers — the core recovery logic (validation → RLS check → findOneAndUpdate → 404/409 handling → optimistic UI) is solid. Items 1 and 3 are the most important to address.

(≧◡≦) / 🐇

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/dashboard-api/src/controllers/project.controller.js (1)

944-969: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate id before querying in deleteRow to avoid false 500s.

Line 944 reads id but deleteRow never validates it. A malformed ID can throw a cast error and incorrectly return Line 993’s 500 instead of a 400 client error.

Suggested patch
 module.exports.deleteRow = async (req, res, next) => {
   try {
     const { projectId, collectionName, id } = req.params;
+    if (!mongoose.isValidObjectId(id)) {
+      return next(new AppError(400, "Invalid document ID format."));
+    }

     const project = await Project.findOne({
       _id: projectId,
       owner: req.user._id,

As per coding guidelines, "**/src/controllers/**/*.{js,ts}: All API endpoints return: { success: bool, data: {}, message: "" }. Use AppError class for errors — never raw throw, never expose MongoDB errors to client."

Also applies to: 991-994

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/dashboard-api/src/controllers/project.controller.js` around lines 944 -
969, The deleteRow handler should validate the route param id before querying to
avoid MongoDB cast errors: add an ObjectId validity check (e.g., using
mongoose.Types.ObjectId.isValid) at the start of deleteRow (before calling
getConnection/getCompiledModel and before Model.findOneAndUpdate) and, if
invalid, call next(new AppError(400, "Invalid id parameter")) so the endpoint
returns a 400 instead of a 500; ensure this validation is applied to the id used
in the findOneAndUpdate call and follows the API response conventions for {
success, data, message } and AppError usage.
apps/web-dashboard/src/components/CollectionTable.jsx (1)

56-59: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clamp deletion countdown to avoid negative “days remaining”.

daysRemaining can go below 0, producing confusing tooltips. Clamp it at 0 before rendering.

Suggested fix
 const getDeletionTooltip = (deletedAt, now) => {
     if (!deletedAt || !now) return "";
-    const daysRemaining = 30 - Math.floor((now - new Date(deletedAt).getTime()) / (1000 * 60 * 60 * 24));
+    const elapsedDays = Math.floor((now - new Date(deletedAt).getTime()) / (1000 * 60 * 60 * 24));
+    const daysRemaining = Math.max(0, 30 - elapsedDays);
     return `Deleted on: ${formatDate(deletedAt)} (${daysRemaining} days until permanent deletion)`;
 };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web-dashboard/src/components/CollectionTable.jsx` around lines 56 - 59,
The tooltip can show negative days because daysRemaining is not clamped; in
getDeletionTooltip(deletedAt, now) compute daysRemaining as currently done and
then clamp it to a minimum of 0 (e.g., daysRemaining = Math.max(0,
daysRemaining)) before using it in the returned string that calls
formatDate(deletedAt), so the tooltip never displays negative “days remaining.”
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@apps/dashboard-api/src/controllers/project.controller.js`:
- Around line 944-969: The deleteRow handler should validate the route param id
before querying to avoid MongoDB cast errors: add an ObjectId validity check
(e.g., using mongoose.Types.ObjectId.isValid) at the start of deleteRow (before
calling getConnection/getCompiledModel and before Model.findOneAndUpdate) and,
if invalid, call next(new AppError(400, "Invalid id parameter")) so the endpoint
returns a 400 instead of a 500; ensure this validation is applied to the id used
in the findOneAndUpdate call and follows the API response conventions for {
success, data, message } and AppError usage.

In `@apps/web-dashboard/src/components/CollectionTable.jsx`:
- Around line 56-59: The tooltip can show negative days because daysRemaining is
not clamped; in getDeletionTooltip(deletedAt, now) compute daysRemaining as
currently done and then clamp it to a minimum of 0 (e.g., daysRemaining =
Math.max(0, daysRemaining)) before using it in the returned string that calls
formatDate(deletedAt), so the tooltip never displays negative “days remaining.”

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6673a33e-925d-4e90-b6a0-f50faef4f237

📥 Commits

Reviewing files that changed from the base of the PR and between 258e3bb and 12c2611.

📒 Files selected for processing (19)
  • apps/dashboard-api/src/__tests__/project.controller.softDelete.test.js
  • apps/dashboard-api/src/controllers/project.controller.js
  • apps/public-api/src/__tests__/softDelete.test.js
  • apps/public-api/src/__tests__/storage.controller.test.js
  • apps/public-api/src/controllers/data.controller.js
  • apps/public-api/src/controllers/storage.controller.js
  • apps/public-api/src/middlewares/api_usage.js
  • apps/public-api/src/middlewares/usageGate.js
  • apps/public-api/src/utils/webhookDispatcher.js
  • apps/web-dashboard/src/components/CollectionTable.jsx
  • apps/web-dashboard/src/components/RecordList.jsx
  • apps/web-dashboard/src/index.css
  • apps/web-dashboard/src/pages/Database.jsx
  • packages/common/src/index.js
  • packages/common/src/models/Webhook.js
  • packages/common/src/queues/trashCleanupQueue.js
  • packages/common/src/utils/input.validation.js
  • packages/common/src/utils/usageCounter.js
  • packages/common/src/utils/webhookDispatcher.js
💤 Files with no reviewable changes (1)
  • apps/public-api/src/utils/webhookDispatcher.js
✅ Files skipped from review due to trivial changes (2)
  • apps/public-api/src/tests/storage.controller.test.js
  • packages/common/src/utils/input.validation.js

@MdTowfikomer
Copy link
Copy Markdown
Contributor Author

Hey @yash-pouranik, could you please review the changes, here is a breakdown of the core improvements in this commit

  • Centralized Utils: Moved webhookDispatcher.js and usageCounter.js from apps/public-api to packages/common.
    Cause Both the Public and Dashboard APIs now trigger webhooks and track usage (especially for recovery events). Centralizing these ensures a single source of truth and prevents logic drift between services.

  • Schema Integration: Formally added recover to the webhookEventConfigSchema. Developers can now explicitly subscribe to and validate recovery-specific webhook subscriptions.

  • Hardened 30-Day Window: APIs now block recovery queries if deletedAt with in the last 30 days, ensuring policy compliance even if background cleanup is pending.

  • Deep Redaction: Webhook dispatcher now recursively scrubs sensitive keys (password, token, secret) from all outgoing payloads.

  • Proactive Queue Sync: Added syncCollectionCleanup to cancel orphaned BullMQ jobs or reschedule them to the next valid deadline immediately upon recovery.

  • Fail-Safe UI: Frontend now uses optimistic updates with a full try/catch rollback to prevent local/server state desync if a recovery request fails.

@yash-pouranik
Copy link
Copy Markdown
Collaborator

@MdTowfikomer
u told-

  • Schema Integration: Formally added recover to the webhookEventConfigSchema. Developers can now explicitly subscribe to and validate recovery-specific webhook subscriptions.
    but from where in UI? please point

ELse I checked and well implemented.

@MdTowfikomer
Copy link
Copy Markdown
Contributor Author

MdTowfikomer commented May 25, 2026

@yash-pouranik ,recover is added in packages/common/src/utils/input.validation.js as a webhook event flag. It lets the backend validate and accept recovery events, and the existing webhook UI can save it like the other event flags, like insert, update, and delete in the same config object, so recover can be saved the same way by the backend

  • backend accepts recover
  • UI does not expose it yet

@yash-pouranik
Copy link
Copy Markdown
Collaborator

OKay got it
Will check whole implementation myself in some hour and get back to you soon or tomorrow.
@MdTowfikomer

@MdTowfikomer
Copy link
Copy Markdown
Contributor Author

Sure, take you time..👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(api): implement recovery endpoint for soft-deleted documents

2 participants