Explore: List query templates #86897

ifrost · 2024-04-24T22:15:33Z

Fixes #86122

For testing please use Query Library App plugin, or create some templates by posting requests directly to the API:

Screen.Recording.2024-04-26.at.14.11.11.mov

Changes:

Shows the list of query templates
Displayed columns:
- the name of the data source, title, and the query
- data source type
- date added
Basic error handling

Out of scope:

Displaying the user who created the query template (Explore: Show user who created the query template #86987)
Query template actions (Explore: Running a query from query library #83097, Explore: Edit query template description #84895, Explore: Starring a query template #84896, Query Library: Private query templates #84900, Explore: Duplicating a query template #84897, Explore: Delete query template #86986)

Follow-ups:

# Conflicts: # docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md # packages/grafana-data/src/types/featureToggles.gen.ts # pkg/services/featuremgmt/registry.go # pkg/services/featuremgmt/toggles_gen.csv # pkg/services/featuremgmt/toggles_gen.go # pkg/services/featuremgmt/toggles_gen.json

This is required to have better UI and an indication of selected state in split view

This is to make it consistent with the toolbar button

This is to avoid confusing UX with 2 button triggering the drawer but with slightly different behavior

To avoid confusion for current users and test it internally a bit more it's behind a feature toggle. Bigger drawer may obstruct the view and add more friction in the UX.

The test was failing because queryLibraryAvailable was set to true for tests. This change makes it more explicit what use case is being tested

# Conflicts: # pkg/services/featuremgmt/toggles_gen.json

This is just moved from the app. To be cleaned up and refactored later.

# Conflicts: # packages/grafana-data/src/types/featureToggles.gen.ts # pkg/services/featuremgmt/registry.go # pkg/services/featuremgmt/toggles_gen.csv # pkg/services/featuremgmt/toggles_gen.go # pkg/services/featuremgmt/toggles_gen.json

# Conflicts: # public/app/features/explore/QueriesDrawer/QueriesDrawerDropdown.tsx # public/app/features/explore/RichHistory/RichHistory.tsx

It will be moved to the kebab

Just to simplify the PR, it's not needed for Explore yet.

gelicia

Scaffolding looks great, confirmed by adding a query to the library app and seeing it in Explore! Exciting stuff!

ifrost · 2024-04-30T10:06:32Z

@grafana/grafana-frontend-platform I'm not 100% how to correctly add a dependency to grafana/runtime. What I'm trying to achieve is to expose the ApiProvider and React hooks from RTK Query to plugins. I want to use it in core (Explore) and in an Application Plugin (Query Library). It requires reactjs/toolkit (for RTK Query) to work, which depends on on a peer dependency of react-redux, which depends on react-is. If I don't add @reactjs/redux or react-redux to dependencies/peerDependencies in grafana-runtime I get this error when bulding grafana-runtime:

[!] Error: 'isValidElementType' is not exported by ../../node_modules/react-is/index.js, imported by ../../node_modules/react-redux/es/components/connect.js
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
../../node_modules/react-redux/es/components/connect.js (8:9)
 6: import hoistStatics from 'hoist-non-react-statics';
 7: import * as React from 'react';
 8: import { isValidElementType, isContextConsumer } from 'react-is';
             ^
 9: import defaultSelectorFactory from '../connect/selectorFactory';
10: import { mapDispatchToPropsFactory } from '../connect/mapDispatchToProps';
Error: 'isValidElementType' is not exported by ../../node_modules/react-is/index.js, imported by ../../node_modules/react-redux/es/components/connect.js
    at error (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:198:30)
    at Module.error (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:12560:16)
    at Module.traceVariable (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:12919:29)
    at ModuleScope.findVariable (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:11571:39)
    at FunctionScope.findVariable (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:6503:38)
    at ChildScope.findVariable (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:6503:38)
    at ReturnValueScope.findVariable (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:6503:38)
    at ChildScope.findVariable (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:6503:38)
    at Identifier.bind (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:7570:40)
    at CallExpression.bind (/Users/ifrost/projects/grafana/node_modules/rollup/dist/shared/rollup.js:5400:23)

The error is coming from rollup which should be able to resolve the dependency from the parent but it fails 🤔

Not sure why though, maybe because react-is resolves dependency conditionally?

if (process.env.NODE_ENV === 'production') {
  module.exports = require('./cjs/react-is.production.min.js');
} else {
  module.exports = require('./cjs/react-is.development.js');
}

Adding dependency/peerDependency cause the module to be marked as external (via externals rollup plugin) and it's bundled correctly, but I'm not sure if that's the right way of doing this 🤔

…/list-query-templates

gelicia · 2024-05-07T15:39:25Z

Had a chat with @ashharrison90 and @joshhunt about this - unfortunately this PR is in the middle of two unknown and emerging practices that the frontend platform team is working on - both app platform and a common interface for generating endpoints. They would ideally like some of the code that is handwritten in here to be generated, to provide guardrails and shape to minimize the possibility of API instability between releases.

If this ends up taking long enough where it becomes a blocker, we can look into marking the query library API as internal and making extremely clear it is unstable and moving forward with it. I communicated to them that I don't consider myself blocked at the moment and it could wait, and I would just work off this branch.

Cc: @ifrost

ifrost · 2024-05-13T08:19:55Z

After having a chat with @grafana/grafana-frontend-platform we have decided to move this functionality back to core and used it from there until there's a consenus reached on how such APIs should be exposed.

Related issues/prs/docs:

public/app/features/query-library/api/query.ts

ifrost added 30 commits April 16, 2024 11:49

Create basic feature toggle

9b2b7f4

Rename context to reflect it contains query history and query library

91448ef

Update icons and variants

21f58ad

Rename hooks

fad3317

Update tests

207f28e

Fix mock

e0a5b49

Add tracking

f4d47e7

Turn button into a toggle

661f657

Make dropdown active as well

b47897b

This is required to have better UI and an indication of selected state in split view

Update Query Library icon

f041ca5

This is to make it consistent with the toolbar button

Hide query history button when query library is available

ffa406e

This is to avoid confusing UX with 2 button triggering the drawer but with slightly different behavior

Make the drawer bigger for query library

8719a8b

To avoid confusion for current users and test it internally a bit more it's behind a feature toggle. Bigger drawer may obstruct the view and add more friction in the UX.

Fix tests

eaf4292

The test was failing because queryLibraryAvailable was set to true for tests. This change makes it more explicit what use case is being tested

Remove active state underline from the dropdown

32fe982

Merge branch 'refs/heads/main' into ifrost/query-library-toggle

13527d4

# Conflicts: # pkg/services/featuremgmt/toggles_gen.json

Add basic types and api methods

9eabc48

This is just moved from the app. To be cleaned up and refactored later.

Move API utils from Query Library app to Grafana packages

2d9a55b

Move API utils from Query Library app to Grafana packages

f1746ce

Move API utils from Query Library app to Grafana packages

5fa506c

Add basic table for query templates

2f1321b

Add sorting

72f4dce

Style cells

783d8d4

Style table cells

b5dcfd2

Allow closing Query Library drawer from the toolbar

076347d

Merge branch 'refs/heads/main' into ifrost/list-query-templates

f6c852a

# Conflicts: # public/app/features/explore/QueriesDrawer/QueriesDrawerDropdown.tsx # public/app/features/explore/RichHistory/RichHistory.tsx

Remove Private Query toggle

48de27e

It will be moved to the kebab

Add empty state

bee1a84

Remove variables detection for now

7ce0839

Just to simplify the PR, it's not needed for Explore yet.

ifrost requested review from a team as code owners April 29, 2024 12:37

ifrost requested review from ashharrison90, tskarhed and leventebalogh and removed request for a team April 29, 2024 12:37

Move reactjs/toolkit to dev dependecies

8855666

gelicia approved these changes Apr 29, 2024

View reviewed changes

Minor clean up and use react-redux as a peer dependency

6f81ec5

gelicia added 2 commits May 6, 2024 11:17

Merge branch 'main' of https://github.com/grafana/grafana into ifrost…

1d61521

…/list-query-templates

Merge branch 'main' of https://github.com/grafana/grafana into ifrost…

95916f3

…/list-query-templates

ifrost added 3 commits May 10, 2024 13:35

Merge branch 'refs/heads/main' into ifrost/list-query-templates

cffb0bc

Move query library code to core features

940bab9

Update code owners

5918e14

ifrost requested a review from tolzhabayev as a code owner May 10, 2024 12:11

ifrost added 4 commits May 10, 2024 14:31

Update export

0e5c27d

Update exports

ed65cf0

Use Redux store instead of APIProvider

f665ad7

Await for query templates to show during the test

a67c7e6

ifrost requested a review from gelicia May 13, 2024 08:15

gelicia reviewed May 14, 2024

View reviewed changes

public/app/features/query-library/api/query.ts Show resolved Hide resolved

gelicia approved these changes May 14, 2024

View reviewed changes

Add more explicit docs that the feature is experimental

bc7f1c7

ifrost merged commit fd218ed into main May 14, 2024
14 checks passed

ifrost deleted the ifrost/list-query-templates branch May 14, 2024 08:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explore: List query templates #86897

Explore: List query templates #86897

ifrost commented Apr 24, 2024 •

edited

gelicia left a comment

ifrost commented Apr 30, 2024

gelicia commented May 7, 2024

ifrost commented May 13, 2024

Explore: List query templates #86897

Explore: List query templates #86897

Conversation

ifrost commented Apr 24, 2024 • edited

gelicia left a comment

Choose a reason for hiding this comment

ifrost commented Apr 30, 2024

gelicia commented May 7, 2024

ifrost commented May 13, 2024

ifrost commented Apr 24, 2024 •

edited