Skip to content

Long-running integration branch: experimental → main#352

Draft
samjewell wants to merge 5 commits into
mainfrom
experimental
Draft

Long-running integration branch: experimental → main#352
samjewell wants to merge 5 commits into
mainfrom
experimental

Conversation

@samjewell

@samjewell samjewell commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Purpose — long-running integration branch

This is a standing, long-lived PR. It is intentionally kept open and is not meant to be merged ad-hoc. It serves as the integration point between our fast-iteration experimental branch and main.

How we work with experimental

  • Feature work is opened as small PRs targeting experimental (not main).
  • Those feature PRs are merged once CI is green with GenAI review only (Cursor Bugbot + Security Agent). This is allowed because the org's required-human-review ruleset is scoped to the default branch (main) only — see ruleset Public repo required PRs (~DEFAULT_BRANCH).
  • experimental is deployed to the ops environment via the existing Plugins - CD (publish.yaml) workflow_dispatch, which explicitly supports deploying non-main branches. This lets us combine several in-flight PRs into one iterable build and collaborate without clobbering each other's dev/ops deploys.

Why this preserves the org control

Everything that reaches customers / releases / the plugin catalogue still flows exclusively through main, and this PR is the gate: promoting experimental → main happens here and requires the normal human code review (1 approval, enforced by the org ruleset that cannot be bypassed). So main protection is fully intact — we've only moved fast-iteration/non-prod deploys onto a long-lived branch instead of short-lived ones.

Current contents

When we want to promote, we review the accumulated diff here and merge. Until then, this PR stays open as the running ledger of what's ahead of main on experimental.

Note

Do not squash/auto-merge this PR casually. It is a deliberate human-review checkpoint.

Made with Cursor


Note

Medium Risk
Changes query editor behavior and filter merge logic for mixed visual/JSON queries; mistakes could drop or mishandle advanced filters, though new hook tests cover preservation.

Overview
Replaces the full JSON-only query editor with a hybrid layout: the visual builder always renders, and queries with unsupported Cube features (e.g. time dimensions, advanced filters) also show a compact Additional query configuration callout via UnsupportedFieldsViewer, which lists reasons and exposes only unsupported query keys in collapsible JSON (not the whole query). JsonQueryViewer is removed.

QueryEditor hoists SQL compilation and SQL Preview to the top level (one preview for all modes). SQL Preview is collapsed by default behind a toggle; unit and e2e tests expand it before asserting on SQL.

The visual Filters UI is limited to equals/notEquals dimension filters (VISUAL_BUILDER_OPERATORS); advanced filters and AND/OR groups stay in query state but are hidden from FilterField. onFiltersChange merges visual filter edits with preserved non-visual filters so JSON-defined filters are not dropped when editing in the builder. Filter limits are documented via an info tooltip instead of inline hint text.

Reviewed by Cursor Bugbot for commit 8333314. Bugbot is set up for automated code reviews on this repo. Configure here.

