fix(dashboard): keep ALL on a dynamic variable that holds a value outside its list

.github/workflows/e2eci.yaml

@@ -2,13 +2,25 @@ name: e2eci
 
 on:
   pull_request:
+    types:
+      - opened
+      - synchronize
+      - reopened
+      - labeled
+  pull_request_target:
+    types:
+      - labeled
+  merge_group:
+    types:
+      - checks_requested
 
 concurrency:
-  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
+  group: e2eci-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}
   cancel-in-progress: true
 
 jobs:
   changes:
+    if: github.event_name != 'merge_group'
     runs-on: ubuntu-latest
     outputs:
       e2e: ${{ steps.changes.outputs.e2e }}
@@ -18,21 +30,17 @@ jobs:
         env:
           GH_TOKEN: ${{ github.token }}
           PR_NUMBER: ${{ github.event.pull_request.number }}
-          FORCE: ${{ contains(github.event.pull_request.labels.*.name, 'safe-to-e2e') }}
+          LABELED: ${{ contains(github.event.pull_request.labels.*.name, 'safe-to-e2e') }}
         run: |
-          CHANGED_FILES=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files" --paginate --jq '.[].filename | select(startswith("tests/integration/") | not)')
-          PATTERN='^(frontend/|tests/|\.github/workflows/e2eci\.yaml$|cmd/enterprise/Dockerfile\.with-web\.integration$)'
-
-          if [ "$FORCE" = "true" ]; then
+          files=$(gh api "repos/$GITHUB_REPOSITORY/pulls/$PR_NUMBER/files" --paginate --jq '.[].filename')
+          if [ "$LABELED" = true ] || grep -qE '^(frontend|tests/e2e)/' <<< "$files"; then
             echo "e2e=true" >> "$GITHUB_OUTPUT"
-          elif printf '%s\n' "$CHANGED_FILES" | grep -qE "$PATTERN"; then
-            echo "e2e=true" >> "$GITHUB_OUTPUT"
-          else
-            echo "e2e=false" >> "$GITHUB_OUTPUT"
           fi
   fmtlint:
     needs: changes
-    if: needs.changes.outputs.e2e == 'true'
+    if: |
+      ((github.event_name == 'pull_request' && ! github.event.pull_request.head.repo.fork && github.event.pull_request.user.login != 'dependabot[bot]' && ! contains(github.event.pull_request.labels.*.name, 'safe-to-test')) ||
+      (github.event_name == 'pull_request_target' && contains(github.event.pull_request.labels.*.name, 'safe-to-test'))) && needs.changes.outputs.e2e == 'true'
     runs-on: ubuntu-latest
     steps:
       - name: checkout
@@ -62,7 +70,9 @@ jobs:
           - chromium
         shard: [1, 2, 3, 4]
     needs: changes
-    if: needs.changes.outputs.e2e == 'true'
+    if: |
+      ((github.event_name == 'pull_request' && ! github.event.pull_request.head.repo.fork && github.event.pull_request.user.login != 'dependabot[bot]' && ! contains(github.event.pull_request.labels.*.name, 'safe-to-test')) ||
+      (github.event_name == 'pull_request_target' && contains(github.event.pull_request.labels.*.name, 'safe-to-test'))) && needs.changes.outputs.e2e == 'true'
     runs-on: ubuntu-latest
     timeout-minutes: 30
     steps:
@@ -171,14 +181,17 @@ jobs:
           name: playwright-artifacts-${{ matrix.project }}-${{ matrix.shard }}
           path: tests/e2e/artifacts/
           retention-days: 5
-  # Required status check. Fails when e2e was needed but did not pass.
+  # Required status check. Fails when e2e was needed but did not pass,
+  # including fork and dependabot PRs still waiting on safe-to-test.
   e2e:
     if: always()
     needs: [changes, fmtlint, test]
     runs-on: ubuntu-latest
     steps:
       - name: verdict
-        if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')
+        if: |
+          github.event_name != 'merge_group' && (needs.changes.result != 'success' ||
+          (needs.changes.outputs.e2e == 'true' && (needs.fmtlint.result != 'success' || needs.test.result != 'success')))
         run: |
-          echo "::error::e2e did not pass."
+          echo "::error::e2e did not pass. Fork and dependabot PRs need the safe-to-test label."
           exit 1

frontend/src/pages/DashboardPage/DashboardContainer/VariablesBar/__tests__/ValueSelector.test.tsx

@@ -193,6 +193,40 @@ describe('ValueSelector', () => {
 
 			expect(onSearch).toHaveBeenLastCalledWith('pay');
 		});
