Skip to content

fix!: TransactionResponse class#103

Merged
jopmiddelkamp merged 6 commits intomasterfrom
feature/fix-transaction-response
Dec 8, 2025
Merged

fix!: TransactionResponse class#103
jopmiddelkamp merged 6 commits intomasterfrom
feature/fix-transaction-response

Conversation

@cuongph87
Copy link
Copy Markdown
Contributor

@cuongph87 cuongph87 commented Dec 2, 2025

Types of changes

  • 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 change)
  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Note

Refactors TransactionResponse (breaking) to match Horizon: adds required fields, changes several types/fields, introduces preconditions/links, and updates tests/fixtures accordingly.

  • Responses:
    • TransactionResponse:
      • Make class sealed; add required fields (id, links, etc.).
      • Change types: created_at to ISO string; fee_charged/max_fee to string; muxed IDs to string.
      • Add fee_meta_xdr, preconditions; remove result_meta_xdr.
      • Update nested types: InnerTransaction.max_fee to string; stricter memo handling (invalid ops throw InvalidOperationException).
    • Links/Preconditions:
      • Extract TransactionResponseLinks (adds transaction link; precedes/succeeds now pages).
      • Add TransactionResponsePreconditions with TimeBounds and optional bounds/signers.
  • Tests & Data:
    • Replace legacy transaction fixtures with transaction.json; add transactionWithTextMemo.json; update muxed/fee-bump fixtures (string fees, new links).
    • Update assertions to new types/fields (e.g., CreatedAt string, FeeMetaXdr, memo handling).
    • Adjust streaming and payment operation tests to new transaction shape.
    • Update test project includes to new/renamed files.

Written by Cursor Bugbot for commit 0a8ab05. Configure here.

@cuongph87 cuongph87 changed the title fix: TransactionResponse class fix!: TransactionResponse class Dec 2, 2025
@jopmiddelkamp jopmiddelkamp requested a review from Copilot December 2, 2025 16:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the TransactionResponse class to introduce breaking changes that improve type safety and consistency with the Stellar Horizon API. The changes address issues #91 and #92 by adding required modifiers to always-present properties, changing type representations to match API contracts, and reorganizing the class structure.

Key Changes

  • Added required modifiers to non-nullable properties to enforce compile-time initialization requirements
  • Changed numeric fee/muxed ID properties to strings to match Horizon API representation
  • Extracted nested classes (TransactionResponseLinks, TransactionResponsePreconditions) into separate files for better organization
  • Updated test data to use current Horizon API responses and improved test coverage

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
StellarDotnetSdk/Responses/TransactionResponse.cs Added required modifiers, changed fee/muxed properties to strings, improved documentation, extracted nested classes
StellarDotnetSdk/Responses/TransactionResponseLinks.cs New file containing extracted TransactionResponseLinks class with required modifiers
StellarDotnetSdk/Responses/TransactionResponsePreconditions.cs New file containing preconditions-related classes with proper nullability
StellarDotnetSdk.Tests/Responses/TransactionDeserializerTest.cs Updated tests to use current test data and validate new property types
StellarDotnetSdk.Tests/TestData/Responses/*.json Replaced outdated test data with current Horizon API responses
Comments suppressed due to low confidence (1)

StellarDotnetSdk.Tests/Responses/TransactionDeserializerTest.cs:1

  • The test now validates CreatedAt as a string but doesn't verify that the string can be successfully parsed as a valid DateTime. Consider adding validation that the ISO 8601 format can be parsed using DateTime.Parse(transaction.CreatedAt) to ensure the string format is correct.
using System.IO;

Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs
Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk.Tests/TestData/Responses/transactionPage.json Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponsePreconditions.cs Outdated
@jopmiddelkamp
Copy link
Copy Markdown
Contributor

bugbot run

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!


Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponsePreconditions.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponsePreconditions.cs Outdated
@cuongph87 cuongph87 force-pushed the feature/fix-transaction-response branch from 9516ddc to d619166 Compare December 4, 2025 04:19
Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponse.cs Outdated
Comment thread StellarDotnetSdk/Responses/TransactionResponsePreconditions.cs Outdated
@cuongph87 cuongph87 force-pushed the feature/fix-transaction-response branch from 73d7861 to 8f6ed31 Compare December 8, 2025 01:49
jopmiddelkamp
jopmiddelkamp previously approved these changes Dec 8, 2025
@cuongph87 cuongph87 force-pushed the feature/fix-transaction-response branch from 0491a17 to 7f62ba6 Compare December 8, 2025 03:08
@cuongph87 cuongph87 force-pushed the feature/fix-transaction-response branch from 7f62ba6 to 6c1bfe7 Compare December 8, 2025 03:08
@cuongph87 cuongph87 force-pushed the feature/fix-transaction-response branch from 6c1bfe7 to a08ea96 Compare December 8, 2025 03:21
@jopmiddelkamp jopmiddelkamp merged commit e172687 into master Dec 8, 2025
1 of 2 checks passed
@jopmiddelkamp jopmiddelkamp deleted the feature/fix-transaction-response branch December 8, 2025 04:38
@cuongph87 cuongph87 added the breaking Breaking change label Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix TransactionResponse class Add support for TransactionResponse.Preconditions

3 participants