fix(fleetcontrol): add missing cursor to getEntitySearchQuery - #1442
Conversation
GetEntitySearch accepted a cursor parameter that was silently dropped: the generated query only declared $query and never declared $cursor or passed cursor to the entitySearch field, so every call re-fetched page one regardless of the cursor passed in. Same bug and fix pattern as 93b8a99 (#1394), which fixed the identical issue in GetFleetMembers. Verified against live NerdGraph that passing the returned nextCursor now actually advances the result set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1442 +/- ##
==========================================
+ Coverage 32.39% 32.97% +0.57%
==========================================
Files 148 148
Lines 6939 6939
==========================================
+ Hits 2248 2288 +40
+ Misses 4480 4435 -45
- Partials 211 216 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hey @ShaneLillieNR, thank you for having this fixed, I think this certainly does look good, just that the unit test is a li'l fancy (unlike the unit tests in the rest of the client), but I get why, so I think that's fine (we might want to streamline this later sometime - I'll take a detailed look too) :) The only double check I'd like you to perform is to test this change with the expected consumer of this change (e.g. the Terraform Provider, or the CLI where you'd like to enable this) so we're sure nothing breaks there. I'm sure there must be a companion PR to enable this (or I'd like you to create one, to fix this as needed) so you might want to test changes in the consumer with this PR and we're sure nothing breaks (it ideally should not :)) and that should give us a 🟢 to merge this. |
Hi @pranav-new-relic I think newrelic/newrelic-cli#1874 should cover what you're looking for? We can cover the exactly details on that when we get there but I think it's enough to prove out that the changes here are good? |
|
newrelic/terraform-provider-newrelic#3171 is a similar PR but for updating the Terraform provider |
newrelic/newrelic-client-go#1442 has merged and shipped in v2.93.2, which carries the GetEntitySearch cursor fix this PR depends on. Point go.mod at the real release instead of the temporary replace-directive pseudo-version used during development, and confirm the full build/vet/unit-test suite is clean with no workarounds needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
newrelic/newrelic-client-go#1442 merged and shipped in v2.93.2, which carries the GetEntitySearch cursor fix this PR depends on. Drop the temporary replace directive and point go.mod at the real release. Re-verified against the same live account/fleets used for the earlier before/after check - unchanged now that a real released version is in place. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
GetEntitySearch(cursor string, query string)inpkg/fleetcontrolaccepted acursorparameter that was a silent no-op: the generatedgetEntitySearchQueryGraphQL document only declared$query: String!and calledentitySearch(query: $query,)— it never declared$cursorand never passedcursor: $cursorto theentitySearchfield. Thevarsmap set"cursor": cursor, but since the query text never referenced$cursor, the value was dropped before the request ever left the client. Every call re-fetched page one regardless of what cursor was passed in.This is the exact same bug already found and fixed for
GetFleetMembersin93b8a99("fix(fleetcontrol): add missing cursor and fix cursor type in getFleetMembersQuery", #1394). That fix addedinclude_arguments: ["cursor"]for thefleetMembersendpoint in.tutone.yml; the equivalent block forentitySearchwas missing it.Changes
.tutone.yml: addinclude_arguments: ["cursor"]to theentitySearchendpoint under thefleetcontrolpackage's["actor", "entityManagement"]queries block.pkg/fleetcontrol/fleetcontrol_api.go:getEntitySearchQuerynow declares$cursor: String,and passescursor: $cursor,toentitySearch(...). Applied by hand rather than a fulltutone generate— the live NerdGraph schema has drifted substantially since the last full regen, and running it here would have pulled ~16k lines of unrelated changes across 60 files (and even broken an unrelated package due to atype-as-identifier codegen bug inentityrelationship_api.go). The diff here mirrors exactly what93b8a99produced forGetFleetMembers.GetEntitySearch/GetEntitySearchWithContext:cursorchanged fromstringto*string, matching the same non-Tutone-covered change93b8a99made forGetFleetMembers(the API expectsnullfor "no cursor", not an empty string).pkg/fleetcontrol/fleetcontrol_integration_test.go: updated the 3 existingGetEntitySearch("", ...)call sites to passnil.pkg/fleetcontrol/entity_search_unit_test.go(new): mocks the NerdGraph endpoint and asserts the actual JSON request body sent over the wire includes the cursor value (and that a nil cursor sendsnull), verifying the fix at the wire level rather than just via generated code inspection.No
types.gochanges were needed —EntityManagementEntitySearchResultalready hasNextCursor.Surveyed the rest of
fleetcontrol_api.gofor other queries with the same dead-cursor bug:GetEntityfetches a single entity by ID and correctly has no cursor;GetEntitySearchandGetFleetMembersare the only two paginated queries in this package, and both are now fixed.Verification
go build ./...,go vet -tags unit ./...,go vet -tags integration ./...all clean.golangci-lint run ./pkg/fleetcontrol/...— 0 issues.go test -tags unit ./pkg/fleetcontrol/...— passes, including the new unit test.GetEntitySearch(nil, "type = 'FLEET'"), took the returnednextCursor, called again with that cursor — page 2 returned a different set of entities and a new cursor, confirming the cursor now actually advances pagination against real data (not just a mocked assertion).Context
Discovered while fixing a pagination bug in
newrelic-cli'sfleetcontrol fleet searchcommand. That CLI-side fix is blocked on this PR merging and a newnewrelic-client-gorelease.Test plan
go build ./...go vet -tags unit ./.../go vet -tags integration ./...golangci-lint run ./pkg/fleetcontrol/...go test -tags unit ./pkg/fleetcontrol/...