Show Manage Automations disabled button with tooltip on Queries page#39302
Show Manage Automations disabled button with tooltip on Queries page#39302
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #39302 +/- ##
=======================================
Coverage 64.37% 64.37%
=======================================
Files 2398 2398
Lines 187405 187471 +66
Branches 8464 8486 +22
=======================================
+ Hits 120639 120683 +44
- Misses 55852 55877 +25
+ Partials 10914 10911 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -0,0 +1 @@ | |||
| - The "Manage automations" button on the queries page is now always visible, and disabled only when the current team has no queries of its own. No newline at end of file | |||
There was a problem hiding this comment.
will modify it to also reference Policies page on the follow-up PR that will tackle the same changes for Policies
| - The "Manage automations" button on the queries page is now always visible, and disabled only when the current team has no queries of its own. | |
| - The "Manage automations" button on the queries and policies pages is now always visible, and disabled only when the current team has no queries of its own. |
lucasmrod
left a comment
lucasmrod
left a comment
There was a problem hiding this comment.
LGTM!
Worth adding a few integration tests that check the new inherited count being returned.
| // determined by passed in fleet.ListOptions, count of total queries returned without limits, and | ||
| // pagination metadata | ||
| func (ds *Datastore) ListQueries(ctx context.Context, opt fleet.ListQueryOptions) ([]*fleet.Query, int, *fleet.PaginationMetadata, error) { | ||
| func (ds *Datastore) ListQueries(ctx context.Context, opt fleet.ListQueryOptions) ([]*fleet.Query, int, int, *fleet.PaginationMetadata, error) { |
There was a problem hiding this comment.
Nit: To improve readability you can use named returns
| func (ds *Datastore) ListQueries(ctx context.Context, opt fleet.ListQueryOptions) ([]*fleet.Query, int, int, *fleet.PaginationMetadata, error) { | |
| func (ds *Datastore) ListQueries(ctx context.Context, opt fleet.ListQueryOptions) (queries []*fleet.Query, total int, inherited int, metdata *fleet.PaginationMetadata, err error) { |
| assert.Equal(t, 1, listQueriesResp.Count) | ||
| assert.Equal(t, 0, listQueriesResp.InheritedQueryCount) | ||
|
|
||
| // list merged team queries | ||
| s.DoJSON("GET", "/api/latest/fleet/queries", nil, http.StatusOK, &listQueriesResp, "team_id", fmt.Sprint(team1.ID), "merge_inherited", "true", "order_key", "team_id", "order_direction", "desc") | ||
| require.Len(t, listQueriesResp.Queries, 2) | ||
| assert.Equal(t, "team1", listQueriesResp.Queries[0].Name) | ||
| assert.Equal(t, "global1", listQueriesResp.Queries[1].Name) | ||
| assert.Equal(t, 2, listQueriesResp.Count) | ||
| assert.Equal(t, 1, listQueriesResp.InheritedQueryCount) |
There was a problem hiding this comment.
@lucasmrod added assertions for InheritedQueryCount in these existing tests instead of adding new tests.
| }; | ||
|
|
||
| return { queryItems, updateQueryItems }; | ||
| return { queryItems, setQueryItems, updateQueryItems }; |
There was a problem hiding this comment.
It seems like we should be able to handle all the queryItems state inside of the custom hook rather than exposing setQueryItems for an external effect. What if you instead made sortedAvailableQueries an input to this custom hook?
There was a problem hiding this comment.
Makes sense, I'll address
There was a problem hiding this comment.
@gillespi314 addressed.
I removed the useCheckboxStateManagement hook - I think it only added unnecessary indirection (and cognitive load) and everything could be handled within the component.
…#39392) <!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #39304 (part of #25080) Implemented similar approach to #39302, with the difference that the list policies endpoint does not include a count, and there is a separate endpoint. I extended the count policies endpoint to include an `inherited_policy_count`. # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually
4 participants
Related issue: Resolves #39303 (child of #25080).
inherited_query_counttoListQueriesResponse(thought of adding a brand new endpoint just for counting, but felt like extending the current one was good enough). In the parent task, it was suggested to"Depend on team list entity endpoint's count field / team entity count endpoint for whether or not to disable the manage automations button", which Rachael approved, so I went for this approach.ManageQueryAutomationsModalnow fetches its own data withmerge_inherited = false(meaning it only fetches non-inherited queries only). Previously, queries were passed down as props to it, which would not show the queries available to automate if the first page of queries were all inherited and the second page contained queries for that team (the user would have to navigate to the second page for the button to be enabled).^ The fact that the modal fetches its own data is similar behavior to what is currently done in
Policies. For queries, I noticed that we would need to add pagination within theManage Automationsmodal, but that can be a follow-up.Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Screen.Recording.2026-02-04.at.11.56.55.AM.mov