+
+		it('commits ALL even when the selection holds a value outside the list', async () => {
+			// A search pick, or one an older time window offered: ALL must still go out as
+			// `__all__`, not as a list that filters the panels to the truncated options.
+			const onChange = jest.fn();
+			const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime });
+			render(
+				<ValueSelector
+					options={OPTIONS}
+					variableType="dynamic"
+					multiSelect
+					showAllOption
+					selection={{ value: ['searched'], allSelected: false }}
+					onChange={onChange}
+					emptyFallback={{ value: null, allSelected: true }}
+					testId="variable-select-env"
+					dynamic={{
+						values: OPTIONS,
+						relatedValues: [],
+						complete: false,
+						onSearch: jest.fn(),
+						onSearchReset: jest.fn(),
+					}}
+				/>,
+			);
+
+			await openDropdown();
+			await user.click(screen.getByText('ALL'));
+			await user.keyboard('{Escape}');
+
+			expect(onChange).toHaveBeenLastCalledWith(
+				expect.objectContaining({ allSelected: true }),
+			);
+		});
 	});
 
 	describe('clearing', () => {

frontend/src/pages/DashboardPage/DashboardContainer/VariablesBar/__tests__/selectionUtils.test.ts

@@ -12,6 +12,7 @@ function commit(
 		values,
 		options: OPTIONS,
 		showAllOption: true,
+		isDynamic: false,
 		emptyFallback: FALLBACK,
 		...overrides,
 	});
@@ -60,6 +61,20 @@ describe('selectionFromCommittedValues', () => {
 		});
 	});
 
+	// A dynamic ALL is sent as `__all__`, so reading it as a list would filter the
+	// panels down to the truncated option set.
+	it('reads every option PLUS a value outside the list as ALL for a dynamic variable', () => {
+		expect(
+			commit([...OPTIONS, 'searched'], { isDynamic: true }).allSelected,
+		).toBe(true);
+	});
+
+	it('does not read a dynamic subset PLUS a value outside the list as ALL', () => {
+		expect(
+			commit(['checkout', 'searched'], { isDynamic: true }).allSelected,
+		).toBe(false);
+	});
+
 	// Derived from the values + options at commit time, never from the old selection.
 	it('recomputes the marker: a typed value the data now offers is a normal pick', () => {
 		expect(

frontend/src/pages/DashboardPage/DashboardContainer/VariablesBar/components/selectors/ValueSelector.tsx

@@ -102,6 +102,7 @@ function ValueSelector({
 			values,
 			options,
 			showAllOption,
+			isDynamic: !!dynamic,
 			emptyFallback,
 		});
 

frontend/src/pages/DashboardPage/DashboardContainer/VariablesBar/utils/selectionUtils.ts

@@ -51,6 +51,7 @@ interface CommittedValues {
 	values: string[];
 	options: string[];
 	showAllOption: boolean;
+	isDynamic: boolean;
 	emptyFallback: VariableSelection;
 }
 
@@ -62,6 +63,7 @@ export function selectionFromCommittedValues({
 	values,
 	options,
 	showAllOption,
+	isDynamic,
 	emptyFallback,
 }: CommittedValues): VariableSelection {
 	if (values.length === 0) {
@@ -71,10 +73,12 @@ export function selectionFromCommittedValues({
 	const customValues = values.filter((value) => !options.includes(value));
 	// ALL re-materializes to the option set, so a set carrying a typed value is not ALL
 	// — the next refetch would expand it back and drop what the user typed.
+	// A dynamic ALL travels as `__all__` and is never re-materialized, so a value outside
+	// its truncated list (a search pick, an older window's pick) loses nothing.
 	const allSelected =
 		showAllOption &&
 		options.length > 0 &&
-		customValues.length === 0 &&
+		(isDynamic || customValues.length === 0) &&
 		options.every((option) => values.includes(option));
 
 	return {

frontend/src/utils/__tests__/sanitizeOrderBy.test.ts

@@ -1,4 +1,3 @@
-import * as Sentry from '@sentry/react';
 import { DataTypes } from 'types/api/queryBuilder/queryAutocompleteResponse';
 import {
 	IBuilderQuery,
@@ -8,10 +7,6 @@ import { DataSource } from 'types/common/queryBuilder';
 import { getParsedAggregationOptionsForOrderBy } from 'utils/aggregationConverter';
 import { sanitizeOrderByForExplorer } from 'utils/sanitizeOrderBy';
 
-jest.mock('@sentry/react', () => ({
-	captureEvent: jest.fn(),
-}));
-
 jest.mock('utils/aggregationConverter', () => ({
 	getParsedAggregationOptionsForOrderBy: jest.fn(),
 }));
@@ -122,7 +117,6 @@ describe('sanitizeOrderByForExplorer', () => {
 
 		const result = sanitizeOrderByForExplorer(query);
 		expect(result).toStrictEqual([]);
-		expect(Sentry.captureEvent).not.toHaveBeenCalled();
 	});
 
 	it('handles missing orderBy by returning an empty array', () => {

frontend/src/utils/sanitizeOrderBy.ts

@@ -1,3 +1,4 @@
+import * as Sentry from '@sentry/react';
 import {
 	IBuilderQuery,
 	OrderByPayload,
@@ -19,5 +20,15 @@ export function sanitizeOrderByForExplorer(
 
 	const current = query.orderBy || [];
 
+	const hasInvalidOrderBy = current.some((o) => !allowed.has(o.columnName));
+
+	if (hasInvalidOrderBy) {
+		Sentry.captureEvent({
+			message: `Invalid orderBy: current: ${JSON.stringify(
+				current,
+			)} - allowed: ${JSON.stringify(Array.from(allowed))}`,
+			level: 'warning',
+		});
+	}
 	return current.filter((o) => allowed.has(o.columnName));
 }