Skip to content

Conversation

@serforin0
Copy link

@serforin0 serforin0 commented Nov 6, 2025

What does this PR do?

(Provide a description of what this PR does.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

Summary by CodeRabbit

  • Bug Fixes

    • Updated Row model's data field to store the complete input map instead of selectively extracting a nested key, improving data preservation.
  • Tests

    • Added test coverage for Row model data field handling across different scenarios.

…'data'

- Changed data assignment from 'map[\"data\"] ?? map' to 'map' in Row.fromMap constructor
- This fixes the TypeError that occurred when a table had a column named 'data'
- Added comprehensive tests to verify the fix and prevent regressions
- Fixes appwrite#281"
…data

- Changed data assignment to use complete map instead of data field
- This fixes TypeError when table has column named data
- Added tests to verify the fix
- Fixes appwrite#281
@coderabbitai
Copy link

coderabbitai bot commented Nov 6, 2025

Walkthrough

This change modifies the Row model's deserialization logic in fromMap. Previously, the data field attempted to extract a nested "data" key from the input map with a fallback to the entire map. The updated implementation stores the entire input map directly in the data field regardless of whether a separate "data" key exists. The toMap serialization method remains unchanged. Two test cases are added to verify correct behavior when a "data" column exists and when it doesn't.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • The core logic change is isolated to a single line in one file
  • The modification is straightforward (assignment of entire map rather than conditional extraction)
  • Test coverage is added to verify the new behavior for both scenarios (with and without "data" column)
  • Focus areas for review:
    • Verify the behavioral change is intentional and understand the motivation for storing the entire input map
    • Confirm that existing code that depends on Row deserialization won't break with this change
    • Validate that the added tests adequately cover edge cases and potential null/empty map scenarios

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the bug being fixed (Row.fromMap data column handling), matching the actual changes in the codebase.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bfb1a4f and 032c96e.

📒 Files selected for processing (2)
  • lib/src/models/row.dart (1 hunks)
  • test/src/models/row_test.dart (1 hunks)
🔇 Additional comments (3)
test/src/models/row_test.dart (2)

30-60: Test correctly verifies the bug fix for columns named "data".

This test validates that when a database column is named "data" with a string value, fromMap correctly stores the entire map in row.data without type errors. The assertions properly verify that both system fields and custom columns (including the "data" column) are accessible.

Note: This test only validates fromMap behavior, not round-trip serialization. Consider adding a test that calls toMap() on the result to verify symmetry.


62-84: Test provides good coverage for the common case without a "data" column.

This test ensures that fromMap works correctly when there's no column named "data", which is the typical scenario. The assertions properly verify that the entire map is stored in row.data and all fields are accessible.

lib/src/models/row.dart (1)

48-49: ****

The original review misunderstands the intended design. The test file at test/src/models/row_test.dart explicitly verifies that the data field is meant to contain the entire input map, including system fields.

The test "fromMap should handle columns named data without type errors" confirms this by expecting:

  • row.data['\$id'] (system field)
  • row.data['data'] (user column with string value)
  • row.data['name'] (user column)
  • row.data['value'] (user column)

The change from map["data"] ?? map to map correctly fixes issue #281 (type errors when a database column is named "data"). The round-trip serialization works as designed: toMap nests the entire data map under a "data" key, and fromMap reconstructs it. This is not asymmetry—it's the intended behavior, fully covered by tests.

Likely an incorrect or invalid review comment.


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.

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

This library is auto-generated by the Appwrite SDK Generator, and does not accept pull requests directly. To learn more about how you can help us improve this SDK, please check the contributing guide.

@github-actions github-actions bot closed this Nov 6, 2025
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