fix: advanced filters could not be cleared from the filter bar

web-common/src/features/dashboards/filters/AdvancedFilter.svelte

@@ -1,17 +1,41 @@
 <script lang="ts">
   import * as Tooltip from "@rilldata/web-common/components/tooltip-v2";
+  import CancelCircle from "@rilldata/web-common/components/icons/CancelCircle.svelte";
   import { m } from "@rilldata/web-common/lib/i18n/gen/messages";
   import { convertExpressionToFilterParam } from "@rilldata/web-common/features/dashboards/url-state/filters/converters";
   import type { V1Expression } from "@rilldata/web-common/runtime-client";
 
-  export let advancedFilter: V1Expression;
+  let {
+    advancedFilter,
+    onRemove,
+  }: {
+    advancedFilter: V1Expression;
+    // The pill cannot be edited, so removing it clears the whole filter.
+    // Read only surfaces leave this unset.
+    onRemove?: () => void;
+  } = $props();
 
-  $: filterText = convertExpressionToFilterParam(advancedFilter);
+  let filterText = $derived(convertExpressionToFilterParam(advancedFilter));
 </script>
 
 <div
-  class="flex flex-none px-2 py-[3px] max-h-[26px] border bg-surface-subtle border-gray-200 text-fg-primary rounded-2xl"
+  class="flex flex-none items-center gap-x-1 px-2 py-[3px] max-h-[26px] border bg-surface-subtle border-gray-200 text-fg-primary rounded-2xl"
 >
