Skip to content

added _deletedAt to $deletedAt translation#602

Merged
abnegate merged 4 commits intomainfrom
feat-_deletedAt-attr
Apr 29, 2026
Merged

added _deletedAt to $deletedAt translation#602
abnegate merged 4 commits intomainfrom
feat-_deletedAt-attr

Conversation

@shimonewman
Copy link
Copy Markdown
Contributor

@shimonewman shimonewman commented Jun 3, 2025

Summary by CodeRabbit

  • New Features
    • Added support for handling a deleted timestamp attribute in the database adapter.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 3, 2025

Warning

Rate limit exceeded

@Copilot has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 49 minutes and 34 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, 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 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.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 94f59146-cf19-4e10-9522-6b2fbc94b524

📥 Commits

Reviewing files that changed from the base of the PR and between 5b44243 and 26e3416.

📒 Files selected for processing (2)
  • src/Database/Adapter/Mongo.php
  • src/Database/Adapter/SQL.php
📝 Walkthrough

Walkthrough

A new mapping was added in the SQL adapter's internal method to associate the attribute key '$deletedAt' with the database column '_deletedAt'. This extends the attribute-to-column translation logic to recognize and handle a deleted timestamp attribute.

Changes

File Change Summary
src/Database/Adapter/SQL.php Added mapping for '$deletedAt' to '_deletedAt' in attribute-to-column conversion logic

Poem

A hop and a skip in SQL's den,
Now $deletedAt is mapped again.
From code to column, smooth and neat,
_deletedAt makes the mapping complete.
With every change, the data flows right—
The rabbit’s code is out of sight! 🐇

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-_deletedAt-attr

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
Review rate limit: 0/1 reviews remaining, refill in 49 minutes and 34 seconds.

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: 1

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3db51ec and 5b44243.

📒 Files selected for processing (1)
  • src/Database/Adapter/SQL.php (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Setup & Build Docker Image

Comment thread src/Database/Adapter/SQL.php
@abnegate abnegate changed the base branch from 0.69.x to main April 29, 2026 04:54
Copilot AI requested a review from abnegate April 29, 2026 04:55
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 29, 2026

Greptile Summary

This PR adds $deletedAt_deletedAt translation support across the SQL and Mongo adapters to enable soft-delete timestamp handling. The SQL side is implemented correctly — document deserialization and the conditional projection in getAttributeProjection both look sound. Two gaps remain in the Mongo adapter.

  • Mongo projection bug: getAttributeProjection adds $deletedAt directly to the MongoDB projection map (since it is absent from Database::INTERNAL_ATTRIBUTES), projecting a non-existent $deletedAt field instead of _deletedAt, so the value is always null on read.
  • Mongo filter datetime bug: buildFilter's datetime normalisation (ISO string → UTCDateTime) only covers $createdAt/$updatedAt; $deletedAt is skipped in schema mode, causing date-range queries to return no results.

Confidence Score: 3/5

Not safe to merge — two P1 bugs in the Mongo adapter will cause silent data loss and broken queries on $deletedAt.

Two P1 findings: wrong field name in Mongo projections (value always null) and missing datetime normalisation in Mongo filters (date queries silently empty in schema mode). SQL changes are correct and lower the floor only slightly.

src/Database/Adapter/Mongo.php — getAttributeProjection and buildFilter both need updates for $deletedAt.

Important Files Changed

Filename Overview
src/Database/Adapter/Mongo.php Adds $deletedAt_deletedAt to getInternalKeyForAttribute, but getAttributeProjection will project the wrong field name and buildFilter skips datetime normalisation for $deletedAt queries when in schema mode.
src/Database/Adapter/SQL.php Adds _deletedAt$deletedAt document deserialization and conditional projection support; the $hasDeletedAt flag correctly prevents injecting _deletedAt into tables that don't have the column.

Comments Outside Diff (2)

  1. src/Database/Adapter/Mongo.php, line 3009-3033 (link)

    P1 $deletedAt projection uses wrong field name

    getAttributeProjection builds $internalKeys from Database::INTERNAL_ATTRIBUTES, which does not include $deletedAt. This means when $deletedAt is in $selections, it is not skipped by the internal-key guard on line 3020 and instead falls through to $projection[$selection] = 1, producing $projection['$deletedAt'] = 1. MongoDB will project the literal key $deletedAt (which does not exist — the field is stored as _deletedAt), so the field will come back null and the translated $deletedAt on the read path will never be populated.

  2. src/Database/Adapter/Mongo.php, line 2705 (link)

    P1 $deletedAt datetime not normalized in query filters

    Line 2705 normalises ISO 8601 string values to UTCDateTime only when !$this->getSupportForAttributes() (schemaless mode) or the attribute is $createdAt/$updatedAt. When getSupportForAttributes() returns true and a caller filters by $deletedAt, the condition is false so string datetime values are not converted to UTCDateTime. The field is then correctly remapped to _deletedAt by escapeMongoFieldName on line 2734, but the value stays as a plain string, causing MongoDB date comparisons to silently return no results (string ≠ UTCDateTime BSON type).

Reviews (2): Last reviewed commit: "fix: only select _deletedAt when explici..." | Re-trigger Greptile

Comment thread src/Database/Adapter/SQL.php Outdated
Comment thread src/Database/Adapter/SQL.php
@abnegate
Copy link
Copy Markdown
Member

@copilot Address greptile comments

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 29, 2026

@copilot Address greptile comments

Addressed in commit 26e3416:

  • P1: Removed '$deletedAt' from the always-selected $internalKeys array in getAttributeProjection. _deletedAt is now only included in the SELECT when $deletedAt is explicitly in the caller's selections — preventing "Unknown column" errors on tables without that column.
  • P2: Added '$deletedAt' => '_deletedAt' to the Mongo adapter's getInternalKeyForAttribute so MongoDB-backed stores translate the field correctly.

Copilot AI requested a review from abnegate April 29, 2026 05:05
@abnegate abnegate merged commit 2214a7e into main Apr 29, 2026
23 checks passed
@abnegate abnegate deleted the feat-_deletedAt-attr branch April 29, 2026 06:31
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.

3 participants