Skip to content

fix: LiquidityPoolResponse class#106

Merged
cuongph87 merged 3 commits intomasterfrom
feature/fix-liquidity-response
Dec 4, 2025
Merged

fix: LiquidityPoolResponse class#106
cuongph87 merged 3 commits intomasterfrom
feature/fix-liquidity-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

Updates LiquidityPoolResponse to use required properties, add last-modified fields, and strongly-typed links; updates tests and fixtures to new Horizon responses.

  • Model (StellarDotnetSdk/Responses/LiquidityPoolResponse.cs):
    • Make class sealed; add XML docs.
    • Mark core properties as required (id, fee_bp, type, total_trustlines, total_shares, reserves, _links, paging_token).
    • Add last_modified_ledger and last_modified_time fields.
    • Change _links to strongly-typed Link<T>: operations -> Link<Page<OperationResponse>>, self -> Link<LiquidityPoolResponse>, transactions -> Link<Page<TransactionResponse>>.
    • Remove effects link support.
  • Tests:
    • Refactor LiquidityPoolDeserializerTest to use shared AssertTestData and validate new fields/typed links.
    • Update fixtures:
      • Replace Responses/liquidityPool.json with Horizon testnet URLs, new pool id, reserves, and new last-modified fields.
      • Add Responses/liquidityPoolPage.json with page and record link structures.

Written by Cursor Bugbot for commit 8bfb9db. Configure here.

@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 fixes the LiquidityPoolResponse class by adding the required modifier to always-present properties and updating the class to use strongly-typed generic Link<T> objects. The changes also add missing properties (LastModifiedLedger and LastModifiedTime), comprehensive XML documentation, and update test data to match real Horizon API responses.

Key Changes:

  • Added required modifiers to all properties that are always present in Horizon API responses
  • Changed Link types to strongly-typed Link<T> with appropriate type parameters
  • Added missing LastModifiedLedger and LastModifiedTime properties
  • Removed the #nullable disable directive, embracing nullable reference types

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
StellarDotnetSdk/Responses/LiquidityPoolResponse.cs Added required modifiers, comprehensive XML documentation, missing properties, strongly-typed links, and made classes sealed
StellarDotnetSdk.Tests/TestData/Responses/liquidityPool.json Updated test data with real Horizon API response format including new fields and corrected structure
StellarDotnetSdk.Tests/TestData/Responses/liquidityPoolPage.json Added missing _links section to the page response test data
StellarDotnetSdk.Tests/Responses/LiquidityPoolDeserializerTest.cs Refactored duplicate assertion code into a shared helper method AssertTestData and updated assertions to match new test data

Comment thread StellarDotnetSdk/Responses/LiquidityPoolResponse.cs
Comment thread StellarDotnetSdk.Tests/TestData/Responses/liquidityPool.json
@jopmiddelkamp
Copy link
Copy Markdown
Contributor

bugbot run

Comment thread StellarDotnetSdk.Tests/TestData/Responses/liquidityPool.json
@cuongph87 cuongph87 force-pushed the feature/fix-liquidity-response branch from 8b73762 to 46f22ea Compare December 4, 2025 03:57
Comment thread StellarDotnetSdk/Responses/LiquidityPoolResponse.cs Outdated
@cuongph87 cuongph87 force-pushed the feature/fix-liquidity-response branch from 9301a1d to 5363251 Compare December 4, 2025 06:12
@cuongph87 cuongph87 merged commit ea984c7 into master Dec 4, 2025
1 check passed
@cuongph87 cuongph87 deleted the feature/fix-liquidity-response branch December 4, 2025 06:13
@cuongph87 cuongph87 added the fix label Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix LiquidityPoolResponse class

3 participants