Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add include_deleted param to ledger_entry API #721

Merged
merged 19 commits into from
Oct 26, 2024
Merged

Add include_deleted param to ledger_entry API #721

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
29468e7
add nfts_by_issuer
May 16, 2024
208d969
update Changelog
Kassaking7 May 16, 2024
a2fe883
added nfts_by_issuer to Request's get_method
Kassaking7 May 22, 2024
20885b3
reformat changelog and add required comment for nfts_by_issuer.py
Kassaking7 May 22, 2024
d70f8fe
Merge branch 'main' into main
Kassaking7 Jun 11, 2024
1038ef7
add include_delete field
Kassaking7 Jun 14, 2024
9b3ae2e
update change log
Kassaking7 Jun 14, 2024
8dd215c
Merge pull request #1 from Kassaking7/add-include-delete-to-ledger-en…
Kassaking7 Jul 3, 2024
f7b8144
fix conflict
Kassaking7 Jul 3, 2024
910fb14
another conflict fixed
Kassaking7 Jul 3, 2024
e957057
Merge branch 'XRPLF:main' into main
Kassaking7 Jul 9, 2024
23dcc9b
add a comment
Kassaking7 Jul 10, 2024
b2b12c1
style fix
Kassaking7 Jul 10, 2024
4baff7c
Merge branch 'XRPLF:main' into main
Kassaking7 Jul 10, 2024
cda547b
fix a style issue
Kassaking7 Jul 10, 2024
945f018
changed to more descriptive comment
Kassaking7 Jul 10, 2024
ce4fe99
Merge branch 'main' into main
ckeshava Jul 22, 2024
1512949
Merge branch 'main' into main
mvadari Oct 25, 2024
28db48a
update CHANGELOG
Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Support for the Price Oracles amendment (XLS-47).
- Add `nfts_by_issuer` clio-only API definition
- Included `ctid` field in the `tx` request.
- Add `include_deleted` to ledger_entry request
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Move the include_deleted entry to the "Unreleased" section.

The changelog entry for adding include_deleted to ledger_entry request is currently under version 2.6.0, which is a released version. Since this is a new feature being added in PR #721, it should be documented under the "Unreleased" section instead.

Apply this diff to move the entry:

 ## [[Unreleased]]
 
 ### BREAKING CHANGE:
 - Remove Python 3.7 support to fix dependency installation and use 3.8 as new default.
+
+### Added
+- Add `include_deleted` to ledger_entry request
 
 ## [2.6.0] - 2024-06-03
 
 ### Added
 - Support for the Price Oracles amendment (XLS-47).
 - Add `nfts_by_issuer` clio-only API definition
 - Included `ctid` field in the `tx` request.
-- Add `include_deleted` to ledger_entry request
 - `from_xrpl` method accepts input dictionary keys exclusively in the proper XRPL format.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Add `include_deleted` to ledger_entry request
## [[Unreleased]]
### BREAKING CHANGE:
- Remove Python 3.7 support to fix dependency installation and use 3.8 as new default.
### Added
- Add `include_deleted` to ledger_entry request
## [2.6.0] - 2024-06-03
### Added
- Support for the Price Oracles amendment (XLS-47).
- Add `nfts_by_issuer` clio-only API definition
- Included `ctid` field in the `tx` request.
- `from_xrpl` method accepts input dictionary keys exclusively in the proper XRPL format.

- `from_xrpl` method accepts input dictionary keys exclusively in the proper XRPL format.

### Fixed
Expand Down
1 change: 1 addition & 0 deletions xrpl/models/requests/ledger_entry.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ class LedgerEntry(Request, LookupByLedgerRequest):
binary: bool = False
nft_page: Optional[str] = None
"""Must be the object ID of the NFToken page, as hexadecimal"""
include_deleted: Optional[bool] = None
Kassaking7 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the xrpl-py client library not contain the deleted_ledger_index parameter? I observed it in the javascript client library

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there's no response class in ledger_entry.py so I assume we don't need it? For js one there's already a response class(interface) so I just add deleted_ledger_index to that class.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Python doesn't have response models.


def _get_errors(self: Self) -> Dict[str, str]:
errors = super()._get_errors()
Expand Down
Loading