-
Notifications
You must be signed in to change notification settings - Fork 155
feat: Add support for include_children in TransactionGetReceiptQuery #1114
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.
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
feat: Add support for include_children in TransactionGetReceiptQuery #1114
Conversation
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! From the Hiero Python SDK Team |
tests/integration/transaction_get_receipt_query_children_e2e_test.py
Outdated
Show resolved
Hide resolved
manishdait
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AntonioCeppellini, Just some minor changes rest looks good to me.
|
Hello, this is the Office Hour Bot. This is a reminder that the Hiero Python SDK Office Hours are scheduled in approximately 4 hours (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request or receive assistance from a maintainer. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here to be notified of any changes. |
tests/integration/transaction_get_receipt_query_children_e2e_test.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Antonio Ceppellini <[email protected]>
Signed-off-by: Antonio Ceppellini <[email protected]>
9e24d20 to
46a3c22
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #1114 +/- ##
==========================================
+ Coverage 91.08% 91.10% +0.01%
==========================================
Files 139 139
Lines 8423 8441 +18
==========================================
+ Hits 7672 7690 +18
Misses 751 751 🚀 New features to boost your workflow:
|
|
@manishdait I've followed your advises :D |
Signed-off-by: Antonio Ceppellini <[email protected]>
…nsactionGetReceiptQuery
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request adds support for querying child transaction receipts through a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Query as TransactionGetReceiptQuery
participant Network as Hedera Network
participant Receipt as TransactionReceipt
Client->>Query: set_include_children(true)
Client->>Query: execute(client)
Query->>Query: _make_request()
Note over Query: Build protobuf request<br/>with include_child_receipts=true
Query->>Network: Send query request
Network-->>Query: Response with receipt<br/>+ child_transaction_receipts[]
Query->>Receipt: _from_proto(receipt)
activate Receipt
Receipt->>Receipt: __init__(...children=None)
deactivate Receipt
alt include_children is True
Query->>Query: _map_receipt_list(children_protos)
loop For each child_proto
Query->>Receipt: _from_proto(child_proto)
end
Query->>Receipt: _set_children([child1, child2, ...])
end
Query-->>Client: Return parent Receipt<br/>with children attached
Client->>Receipt: receipt.children
Receipt-->>Client: list[TransactionReceipt]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
please rebase, i was just looking through this should nearly be reviewed soon |
…nsactionGetReceiptQuery Signed-off-by: AntonioCeppellini <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/integration/transaction_get_receipt_query_children_e2e_test.py (1)
22-22: Consider removing or aliasing the unused import.The static analysis tool flags
F811: Redefinition of unused 'env' from line 22because theenvfixture parameter in each test function shadows this import. If this import is only for pytest fixture registration/discovery, consider either:
- Removing it if the fixture is already available via
conftest.py- Using a different pattern like
from tests.integration.utils import env as _envto avoid shadowingsrc/hiero_sdk_python/transaction/transaction_receipt.py (1)
37-53: Consider updating the docstring to document thechildrenparameter.The constructor now accepts an optional
childrenparameter, but the docstring (lines 43-49) doesn't mention it. For API completeness, consider adding documentation for this parameter.🔎 Suggested docstring update:
""" Initializes the TransactionReceipt with the provided protobuf receipt. Args: receipt_proto (transaction_receipt_pb2.TransactionReceiptProto, optional): The protobuf transaction receipt. transaction_id (TransactionId, optional): The transaction ID associated with this receipt. + children (list[TransactionReceipt], optional): Child receipts associated with this receipt. """
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)examples/query/transaction_get_receipt_query.py(3 hunks)src/hiero_sdk_python/query/transaction_get_receipt_query.py(5 hunks)src/hiero_sdk_python/transaction/transaction_receipt.py(4 hunks)tests/integration/transaction_get_receipt_query_children_e2e_test.py(1 hunks)tests/unit/get_receipt_query_test.py(4 hunks)tests/unit/test_transaction_receipt.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
tests/unit/test_transaction_receipt.py (1)
src/hiero_sdk_python/transaction/transaction_receipt.py (4)
TransactionReceipt(24-248)transaction_id(123-130)children(211-218)_set_children(220-227)
src/hiero_sdk_python/query/transaction_get_receipt_query.py (4)
src/hiero_sdk_python/transaction/transaction_receipt.py (5)
transaction_id(123-130)TransactionReceipt(24-248)_from_proto(239-248)children(211-218)_set_children(220-227)tests/unit/conftest.py (1)
transaction_id(42-44)src/hiero_sdk_python/consensus/topic_message.py (1)
_from_proto(145-176)src/hiero_sdk_python/consensus/topic_info.py (1)
_from_proto(79-123)
tests/integration/transaction_get_receipt_query_children_e2e_test.py (4)
tests/unit/query_test.py (1)
query(15-17)src/hiero_sdk_python/query/transaction_get_receipt_query.py (4)
TransactionGetReceiptQuery(15-285)execute(226-259)set_transaction_id(53-68)set_include_children(70-87)src/hiero_sdk_python/response_code.py (1)
ResponseCode(4-387)src/hiero_sdk_python/transaction/transaction_receipt.py (1)
children(211-218)
tests/unit/get_receipt_query_test.py (3)
src/hiero_sdk_python/query/transaction_get_receipt_query.py (3)
TransactionGetReceiptQuery(15-285)execute(226-259)_make_request(101-138)src/hiero_sdk_python/transaction/transaction_receipt.py (3)
transaction_id(123-130)TransactionReceipt(24-248)children(211-218)src/hiero_sdk_python/response_code.py (1)
ResponseCode(4-387)
src/hiero_sdk_python/transaction/transaction_receipt.py (2)
tests/unit/conftest.py (1)
transaction_id(42-44)src/hiero_sdk_python/transaction/transaction_id.py (1)
TransactionId(8-156)
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
49-49: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.8)
tests/integration/transaction_get_receipt_query_children_e2e_test.py
45-47: Avoid specifying long messages outside the exception class
(TRY003)
50-50: Redefinition of unused env from line 22
(F811)
69-69: Redefinition of unused env from line 22
(F811)
87-87: Redefinition of unused env from line 22
(F811)
107-107: Redefinition of unused env from line 22
(F811)
tests/unit/get_receipt_query_test.py
43-43: Do not catch blind exception: Exception
(BLE001)
88-88: Do not catch blind exception: Exception
(BLE001)
⏰ 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). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.10)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (19)
CHANGELOG.md (1)
49-49: LGTM!The changelog entry correctly documents the new feature and follows the existing format used elsewhere in this file.
tests/unit/test_transaction_receipt.py (1)
1-30: LGTM!The tests appropriately cover the
childrenproperty default behavior and the_set_childreninternal setter. Using identity checks (is) correctly verifies that the exact child instances are preserved.examples/query/transaction_get_receipt_query.py (2)
65-79: LGTM!The helper function is well-structured and provides clear output for demonstrating the
include_childrenfeature.
111-119: LGTM!Good demonstration of the new
set_include_children(True)API, and the updated step description clearly indicates the feature being exercised.src/hiero_sdk_python/query/transaction_get_receipt_query.py (4)
27-41: LGTM!The constructor properly initializes
include_childrenwith a backward-compatible default ofFalse.
70-87: LGTM!The setter follows the established pattern with frozen-state validation and method chaining support.
127-128: LGTM!Correctly propagates the
include_childrenflag to the protobuf'sinclude_child_receiptsfield.
248-259: Child receipts correctly mapped; verify transaction_id semantics.The implementation correctly creates the parent receipt and conditionally attaches child receipts. Note that child receipts are constructed using the parent's
transaction_id—this is acceptable sinceTransactionReceipt._from_prototreatstransaction_idas optional context, and child receipts derive from the parent transaction.Minor: Line 259 has trailing whitespace.
🔎 Apply this diff to remove trailing whitespace:
- return parent + return parenttests/unit/get_receipt_query_test.py (3)
137-152: LGTM!Good test coverage verifying that
include_child_receiptsis correctly set in the protobuf request.
155-195: LGTM!Thorough test verifying end-to-end behavior: child receipts are mapped correctly with proper statuses when
include_childrenis enabled.
198-229: LGTM!Important backward-compatibility test ensuring
childrenremains empty wheninclude_childrenis not explicitly requested.tests/integration/transaction_get_receipt_query_children_e2e_test.py (5)
25-47: LGTM!The helper function provides robust fallback logic for extracting transaction IDs across different transaction types. The approach of checking multiple attributes is appropriate for E2E test flexibility.
50-65: LGTM!The helper correctly implements the transfer pattern with
Hbar().to_tinybars()as suggested in previous reviews.
68-83: LGTM!Good test coverage for the default behavior. The assertion
receipt.children == []correctly validates that children defaults to an empty list wheninclude_childrenis not set.
86-103: LGTM!The test correctly validates that
include_children=Trueresults in a list response. The decision not to assert on list contents is appropriate since simple transfers typically don't produce child receipts.
106-184: LGTM!Well-structured contract execution test that:
- Properly handles the non-deterministic nature of child receipts (as documented)
- Uses defensive
pytest.skipfor transaction ID extraction failures- Validates child receipt integrity when children exist
The local imports are acceptable for test isolation.
src/hiero_sdk_python/transaction/transaction_receipt.py (3)
210-218: LGTM!Clean property implementation with clear documentation explaining the return behavior.
220-227: LGTM!The internal setter follows the appropriate pattern for post-construction modification by the query class. The underscore prefix correctly signals internal usage.
238-248: LGTM!The
_from_protomethod correctly omits thechildrenparameter since child receipts come from a separate field in the response (child_transaction_receipts) and are populated by the query class via_set_children.
|
thank you @AntonioCeppellini excellent work! |
…iero-ledger#1114) Signed-off-by: Antonio Ceppellini <[email protected]> Signed-off-by: AntonioCeppellini <[email protected]> Signed-off-by: prajeeta pal <[email protected]>
Description:
This PR add support for include_children in TransactionGetReceiptQuery
TransactionGetReceiptQuerywe have the new fieldinclude_childrenthat is a bool.set_include_children.TransactionReceiptnew propertychildren_set_childrenused in the queryRelated issue(s):
Fixes #1100
Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.