Ready for Review ObjectsApi #128
Conversation
Co-authored-by: JackTelford <142545189+JackTelford@users.noreply.github.com>
Co-authored-by: JackTelford <142545189+JackTelford@users.noreply.github.com>
|
Files Step 1 — Collect endpoints from Old Step 2 — Compare with New Output (list only findings) MISSING → HTTP: GET | URL: /api/v2/example | Old Method: GetExampleAsyncWithHttpInfo For duplicates/overloads: DUPLICATE → HTTP: POST | URL: /api/v2/example | New Methods: CreateExampleAsync, CreateExampleAsync_v2 If there are no findings, return: ✅ All endpoints migrated 1:1 (no missing, no duplicates) If a method exists in the Old file but not in the New file, add a single corresponding method to the New file, following the existing code style and patterns used in the New file |
Co-authored-by: JackTelford <142545189+JackTelford@users.noreply.github.com>
I've completed the endpoint analysis and migration. Here are the findings: MISSING → HTTP: GET | URL: /api/v2/authorization/divisions/query | Old Method: GetAuthorizationDivisionsQueryAsyncWithHttpInfo MISSING → HTTP: POST | URL: /api/v2/authorization/divisions/{divisionId}/restore | Old Method: PostAuthorizationDivisionRestoreAsyncWithHttpInfo Both missing methods have now been added to the new ObjectsApi.cs following the established patterns:
✅ All 11 endpoints now migrated 1:1 (no missing, no duplicates) Commit: f4d9e64 |
MikeAlhayek
left a comment
There was a problem hiding this comment.
Any parameter that is a collection should use plural. Expand to expands. Also in the foreach, name the variable for the current item the single version of the variable. For example expand in expands
| ArgumentNullException.ThrowIfNull(body); | ||
|
|
||
| var uri = UriHelper.GetUri($"api/v2/authorization/divisions/{divisionId}", null); | ||
| var uri = UriHelper.GetUri($"api/v2/authorization/divisions/{Uri.EscapeDataString(divisionId)}", null); |
There was a problem hiding this comment.
Since there is no parameter, there is no need to use GetUri
There was a problem hiding this comment.
Removed unnecessary UriHelper.GetUri calls when no query parameters are present, now using direct string URIs instead.
|
|
||
| if (id != null) | ||
| { | ||
| foreach (var item in id) |
There was a problem hiding this comment.
Updated parameter name from "id" to "ids" for collection parameters in GetAuthorizationDivisionsQueryAsync method.
Updated all collection parameters to use plural names (expand → expands, id → ids) and updated foreach loop variables to use singular names (item → expand/id). Changes applied to GetAuthorizationDivisionsAsync and GetAuthorizationDivisionsQueryAsync methods. |
…, fix formatting, remove unnecessary UriHelper calls Co-authored-by: MikeAlhayek <24724371+MikeAlhayek@users.noreply.github.com>
…s and add newline at end of IObjectsApi.cs Co-authored-by: MikeAlhayek <24724371+MikeAlhayek@users.noreply.github.com>
…line at end of ObjectsApi.cs Co-authored-by: MikeAlhayek <24724371+MikeAlhayek@users.noreply.github.com>
Updated ObjectsApi to follow the established pattern - replaced global HttpClient instance with per-method HttpClient creation using IHttpClientFactory, matching the patterns in FlowsApi and AlertingApi. Commit: addba96 |
|
@copilot Audit the oldapi and the new api file and make sure we have a 1:1 match and No duplicates then apply the rules from the above and |
|
@copilot continue where you left off at. |
Completed comprehensive audit of ObjectsApi migration: Audit ResultsStep 1 - Old API Analysis: Found 11 async endpoints with Step 2 - New API Analysis: Found 11 corresponding async methods implementing identical HTTP endpoints with modern HttpClient pattern. Step 3 - 1:1 Mapping Verification:
Step 4 - Missing Models Check:
Step 5 - Rules Applied: Conclusion: Migration is complete and correct with full feature parity. Commit: addba96 |
MikeAlhayek
left a comment
There was a problem hiding this comment.
Apply the following feedback everywhere
|
|
||
| ArgumentNullException.ThrowIfNull(body); | ||
|
|
||
| var uri = $"api/v2/authorization/divisions/{Uri.EscapeDataString(divisionId)}/restore"; |
There was a problem hiding this comment.
You don't need uri variable here. Apply the same feedback everywhere
There was a problem hiding this comment.
Removed unnecessary uri variables throughout ObjectsApi - now passing URI strings directly to HttpClient methods instead of storing in variables first. Applied to CreateAuthorizationDivisionAsync, UpdateAuthorizationDivisionAsync, CreateAuthorizationDivisionObjectsAsync, and PostAuthorizationDivisionRestoreAsync methods. Commit: ac43793
MikeAlhayek
left a comment
There was a problem hiding this comment.
@copilot please apply the same feedback everywhere.
Also, please resolve the merge conflicts and request my review once everything is done and the project builds
| ArgumentNullException.ThrowIfNull(body); | ||
|
|
||
| var uri = UriHelper.GetUri("api/v2/authorization/divisions", null); | ||
| var uri = "api/v2/authorization/divisions"; |
There was a problem hiding this comment.
This variable is not needed. You can just pass this value to the request call.
There was a problem hiding this comment.
Removed unnecessary uri variable from CreateAuthorizationDivisionAsync method - now passing the URI string directly to PostAsJsonAsync call. Applied same pattern to all methods where uri variables were not needed. Commit: ac43793
…mentation to IObjectsApi interface Co-authored-by: MikeAlhayek <24724371+MikeAlhayek@users.noreply.github.com>
Applied all feedback:
|
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.