Skip to content

fix: return the mapped document type for an empty id - #974

Merged
loks0n merged 1 commit into
mainfrom
fix/typed-empty-document
Sep 16, 2026
Merged

loks0n merged 1 commit into
mainfrom
fix/typed-empty-document

Conversation

@loks0n

@loks0n loks0n commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

getDocument() short-circuits on an empty id before it looks the collection up, and returned a plain Document there even when the collection is mapped to a subclass with setDocumentType(). Every other empty result (missing row, denied read, negative cache) already comes back as the mapped class, so a caller relying on that class, such as an HTTP resource typed to return the current user or team, hit a TypeError only on the empty-id path.

Unit test added (tests/unit/EmptyDocumentTypeTest.php, Memory adapters, no services); it fails on main with Failed asserting that an instance of class Utopia\Database\Document is an instance of class Tests\Unit\TypedUser.

Lets appwrite-labs/cloud drop the per-request new Project($doc->getArrayCopy()) rebuild (appwrite-labs/cloud#5861).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Custom document types configured for a collection are now returned when requesting an empty document ID.
    • Requests for missing documents continue to return the configured custom document type when available.
    • Unmapped collections continue to return a standard document.

getDocument() short-circuits on an empty id before it looks the collection
up, and returned a plain Document there even when the collection is mapped
to a subclass with setDocumentType(). Every other empty result (missing row,
denied read, negative cache) already comes back as the mapped class, so a
caller relying on that class, such as an HTTP resource typed to return the
current user or team, hit a TypeError only on the empty-id path.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 0b4c2a60-0df9-49e2-9b70-15b7d57a07d6

📥 Commits

Reviewing files that changed from the base of the PR and between 54364c6 and f528967.

📒 Files selected for processing (2)
  • src/Database/Database.php
  • tests/unit/EmptyDocumentTypeTest.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

getDocument() now returns the configured collection document type for empty and missing document IDs. Tests cover mapped collections and the plain Document fallback for unmapped collections.

Changes

Mapped document instance handling

Layer / File(s) Summary
Document creation and validation
src/Database/Database.php, tests/unit/EmptyDocumentTypeTest.php
Empty-ID lookups now use createDocumentInstance(). Tests verify mapped collections return empty TypedUser instances for empty and missing IDs, while unmapped collections return a plain Document.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f5289

The empty-document lookup preserves mapped document types while retaining the plain Document fallback for unmapped collections. The change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. 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 clearly and concisely describes the main change: returning the mapped document type when the document ID is empty.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/typed-empty-document

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.


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.

@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with no actionable correctness, security, or test-quality issues identified.

Summary

The PR makes getDocument() preserve the configured collection-specific document type when an empty ID short-circuits the read.

  • Uses the existing custom-document factory for empty-ID results.
  • Adds coverage for mapped and unmapped collections.
  • Keeps empty-result semantics consistent with missing-record paths.

Reviews (1) · Last reviewed commit: "fix: return the mapped document type for..."

@loks0n
loks0n merged commit 9f601bf into main Sep 16, 2026
22 checks passed
@loks0n
loks0n deleted the fix/typed-empty-document branch September 16, 2026 09:52
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.

1 participant