adrapereira and others added 3 commits May 28, 2026 16:39
…141) (#351)

* feat: show visual editor alongside read-only JSON for unsupported features

Previously, when a query contained features the visual builder cannot
represent (time dimensions, AND/OR filter groups, advanced operators,
template-variable filter values), the entire visual editor was replaced
by a read-only JSON viewer, leaving users unable to edit anything visually.

This makes the layout hybrid: the visual editor is always rendered, and
when unsupported features are detected, a compact collapsible JSON callout
(UnsupportedFieldsViewer) shows only the unsupported top-level keys.

- Always render the visual editor; show UnsupportedFieldsViewer additively
- New UnsupportedFieldsViewer replaces JsonQueryViewer (now removed); it
  shows only unsupported keys (via getUnsupportedQueryKeys), collapsed by default
- SQLPreview is now collapsed by default behind a "SQL Preview" toggle and
  rendered once at the top level regardless of supported state
- Filters label gains a compact tooltip; the visual FilterField only receives
  equals/notEquals filters so advanced operators no longer leak into it
- onFiltersChange merges visual-builder edits back with non-visual filters
  (AND/OR groups, advanced operators, template-variable filters) so they
  are no longer silently dropped

Reimplements the intent of the now-closed #141 cleanly on top of current main.

Co-authored-by: Cursor <cursoragent@cursor.com>

* test(e2e): expand collapsed SQL preview before asserting

The SQL preview is now collapsed by default behind a "SQL Preview" toggle,
so the Playwright specs must click the toggle before the generated SQL and
"Edit SQL in Explore" button become visible.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Template variable filters dropped
    • Fixed by excluding template variable filters from FilterField and preserving them in onFiltersChange via the new filterHasTemplateVariables check.

Create PR

Or push these changes by commenting:

@cursor push 49471fd43b
Preview (49471fd43b)
diff --git a/src/components/QueryEditor.tsx b/src/components/QueryEditor.tsx
--- a/src/components/QueryEditor.tsx
+++ b/src/components/QueryEditor.tsx
@@ -11,7 +11,7 @@
 import { FilterField } from './FilterField/FilterField';
 import { useQueryEditorHandlers } from '../hooks/useQueryEditorHandlers';
 import { buildCubeQueryJson } from '../utils/buildCubeQuery';
-import { detectUnsupportedFeatures, getUnsupportedQueryKeys } from '../utils/detectUnsupportedFeatures';
+import { detectUnsupportedFeatures, getUnsupportedQueryKeys, filterHasTemplateVariables } from '../utils/detectUnsupportedFeatures';
 import { decorateWithViewSelection, getViewSelectionState } from '../utils/viewSelection';
 import { UnsupportedFieldsViewer } from './UnsupportedFieldsViewer';
 
@@ -216,7 +216,8 @@
       >
         <FilterField
           filters={query.filters?.filter(
-            (f): f is CubeFilter => isCubeFilter(f) && VISUAL_BUILDER_OPERATORS.has(f.operator)
+            (f): f is CubeFilter =>
+              isCubeFilter(f) && VISUAL_BUILDER_OPERATORS.has(f.operator) && !filterHasTemplateVariables(f)
           )}
           dimensions={dimensionOptions}
           onChange={onFiltersChange}

diff --git a/src/hooks/useQueryEditorHandlers.ts b/src/hooks/useQueryEditorHandlers.ts
--- a/src/hooks/useQueryEditorHandlers.ts
+++ b/src/hooks/useQueryEditorHandlers.ts
@@ -1,6 +1,7 @@
 import type { TQueryOrderArray } from '@cubejs-client/core';
 import { ChangeEvent } from 'react';
 import { CubeQuery, CubeFilter, CubeFilterItem, Order, DEFAULT_ORDER, VISUAL_BUILDER_OPERATORS, isCubeFilter } from '../types';
+import { filterHasTemplateVariables } from '../utils/detectUnsupportedFeatures';
 import { SelectableValue } from '@grafana/data';
 import { normalizeOrder } from '../utils/normalizeOrder';
 
@@ -91,7 +92,10 @@
     // non-visual-builder filters (advanced operators, AND/OR groups,
     // template-variable filters) so editing in the visual builder doesn't drop them.
     const nonVisualFilters = (query.filters ?? []).filter(
-      (f: CubeFilterItem) => !isCubeFilter(f) || !VISUAL_BUILDER_OPERATORS.has(f.operator)
+      (f: CubeFilterItem) =>
+        !isCubeFilter(f) ||
+        !VISUAL_BUILDER_OPERATORS.has(f.operator) ||
+        filterHasTemplateVariables(f)
     );
     const merged = [...filters, ...nonVisualFilters];
     updateQueryAndRun({ filters: merged.length > 0 ? merged : undefined });

diff --git a/src/utils/detectUnsupportedFeatures.ts b/src/utils/detectUnsupportedFeatures.ts
--- a/src/utils/detectUnsupportedFeatures.ts
+++ b/src/utils/detectUnsupportedFeatures.ts
@@ -1,8 +1,16 @@
-import { CubeFilterItem, CubeQuery, VISUAL_BUILDER_OPERATORS, isCubeFilter, isCubeAndFilter, isCubeOrFilter } from '../types';
+import { CubeFilter, CubeFilterItem, CubeQuery, VISUAL_BUILDER_OPERATORS, isCubeFilter, isCubeAndFilter, isCubeOrFilter } from '../types';
 
-const TEMPLATE_VARIABLE_PATTERN = /(?:\$(?:[a-zA-Z_]\w*|\{[a-zA-Z_]\w*(?::[^}]+)?\})|\[\[[^\]]+\]\])/;
+export const TEMPLATE_VARIABLE_PATTERN = /(?:\$(?:[a-zA-Z_]\w*|\{[a-zA-Z_]\w*(?::[^}]+)?\})|\[\[[^\]]+\]\])/;
 
 /**
+ * Returns true if the given flat CubeFilter contains a template variable
+ * (e.g. $var, ${var}, ${var:raw}, or [[var]]) in any of its values.
+ */
+export function filterHasTemplateVariables(filter: CubeFilter): boolean {
+  return filter.values?.some((v) => TEMPLATE_VARIABLE_PATTERN.test(v)) ?? false;
+}
+
+/**
  * Detects query features that the visual builder cannot represent.
  *
  * Uses a blocklist approach: we check for specific patterns we know are

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 8333314. Configure here.

(f: CubeFilterItem) => !isCubeFilter(f) || !VISUAL_BUILDER_OPERATORS.has(f.operator)
);
const merged = [...filters, ...nonVisualFilters];
updateQueryAndRun({ filters: merged.length > 0 ? merged : undefined });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Template variable filters dropped

High Severity

Hybrid mode shows equals/notEquals filters (including dashboard variable values) in the visual FilterField, but onFiltersChange only re-attaches advanced operators and AND/OR groups as non-visual filters. Clearing or replacing visual filters removes template-variable filters from the saved query even though they are flagged unsupported elsewhere.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8333314. Configure here.

@samjewell samjewell self-assigned this Jun 4, 2026
@samjewell samjewell marked this pull request as draft June 4, 2026 09:09
A dashboard-variable filter (e.g. {operator: equals, values: ['$var']})
uses a visual operator, so it was both handed to the visual FilterField
and excluded from onFiltersChange's preserved non-visual set. FilterField
can't render a $var value as a selectable chip, so editing any filter in
the visual builder could silently drop it.

- Exclude template-variable filters from FilterField's input so they're
  never echoed back by its onChange (avoids dropping and avoids duplication)
- Preserve them in onFiltersChange regardless of operator
- Export a shared TEMPLATE_VARIABLE_PATTERN + filterValuesContainTemplateVariable
  helper from detectUnsupportedFeatures so detection and filter handling agree
- Add unit tests covering $var, ${var:format}, and [[var]] syntaxes

Fixes the Cursor Bugbot finding on #351.

Co-authored-by: Cursor <cursoragent@cursor.com>
@samjewell

samjewell commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

This deploy to the Ops environment (catalog) from this branch worked fine:
https://github.com/grafana/grafana-cube-datasource/actions/runs/26919203685

And the plugin itself at version 0.6.1+833331455849de141f8cdc219a530b74f17bda3b is also working fine in our ops environment :-)

This image happens to show the feature which is already on this feature branch, and the screenshot was taken from our Ops instance.
image

So this works as a way to get plugin improvements merged to a feature branch, then built and deployed to our ops env. Not sure exactly how we can reference the build artifacts from a grafana instance outside our Ops hosted-grafana environment though - that's another question, and something we might need to look into. cc @adrapereira

@cla-assistant

cla-assistant Bot commented Jun 8, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cla-assistant

cla-assistant Bot commented Jun 8, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ samjewell
❌ adrapereira
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants