Skip to content

Fix bulk update encoding with defaults - #732

Merged
abnegate merged 2 commits into
mainfrom
fix-bulk-update-encode
Oct 7, 2025
Merged

Fix bulk update encoding with defaults#732
abnegate merged 2 commits into
mainfrom
fix-bulk-update-encode

Conversation

@abnegate

@abnegateabnegate commented Oct 7, 2025

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes

    • Successive document updates no longer reset attributes with default values; updates now preserve defaults and previously set values for single and batch operations.
  • Tests

    • Added end-to-end tests verifying defaults persist across successive batch and single-document updates.
    • Minor test cleanup to improve readability of assertions.

@coderabbitai

coderabbitaiBot commented Oct 7, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

UpdateDocuments now suppresses default application during encoding by passing applyDefaults = false; encode is publicly exposed with a new third parameter to control defaults. Two e2e tests added to ensure successive updates do not reset defaulted attributes; two inline comments removed in relationship tests.

Changes

Cohort / File(s)Summary
Core: encoding API
src/Database/Database.php
Made encode public and added bool $applyDefaults = true param with docblock; behavior now allows skipping default application when false.
Core: update flow
src/Database/Database.php
updateDocuments now calls encode(..., applyDefaults: false) during update paths so defaults are not applied/re-applied before validation/persistence.
E2E: defaults persistence tests
tests/e2e/Adapter/Scopes/DocumentTests.php
Added two tests: testUpdateDocumentsSuccessiveCallsDoNotResetDefaults and testUpdateDocumentSuccessiveCallsDoNotResetDefaults to verify successive updates do not reset attributes with defaults.
Test cleanup
tests/e2e/Adapter/Scopes/RelationshipTests.php
Removed two inline comments adjacent to assertEquals calls; no behavioral changes.

Sequence Diagram(s)

sequenceDiagram
autonumber
actor Client
participant DB as Database::updateDocuments
participant V as Validator
participant E as Encoder
participant A as Adapter
Client->>DB: updateDocuments(collection, ids, updates)
DB->>V: validate(updates) on raw input
alt validation fails
V-->>DB: validation error
DB-->>Client: error
else validation passes
loop for each document id
DB->>E: encode(document, updates, applyDefaults=false)
E-->>DB: encodedDocument (defaults not applied)
DB->>A: adapter.update(encodedDocument)
A-->>DB: result
end
DB-->>Client: aggregated result
end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • ArnabChatterjee20k
  • fogelito

Poem

I hop through code and nibble lines,
Defaults stay put, no fearful signs.
Encode I hold till just the right beat,
Successive updates — no retreat.
A carrot cheer for tests complete! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title Check✅ PassedThe title succinctly captures the primary change, which is correcting how bulk updates handle encoding of default values during the update process. It is concise, clearly reflects the core update without extraneous detail, and aligns with the modifications to the updateDocuments method.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-bulk-update-encode

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 939c120 and b1f51f1.

📒 Files selected for processing (1)
  • src/Database/Database.php (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Database/Database.php (2)
src/Database/Document.php (1)
  • Document (12-470)
src/Database/Validator/Queries/Document.php (1)
  • Document (10-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Adapter Tests (SharedTables/Postgres)
  • GitHub Check: Adapter Tests (Pool)
  • GitHub Check: Adapter Tests (Mirror)
  • GitHub Check: Adapter Tests (SharedTables/MySQL)
  • GitHub Check: Adapter Tests (SharedTables/MariaDB)
  • GitHub Check: Adapter Tests (SQLite)
  • GitHub Check: Adapter Tests (SharedTables/SQLite)
  • GitHub Check: Adapter Tests (MySQL)
  • GitHub Check: Adapter Tests (Postgres)
  • GitHub Check: Adapter Tests (MariaDB)
  • GitHub Check: Unit Test
🔇 Additional comments (1)
src/Database/Database.php (1)

7065-7119: encode() signature change safe: no subclasses override encode(), so the updated method signature is compatible.


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.

@abnegate
abnegate merged commit 35c978d into mainOct 7, 2025
15 checks passed
@abnegate
abnegate deleted the fix-bulk-update-encode branch October 7, 2025 03:09
@abnegateabnegate mentioned this pull request Oct 7, 2025
2 tasks
Sign up for freeto 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

@abnegate