Replace JsonElement returns with typed response DTOs and tuple request inputs#11
Merged
stephanstapel merged 1 commit intomainfrom Dec 30, 2025
Conversation
stephanstapel
deleted the
codex/remove-createcatalogcartbasicasync-and-bulkbatchasync
branch
There was a problem hiding this comment.
Pull request overview
This pull request refactors the BrickOwlSharp client library to replace unstructured JsonElement return types with strongly-typed response DTOs for improved type safety and developer experience. The PR also simplifies request construction for batch and cart endpoints by accepting tuple-style inputs instead of requiring pre-serialized JSON payloads.
- Introduced 14 new response DTO classes (e.g.,
CatalogBulkResponse,UserDetailsResponse,BulkBatchResponse) to replaceJsonElementreturns - Changed
BulkBatchAsyncandCreateCatalogCartBasicAsyncto accept structured tuple inputs with internal JSON serialization - Updated all demo projects to use the new typed responses and tuple-based request construction
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| BrickOwlSharp.Client/UserDetailsResponse.cs | New DTO for user details with extension data support |
| BrickOwlSharp.Client/UserAddressesResponse.cs | New DTO for user addresses with extension data support |
| BrickOwlSharp.Client/TokenDetailsResponse.cs | New DTO for token details with extension data support |
| BrickOwlSharp.Client/OrderTaxSchemesResponse.cs | New DTO for order tax schemes with extension data support |
| BrickOwlSharp.Client/InvoiceTransactionsResponse.cs | New DTO for invoice transactions with extension data support |
| BrickOwlSharp.Client/CatalogSearchResponse.cs | New DTO for catalog search results with extension data support |
| BrickOwlSharp.Client/CatalogFieldOptionListResponse.cs | New DTO for catalog field options with extension data support |
| BrickOwlSharp.Client/CatalogConditionListResponse.cs | New DTO for catalog conditions with extension data support |
| BrickOwlSharp.Client/CatalogBulkResponse.cs | New DTO for bulk catalog data with extension data support |
| BrickOwlSharp.Client/CatalogBulkLookupResponse.cs | New DTO for bulk lookup results with extension data support |
| BrickOwlSharp.Client/CatalogCartBasicResponse.cs | New DTO for cart responses with typed properties (Currency, CartTotal, Items) |
| BrickOwlSharp.Client/CatalogCartBasicItemResponse.cs | New DTO for individual cart items with typed properties (Boid, Quantity, Price) |
| BrickOwlSharp.Client/BulkBatchResponse.cs | New DTO for batch responses containing a list of response items |
| BrickOwlSharp.Client/BulkBatchResponseItem.cs | New DTO for individual batch response items with endpoint, status, and body |
| BrickOwlSharp.Client/IBrickOwlClient.cs | Updated interface signatures to use new DTOs and tuple parameters for batch and cart methods |
| BrickOwlSharp.Client/BrickOwlClient.cs | Implemented tuple-to-JSON serialization logic for batch and cart methods, updated all affected methods to return typed DTOs |
| BrickOwlSharp.Demos/UserDemo.cs | Updated to use typed responses and access AdditionalData instead of JsonElement enumeration |
| BrickOwlSharp.Demos/OrderManagementDemo.cs | Updated to use OrderTaxSchemesResponse instead of JsonElement |
| BrickOwlSharp.Demos/InvoiceDemo.cs | Updated to use InvoiceTransactionsResponse instead of JsonElement |
| BrickOwlSharp.Demos/CatalogAdvancedDemo.cs | Updated to construct tuple-based requests and consume typed responses for cart and batch operations |
Comments suppressed due to low confidence (6)
BrickOwlSharp.Client/BrickOwlClient.cs:673
- Disposable 'FormUrlEncodedContent' is created but not disposed.
HttpContent content = new FormUrlEncodedContent(formData);
BrickOwlSharp.Client/BrickOwlClient.cs:92
- These 'if' statements can be combined.
if (disposing)
{
if (_disposeHttpClient)
{
_httpClient.Dispose();
}
}
BrickOwlSharp.Client/BrickOwlClient.cs:454
- The expression 'A == true' can be simplified to 'A'.
url = AppendOptionalParam(url, "active_only", activeOnly.Value == true ? 1 : 0);
BrickOwlSharp.Client/BrickOwlClient.cs:184
- Both branches of this 'if' statement return - consider using '?' to express intent better.
if (string.Equals(result.Status, "success", StringComparison.OrdinalIgnoreCase))
{
return true;
}
else
{
return false;
}
BrickOwlSharp.Client/BrickOwlClient.cs:615
- Both branches of this 'if' statement return - consider using '?' to express intent better.
if (string.Equals(result.Status, "success", StringComparison.OrdinalIgnoreCase))
{
return true;
}
else
{
return false;
}
BrickOwlSharp.Client/BrickOwlClient.cs:755
- Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
if (rawValue is decimal v)
{
value = v.ToString(CultureInfo.InvariantCulture);
}
else
{
value = rawValue.ToString();
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+323
to
+347
| var itemList = new List<Dictionary<string, string>>(); | ||
| foreach (var item in items ?? Enumerable.Empty<(string DesignId, int? ColorId, string Boid, int Quantity)>()) | ||
| { | ||
| var itemDictionary = new Dictionary<string, string> | ||
| { | ||
| { "qty", item.Quantity.ToString() } | ||
| }; | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(item.DesignId)) | ||
| { | ||
| itemDictionary["design_id"] = item.DesignId; | ||
| } | ||
|
|
||
| if (item.ColorId.HasValue) | ||
| { | ||
| itemDictionary["color_id"] = item.ColorId.Value.ToString(); | ||
| } | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(item.Boid)) | ||
| { | ||
| itemDictionary["boid"] = item.Boid; | ||
| } | ||
|
|
||
| itemList.Add(itemDictionary); | ||
| } |
There was a problem hiding this comment.
The tuple parameter for CreateCatalogCartBasicAsync allows both DesignId/ColorId and Boid to be null/empty simultaneously. According to typical catalog cart APIs, at least one identification method (either DesignId with ColorId, or Boid) should be required. Consider adding validation to ensure that each item has either a non-empty DesignId or a non-empty Boid before processing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
JsonElementresponses and provide typed DTOs for clearer deserialization and stronger typing.Description
Task<JsonElement>return types with concrete DTOs such asCatalogBulkResponse,CatalogSearchResponse,OrderTaxSchemesResponse,UserDetailsResponse, andInvoiceTransactionsResponse, and added the corresponding response classes (one per file).BulkBatchAsyncto acceptIEnumerable<(string Endpoint, string RequestMethod, IEnumerable<Dictionary<string,string>> Parameters)>andCreateCatalogCartBasicAsyncto acceptIEnumerable<(string DesignId, int? ColorId, string Boid, int Quantity)>, with internal serialization of the payloads.IBrickOwlClientsignatures andBrickOwlClientimplementations to use the new DTO types and to callExecuteGet<T>/ExecutePost<T>with the DTOs.CatalogAdvancedDemo,InvoiceDemo,OrderManagementDemo,UserDemo) to construct tuple inputs and consume the new typed responses.Testing
Codex Task