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.

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

Confusion in SDK's Handling of MerchantPageDTO and MerchantDTO #24

Open
mathmul opened this issue Jun 19, 2024 · 1 comment
Open

Confusion in SDK's Handling of MerchantPageDTO and MerchantDTO #24

mathmul opened this issue Jun 19, 2024 · 1 comment

Comments

@mathmul
Copy link

mathmul commented Jun 19, 2024

Description

There appears to be a potential misunderstanding or error in how MerchantPageDTO and MerchantDTO are used and documented in the SDK. Specifically, the API method searchMerchant is documented to return Result<MerchantPageDTO>, which seems like something you'd expect, but is misleading given the structure and inheritance of MerchantPageDTO and MerchantDTO.

Issue Details

  • MerchantPageDTO is expected to represent a paginated dataset of merchants, as suggested by its naming convention. However, it does not contain a dataset or list of MerchantDTO objects. Instead, MerchantPageDTO only includes individual merchant attributes and is extended by MerchantDTO.
  • The SDK method searchMerchant suggests it returns a Result<MerchantPageDTO>, but based on typical API and pagination patterns, one would expect this method to return a paginated response containing multiple MerchantDTO objects, as documented in the example response (200 OK).
  • This creates confusion and may lead to incorrect implementation or usage of the SDK.

Expected Behavior

Ideally, MerchantPageDTO should either:

  • Be renamed to better reflect its role as a base class for an individual merchant's data, not a paginated response, and not be used in searchMerchant method, and another DTO should be created for that.
  • Be adjusted to actually represent a paginated response containing multiple MerchantDTO objects if that was the original intent.

Suggested Fix

  1. All the properties, getters and setter from MerchantPageDTO should be moved to MerchantDTO
  2. MerchantDTO should no longer inherit from MerchantPageDTO
  3. MerchantPageDTO should be a DTO that contains a dataset/list of MerchantDTO objects
  4. Ideally, searchMerchant method would be renamed to searchMerchants
  5. Synchronize the documentation and method signatures in the SDK to accurately reflect the data structures being returned and handled

Steps to Reproduce

  1. Call the searchMerchant method with standard pagination parameters.
  2. Observe the returned Result<MerchantPageDTO> and note the lack of pagination handling or dataset encapsulation typically expected in such responses.

Possible Solutions

  • If MerchantPageDTO is meant to represent a single merchant, rename and refactor accordingly, and adjust the SDK to return a more appropriately named class for paginated responses.
  • If a paginated list of merchants was intended, refactor MerchantPageDTO to include a List<MerchantDTO> or similar dataset and adjust all related API methods to handle this correctly.
@mathmul
Copy link
Author

mathmul commented Jul 25, 2024

Over a month since posting this and not even a comment? Has anyone else had this issue? Better yet, has anyone successfully implemented this part of the SDK?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant