Fix error in account_objects request type filter

[ckeshava]

High Level Overview of Change

fix #888: Allow canonical case-insensitive ledger-object names in type filter. Please refer to the issue description for more context on this issue.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Tests (You added tests for code that already exists, or your new feature included in this PR)
• Documentation Updates
• Release

Did you update CHANGELOG.md?

• Yes
• No, this change does not impact library users

Test Plan

Added a new test with canonical str inputs to the type filer in the account_objects request.

[coderabbitai]

Walkthrough

The AccountObjects request model now accepts canonical ledger entry names as strings (e.g., "Escrow") in addition to the AccountObjectType enum, aligning with rippled 2.4.0's ledger entry naming support. A test validates both enum and string type filtering.

Changes

Cohort / File(s)	Summary
Model type expansion xrpl/models/requests/account_objects.py	Modified type field to accept Union[AccountObjectType, str] instead of only AccountObjectType, enabling canonical ledger entry names as string values
Test validation tests/integration/reqs/test_account_objects.py	Added async test test_type_filter validating type filtering with exact enum match ("Escrow") and case-insensitive string variant ("mPtOkeNisSuance")
Documentation CHANGELOG.md	Added Fixed subsection under Unreleased documenting the type filter validation fix (issue #888)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Straightforward type annotation change adding Union support
• Test method validates the behavioral change directly
• Consistent pattern across files with minimal logic density

Suggested reviewers

• achowdhry-ripple
• pdp2121

Poem

🐰 A string now dances where enums stood tall,
Canonical names answer the call,
"Escrow" and "mPtOkeNisSuance" align,
The account_objects request now feels just fine!
Tests verify all with a whisker-twitch smile! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: fixing an error in the account_objects request type filter, which aligns with the PR's primary objective of enabling canonical case-insensitive ledger-object names in the type field.
Description check	✅ Passed	The description follows the template structure with High Level Overview, Type of Change selection, CHANGELOG.md update confirmation, and Test Plan. All required sections are completed with relevant information about the bug fix.
Linked Issues check	✅ Passed	The PR successfully addresses issue #888 by modifying AccountObjects.type to accept Union[AccountObjectType, str], enabling canonical ledger entry names like 'Escrow' to be used as strings in the type filter.
Out of Scope Changes check	✅ Passed	All changes (type annotation update in AccountObjects model, test additions, and CHANGELOG update) are directly related to fixing the type filter validation issue and include no unrelated modifications.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 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 1464e9e and 0f4613f.

📒 Files selected for processing (3)

• CHANGELOG.md (1 hunks)
• tests/integration/reqs/test_account_objects.py (1 hunks)
• xrpl/models/requests/account_objects.py (2 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2024-10-30T20:32:03.246Z

Learnt from: ckeshava
Repo: XRPLF/xrpl-py PR: 759
File: xrpl/models/transactions/credential_accept.py:27-39
Timestamp: 2024-10-30T20:32:03.246Z
Learning: In the `xrpl` codebase, when defining required fields in dataclasses (e.g., `account: str = REQUIRED`), it's necessary to include `# type: ignore` to prevent `mypy` errors.

Applied to files:

• xrpl/models/requests/account_objects.py

📚 Learning: 2025-06-04T22:17:47.822Z

Learnt from: ckeshava
Repo: XRPLF/xrpl-py PR: 814
File: xrpl/models/requests/vault_info.py:16-31
Timestamp: 2025-06-04T22:17:47.822Z
Learning: In the xrpl-py library, Request classes do not perform client-side validation on their contents. The library relies on the rippled node to return appropriate error codes for malformed requests, maintaining a consistent pattern across all Request models.

Applied to files:

• xrpl/models/requests/account_objects.py

📚 Learning: 2024-11-04T19:41:04.808Z

Learnt from: mvadari
Repo: XRPLF/xrpl-py PR: 759
File: tests/integration/transactions/test_credential.py:31-37
Timestamp: 2024-11-04T19:41:04.808Z
Learning: The `is_cred_object_present` function in `tests/integration/transactions/test_credential.py` assumes that the fields `'Issuer'`, `'Subject'`, and `'CredentialType'` are always present in the account object dictionaries within `account_objects`.

Applied to files:

• tests/integration/reqs/test_account_objects.py

🧬 Code graph analysis (1)

tests/integration/reqs/test_account_objects.py (3)

xrpl/models/requests/account_objects.py (1)

• AccountObjects (50-73)

xrpl/wallet/main.py (1)

• address (24-29)

xrpl/models/response.py (1)

• is_successful (74-82)

⏰ 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). (8)

• GitHub Check: Integration test (3.14)
• GitHub Check: Integration test (3.12)
• GitHub Check: Integration test (3.9)
• GitHub Check: Integration test (3.10)
• GitHub Check: Integration test (3.11)
• GitHub Check: Integration test (3.13)
• GitHub Check: Integration test (3.8)
• GitHub Check: semgrep-cloud-platform/scan

🔇 Additional comments (4)

xrpl/models/requests/account_objects.py (2)

12-12: LGTM! Import correctly added.

The Union import is necessary for the type annotation change and follows standard Python typing conventions.

---

68-68: LGTM! Type annotation correctly updated.

The change from Optional[AccountObjectType] to Optional[Union[AccountObjectType, str]] correctly implements support for canonical ledger entry names as strings (e.g., "Escrow", "MPTokenIssuance") while maintaining backward compatibility with the enum. This aligns with rippled 2.4.0's support for canonical names and follows the library's pattern of delegating validation to the server.

Based on learnings, Request classes in this library don't perform client-side validation and rely on rippled to return appropriate error codes for invalid inputs.

CHANGELOG.md (1)

10-12: LGTM! Changelog properly updated.

The changelog entry correctly documents the fix under the "Fixed" category and properly references issue #888. The format follows Keep a Changelog conventions.

tests/integration/reqs/test_account_objects.py (1)

17-36: LGTM! Test effectively validates the fix.

The test correctly verifies that string-based type filters are accepted without validation errors, addressing issue #888. Testing both exact ("Escrow") and case-insensitive ("mPtOkeNisSuance") inputs ensures the feature works as expected with rippled 2.4.0's canonical name support.

The test appropriately focuses on verifying that the request succeeds rather than validating the server's filtering behavior, which is consistent with integration testing practices and the library's pattern of delegating validation to the server.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Patel-Raj11]

I think same applies to LedgerEntryType based on XRPLF/rippled#5271. Can you check and update LedgerData request as well?

[mvadari]

Instead of just using a str there should probably be another enum for the actual ledger entry types