+  {#if onRemove}
+    <!-- A sibling of the tooltip trigger, which is a button of its own. -->
+    <button
+      class="text-inherit"
+      aria-label={m.filter_advanced_remove()}
+      type="button"
+      onpointerdown={(e) => e.stopPropagation()}
+      onclick={(e) => {
+        e.stopPropagation();
+        onRemove();
+      }}
+    >
+      <CancelCircle size="16px" />
+    </button>
+  {/if}
   <Tooltip.Root>
     <Tooltip.Trigger>
       <span class="font-bold mr-1">{m.filter_advanced_beta()}</span>

web-common/src/features/dashboards/filters/CanvasExpressionFilters.spec.ts

@@ -1,5 +1,6 @@
 import { useCanvasFiltersVariant } from "@rilldata/web-common/features/dashboards/filters/test/canvas-filters-variant";
 import {
+  testAdvancedFilters,
   testDimensionFilters,
   testMeasureFilters,
 } from "@rilldata/web-common/features/dashboards/filters/test/expression-filters-suite";
@@ -71,6 +72,7 @@ describe("CanvasExpressionFilters", () => {
 
   testDimensionFilters(variant);
   testMeasureFilters(variant);
+  testAdvancedFilters(variant);
   // We need to fix canvas navigation before enabling this.
   // Check CanvasInitialization.svelte for more details.
   // testURLNavigationFlows(variant);

web-common/src/features/dashboards/filters/ExploreExpressionFilters.spec.ts

@@ -1,5 +1,6 @@
 import { useExploreFiltersVariant } from "@rilldata/web-common/features/dashboards/filters/test/explore-filters-variant";
 import {
+  testAdvancedFilters,
   testDimensionFilters,
   testMeasureFilters,
   testUrlLengthLimit,
@@ -75,4 +76,5 @@ describe("ExploreExpressionFilters", () => {
   testUrlLengthLimit(variant);
   testMeasureFilters(variant);
   testURLNavigationFlows(variant);
+  testAdvancedFilters(variant);
 });

web-common/src/features/dashboards/filters/ExpressionFilterManager.spec.ts

@@ -1382,6 +1382,26 @@ describe("clear", () => {
     expect(filterManager.sortedFilterManagers.measures).toEqual([]);
   });
 
+  it("resets the complex flag", () => {
+    const filterManager = createFilterManager();
+
+    // A top level OR has no chip, so the bar shows the read only advanced filter.
+    filterManager.storeSync.setUrlParams(
+      perMetricsViewParams({
+        [AD_BIDS_METRICS_NAME]: `${AD_BIDS_PUBLISHER_DIMENSION} IN ('Google') OR ${AD_BIDS_DOMAIN_DIMENSION} IN ('google.com')`,
+      }),
+    );
+    expect(filterManager.isComplexFilter).toBe(true);
+
+    filterManager.clear();
+
+    // The navigation after a clear does not parse the params again, so `clear` resets the flag itself.
+    expect(filterManager.isComplexFilter).toBe(false);
+    expect(filterManager.topLevelJoiner.expr).toEqual({});
+    expect(filterManager.sortedFilterManagers.dimensions).toEqual([]);
+    expect(filterManager.sortedFilterManagers.measures).toEqual([]);
+  });
+
   it("keeps the required and pinned chips", () => {
     const yamlConfigProvider = new YAMLConfigProvider();
     yamlConfigProvider.requiredFilters = { [AD_BIDS_DOMAIN_DIMENSION]: true };

web-common/src/features/dashboards/filters/ExpressionFilterManager.svelte.ts

@@ -316,6 +316,9 @@ export class ExpressionFilterManager implements UrlParamsStore {
 
   public clear() {
     this.temporaryFilterName = undefined;
+    // `setUrlParams` is the only other writer of this flag,
+    // and the navigation after a clear skips it because the params already match the cleared state.
+    this.isComplexFilter = false;
     this.topLevelJoiner.clear();
     // Clearing goes through the param rather than the managers, so it has to report itself.
     this.events.emit("filter-changed", { source: undefined });

web-common/src/features/dashboards/filters/ExpressionFilters.svelte

@@ -89,7 +89,10 @@
   <div class="relative flex flex-row flex-wrap gap-x-2 gap-y-2">
     {#if expressionFilterManager.isComplexFilter}
       {#each Object.entries(expressionFilterManager.exprByMetricsView) as [mv, expr] (mv)}
-        <AdvancedFilter advancedFilter={expr} />
+        <AdvancedFilter
+          advancedFilter={expr}
+          onRemove={() => expressionFilterManager.clear()}
+        />
       {/each}
     {:else}
       {#if !hasFilters}
@@ -135,14 +138,16 @@
         {excludedDimensions}
         {excludedMeasures}
       />
+    {/if}
 
-      <!-- if filters are present, place a chip at the end of the flex container
-      that enables clearing all filters -->
-      {#if hasClearableFilters}
-        <Button type="text" onClick={() => expressionFilterManager.clear()}>
-          {m.dashboard_clear_filters()}
-        </Button>
-      {/if}
+    <!-- if filters are present, place a chip at the end of the flex container
+    that enables clearing all filters.
+    An advanced filter is gated on the flag rather than on its pill: a param the chips cannot
+    show may also parse to no pill at all, and it still has to be clearable. -->
+    {#if expressionFilterManager.isComplexFilter || hasClearableFilters}
+      <Button type="text" onClick={() => expressionFilterManager.clear()}>
+        {m.dashboard_clear_filters()}
+      </Button>
     {/if}
   </div>
 </div>

web-common/src/features/dashboards/filters/StandaloneExpressionFilters.spec.ts

@@ -1,4 +1,5 @@
 import {
+  testAdvancedFilters,
   testDimensionFilters,
   testMeasureFilters,
 } from "@rilldata/web-common/features/dashboards/filters/test/expression-filters-suite";
@@ -69,4 +70,5 @@ describe("StandaloneExpressionFilters", () => {
 
   testDimensionFilters(variant);
   testMeasureFilters(variant);
+  testAdvancedFilters(variant);
 });

web-common/src/features/dashboards/filters/VerticalExpressionFilters.svelte

@@ -54,7 +54,10 @@
   <div class="relative flex flex-row flex-wrap gap-x-2 gap-y-2">
     {#if expressionFilterManager.isComplexFilter}
       {#each Object.entries(expressionFilterManager.exprByMetricsView) as [mv, expr] (mv)}
-        <AdvancedFilter advancedFilter={expr} />
+        <AdvancedFilter
+          advancedFilter={expr}
+          onRemove={() => expressionFilterManager.clear()}
+        />
       {/each}
     {:else}
       {#each sortedDimensionManagers as dimensionManager (dimensionManager.name)}
@@ -83,7 +86,7 @@
   </div>
 
   <div class="ml-auto">
-    {#if hasFilters}
+    {#if hasFilters || expressionFilterManager.isComplexFilter}
       <Button type="text" onClick={() => expressionFilterManager.clear()}>
         {m.dashboard_clear_filters()}
       </Button>

web-common/src/features/dashboards/filters/test/CanvasExpressionFiltersTest.svelte

@@ -1,4 +1,5 @@
 <script lang="ts">
+  import * as Tooltip from "@rilldata/web-common/components/tooltip-v2";
   import CanvasDashboardWrapper from "@rilldata/web-common/features/canvas/CanvasDashboardWrapper.svelte";
   import CanvasProvider from "@rilldata/web-common/features/canvas/CanvasProvider.svelte";
   import { DEFAULT_DASHBOARD_WIDTH } from "@rilldata/web-common/features/canvas/layout-util";
@@ -15,13 +16,16 @@
   const runtimeClient = useRuntimeClient();
 </script>
 
-<CanvasProvider {canvasName} instanceId={runtimeClient.instanceId}>
-  <!-- `CanvasDashboardEmbed` reads both of these off the spec; they are fixed here. -->
-  <CanvasDashboardWrapper
-    {canvasName}
-    maxWidth={DEFAULT_DASHBOARD_WIDTH}
-    filtersEnabled
-  >
-    <div>Dashboard loaded!</div>
-  </CanvasDashboardWrapper>
-</CanvasProvider>
+<!-- The app layout supplies the tooltip provider, which the advanced filter pill needs. -->
+<Tooltip.Provider>
+  <CanvasProvider {canvasName} instanceId={runtimeClient.instanceId}>
+    <!-- `CanvasDashboardEmbed` reads both of these off the spec; they are fixed here. -->
+    <CanvasDashboardWrapper
+      {canvasName}
+      maxWidth={DEFAULT_DASHBOARD_WIDTH}
+      filtersEnabled
+    >
+      <div>Dashboard loaded!</div>
+    </CanvasDashboardWrapper>
+  </CanvasProvider>
+</Tooltip.Provider>

web-common/src/features/dashboards/filters/test/ExploreExpressionFiltersTest.svelte

@@ -1,4 +1,5 @@
 <script lang="ts">
+  import * as Tooltip from "@rilldata/web-common/components/tooltip-v2";
   import Filters from "@rilldata/web-common/features/dashboards/filters/Filters.svelte";
   import DashboardStateManager from "@rilldata/web-common/features/dashboards/state-managers/loaders/DashboardStateManager.svelte";
   import StateManagersProvider from "@rilldata/web-common/features/dashboards/state-managers/StateManagersProvider.svelte";
@@ -12,13 +13,16 @@
   export let hasTimeSeries: boolean = false;
 </script>
 
-<StateManagersProvider metricsViewName={AD_BIDS_METRICS_NAME} {exploreName}>
-  <DashboardStateManager {exploreName}>
-    <Filters
-      timeRanges={[]}
-      metricsViewName={AD_BIDS_METRICS_NAME}
-      {hasTimeSeries}
-    />
-    <div>Dashboard loaded!</div>
-  </DashboardStateManager>
-</StateManagersProvider>
+<!-- The app layout supplies the tooltip provider, which the advanced filter pill needs. -->
+<Tooltip.Provider>
+  <StateManagersProvider metricsViewName={AD_BIDS_METRICS_NAME} {exploreName}>
+    <DashboardStateManager {exploreName}>
+      <Filters
+        timeRanges={[]}
+        metricsViewName={AD_BIDS_METRICS_NAME}
+        {hasTimeSeries}
+      />
+      <div>Dashboard loaded!</div>
+    </DashboardStateManager>
+  </StateManagersProvider>
+</Tooltip.Provider>

web-common/src/features/dashboards/filters/test/StandaloneExpressionFiltersTest.svelte

@@ -1,4 +1,5 @@
 <script lang="ts">
+  import * as Tooltip from "@rilldata/web-common/components/tooltip-v2";
   import { ExpressionFilterManager } from "@rilldata/web-common/features/dashboards/filters/ExpressionFilterManager.svelte.ts";
   import ExpressionFilters from "@rilldata/web-common/features/dashboards/filters/ExpressionFilters.svelte";
   import { YAMLConfigProvider } from "@rilldata/web-common/features/dashboards/providers/YAMLConfigProvider.svelte.ts";
@@ -13,9 +14,12 @@
    */
   let {
     metricsViewNames,
+    initUrlSearch,
     onManagerCreated,
   }: {
     metricsViewNames: string[];
+    // Url search the bar starts from, for a test that begins with a filter already applied.
+    initUrlSearch?: string;
     onManagerCreated?: (
       expressionFilterManager: ExpressionFilterManager,
     ) => void;
@@ -36,15 +40,21 @@
   // svelte-ignore state_referenced_locally
   onManagerCreated?.(expressionFilterManager);
 
-  expressionFilterManager.storeSync.setUrlParams(new URLSearchParams());
+  // svelte-ignore state_referenced_locally
+  expressionFilterManager.storeSync.setUrlParams(
+    new URLSearchParams(initUrlSearch),
+  );
 </script>
 
-<ExpressionFilters
-  {expressionFilterManager}
-  timeStart={undefined}
-  timeEnd={undefined}
-  timeControlsReady
-/>
-{#if metricsViewsProvider.ready}
-  <div>Dashboard loaded!</div>
-{/if}
+<!-- The app layout supplies the tooltip provider, which the advanced filter pill needs. -->
+<Tooltip.Provider>
+  <ExpressionFilters
+    {expressionFilterManager}
+    timeStart={undefined}
+    timeEnd={undefined}
+    timeControlsReady
+  />
+  {#if metricsViewsProvider.ready}
+    <div>Dashboard loaded!</div>
+  {/if}
+</Tooltip.Provider>

web-common/src/features/dashboards/filters/test/expression-filters-suite.ts

@@ -13,6 +13,7 @@ import {
   getSelectAllButton,
   isMeasureFilterFormOpen,
   pressEnter,
+  removeAdvancedFilter,
   removeDimensionFilter,
   removeMeasureFilter,
   selectDimensionFilterMode,
@@ -22,6 +23,7 @@ import {
   toggleMeasureFilter,
   toggleSelectAll,
   typeInDimensionFilterSearch,
+  waitForAdvancedFilter,
   waitForDimensionFilterResultCount,
   waitForDimensionFilterResults,
   waitForEmptyFilters,
@@ -34,8 +36,10 @@ import {
   createInExpression,
   createLikeExpression,
   createSubQueryExpression,
+  getAllIdentifiers,
 } from "@rilldata/web-common/features/dashboards/stores/filter-utils";
 import {
+  AD_BIDS_DOMAIN_DIMENSION,
   AD_BIDS_IMPRESSIONS_MEASURE,
   AD_BIDS_PUBLISHER_DIMENSION,
 } from "@rilldata/web-common/features/dashboards/stores/test-data/data";
@@ -1143,6 +1147,58 @@ export function testURLNavigationFlows(variant: ExpressionFiltersVariant) {
   });
 }
 
+/**
+ * A filter the chips cannot show falls back to the read only advanced pill.
+ * The pill cannot be edited, so clearing is the only way out of it.
+ */
+export function testAdvancedFilters(variant: ExpressionFiltersVariant) {
+  const {
+    initialUrlSearch,
+    urlSearchWithFilter,
+    assertWhereFilter,
+    assertUrlSearch,
+  } = variantAssertions(variant);
+
+  describe("Advanced filters", () => {
+    // A top level OR has no chip of its own.
+    const orFilter = `${AD_BIDS_PUBLISHER_DIMENSION} IN ('Facebook') OR ${AD_BIDS_DOMAIN_DIMENSION} IN ('google.com')`;
+
+    it("Should clear an advanced filter with the clear button", async () => {
+      await variant.render(urlSearchWithFilter(orFilter));
+      await waitForAdvancedFilter(orFilter);
+
+      // The filter reaches the consumer even though no chip can show it.
+      // Only the identifiers are checked, since the explore state holds the OR as is while a
+      // standalone bar wraps it in an AND.
+      expect(
+        getAllIdentifiers(variant.expressionFilterManager.getWhereFilter()),
+      ).toEqual([AD_BIDS_PUBLISHER_DIMENSION, AD_BIDS_DOMAIN_DIMENSION]);
+      // Nothing can be added next to the pill.
+      expect(
+        screen.queryByLabelText("Add filter button"),
+      ).not.toBeInTheDocument();
+
+      await clearFilters();
+
+      expect(screen.queryByText("Advanced (BETA)")).not.toBeInTheDocument();
+      expect(screen.getByLabelText("Add filter button")).toBeVisible();
+      assertWhereFilter(createAndExpression([]));
+      assertUrlSearch(initialUrlSearch);
+    });
+
+    it("Should clear an advanced filter from its pill", async () => {
+      await variant.render(urlSearchWithFilter(orFilter));
+      await waitForAdvancedFilter(orFilter);
+
+      await removeAdvancedFilter();
+
+      await waitForEmptyFilters();
+      assertWhereFilter(createAndExpression([]));
+      assertUrlSearch(initialUrlSearch);
+    });
+  });
+}
+
 /** The expression a measure filter on `impressions`, split by `publisher`, produces. */
 export function measureFilterExpression(operation: V1Operation, value: number) {
   return createSubQueryExpression(