From 745f171a5c8ccfae0082572ecda87ae48eb1595c Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Fri, 4 Sep 2026 14:11:29 -0400 Subject: [PATCH] feat: Allow configuring dashboard filters as required --- .changeset/required-dashboard-filters.md | 7 + packages/api/openapi.json | 50 +- .../dashboards/saveDashboard.int.test.ts | 149 +++++- .../api/src/mcp/prompts/dashboards/content.ts | 2 + .../api/src/mcp/tools/dashboards/schemas.ts | 22 + .../api/__tests__/dashboard.int.test.ts | 110 +++++ .../__tests__/dashboards.int.test.ts | 145 +++++- .../src/routers/external-api/v2/dashboards.ts | 59 ++- packages/api/src/utils/externalApi.ts | 12 +- packages/app/eslint.config.mjs | 2 + packages/app/src/DBDashboardPage.tsx | 133 ++++- packages/app/src/DashboardFilters.tsx | 66 ++- .../ServicesDashboardPage.tsx | 1 + .../src/__tests__/DashboardFilters.test.ts | 25 + .../DashboardFilterEditForm.tsx | 45 +- .../DashboardFiltersList.tsx | 15 + .../__tests__/DashboardFiltersModal.test.tsx | 127 +++++ .../__tests__/filterFormState.test.ts | 108 +++++ .../DashboardFiltersModal/filterFormState.ts | 43 +- .../DashboardFiltersModal/index.tsx | 4 + .../__tests__/useDashboardFilters.test.tsx | 87 ++++ .../usePresetDashboardFilters.test.tsx | 1 + .../app/src/hooks/useDashboardFilters.tsx | 28 +- .../dashboard-required-filters.spec.ts | 453 ++++++++++++++++++ .../tests/e2e/page-objects/DashboardPage.ts | 102 +++- .../__tests__/dashboardFilterValues.test.ts | 253 ++++++++++ .../src/__tests__/filters.test.ts | 92 ++++ .../common-utils/src/dashboardFilterValues.ts | 52 ++ packages/common-utils/src/filters.ts | 34 ++ packages/common-utils/src/types.ts | 12 + 30 files changed, 2132 insertions(+), 107 deletions(-) create mode 100644 .changeset/required-dashboard-filters.md create mode 100644 packages/app/tests/e2e/features/dashboard-required-filters.spec.ts diff --git a/.changeset/required-dashboard-filters.md b/.changeset/required-dashboard-filters.md new file mode 100644 index 0000000000..d32723a452 --- /dev/null +++ b/.changeset/required-dashboard-filters.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/api': patch +'@hyperdx/app': patch +--- + +feat: Allow configuring dashboard filters as required diff --git a/packages/api/openapi.json b/packages/api/openapi.json index fc0e35c345..0a7b87fb39 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -3160,7 +3160,7 @@ ] }, "FilterInput": { - "description": "A drop-down filter on a dashboard. Depending on type, filters can either broadcast\nselected value(s) to every tile (isBroadcastEnabled), expose selected\nvalues as a variable (isVariableEnabled) or both. Typically a filter should\ndo just one of these things.\n", + "description": "A drop-down filter on a dashboard. Depending on type, filters can either broadcast\nselected value(s) to every tile (isBroadcastEnabled), expose selected\nvalues as a variable (isVariableEnabled) or both. Typically a filter should\ndo just one of these things. Any filter may additionally be marked\nrequired via minSelections, which blocks the tiles that read it until it\nhas a selected value - or every tile on the dashboard, with\nisGlobalRequirement.\n", "oneOf": [ { "$ref": "#/components/schemas/QueryExpressionFilterInput" @@ -3271,6 +3271,22 @@ "pattern": "^[a-zA-Z][a-zA-Z0-9_]*$", "description": "Token tiles reference this filter's selected value by, as\n`$variableName`. Must start with a letter and may contain only\nletters, numbers, and underscores. Defaults to the display name with\nwhitespace replaced by underscores and remaining illegal characters\nremoved, so a variable-enabled filter whose name derives nothing\nusable must send this field explicitly. Variable names must be unique\nacross a dashboard's variable-enabled filters. Names the variable\nonly, so the field is rejected when isVariableEnabled is not true, and\nis omitted from responses for such a filter.\n", "example": "environment" + }, + "minSelections": { + "type": "integer", + "enum": [ + 0, + 1 + ], + "default": 0, + "description": "Minimum number of values that must be selected before tiles load.\nSet to 1 to make the filter required. Only 0 and 1 are accepted.\nOmit the field (or send 0) for the default optional behavior.\n", + "example": 1 + }, + "isGlobalRequirement": { + "type": "boolean", + "default": false, + "description": "Widens a required filter's block to every tile on the dashboard. False\n(the default) blocks only the tiles that read the filter: those\nreferencing its variableName, and those its broadcast applies to.\nIgnored unless minSelections is 1.\n", + "example": true } } }, @@ -3337,6 +3353,22 @@ "pattern": "^[a-zA-Z][a-zA-Z0-9_]*$", "description": "Token tiles reference this filter's selected value by, as\n`$variableName`. Must start with a letter and may contain only\nletters, numbers, and underscores. Defaults to the display name with\nwhitespace replaced by underscores and remaining illegal characters\nremoved, so a variable-enabled filter whose name derives nothing\nusable must send this field explicitly. Variable names must be unique\nacross a dashboard's variable-enabled filters. Names the variable\nonly, so the field is rejected when isVariableEnabled is not true, and\nis omitted from responses for such a filter.\n", "example": "environment" + }, + "minSelections": { + "type": "integer", + "enum": [ + 0, + 1 + ], + "default": 0, + "description": "Minimum number of values that must be selected before tiles load.\nSet to 1 to make the filter required. Only 0 and 1 are accepted.\nOmit the field (or send 0) for the default optional behavior.\n", + "example": 1 + }, + "isGlobalRequirement": { + "type": "boolean", + "default": false, + "description": "Widens a required filter's block to every tile on the dashboard. False\n(the default) blocks only the tiles that read the filter: those\nreferencing its variableName, and those its broadcast applies to.\nIgnored unless minSelections is 1.\n", + "example": true } } }, @@ -3406,6 +3438,22 @@ "pattern": "^[a-zA-Z][a-zA-Z0-9_]*$", "description": "Token that tiles reference this filter's selected value by, as\n`$variableName`. Must start with a letter and may contain only\nletters, numbers, and underscores. Defaults to the display name with\nwhitespace replaced by underscores and remaining illegal characters\nremoved. Variable names must be unique across a dashboard's\nvariable-enabled filters.\n", "example": "pod" + }, + "minSelections": { + "type": "integer", + "enum": [ + 0, + 1 + ], + "default": 0, + "description": "Minimum number of values that must be selected before tiles load.\nSet to 1 to make the filter required. Only 0 and 1 are accepted.\nOmit the field (or send 0) for the default optional behavior.\n", + "example": 1 + }, + "isGlobalRequirement": { + "type": "boolean", + "default": false, + "description": "Widens a required filter's block to every tile on the dashboard. False\n(the default) blocks only the tiles that read the filter: those\nreferencing its variableName, and those its broadcast applies to.\nIgnored unless minSelections is 1.\n", + "example": true } } }, diff --git a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts index 250621e5d9..08a68e0110 100644 --- a/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts +++ b/packages/api/src/mcp/__tests__/dashboards/saveDashboard.int.test.ts @@ -1855,6 +1855,14 @@ describe('MCP Dashboard Tools - clickstack_save_dashboard', () => { }, }); + const staticFilter = (overrides: Record = {}) => ({ + type: 'STATIC_LIST' as const, + name: 'Environment', + options: ['prod', 'staging', 'dev'], + variableName: 'env', + ...overrides, + }); + it('should round-trip filters on create, get, and update', async () => { const sourceId = ctx.traceSource._id.toString(); @@ -2579,15 +2587,144 @@ describe('MCP Dashboard Tools - clickstack_save_dashboard', () => { }); }); - describe('static list filters', () => { - const staticFilter = (overrides: Record = {}) => ({ - type: 'STATIC_LIST' as const, - name: 'Environment', - options: ['prod', 'staging', 'dev'], - variableName: 'env', + describe('required filters', () => { + const queryFilter = (overrides: Record = {}) => ({ + type: 'QUERY_EXPRESSION' as const, + name: 'Service', + expression: 'ServiceName', + sourceId: ctx.traceSource._id.toString(), ...overrides, }); + it.each([ + ['QUERY_EXPRESSION', queryFilter], + ['STATIC_LIST', staticFilter], + ] as const)( + 'round-trips a required %s filter through create, get, and update', + async (_label, makeFilter) => { + const sourceId = ctx.traceSource._id.toString(); + + const createResult = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: 'Required filter round-trip', + tiles: [traceTile(sourceId)], + filters: [makeFilter({ minSelections: 1 })], + }, + ); + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + expect(created.filters[0].minSelections).toBe(1); + + const getResult = await callTool( + ctx.client!, + 'clickstack_get_dashboard', + { id: created.id }, + ); + const fetched = JSON.parse(getFirstText(getResult)); + expect(fetched.filters[0].minSelections).toBe(1); + + const updateResult = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + id: created.id, + name: fetched.name, + tiles: fetched.tiles, + filters: fetched.filters, + }, + ); + expect(updateResult.isError).toBeFalsy(); + expect( + JSON.parse(getFirstText(updateResult)).filters[0].minSelections, + ).toBe(1); + }, + ); + + it('omits minSelections for a filter that is not required', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: 'Optional filter', + tiles: [traceTile(sourceId)], + filters: [queryFilter(), queryFilter({ minSelections: 0 })], + }, + ); + + expect(result.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(result)); + for (const filter of created.filters) { + expect(filter).not.toHaveProperty('minSelections'); + } + }); + + it.each([2, -1, 1.5])('rejects minSelections %s', async value => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: 'Bad minSelections', + tiles: [traceTile(sourceId)], + filters: [queryFilter({ minSelections: value })], + }, + ); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('minSelections'); + }); + + it('round-trips a dashboard-wide requirement through create and get', async () => { + const sourceId = ctx.traceSource._id.toString(); + + const createResult = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: 'Dashboard-wide requirement', + tiles: [traceTile(sourceId)], + filters: [ + queryFilter({ minSelections: 1, isGlobalRequirement: true }), + ], + }, + ); + expect(createResult.isError).toBeFalsy(); + const created = JSON.parse(getFirstText(createResult)); + expect(created.filters[0].isGlobalRequirement).toBe(true); + + const getResult = await callTool( + ctx.client!, + 'clickstack_get_dashboard', + { id: created.id }, + ); + expect( + JSON.parse(getFirstText(getResult)).filters[0].isGlobalRequirement, + ).toBe(true); + }); + + it('omits the scope for a filter that is not required', async () => { + const sourceId = ctx.traceSource._id.toString(); + const result = await callTool( + ctx.client!, + 'clickstack_save_dashboard', + { + name: 'Optional filter with a scope', + tiles: [traceTile(sourceId)], + filters: [queryFilter({ isGlobalRequirement: true })], + }, + ); + + expect(result.isError).toBeFalsy(); + expect(JSON.parse(getFirstText(result)).filters[0]).not.toHaveProperty( + 'isGlobalRequirement', + ); + }); + }); + + describe('static list filters', () => { it('round-trips a static filter through create, get, and save', async () => { const sourceId = ctx.traceSource._id.toString(); diff --git a/packages/api/src/mcp/prompts/dashboards/content.ts b/packages/api/src/mcp/prompts/dashboards/content.ts index d94c7f94de..6f335870e1 100644 --- a/packages/api/src/mcp/prompts/dashboards/content.ts +++ b/packages/api/src/mcp/prompts/dashboards/content.ts @@ -114,6 +114,8 @@ Apply these before calling clickstack_save_dashboard. Each rule is enforced by t 9d. USE A STATIC_LIST FILTER WHEN THE DROPDOWN SHOULD OFFER A FIXED HAND-AUTHORED LIST. When the values are a business list (environments, tenants, tiers) or a curated subset rather than derivable from the data, declare filters: [{ type: "STATIC_LIST", name, options: ["prod", "staging"], variableName }]. It takes no expression, sourceId, or where, and it is always variable-only (there is nothing to broadcast), so tiles must reference $variableName, typically via $__filter(, $) with the column passed explicitly (the one-argument $__filter($var) form fails because the filter has no expression of its own). +9e. MAKE A FILTER REQUIRED ONLY WHEN AN UNSCOPED VIEW IS MEANINGLESS. minSelections: 1 blocks the tiles that read the filter - the ones referencing its $variableName, and the ones its broadcast applies to - until the user picks a value. isGlobalRequirement: true widens that to every tile on the dashboard. Consider pairing either form with savedFilterValues so the dashboard opens on a sensible default rather than blocked. + 10. UPDATE IS REPLACE, NOT MERGE. clickstack_save_dashboard with an id overwrites tiles, containers, and filters in their entirety. Call clickstack_get_dashboard first when you only want to add or rename one entry; do not send a partial set or you will silently drop everything you omitted. 11. GROUP RELATED TILES INTO CONTAINERS. REQUIRED at five or more tiles, no exceptions. An ungrouped wall of nine or ten tiles is a readability failure even when each tile is correct in isolation. Containers are the right way to introduce structure; markdown tiles for section labels are not. diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index 2c123c9ef3..a944ff5fff 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -1225,6 +1225,28 @@ const mcpDashboardFilterBaseShape = { 'Human-readable filter label shown in the dashboard filter bar dropdown.', ), variableName: variableNameSchema.describe(VARIABLE_NAME_DESCRIPTION), + minSelections: z + .number() + .int() + .min(0) + .max(1) + .optional() + .describe( + 'Set to 1 to make this filter REQUIRED. Until the user picks at least one ' + + 'value for it, the tiles that read this filter refuse to load and show a ' + + '"missing required filters" message instead. By default, tiles that ' + + 'reference this filter by its variable or receive a broadcasted value ' + + 'from this filter will be blocked. isGlobalRequirement widens the block ' + + 'to all tiles.Only 0 and 1 are accepted; omit the field (or pass 0) for ' + + 'the normal optional behavior, which is usually what you want.', + ), + isGlobalRequirement: z + .boolean() + .optional() + .describe( + 'Widens a REQUIRED filter (minSelections: 1) to block EVERY tile on the dashboard, ' + + 'not just the ones that read it. Ignored unless minSelections is 1.', + ), }; const mcpQueryExpressionFilterSchema = z diff --git a/packages/api/src/routers/api/__tests__/dashboard.int.test.ts b/packages/api/src/routers/api/__tests__/dashboard.int.test.ts index 88d985856d..12c9647a75 100644 --- a/packages/api/src/routers/api/__tests__/dashboard.int.test.ts +++ b/packages/api/src/routers/api/__tests__/dashboard.int.test.ts @@ -1100,6 +1100,116 @@ describe('dashboard router', () => { }); }); + describe('required filters', () => { + const makeFilter = (overrides = {}) => ({ + id: new Types.ObjectId().toString(), + type: 'QUERY_EXPRESSION' as const, + name: 'Service Name', + expression: 'ServiceName', + source: new Types.ObjectId().toString(), + ...overrides, + }); + + it('persists minSelections on create', async () => { + const filter = makeFilter({ minSelections: 1 }); + + const created = await agent + .post('/dashboards') + .send({ ...MOCK_DASHBOARD, filters: [filter] }) + .expect(200); + + expect(created.body.filters).toEqual([filter]); + + const stored = await Dashboard.findById(created.body.id).lean(); + expect(stored?.filters).toEqual([filter]); + }); + + it('persists minSelections on patch', async () => { + const created = await agent + .post('/dashboards') + .send({ ...MOCK_DASHBOARD, filters: [makeFilter()] }) + .expect(200); + + const filter = makeFilter({ + id: created.body.filters[0].id, + minSelections: 1, + }); + await agent + .patch(`/dashboards/${created.body.id}`) + .send({ filters: [filter] }) + .expect(200); + + const stored = await Dashboard.findById(created.body.id).lean(); + expect(stored?.filters?.[0].minSelections).toBe(1); + }); + + // Absence is meaningful: the client reads a missing value as not required, + // so the server must not materialize one. + it('leaves minSelections absent when it is not sent', async () => { + const created = await agent + .post('/dashboards') + .send({ ...MOCK_DASHBOARD, filters: [makeFilter()] }) + .expect(200); + + const stored = await Dashboard.findById(created.body.id).lean(); + expect(stored?.filters?.[0]).not.toHaveProperty('minSelections'); + }); + + it.each([2, -1, 1.5, '1'])('rejects minSelections %s', async value => { + await agent + .post('/dashboards') + .send({ + ...MOCK_DASHBOARD, + filters: [makeFilter({ minSelections: value })], + }) + .expect(400); + }); + + it('persists a dashboard-wide requirement', async () => { + const filter = makeFilter({ + minSelections: 1, + isGlobalRequirement: true, + }); + + const created = await agent + .post('/dashboards') + .send({ ...MOCK_DASHBOARD, filters: [filter] }) + .expect(200); + + expect(created.body.filters).toEqual([filter]); + + const stored = await Dashboard.findById(created.body.id).lean(); + expect(stored?.filters).toEqual([filter]); + }); + + // Absence is what scopes the block to the filter's own tiles, so the server + // must not materialize the flag. + it('leaves the scope absent when it is not sent', async () => { + const created = await agent + .post('/dashboards') + .send({ + ...MOCK_DASHBOARD, + filters: [makeFilter({ minSelections: 1 })], + }) + .expect(200); + + const stored = await Dashboard.findById(created.body.id).lean(); + expect(stored?.filters?.[0]).not.toHaveProperty('isGlobalRequirement'); + }); + + it('rejects a non-boolean scope', async () => { + await agent + .post('/dashboards') + .send({ + ...MOCK_DASHBOARD, + filters: [ + makeFilter({ minSelections: 1, isGlobalRequirement: 'true' }), + ], + }) + .expect(400); + }); + }); + describe('static-list filters', () => { const makeStaticFilter = (overrides = {}) => ({ id: new Types.ObjectId().toString(), diff --git a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts index d429ecdfd7..8c322ddb9b 100644 --- a/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/dashboards.int.test.ts @@ -5329,6 +5329,29 @@ describe('External API v2 Dashboards - new format', () => { ...overrides, }); + const staticFilterInput = (overrides = {}) => ({ + id: new ObjectId().toString(), + type: 'STATIC_LIST' as const, + name: 'Environment', + options: ['prod', 'staging', 'dev'], + isBroadcastEnabled: false, + isVariableEnabled: true, + variableName: 'env', + ...overrides, + }); + + const promqlFilterInput = (overrides = {}) => ({ + id: new ObjectId().toString(), + type: 'PROMETHEUS_LABEL' as const, + name: 'Pod', + sourceId: promqlSource._id.toString(), + label: 'pod', + isBroadcastEnabled: false, + isVariableEnabled: true, + variableName: 'pod', + ...overrides, + }); + const sendFilters = async (filters: Record[]) => { const payload = createMockDashboardWithIds( traceSource._id.toString(), @@ -5566,18 +5589,108 @@ describe('External API v2 Dashboards - new format', () => { }); }); - describe('static-list filters', () => { - const staticFilterInput = (overrides = {}) => ({ - id: new ObjectId().toString(), - type: 'STATIC_LIST' as const, - name: 'Environment', - options: ['prod', 'staging', 'dev'], - isBroadcastEnabled: false, - isVariableEnabled: true, - variableName: 'env', - ...overrides, + describe('required filters', () => { + // Named factories rather than a keyed map, so the label `it.each` prints + // does not have to come from a lookup evaluated at collection time + // (`promqlFilterInput` reads a source created in `beforeEach`). + const variants = [ + ['QUERY_EXPRESSION', filterInput], + ['STATIC_LIST', staticFilterInput], + ['PROMETHEUS_LABEL', promqlFilterInput], + ] as const; + + it.each(variants)( + 'accepts a required %s filter', + async (_type, makeFilter) => { + const response = await sendFilters([ + makeFilter({ minSelections: 1 }), + ]); + expect(response.status).toBe(200); + expect(response.body.data.filters[0].minSelections).toBe(1); + }, + ); + + // Not required is the absence of the field, so a GET response can be PUT + // back verbatim - the same policy `variableName` already follows. + it.each([undefined, 0])( + 'omits the key from responses when minSelections is %s', + async minSelections => { + const response = await sendFilters([filterInput({ minSelections })]); + expect(response.status).toBe(200); + expect(response.body.data.filters[0]).not.toHaveProperty( + 'minSelections', + ); + }, + ); + + it.each([2, -1, 1.5, '1'])('rejects minSelections %s', async value => { + await expectFilters([filterInput({ minSelections: value })], 400); + }); + + it('round-trips a required filter through GET and back', async () => { + const response = await sendFilters([filterInput({ minSelections: 1 })]); + expect(response.status).toBe(200); + + const dashboard = await authRequest( + 'get', + `${BASE_URL}/${response.body.data.id}`, + ).expect(200); + expect(dashboard.body.data.filters[0].minSelections).toBe(1); + + await authRequest('put', `${BASE_URL}/${response.body.data.id}`) + .send(omit(dashboard.body.data, 'id')) + .expect(200); + }); + + it.each(variants)( + 'accepts a dashboard-wide requirement on a %s filter', + async (_type, makeFilter) => { + const response = await sendFilters([ + makeFilter({ minSelections: 1, isGlobalRequirement: true }), + ]); + expect(response.status).toBe(200); + expect(response.body.data.filters[0].isGlobalRequirement).toBe(true); + }, + ); + + // Same policy as minSelections: the scope means nothing without a + // requirement, so it never appears on an optional filter's response. + it('omits the scope from responses when the filter is not required', async () => { + const response = await sendFilters([ + filterInput({ isGlobalRequirement: true }), + ]); + expect(response.status).toBe(200); + expect(response.body.data.filters[0]).not.toHaveProperty( + 'isGlobalRequirement', + ); + }); + + it('rejects a non-boolean scope', async () => { + await expectFilters( + [filterInput({ minSelections: 1, isGlobalRequirement: 'true' })], + 400, + ); + }); + + it('round-trips a dashboard-wide requirement through GET and back', async () => { + const response = await sendFilters([ + filterInput({ minSelections: 1, isGlobalRequirement: true }), + ]); + expect(response.status).toBe(200); + + const dashboard = await authRequest( + 'get', + `${BASE_URL}/${response.body.data.id}`, + ).expect(200); + expect(dashboard.body.data.filters[0].isGlobalRequirement).toBe(true); + + await authRequest('put', `${BASE_URL}/${response.body.data.id}`) + .send(omit(dashboard.body.data, 'id')) + .expect(200); }); + }); + describe('static-list filters', () => { it('accepts a static-list filter and round-trips it through GET', async () => { const response = await sendFilters([staticFilterInput()]); expect(response.status).toBe(200); @@ -5704,18 +5817,6 @@ describe('External API v2 Dashboards - new format', () => { }); describe('promql-label filters', () => { - const promqlFilterInput = (overrides = {}) => ({ - id: new ObjectId().toString(), - type: 'PROMETHEUS_LABEL' as const, - name: 'Pod', - sourceId: promqlSource._id.toString(), - label: 'pod', - isBroadcastEnabled: false, - isVariableEnabled: true, - variableName: 'pod', - ...overrides, - }); - it('accepts a promql-label filter and round-trips it through GET', async () => { const response = await sendFilters([promqlFilterInput()]); expect(response.status).toBe(200); diff --git a/packages/api/src/routers/external-api/v2/dashboards.ts b/packages/api/src/routers/external-api/v2/dashboards.ts index a262d2d9de..80d3b1c225 100644 --- a/packages/api/src/routers/external-api/v2/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/dashboards.ts @@ -1764,7 +1764,10 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * A drop-down filter on a dashboard. Depending on type, filters can either broadcast * selected value(s) to every tile (isBroadcastEnabled), expose selected * values as a variable (isVariableEnabled) or both. Typically a filter should - * do just one of these things. + * do just one of these things. Any filter may additionally be marked + * required via minSelections, which blocks the tiles that read it until it + * has a selected value - or every tile on the dashboard, with + * isGlobalRequirement. * oneOf: * - $ref: '#/components/schemas/QueryExpressionFilterInput' * - $ref: '#/components/schemas/StaticListFilterInput' @@ -1867,6 +1870,24 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * only, so the field is rejected when isVariableEnabled is not true, and * is omitted from responses for such a filter. * example: "environment" + * minSelections: + * type: integer + * enum: [0, 1] + * default: 0 + * description: | + * Minimum number of values that must be selected before tiles load. + * Set to 1 to make the filter required. Only 0 and 1 are accepted. + * Omit the field (or send 0) for the default optional behavior. + * example: 1 + * isGlobalRequirement: + * type: boolean + * default: false + * description: | + * Widens a required filter's block to every tile on the dashboard. False + * (the default) blocks only the tiles that read the filter: those + * referencing its variableName, and those its broadcast applies to. + * Ignored unless minSelections is 1. + * example: true * * StaticListFilterInput: * type: object @@ -1935,6 +1956,24 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * only, so the field is rejected when isVariableEnabled is not true, and * is omitted from responses for such a filter. * example: "environment" + * minSelections: + * type: integer + * enum: [0, 1] + * default: 0 + * description: | + * Minimum number of values that must be selected before tiles load. + * Set to 1 to make the filter required. Only 0 and 1 are accepted. + * Omit the field (or send 0) for the default optional behavior. + * example: 1 + * isGlobalRequirement: + * type: boolean + * default: false + * description: | + * Widens a required filter's block to every tile on the dashboard. False + * (the default) blocks only the tiles that read the filter: those + * referencing its variableName, and those its broadcast applies to. + * Ignored unless minSelections is 1. + * example: true * * PrometheusLabelFilterInput: * type: object @@ -2004,6 +2043,24 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * removed. Variable names must be unique across a dashboard's * variable-enabled filters. * example: "pod" + * minSelections: + * type: integer + * enum: [0, 1] + * default: 0 + * description: | + * Minimum number of values that must be selected before tiles load. + * Set to 1 to make the filter required. Only 0 and 1 are accepted. + * Omit the field (or send 0) for the default optional behavior. + * example: 1 + * isGlobalRequirement: + * type: boolean + * default: false + * description: | + * Widens a required filter's block to every tile on the dashboard. False + * (the default) blocks only the tiles that read the filter: those + * referencing its variableName, and those its broadcast applies to. + * Ignored unless minSelections is 1. + * example: true * * Filter: * allOf: diff --git a/packages/api/src/utils/externalApi.ts b/packages/api/src/utils/externalApi.ts index 2f2786fdbb..79de4ec472 100644 --- a/packages/api/src/utils/externalApi.ts +++ b/packages/api/src/utils/externalApi.ts @@ -1,5 +1,6 @@ import { isFilterBroadcastEnabled, + isFilterRequired, isFilterVariableEnabled, } from '@hyperdx/common-utils/dist/filters'; import { @@ -226,19 +227,26 @@ export function translateExternalChartToTileConfig( export function translateFilterToExternalFilter( filter: DashboardFilter, ): ExternalDashboardFilterWithId { + // Omitted from responses unless the filter is required, so a GET response + // can be PUT back verbatim. + const omittedKeys = isFilterRequired(filter) + ? [] + : (['minSelections', 'isGlobalRequirement'] as const); + switch (filter.type) { case 'STATIC_LIST': - return filter; + return omit(filter, ...omittedKeys); case 'PROMETHEUS_LABEL': return { - ...omit(filter, 'source'), + ...omit(filter, 'source', ...omittedKeys), sourceId: filter.source.toString(), }; case 'QUERY_EXPRESSION': { // Ignore variableName and appliesToSourceIds if the filter is not in a mode that uses them const ignoredKeys = [ + ...omittedKeys, ...(isFilterVariableEnabled(filter) ? [] : (['variableName'] as const)), ...(isFilterBroadcastEnabled(filter) ? [] diff --git a/packages/app/eslint.config.mjs b/packages/app/eslint.config.mjs index ee6aad4801..dc9b5188bb 100644 --- a/packages/app/eslint.config.mjs +++ b/packages/app/eslint.config.mjs @@ -272,6 +272,8 @@ export default [ assertFunctionNames: [ 'expectFieldSuggestion', 'expectValueSuggestion', + 'expectTileBlockedOn', + 'expectTileLoaded', ], }, ], diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index de30e23d4f..b60d4890c4 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -34,6 +34,7 @@ import { Granularity, isTimeSeriesDisplayType, } from '@hyperdx/common-utils/dist/core/utils'; +import { getBlockingRequiredFilters } from '@hyperdx/common-utils/dist/dashboardFilterValues'; import { displayTypeRequiresSource, isBuilderChartConfig, @@ -382,6 +383,36 @@ const whereLanguageParser = parseAsString.withDefault( typeof window !== 'undefined' ? (getStoredLanguage() ?? 'lucene') : 'lucene', ); +/** + * A tile that cannot draw its chart yet, explaining why in its place. Keeps the + * chrome identical to a rendered tile so the toolbar stays usable. + */ +const TilePlaceholder = ({ + title, + toolbarItems, + children, + 'data-testid': dataTestId, +}: { + title: React.ReactNode; + toolbarItems?: React.ReactNode[]; + children: React.ReactNode; + 'data-testid'?: string; +}) => ( + + + + {children} + + + +); + const Tile = ({ chart, dateRange, @@ -395,6 +426,7 @@ const Tile = ({ onTimeRangeSelect, filters, variables, + unsatisfiedRequiredFilters, showAlertAnnotations, showReleaseAnnotations, isLive, @@ -426,6 +458,8 @@ const Tile = ({ onTimeRangeSelect: (start: Date, end: Date) => void; filters?: Filter[]; variables?: ChartVariable[]; + /** The dashboard's required filters that have nothing selected. */ + unsatisfiedRequiredFilters?: DashboardFilter[]; // When true, draw alert firing/recovery annotations on this tile's chart. showAlertAnnotations?: boolean; // When true, draw release markers on this tile's chart. @@ -560,6 +594,49 @@ const Tile = ({ [serializedTileVariables], ); + // Whether a broadcast filter reaches this tile's query at all. PromQL tiles + // are handed no filters, and a raw-SQL tile drops them without a source or + // without a macro to apply them. + const consumesBroadcastFilters = useMemo(() => { + if (isPromqlSavedChartConfig(chart.config)) return false; + if (isRawSqlSavedChartConfig(chart.config)) { + return ( + !!chart.config.source && + !isMissingFiltersMacro(chart.config.sqlTemplate) + ); + } + return true; + }, [chart.config]); + + // Serialized for the same reason as `tileVariables`: any change to the + // dashboard's filters hands this tile a new array, and only a change to the + // names this tile is blocked on should churn the render memo below. + const serializedMissingRequiredFilterNames = useMemo( + () => + JSON.stringify( + getBlockingRequiredFilters(unsatisfiedRequiredFilters ?? [], { + sourceId: chart.config.source, + referencedVariableNames: tileVariables?.map( + variable => variable.name, + ), + consumesBroadcastFilters, + }).map(filter => filter.name), + ), + [ + unsatisfiedRequiredFilters, + chart.config.source, + tileVariables, + consumesBroadcastFilters, + ], + ); + const missingRequiredFilterNames = useMemo( + () => JSON.parse(serializedMissingRequiredFilterNames), + [serializedMissingRequiredFilterNames], + ); + const isBlockedByRequiredFilters = + missingRequiredFilterNames.length > 0 && + displayTypeRequiresSource(chart.config.displayType); + useEffect(() => { if (isPromqlSavedChartConfig(chart.config)) { if (source != null) { @@ -692,7 +769,9 @@ const Tile = ({ const alertAnnotations = useAlertAnnotations( alert?.id, isFullscreen ? fullscreenDateRange : dateRange, - showAlertAnnotations && tileCanDrawAnnotations, + showAlertAnnotations && + tileCanDrawAnnotations && + !isBlockedByRequiredFilters, ); // Release markers, over the same visible window. Scoped to this tile: the @@ -709,7 +788,9 @@ const Tile = ({ : undefined; const releaseAnnotations = useReleaseAnnotations( isFullscreen ? fullscreenDateRange : dateRange, - showReleaseAnnotations && tileCanDrawAnnotations, + showReleaseAnnotations && + tileCanDrawAnnotations && + !isBlockedByRequiredFilters, { source, where: builderConfig?.where, @@ -1147,7 +1228,11 @@ const Tile = ({ // The fullscreen view is always visible, so it should always load. // In the tile (grid) view, gate data fetching on viewport visibility. - const chartEnabled = isFullscreenView ? true : hasBeenVisible; + const chartEnabled = isBlockedByRequiredFilters + ? false + : isFullscreenView + ? true + : hasBeenVisible; // Use the fullscreen-local date range and granularity when rendering // inside the fullscreen modal so that changing them does not affect @@ -1178,24 +1263,26 @@ const Tile = ({ } > - {isSourceMissing ? ( - - - - The data source for this tile no longer exists. Edit the tile - to select a new source. - - - + {isBlockedByRequiredFilters ? ( + + {`Missing required filters: ${missingRequiredFilterNames.join( + ', ', + )}. Select a value for each to load this tile.`} + + ) : isSourceMissing ? ( + + The data source for this tile no longer exists. Edit the tile to + select a new source. + ) : isSourceUnset ? ( - - - - The data source for this tile is not set. Edit the tile to - select a data source. - - - + + The data source for this tile is not set. Edit the tile to select + a data source. + ) : ( <> {(effectiveQueriedConfig?.displayType === DisplayType.Line || @@ -1420,6 +1507,8 @@ const Tile = ({ filterWarning, isSourceMissing, isSourceUnset, + isBlockedByRequiredFilters, + missingRequiredFilterNames, hasBeenVisible, annotations, isLive, @@ -1856,6 +1945,7 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { ignoredVariableNames, getFilterQueriesForSource, variables, + unsatisfiedRequiredFilters, } = useDashboardFilters(filters); const dashboardReady = @@ -2305,6 +2395,7 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { ...getFilterQueriesForSource(tileSourceId), ]} variables={variables} + unsatisfiedRequiredFilters={unsatisfiedRequiredFilters} onTimeRangeSelect={onTimeRangeSelect} showAlertAnnotations={showAlertAnnotations} showReleaseAnnotations={showReleaseAnnotations} @@ -2408,6 +2499,7 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { showReleaseAnnotations, getFilterQueriesForSource, variables, + unsatisfiedRequiredFilters, moveTargetContainers, handleMoveTileToGroup, selectedTileIds, @@ -3362,6 +3454,7 @@ function DBDashboardPage({ presetConfig }: { presetConfig?: Dashboard }) { onRemoveFilter={handleRemoveFilter} isLoading={isSavingDashboard || isFetchingDashboard} showVariableOptions + showRequiredFilterOptions variables={variables} /> )} diff --git a/packages/app/src/DashboardFilters.tsx b/packages/app/src/DashboardFilters.tsx index e3a4966b12..1eb89262bf 100644 --- a/packages/app/src/DashboardFilters.tsx +++ b/packages/app/src/DashboardFilters.tsx @@ -4,6 +4,8 @@ import { getFilterBroadcastTarget, getFilterVariableName, getPendingFilterValuesVariables, + isFilterGlobalRequirement, + isFilterRequired, isFilterVariableEnabled, isQueryExpressionFilter, isStaticListFilter, @@ -49,6 +51,15 @@ export const getPendingVariablesTooltip = ( } no selected value.`; }; +/** + * Explain a required filter that has nothing selected, naming how far the block + * it imposes reaches. + */ +export const getRequiredFilterTooltip = (filter: DashboardFilter): string => + isFilterGlobalRequirement(filter) + ? 'Required filter. No tile on this dashboard loads until it has a selection.' + : 'Required filter. Tiles that use this filter do not load until it has a selection.'; + /** * Describe what a filter does with the value you pick: broadcast it as a * condition, expose it as a variable, both, or neither. @@ -87,6 +98,25 @@ export const getFilterEffect = ( return { hasEffect: true, tooltip: parts.join(', ') }; }; +/** One of the caution icons a filter's label row can carry, with its tooltip. */ +const FilterCaution = ({ + label, + testId, + variant = 'warning', +}: { + label: string; + testId: string; + variant?: 'warning' | 'danger'; +}) => ( + + + +); + const DashboardFilterSelect = ({ filter, onChange, @@ -99,6 +129,7 @@ const DashboardFilterSelect = ({ }: DashboardFilterSelectProps) => { const valuesOrEmptyMemo = useMemo(() => values ?? [], [values]); const effect = getFilterEffect(filter); + const isMissingRequiredValue = isFilterRequired(filter) && value.length === 0; return ( @@ -121,36 +152,27 @@ const DashboardFilterSelect = ({ /> )} + {isMissingRequiredValue && ( + + )} {!!pendingVariables?.length && ( - - - + testId={`dashboard-filter-pending-variable-${filter.name}`} + /> )} {isError && ( - - - + testId={`dashboard-filter-error-${filter.name}`} + variant="danger" + /> )}
diff --git a/packages/app/src/ServicesDashboardPage/ServicesDashboardPage.tsx b/packages/app/src/ServicesDashboardPage/ServicesDashboardPage.tsx index b1f13b5bf7..b889b2e341 100644 --- a/packages/app/src/ServicesDashboardPage/ServicesDashboardPage.tsx +++ b/packages/app/src/ServicesDashboardPage/ServicesDashboardPage.tsx @@ -460,6 +460,7 @@ function ServicesDashboardPage() { source={source} isLoading={isFetchingFilters || isFiltersMutationPending} showVariableOptions={false} + showRequiredFilterOptions={false} /> ); diff --git a/packages/app/src/__tests__/DashboardFilters.test.ts b/packages/app/src/__tests__/DashboardFilters.test.ts index 1284060dbd..d063d63a9c 100644 --- a/packages/app/src/__tests__/DashboardFilters.test.ts +++ b/packages/app/src/__tests__/DashboardFilters.test.ts @@ -3,6 +3,7 @@ import { DashboardFilter } from '@hyperdx/common-utils/dist/types'; import { getFilterEffect, getPendingVariablesTooltip, + getRequiredFilterTooltip, } from '@/DashboardFilters'; const baseFilter: DashboardFilter = { @@ -131,3 +132,27 @@ describe('getPendingVariablesTooltip', () => { ); }); }); + +describe('getRequiredFilterTooltip', () => { + it('warns only about the tiles that use the filter by default', () => { + for (const isGlobalRequirement of [undefined, false]) { + expect( + getRequiredFilterTooltip({ + ...baseFilter, + minSelections: 1, + isGlobalRequirement, + }), + ).toContain('Tiles that use this filter do not load'); + } + }); + + it('warns that the whole dashboard is held back by a global requirement', () => { + expect( + getRequiredFilterTooltip({ + ...baseFilter, + minSelections: 1, + isGlobalRequirement: true, + }), + ).toContain('No tile on this dashboard loads'); + }); +}); diff --git a/packages/app/src/components/DashboardFiltersModal/DashboardFilterEditForm.tsx b/packages/app/src/components/DashboardFiltersModal/DashboardFilterEditForm.tsx index eebae6d5fe..eb97db3f19 100644 --- a/packages/app/src/components/DashboardFiltersModal/DashboardFilterEditForm.tsx +++ b/packages/app/src/components/DashboardFiltersModal/DashboardFilterEditForm.tsx @@ -10,10 +10,20 @@ import { DashboardFilterType, TSource, } from '@hyperdx/common-utils/dist/types'; -import { Alert, Button, Group, Modal, Stack, TextInput } from '@mantine/core'; +import { + Alert, + Box, + Button, + Divider, + Group, + Modal, + Stack, + TextInput, +} from '@mantine/core'; import { IconAlertTriangle } from '@tabler/icons-react'; import { ErrorBoundary } from '@/components/Error/ErrorBoundary'; +import { CheckBoxControlled } from '@/components/InputControlled'; import SelectControlled from '@/components/SelectControlled'; import { SqlVariablesProvider } from '@/components/SQLEditor/variableCompletions'; import { IS_PROMQL_ENABLED } from '@/config'; @@ -56,6 +66,8 @@ interface DashboardFilterEditFormProps { source?: TSource; /** Whether the broadcast / variable controls are available. */ showVariableOptions: boolean; + /** Whether to show the options for marking a filter as required. */ + showRequiredFilterOptions: boolean; /** The dashboard's current variable state, if any */ variables?: ChartVariable[]; onSave: (filter: DashboardFilter) => void; @@ -74,6 +86,7 @@ export const DashboardFilterEditForm = ({ source: presetSource, filters, showVariableOptions, + showRequiredFilterOptions, variables, onSave, onClose, @@ -131,9 +144,9 @@ export const DashboardFilterEditForm = ({ const hasEditedVariableName = !!filter?.variableName || !!formState.dirtyFields.variableName; - const [formFilterType, formFilterName] = useWatch({ + const [formFilterType, formFilterName, isRequired] = useWatch({ control, - name: ['type', 'name'], + name: ['type', 'name', 'isRequired'], }); const derivedVariableName = deriveVariableName(formFilterName ?? ''); @@ -239,6 +252,32 @@ export const DashboardFilterEditForm = ({ )} + {showRequiredFilterOptions && ( + + + + {!!isRequired && ( + + + + )} + + )} + {formState.errors.root && ( , + tooltip: isGlobal + ? 'Every tile is blocked until this filter has a selection' + : 'The tiles that use this filter are blocked until it has a selection', + label: 'Required', + testId: `dashboard-filter-required-attr-${filter.name}`, + }); + } + const variableName = showVariableOptions && isFilterVariableEnabled(filter) ? getFilterVariableName(filter) diff --git a/packages/app/src/components/DashboardFiltersModal/__tests__/DashboardFiltersModal.test.tsx b/packages/app/src/components/DashboardFiltersModal/__tests__/DashboardFiltersModal.test.tsx index 8691aac91b..1f9d5e5baa 100644 --- a/packages/app/src/components/DashboardFiltersModal/__tests__/DashboardFiltersModal.test.tsx +++ b/packages/app/src/components/DashboardFiltersModal/__tests__/DashboardFiltersModal.test.tsx @@ -70,6 +70,7 @@ const renderModal = ( opened filters={[]} showVariableOptions + showRequiredFilterOptions onClose={jest.fn()} onSaveFilter={onSaveFilter} onRemoveFilter={jest.fn()} @@ -321,4 +322,130 @@ describe('DashboardFiltersModal', () => { expect(screen.queryByTestId('filter-type-picker')).toBeNull(); }); + + it.each(['Queried values', 'Static values'] as const)( + 'offers the required control on a %s filter', + async label => { + const { user } = renderModal(); + + await user.click(screen.getByTestId('add-filter-button')); + await selectFilterType(user, label); + + expect( + screen.getByTestId('filter-required-checkbox'), + ).toBeInTheDocument(); + }, + ); + + // A dashboard that draws its tiles without consulting the filters enforces + // nothing, so offering the control there would promise something that never + // happens. + it('hides the required control where a requirement is not enforced', async () => { + const { user } = renderModal({ showRequiredFilterOptions: false }); + + await user.click(screen.getByTestId('add-filter-button')); + + expect(screen.queryByTestId('filter-required-checkbox')).toBeNull(); + }); + + // Requiring a value and publishing a variable are unrelated: a filter that + // only broadcasts can still be required. + it('offers the required control where variables are unavailable', async () => { + const { user } = renderModal({ showVariableOptions: false }); + + await user.click(screen.getByTestId('add-filter-button')); + + expect(screen.getByTestId('filter-required-checkbox')).toBeInTheDocument(); + }); + + it('saves a required filter and reopens it checked', async () => { + const { onSaveFilter, user } = renderModal(); + + await user.click(screen.getByTestId('add-filter-button')); + await selectFilterType(user, 'Static values'); + await user.type(screen.getByTestId('filter-name-input'), 'Environment'); + await user.type(screen.getByTestId('filter-options-input'), 'prod{Enter}'); + await user.click(screen.getByTestId('filter-required-checkbox')); + await user.click(screen.getByTestId('save-filter-button')); + + await waitFor(() => expect(onSaveFilter).toHaveBeenCalledTimes(1)); + const saved: DashboardFilter = onSaveFilter.mock.calls[0][0]; + expect(saved.minSelections).toBe(1); + expect(saved.isGlobalRequirement).toBeUndefined(); + + renderModal({ filters: [saved] }); + await user.click( + screen.getAllByTestId(`edit-filter-button-${saved.name}`)[0], + ); + + expect(screen.getByTestId('filter-required-checkbox')).toBeChecked(); + }); + + it('offers the requirement scope, unchecked, once the filter is required', async () => { + const { user } = renderModal(); + + await user.click(screen.getByTestId('add-filter-button')); + await selectFilterType(user, 'Static values'); + + expect( + screen.queryByTestId('filter-global-requirement-checkbox'), + ).toBeNull(); + + await user.click(screen.getByTestId('filter-required-checkbox')); + + expect( + screen.getByTestId('filter-global-requirement-checkbox'), + ).not.toBeChecked(); + }); + + it('saves a dashboard-wide requirement and reopens it checked', async () => { + const { onSaveFilter, user } = renderModal(); + + await user.click(screen.getByTestId('add-filter-button')); + await selectFilterType(user, 'Static values'); + await user.type(screen.getByTestId('filter-name-input'), 'Environment'); + await user.type(screen.getByTestId('filter-options-input'), 'prod{Enter}'); + await user.click(screen.getByTestId('filter-required-checkbox')); + await user.click(screen.getByTestId('filter-global-requirement-checkbox')); + await user.click(screen.getByTestId('save-filter-button')); + + await waitFor(() => expect(onSaveFilter).toHaveBeenCalledTimes(1)); + const saved: DashboardFilter = onSaveFilter.mock.calls[0][0]; + expect(saved.minSelections).toBe(1); + expect(saved.isGlobalRequirement).toBe(true); + + renderModal({ filters: [saved] }); + await user.click( + screen.getAllByTestId(`edit-filter-button-${saved.name}`)[0], + ); + + expect( + screen.getByTestId('filter-global-requirement-checkbox'), + ).toBeChecked(); + }); + + it('marks a required filter in the list, whatever its reach', () => { + renderModal({ + filters: [ + { ...EXISTING_STATIC_FILTER, minSelections: 1 }, + { ...EXISTING_FILTER, minSelections: 1, isGlobalRequirement: true }, + ], + }); + + for (const { name } of [EXISTING_STATIC_FILTER, EXISTING_FILTER]) { + expect( + screen.getByTestId(`dashboard-filter-required-attr-${name}`), + ).toHaveTextContent(/^Required$/); + } + }); + + it('leaves an optional filter unmarked', () => { + renderModal({ filters: [EXISTING_STATIC_FILTER] }); + + expect( + screen.queryByTestId( + `dashboard-filter-required-attr-${EXISTING_STATIC_FILTER.name}`, + ), + ).toBeNull(); + }); }); diff --git a/packages/app/src/components/DashboardFiltersModal/__tests__/filterFormState.test.ts b/packages/app/src/components/DashboardFiltersModal/__tests__/filterFormState.test.ts index 5eea4eb94e..718c0e6b4e 100644 --- a/packages/app/src/components/DashboardFiltersModal/__tests__/filterFormState.test.ts +++ b/packages/app/src/components/DashboardFiltersModal/__tests__/filterFormState.test.ts @@ -1,3 +1,5 @@ +import { DashboardFilter } from '@hyperdx/common-utils/dist/types'; + import { FilterFormValues, toFormValues, @@ -190,3 +192,109 @@ describe('toSavedFilter', () => { ).toThrow(); }); }); + +/** The minimum a form needs to save, per filter type. */ +const byType: Record> = { + QUERY_EXPRESSION: { + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + source: 'logs', + }, + STATIC_LIST: { + type: 'STATIC_LIST', + name: 'Environment', + options: ['prod'], + }, + PROMETHEUS_LABEL: { + type: 'PROMETHEUS_LABEL', + name: 'Job', + source: 'prom', + label: 'job', + }, +}; +const variants = Object.entries(byType); + +describe('minSelections', () => { + it.each(variants)( + 'reads a stored minimum back onto the %s form', + (_type, filter) => { + const saved = toSavedFilter(formValues({ ...filter, isRequired: true })); + + expect(saved.minSelections).toBe(1); + expect(toFormValues(saved).isRequired).toBe(true); + }, + ); + + // Absent rather than 0, so an optional filter's API payload is unchanged. + it.each(variants)( + 'emits no key at all for an optional %s filter', + (_type, filter) => { + const saved = toSavedFilter(formValues({ ...filter, isRequired: false })); + + expect(saved).not.toHaveProperty('minSelections'); + expect(toFormValues(saved).isRequired).toBe(false); + }, + ); + + it('never leaks the form-only isRequired field into the saved filter', () => { + expect( + toSavedFilter( + formValues({ ...byType.QUERY_EXPRESSION, isRequired: true }), + ), + ).not.toHaveProperty('isRequired'); + }); + + it('defaults a new filter to optional', () => { + expect(toFormValues().isRequired).toBe(false); + }); +}); + +describe('isGlobalRequirement', () => { + it('defaults a new filter to blocking only the tiles that use it', () => { + expect(toFormValues().isGlobalRequirement).toBe(false); + }); + + it.each(variants)( + 'stores a dashboard-wide %s requirement and reads it back onto the form', + (_type, filter) => { + const saved = toSavedFilter( + formValues({ ...filter, isRequired: true, isGlobalRequirement: true }), + ); + + expect(saved.isGlobalRequirement).toBe(true); + expect(toFormValues(saved).isGlobalRequirement).toBe(true); + }, + ); + + // Absent rather than false, so a required filter's payload carries only the + // field that turns the requirement on. + it.each(variants)( + 'emits no key for a %s filter that blocks only its own tiles', + (_type, filter) => { + const saved = toSavedFilter( + formValues({ ...filter, isRequired: true, isGlobalRequirement: false }), + ); + + expect(saved).not.toHaveProperty('isGlobalRequirement'); + expect(toFormValues(saved).isGlobalRequirement).toBe(false); + }, + ); + + // The scope is meaningless without a requirement, so an unchecked "Required" + // must not leave the wider block behind on the stored filter. + it.each(variants)( + 'emits no key for an optional %s filter', + (_type, filter) => { + expect( + toSavedFilter( + formValues({ + ...filter, + isRequired: false, + isGlobalRequirement: true, + }), + ), + ).not.toHaveProperty('isGlobalRequirement'); + }, + ); +}); diff --git a/packages/app/src/components/DashboardFiltersModal/filterFormState.ts b/packages/app/src/components/DashboardFiltersModal/filterFormState.ts index 6b3c1bbc0d..d47610d385 100644 --- a/packages/app/src/components/DashboardFiltersModal/filterFormState.ts +++ b/packages/app/src/components/DashboardFiltersModal/filterFormState.ts @@ -2,6 +2,8 @@ import { Control } from 'react-hook-form'; import { getFilterVariableName, isFilterBroadcastEnabled, + isFilterGlobalRequirement, + isFilterRequired, isFilterVariableEnabled, } from '@hyperdx/common-utils/dist/filters'; import { @@ -21,14 +23,29 @@ import { getStoredLanguage } from '@/components/SearchInput/SearchWhereInput'; */ export type FilterFormValues = { type: DashboardFilter['type']; -} & Omit & + /** Boolean version of the persisted numeric `minSelections`. */ + isRequired: boolean; + /** Always set, unlike the stored field, whose default lives in `isFilterGlobalRequirement`. */ + isGlobalRequirement: boolean; +} & Omit< + QueryExpressionDashboardFilter, + 'type' | 'minSelections' | 'isGlobalRequirement' +> & Omit< StaticListDashboardFilter, - 'type' | 'isBroadcastEnabled' | 'isVariableEnabled' + | 'type' + | 'isBroadcastEnabled' + | 'isVariableEnabled' + | 'minSelections' + | 'isGlobalRequirement' > & Omit< PromqlLabelDashboardFilter, - 'type' | 'isBroadcastEnabled' | 'isVariableEnabled' + | 'type' + | 'isBroadcastEnabled' + | 'isVariableEnabled' + | 'minSelections' + | 'isGlobalRequirement' >; export type FilterFormControl = Control; @@ -48,6 +65,8 @@ export const toFormValues = ( variableName: filter?.variableName ?? '', isBroadcastEnabled: filter ? isFilterBroadcastEnabled(filter) : true, isVariableEnabled: filter ? isFilterVariableEnabled(filter) : false, + isRequired: filter ? isFilterRequired(filter) : false, + isGlobalRequirement: filter ? isFilterGlobalRequirement(filter) : false, // QUERY_EXPRESSION fields expression: queried?.expression ?? '', @@ -68,24 +87,35 @@ export const toFormValues = ( /** Normalizes the form values into the filter that gets stored. */ export const toSavedFilter = (values: FilterFormValues): DashboardFilter => { + // Pulled out of the spread below so neither key is stored unless it's relevant + const { isRequired, isGlobalRequirement, ...rest } = values; + const requirement = isRequired + ? { + minSelections: 1, + ...(isGlobalRequirement ? { isGlobalRequirement: true } : {}), + } + : {}; + if (values.type === 'STATIC_LIST') { return DashboardFilterSchema.parse({ - ...values, + ...rest, options: values.options.map(option => option.trim()), isBroadcastEnabled: false, isVariableEnabled: true, variableName: getFilterVariableName(values), + ...requirement, }); } if (values.type === 'PROMETHEUS_LABEL') { return DashboardFilterSchema.parse({ - ...values, + ...rest, label: values.label.trim(), match: values.match?.trim() || undefined, isBroadcastEnabled: false, isVariableEnabled: true, variableName: getFilterVariableName(values), + ...requirement, }); } @@ -94,12 +124,13 @@ export const toSavedFilter = (values: FilterFormValues): DashboardFilter => { const isVariableEnabled = isFilterVariableEnabled(values); return DashboardFilterSchema.parse({ - ...values, + ...rest, where: trimmedWhere || undefined, whereLanguage: trimmedWhere ? (values.whereLanguage ?? 'sql') : undefined, appliesToSourceIds: appliesTo?.length ? appliesTo : undefined, isBroadcastEnabled: isFilterBroadcastEnabled(values), isVariableEnabled, variableName: isVariableEnabled ? getFilterVariableName(values) : undefined, + ...requirement, }); }; diff --git a/packages/app/src/components/DashboardFiltersModal/index.tsx b/packages/app/src/components/DashboardFiltersModal/index.tsx index ae2566dee6..3f9ac4cc88 100644 --- a/packages/app/src/components/DashboardFiltersModal/index.tsx +++ b/packages/app/src/components/DashboardFiltersModal/index.tsx @@ -16,6 +16,8 @@ interface DashboardFiltersEditModalProps { source?: TSource; /** Whether to offer the broadcast / variable controls. */ showVariableOptions: boolean; + /** Whether to show the options for marking a filter as required. */ + showRequiredFilterOptions: boolean; /** The dashboard's variables, if any */ variables?: ChartVariable[]; onClose: () => void; @@ -29,6 +31,7 @@ const DashboardFiltersModal = ({ isLoading, source, showVariableOptions, + showRequiredFilterOptions, variables, onClose, onSaveFilter, @@ -69,6 +72,7 @@ const DashboardFiltersModal = ({ filters={filters} source={source} showVariableOptions={showVariableOptions} + showRequiredFilterOptions={showRequiredFilterOptions} variables={variables} onSave={handleSaveFilter} onCancel={stopEditing} diff --git a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx index 74ecc083cf..ec0d06fb6f 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx @@ -940,4 +940,91 @@ describe('useDashboardFilters', () => { ]); }); }); + + describe('unsatisfiedRequiredFilters', () => { + const required = ( + overrides: Partial = {}, + ): QueryExpressionDashboardFilter => ({ + ...mockFilters[0], + minSelections: 1, + ...overrides, + }); + + it('is empty when nothing is required', () => { + const { result } = renderHook(() => useDashboardFilters(mockFilters)); + + expect(result.current.unsatisfiedRequiredFilters).toEqual([]); + }); + + it('reports a required filter with no selection', () => { + const { result } = renderHook(() => + useDashboardFilters([required(), mockFilters[1]]), + ); + + expect(result.current.unsatisfiedRequiredFilters.map(f => f.id)).toEqual([ + 'filter1', + ]); + }); + + it('clears once an expression-keyed value arrives', () => { + const filters = [required()]; + const { result, rerender } = renderHook(() => + useDashboardFilters(filters), + ); + + act(() => { + result.current.setFilterValue('filter1', ['prod']); + }); + rerender(); + + expect(result.current.unsatisfiedRequiredFilters).toEqual([]); + }); + + it('clears once a variable-keyed value arrives', () => { + const filters = [ + required({ isVariableEnabled: true, variableName: 'env' }), + ]; + const { result, rerender } = renderHook(() => + useDashboardFilters(filters), + ); + + act(() => { + result.current.setFilterValue('filter1', ['prod']); + }); + rerender(); + + expect(mockState).toEqual([ + { type: 'variable', name: 'env', values: ['prod'] }, + ]); + expect(result.current.unsatisfiedRequiredFilters).toEqual([]); + }); + + // An exclusion narrows the data but chooses nothing, so it cannot satisfy + // "at least one value selected". + it('still reports a filter whose only value is an exclusion', () => { + mockState = [{ type: 'sql', condition: "environment NOT IN ('dev')" }]; + + const { result } = renderHook(() => useDashboardFilters([required()])); + + expect(selectionFor(result, 'filter1')?.excluded.size).toBe(1); + expect(result.current.unsatisfiedRequiredFilters.map(f => f.id)).toEqual([ + 'filter1', + ]); + }); + + it('reports every unsatisfied filter in declaration order', () => { + const { result } = renderHook(() => + useDashboardFilters([ + required({ id: 'filter1' }), + mockFilters[1], + required({ ...mockFilters[2], minSelections: 1 }), + ]), + ); + + expect(result.current.unsatisfiedRequiredFilters.map(f => f.id)).toEqual([ + 'filter1', + 'filter3', + ]); + }); + }); }); diff --git a/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx index a53d690e7a..2ccd430ffd 100644 --- a/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx @@ -118,6 +118,7 @@ describe('usePresetDashboardFilters', () => { ignoredVariableNames: [], getFilterQueriesForSource: jest.fn().mockReturnValue(mockFilterQueries), variables: [], + unsatisfiedRequiredFilters: [], }); }); diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index 7a176500ea..a91ed71180 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -3,16 +3,17 @@ import { useQueryState } from 'nuqs'; import { FilterSelection, filterSelectionKey, + getUnsatisfiedRequiredFilters, parseDashboardFilterValues, ParsedDashboardFilterValues, resolveFilterSelection, serializeDashboardFilterValues, } from '@hyperdx/common-utils/dist/dashboardFilterValues'; import { + doesFilterApplyToSource, FilterState, filtersToQuery, getDashboardVariableFilters, - getFilterBroadcastTarget, getFilterExpression, isFilterBroadcastEnabled, isQueryExpressionFilter, @@ -25,21 +26,6 @@ import { import { dashboardFilterValuesParser } from '@/utils/queryParsers'; -/** - * Whether a filter definition broadcasts its selected value onto a tile - * whose source is `sourceId`. - */ -const definitionAppliesToSource = ( - definition: DashboardFilter, - sourceId: string | undefined, -): boolean => { - const target = getFilterBroadcastTarget(definition); - if (!target) return false; - const appliesTo = target.appliesToSourceIds; - if (!appliesTo || appliesTo.length === 0) return true; - return !!sourceId && appliesTo.includes(sourceId); -}; - const hasSelection = (selection: FilterSelection): boolean => selection.included.size > 0 || selection.excluded.size > 0 || @@ -155,11 +141,13 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { ignoredExpressions, ignoredVariableNames, variables, + unsatisfiedRequiredFilters, } = useMemo<{ selectionByFilterId: ReadonlyMap; ignoredExpressions: string[]; ignoredVariableNames: string[]; variables: ChartVariable[]; + unsatisfiedRequiredFilters: DashboardFilter[]; }>(() => { const parsed = parseDashboardFilterValues(filterValueEntries ?? []); @@ -204,6 +192,10 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { variables, ignoredExpressions, ignoredVariableNames, + unsatisfiedRequiredFilters: getUnsatisfiedRequiredFilters( + filters, + selectionByFilterId, + ), }; }, [filterValueEntries, filters]); @@ -251,7 +243,7 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { const getFilterQueriesForSource = useCallback( (sourceId: string | undefined): Filter[] => getFiltersQueriesFor(definition => - definitionAppliesToSource(definition, sourceId), + doesFilterApplyToSource(definition, sourceId), ), [getFiltersQueriesFor], ); @@ -292,6 +284,8 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { getFilterQueriesForSource, /** The dashboard's variable-enabled filters and their currently selected values. */ variables, + /** Filters marked required that have nothing selected, if any. */ + unsatisfiedRequiredFilters, }; }; diff --git a/packages/app/tests/e2e/features/dashboard-required-filters.spec.ts b/packages/app/tests/e2e/features/dashboard-required-filters.spec.ts new file mode 100644 index 0000000000..c05b5d6b1e --- /dev/null +++ b/packages/app/tests/e2e/features/dashboard-required-filters.spec.ts @@ -0,0 +1,453 @@ +/** + * A dashboard filter marked required (`minSelections: 1`) blocks tiles until it + * has a selected value: each blocked tile renders a "Missing required filters" + * placeholder instead of querying. By default only the tiles that read the + * filter are blocked - the ones referencing its variable, and the ones its + * broadcast reaches. Checking "Block every tile" (`isGlobalRequirement: true`) + * widens that to the whole dashboard. Markdown tiles are never blocked. + */ +import { DisplayType } from '@hyperdx/common-utils/dist/types'; + +import { DashboardPage } from '../page-objects/DashboardPage'; +import { expect, test } from '../utils/base-test'; +import { + DEFAULT_LOGS_SOURCE_NAME, + DEFAULT_TRACES_SOURCE_NAME, + E2E_PROMQL_METRIC_NAME, + PROMQL_SOURCE_NAME, +} from '../utils/constants'; + +const ENVIRONMENTS = ['prod', 'staging', 'dev']; + +test.describe( + 'Required dashboard filters', + { tag: ['@dashboard', '@full-stack'] }, + () => { + let dashboardPage: DashboardPage; + + test.beforeEach(async ({ page }) => { + dashboardPage = new DashboardPage(page); + await dashboardPage.goto(); + }); + + /** A tile shows the placeholder, naming the filters it is waiting on. */ + const expectTileBlockedOn = async ( + tileIndex: number, + filterNames: string, + ) => { + const placeholder = + dashboardPage.getTileMissingRequiredFilters(tileIndex); + await expect(placeholder).toBeVisible({ timeout: 20000 }); + await expect(placeholder).toContainText( + `Missing required filters: ${filterNames}`, + ); + }; + + /** A tile is past the placeholder and has rendered its value. */ + const expectTileLoaded = async (tileIndex: number) => { + await expect( + dashboardPage.getTileMissingRequiredFilters(tileIndex), + ).toHaveCount(0, { timeout: 20000 }); + await expect(dashboardPage.getNumberTileValue(tileIndex)).toBeVisible({ + timeout: 30000, + }); + }; + + test('blocks a data tile but never a markdown tile', async () => { + test.setTimeout(150000); + + await test.step('Create a data tile, a markdown tile, and a required filter', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.addNumberTile( + 'Count tile', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.addMarkdownTile('Notes', 'Runbook notes'); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addStaticListFilterToDashboard( + 'Environment', + ENVIRONMENTS, + { variableName: 'env', required: true, globalRequirement: true }, + ); + await dashboardPage.closeFiltersModal(); + }); + + await test.step('The data tile is blocked and the markdown tile still renders', async () => { + await expectTileBlockedOn(0, 'Environment'); + // No chart component is mounted at all, which is what keeps the tile + // from querying rather than merely hiding a result it fetched. + await expect(dashboardPage.getNumberTileValue(0)).toHaveCount(0); + await expect(dashboardPage.getTileError(0)).toHaveCount(0); + + await expect( + dashboardPage.getTileMissingRequiredFilters(1), + ).toHaveCount(0); + await expect( + dashboardPage.getTile(1).getByText('Runbook notes'), + ).toBeVisible(); + }); + + await test.step('Selecting a value unblocks the data tile', async () => { + await dashboardPage.toggleFilterValue('Environment', 'prod'); + + await expectTileLoaded(0); + }); + + await test.step('Clearing the value blocks it again', async () => { + await dashboardPage.toggleFilterValue('Environment', 'prod'); + + await expectTileBlockedOn(0, 'Environment'); + }); + }); + + // "Block every tile" ignores scope entirely: a filter scoped away from a + // tile still blocks it. + test('blocks every tile, including one the filter does not apply to', async () => { + test.setTimeout(150000); + + await test.step('Scope a required filter to traces, and add a logs tile', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.addNumberTile( + 'Logs count', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addFilterToDashboard( + 'Service', + DEFAULT_TRACES_SOURCE_NAME, + 'ServiceName', + undefined, + [DEFAULT_TRACES_SOURCE_NAME], + { required: true, globalRequirement: true }, + ); + await dashboardPage.closeFiltersModal(); + }); + + await test.step('The logs tile is blocked even though the filter never reaches it', async () => { + await expectTileBlockedOn(0, 'Service'); + }); + + await test.step('Satisfying the filter loads it', async () => { + await dashboardPage.toggleFilterValue('Service', 'frontend'); + + await expectTileLoaded(0); + }); + }); + + test('blocks only the tiles a broadcast reaches', async () => { + test.setTimeout(180000); + + await test.step('Add a logs tile, a traces tile, and a traces-only scoped requirement', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.addNumberTile( + 'Logs count', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.addNumberTile( + 'Traces count', + DEFAULT_TRACES_SOURCE_NAME, + ); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addFilterToDashboard( + 'Service', + DEFAULT_TRACES_SOURCE_NAME, + 'ServiceName', + undefined, + [DEFAULT_TRACES_SOURCE_NAME], + { required: true }, + ); + await dashboardPage.closeFiltersModal(); + }); + + await test.step('Only the traces tile is blocked', async () => { + await expectTileBlockedOn(1, 'Service'); + + await expectTileLoaded(0); + }); + + await test.step('Satisfying the filter loads the traces tile too', async () => { + await dashboardPage.toggleFilterValue('Service', 'frontend'); + + await expectTileLoaded(1); + }); + }); + + // Being in a broadcast's source scope is not the same as reading it: a + // PromQL tile is handed no filters, and a raw-SQL tile without the + // $__filters macro drops them. + test('does not block a tile that ignores broadcast filters', async () => { + test.setTimeout(240000); + + const rawSql = `SELECT ServiceName, count() AS count FROM default.e2e_otel_logs WHERE Timestamp >= fromUnixTimestamp64Milli({startDateMilliseconds:Int64}) AND Timestamp <= fromUnixTimestamp64Milli({endDateMilliseconds:Int64}) GROUP BY ServiceName LIMIT 200`; + + await test.step('Add a builder tile, a PromQL tile, and a macro-less raw SQL tile', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.addNumberTile( + 'Logs count', + DEFAULT_LOGS_SOURCE_NAME, + ); + + await dashboardPage.addTile(); + await expect(dashboardPage.chartEditor.nameInput).toBeVisible(); + await dashboardPage.chartEditor.waitForDataToLoad(); + await dashboardPage.chartEditor.switchToPromqlMode(); + await dashboardPage.chartEditor.selectPromqlSource(PROMQL_SOURCE_NAME); + await dashboardPage.chartEditor.setChartName('PromQL tile'); + await dashboardPage.chartEditor.replacePromqlExpression( + E2E_PROMQL_METRIC_NAME, + ); + await dashboardPage.chartEditor.save(); + await expect(dashboardPage.getTiles()).toHaveCount(2, { + timeout: 10000, + }); + + await dashboardPage.addTile(); + await expect(dashboardPage.chartEditor.nameInput).toBeVisible(); + await dashboardPage.chartEditor.waitForDataToLoad(); + await dashboardPage.chartEditor.setChartType(DisplayType.Table); + await dashboardPage.chartEditor.setChartName('Raw SQL tile'); + await dashboardPage.chartEditor.switchToSqlMode(); + await dashboardPage.chartEditor.typeSqlQuery(rawSql); + await dashboardPage.chartEditor.runQuery(false); + await dashboardPage.saveTile(); + }); + + await test.step('Add an unscoped required filter', async () => { + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addFilterToDashboard( + 'Service', + DEFAULT_LOGS_SOURCE_NAME, + 'ServiceName', + undefined, + undefined, + { required: true }, + ); + await dashboardPage.closeFiltersModal(); + }); + + await test.step('Only the builder tile is blocked', async () => { + await expectTileBlockedOn(0, 'Service'); + + await expect( + dashboardPage.getTileMissingRequiredFilters(1), + ).toHaveCount(0); + await expect( + dashboardPage.getTile(1).locator('.recharts-responsive-container'), + ).toBeVisible({ timeout: 30000 }); + + await expect( + dashboardPage.getTileMissingRequiredFilters(2), + ).toHaveCount(0); + await expect( + dashboardPage.getTile(2).getByTitle('frontend', { exact: true }), + ).toBeVisible({ timeout: 30000 }); + }); + + await test.step('Satisfying the filter loads the builder tile', async () => { + await dashboardPage.toggleFilterValue('Service', 'frontend'); + + await expectTileLoaded(0); + }); + }); + + // A static-list filter broadcasts nothing, so a variable reference is the + // only thing that can put a tile in its scope. + test('blocks only the tiles that reference the variable', async () => { + test.setTimeout(180000); + + await test.step('Add a scoped requirement and one tile that reads it', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addStaticListFilterToDashboard( + 'Environment', + ENVIRONMENTS, + { variableName: 'env', required: true }, + ); + await dashboardPage.closeFiltersModal(); + await dashboardPage.addNumberTile( + 'Unrelated count', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.addNumberTile( + 'Env count', + DEFAULT_LOGS_SOURCE_NAME, + { sqlSeriesWhere: '$__filter(ServiceName, $env)' }, + ); + }); + + await test.step('Only the referencing tile is blocked', async () => { + await expectTileBlockedOn(1, 'Environment'); + + await expectTileLoaded(0); + }); + + await test.step('Selecting a value unblocks it', async () => { + await dashboardPage.toggleFilterValue('Environment', 'prod'); + + await expectTileLoaded(1); + }); + }); + + test('flags the unsatisfied filter in the filter bar', async () => { + test.setTimeout(150000); + + await test.step('Create one required and one optional filter', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addStaticListFilterToDashboard( + 'Environment', + ENVIRONMENTS, + { variableName: 'env', required: true }, + ); + await dashboardPage.addStaticListFilterToDashboard( + 'Tier', + ['gold', 'silver'], + { variableName: 'tier' }, + ); + await dashboardPage.closeFiltersModal(); + }); + + await test.step('Only the required filter carries the caution icon', async () => { + await expect( + dashboardPage.getFilterRequiredWarning('Environment'), + ).toBeVisible({ timeout: 20000 }); + await expect( + dashboardPage.getFilterRequiredWarning('Tier'), + ).toHaveCount(0); + }); + + await test.step('The icon clears on selection and returns when cleared', async () => { + await dashboardPage.toggleFilterValue('Environment', 'prod'); + await expect( + dashboardPage.getFilterRequiredWarning('Environment'), + ).toHaveCount(0, { timeout: 20000 }); + + await dashboardPage.toggleFilterValue('Environment', 'prod'); + await expect( + dashboardPage.getFilterRequiredWarning('Environment'), + ).toBeVisible({ timeout: 20000 }); + }); + }); + + test('stays blocked until every required filter has a value', async () => { + test.setTimeout(150000); + + await test.step('Create two required filters over one tile', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.addNumberTile( + 'Count tile', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addStaticListFilterToDashboard( + 'Environment', + ENVIRONMENTS, + { variableName: 'env', required: true, globalRequirement: true }, + ); + await dashboardPage.addStaticListFilterToDashboard( + 'Tier', + ['gold', 'silver'], + { variableName: 'tier', required: true, globalRequirement: true }, + ); + await dashboardPage.closeFiltersModal(); + }); + + await test.step('Both are named while neither has a value', async () => { + await expectTileBlockedOn(0, 'Environment, Tier'); + }); + + await test.step('Satisfying one leaves the tile blocked on the other', async () => { + await dashboardPage.toggleFilterValue('Environment', 'prod'); + + await expectTileBlockedOn(0, 'Tier'); + await expect( + dashboardPage.getTileMissingRequiredFilters(0), + ).not.toContainText('Environment'); + }); + + await test.step('Satisfying the second loads the tile', async () => { + await dashboardPage.toggleFilterValue('Tier', 'gold'); + + await expectTileLoaded(0); + }); + }); + + test('opens unblocked when the selection is saved as the default', async ({ + page, + }) => { + test.setTimeout(150000); + + await test.step('Save a selection as the dashboard default', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.addNumberTile( + 'Count tile', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addStaticListFilterToDashboard( + 'Environment', + ENVIRONMENTS, + { variableName: 'env', required: true, globalRequirement: true }, + ); + await dashboardPage.closeFiltersModal(); + await dashboardPage.toggleFilterValue('Environment', 'prod'); + await dashboardPage.saveQueryAndFiltersAsDefault(); + }); + + await test.step('A fresh visit with no filters in the URL is not blocked', async () => { + const url = new URL(page.url()); + url.search = ''; + await page.goto(url.toString()); + await dashboardPage.waitForLoaded(); + + await expect( + dashboardPage.getFilterPill('Environment', 'prod'), + ).toBeVisible({ timeout: 20000 }); + await expectTileLoaded(0); + }); + }); + + test('round-trips the required option through an edit', async () => { + test.setTimeout(150000); + + await test.step('Save a required filter over a tile', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.addNumberTile( + 'Count tile', + DEFAULT_LOGS_SOURCE_NAME, + ); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addStaticListFilterToDashboard( + 'Environment', + ENVIRONMENTS, + { variableName: 'env', required: true, globalRequirement: true }, + ); + }); + + await test.step('The list marks it required and the edit form reopens checked', async () => { + await expect( + dashboardPage.getFilterItemByName('Environment'), + ).toContainText('Required'); + + await dashboardPage.openEditFilterForm('Environment'); + await expect(dashboardPage.requiredFilterCheckbox).toBeChecked(); + await expect(dashboardPage.globalRequirementCheckbox).toBeChecked(); + // The scope only exists under a checked "Required". + await dashboardPage.requiredFilterCheckbox.uncheck(); + await expect(dashboardPage.globalRequirementCheckbox).toHaveCount(0); + await dashboardPage.requiredFilterCheckbox.check(); + await dashboardPage.page.getByTestId('save-filter-button').click(); + }); + + await test.step('Unchecking it lets the tile load with nothing selected', async () => { + await dashboardPage.setFilterRequiredForSavedFilter( + 'Environment', + false, + ); + await dashboardPage.closeFiltersModal(); + + await expectTileLoaded(0); + }); + }); + }, +); diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index 432b2d3472..93ebe88383 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -20,6 +20,12 @@ export type FilterWhereOptions = { language?: 'sql' | 'lucene'; }; +/** The "Required" checkbox and the "Block every tile" one nested under it. */ +export type FilterRequirementOptions = { + required?: boolean; + globalRequirement?: boolean; +}; + /** * Config format tile config, as accepted by the external dashboard API. * Used with verifyTileFormFromConfig @@ -98,6 +104,8 @@ export class DashboardPage { readonly appliesToSourceSelector: Locator; readonly broadcastFilterCheckbox: Locator; readonly variableEnabledCheckbox: Locator; + readonly requiredFilterCheckbox: Locator; + readonly globalRequirementCheckbox: Locator; readonly variableNameInput: Locator; private readonly saveButton: Locator; private readonly tileSourceSelector: Locator; @@ -161,6 +169,10 @@ export class DashboardPage { this.variableEnabledCheckbox = page.getByTestId( 'filter-variable-enabled-checkbox', ); + this.requiredFilterCheckbox = page.getByTestId('filter-required-checkbox'); + this.globalRequirementCheckbox = page.getByTestId( + 'filter-global-requirement-checkbox', + ); this.variableNameInput = page.getByTestId('filter-variable-name-input'); this.saveButton = page.getByTestId('chart-save-button'); @@ -890,6 +902,22 @@ export class DashboardPage { .click(); } + /** + * Set the filter form's requirement checkboxes, leaving either alone when the + * caller says nothing about it. The nested "Block every tile" box only exists + * while "Required" is checked, so the order matters. + */ + private async setFilterRequirement(options?: FilterRequirementOptions) { + if (options?.required !== undefined) { + await this.requiredFilterCheckbox.setChecked(options.required); + } + if (options?.globalRequirement !== undefined) { + await this.globalRequirementCheckbox.setChecked( + options.globalRequirement, + ); + } + } + async fillFilterForm( name: string, sourceName: string, @@ -900,7 +928,7 @@ export class DashboardPage { isBroadcastEnabled?: boolean; isVariableEnabled?: boolean; variableName?: string; - }, + } & FilterRequirementOptions, whereOptions?: FilterWhereOptions, ) { const filterNameInput = this.page.getByTestId('filter-name-input'); @@ -941,6 +969,8 @@ export class DashboardPage { } } + await this.setFilterRequirement(variableOptions); + if (appliesToSourceNames && appliesToSourceNames.length > 0) { for (const appliesName of appliesToSourceNames) { await this.appliesToSourceSelector.click(); @@ -966,7 +996,7 @@ export class DashboardPage { isBroadcastEnabled?: boolean; isVariableEnabled?: boolean; variableName?: string; - }, + } & FilterRequirementOptions, whereOptions?: FilterWhereOptions, ) { await this.addFiltersButton.click(); @@ -1083,6 +1113,33 @@ export class DashboardPage { await this.page.getByTestId('save-filter-button').click(); } + /** + * Toggle "Required" on an already-saved filter and save. Assumes the filters + * list modal is open. + */ + async setFilterRequiredForSavedFilter(filterName: string, enabled: boolean) { + await this.page.getByTestId(`edit-filter-button-${filterName}`).click(); + await this.requiredFilterCheckbox.setChecked(enabled); + await this.page.getByTestId('save-filter-button').click(); + await this.getFilterItemByName(filterName).waitFor({ + state: 'visible', + timeout: 10000, + }); + } + + /** The caution icon shown when a required filter has no selected value. */ + getFilterRequiredWarning(filterName: string): Locator { + return this.page.getByTestId(`dashboard-filter-required-${filterName}`); + } + + /** + * The placeholder a tile renders instead of its chart while a required + * filter has no selected value. + */ + getTileMissingRequiredFilters(tileIndex = 0): Locator { + return this.getTile(tileIndex).getByTestId('tile-missing-required-filters'); + } + /** * The warning shown in the filter edit form when a filter both broadcasts and * is a variable, with no "Applies to sources" scope to limit the broadcast. @@ -1206,18 +1263,44 @@ export class DashboardPage { /** * Create a Number tile that counts events from `sourceName`. The tile editor's * default aggregation is "Count of Events", so no agg configuration is needed. - * Leaves exactly one tile on the dashboard. + * + * `sqlSeriesWhere` sets the tile's own SQL filter, the usual place a tile + * references a `$variable`. */ - async addNumberTile(name: string, sourceName: string) { + async addNumberTile( + name: string, + sourceName: string, + options?: { sqlSeriesWhere?: string }, + ) { + const existingTiles = await this.getTiles().count(); await this.addTile(); await expect(this.chartEditor.nameInput).toBeVisible(); await this.chartEditor.waitForDataToLoad(); await this.chartEditor.setChartType(DisplayType.Number); await this.chartEditor.setChartName(name); await this.chartEditor.selectSource(sourceName); + if (options?.sqlSeriesWhere != null) { + await this.chartEditor.setSqlWhere(options.sqlSeriesWhere, 'series'); + } await this.chartEditor.runQuery(false); await this.chartEditor.save(); - await expect(this.getTiles()).toHaveCount(1, { timeout: 10000 }); + await expect(this.getTiles()).toHaveCount(existingTiles + 1, { + timeout: 10000, + }); + } + + /** Create a Markdown tile, which needs neither a source nor a query. */ + async addMarkdownTile(name: string, content: string) { + const existingTiles = await this.getTiles().count(); + await this.addTile(); + await expect(this.chartEditor.nameInput).toBeVisible(); + await this.chartEditor.setChartType(DisplayType.Markdown); + await this.chartEditor.setChartName(name); + await this.markdownTextarea.first().fill(content); + await this.chartEditor.save(); + await expect(this.getTiles()).toHaveCount(existingTiles + 1, { + timeout: 10000, + }); } /** Locator for the rendered value of a Number tile. */ @@ -1344,7 +1427,7 @@ export class DashboardPage { async addStaticListFilterToDashboard( name: string, options: string[], - variableOptions?: { variableName?: string }, + variableOptions?: { variableName?: string } & FilterRequirementOptions, ) { await this.addFiltersButton.click(); await this.selectFilterType('Static values'); @@ -1355,6 +1438,7 @@ export class DashboardPage { if (variableOptions?.variableName !== undefined) { await this.variableNameInput.fill(variableOptions.variableName); } + await this.setFilterRequirement(variableOptions); await this.page.getByTestId('save-filter-button').click(); await this.getFilterItemByName(name).waitFor({ state: 'visible', @@ -1420,7 +1504,10 @@ export class DashboardPage { name: string, sourceName: string, label: string, - variableOptions?: { variableName?: string; match?: string }, + variableOptions?: { + variableName?: string; + match?: string; + } & FilterRequirementOptions, ) { await this.addFiltersButton.click(); await this.selectFilterType('PromQL label values'); @@ -1435,6 +1522,7 @@ export class DashboardPage { if (variableOptions?.variableName !== undefined) { await this.variableNameInput.fill(variableOptions.variableName); } + await this.setFilterRequirement(variableOptions); await this.page.getByTestId('save-filter-button').click(); await this.getFilterItemByName(name).waitFor({ state: 'visible', diff --git a/packages/common-utils/src/__tests__/dashboardFilterValues.test.ts b/packages/common-utils/src/__tests__/dashboardFilterValues.test.ts index 7ef7d08cd8..09af938130 100644 --- a/packages/common-utils/src/__tests__/dashboardFilterValues.test.ts +++ b/packages/common-utils/src/__tests__/dashboardFilterValues.test.ts @@ -1,11 +1,14 @@ import { filterSelectionKey, + getBlockingRequiredFilters, + getUnsatisfiedRequiredFilters, parseDashboardFilterValues, resolveFilterSelection, serializeDashboardFilterValues, } from '@/dashboardFilterValues'; import { FilterState, filtersToQuery } from '@/filters'; import type { + DashboardFilter, DashboardFilterValue, PromqlLabelDashboardFilter, QueryExpressionDashboardFilter, @@ -485,4 +488,254 @@ describe('dashboardFilterValues', () => { ); }); }); + + describe('getUnsatisfiedRequiredFilters', () => { + const required = ( + overrides: Partial = {}, + ) => filter({ minSelections: 1, ...overrides }); + + it.each([ + ['no minimum', undefined], + ['an explicit zero minimum', 0], + ])('returns nothing for a filter with %s', (_label, minSelections) => { + expect( + getUnsatisfiedRequiredFilters( + [filter({ minSelections }), staticFilter({ minSelections })], + new Map(), + ), + ).toEqual([]); + }); + + // The scope is only ever read for a filter that is required, so a payload + // carrying it alone must still leave the filter optional. + it('returns nothing for a filter that is only scoped', () => { + expect( + getUnsatisfiedRequiredFilters( + [filter({ isGlobalRequirement: true })], + new Map(), + ), + ).toEqual([]); + }); + + it('returns nothing for a missing or empty filter list', () => { + expect(getUnsatisfiedRequiredFilters(undefined, new Map())).toEqual([]); + expect(getUnsatisfiedRequiredFilters([], new Map())).toEqual([]); + }); + + it('returns a required filter with no selection', () => { + expect(getUnsatisfiedRequiredFilters([required()], new Map())).toEqual([ + required(), + ]); + }); + + it('omits a required filter once a value is included', () => { + expect( + getUnsatisfiedRequiredFilters( + [required()], + new Map([['f1', included('api')]]), + ), + ).toEqual([]); + }); + + // A `NOT IN` narrows the data without choosing a value. + it('still reports a filter whose selection only excludes', () => { + expect( + getUnsatisfiedRequiredFilters( + [required()], + new Map([ + [ + 'f1', + { + included: new Set(), + excluded: new Set(['api']), + }, + ], + ]), + ), + ).toEqual([required()]); + }); + + it('preserves filter order and skips optional filters', () => { + const filters = [ + required({ id: 'a', name: 'A' }), + filter({ id: 'b', name: 'B' }), + required({ id: 'c', name: 'C' }), + ]; + + expect( + getUnsatisfiedRequiredFilters(filters, new Map()).map(f => f.name), + ).toEqual(['A', 'C']); + }); + }); + + describe('getBlockingRequiredFilters', () => { + const names = ( + filters: DashboardFilter[], + tile: { + sourceId?: string; + referencedVariableNames?: string[]; + consumesBroadcastFilters?: boolean; + }, + ) => + getBlockingRequiredFilters(filters, { + consumesBroadcastFilters: true, + ...tile, + }).map(f => f.name); + + it('blocks any tile on a global requirement', () => { + const filters = [ + filter({ minSelections: 1, isGlobalRequirement: true }), + staticFilter({ minSelections: 1, isGlobalRequirement: true }), + ]; + + expect( + names(filters, { sourceId: 'unrelated', referencedVariableNames: [] }), + ).toEqual(['Service', 'Environment']); + expect(names(filters, {})).toEqual(['Service', 'Environment']); + }); + + it('blocks only the tiles that reference the filter by default', () => { + const filters = [staticFilter({ minSelections: 1 })]; + + expect( + names(filters, { sourceId: 'logs', referencedVariableNames: ['env'] }), + ).toEqual(['Environment']); + expect( + names(filters, { sourceId: 'logs', referencedVariableNames: ['tier'] }), + ).toEqual([]); + expect(names(filters, {})).toEqual([]); + }); + + it('treats an explicit false the same as an absent flag', () => { + const filters = [ + staticFilter({ minSelections: 1, isGlobalRequirement: false }), + ]; + + expect(names(filters, { referencedVariableNames: ['env'] })).toEqual([ + 'Environment', + ]); + expect(names(filters, { referencedVariableNames: [] })).toEqual([]); + }); + + // A filter with no explicit variableName is still referenced by the token + // derived from its display name. + it('matches a filter by its derived variable name', () => { + const filters = [ + staticFilter({ minSelections: 1, variableName: undefined }), + ]; + + expect( + names(filters, { referencedVariableNames: ['Environment'] }), + ).toEqual(['Environment']); + }); + + it('ignores a variable reference to a filter that publishes no variable', () => { + const filters = [ + filter({ + minSelections: 1, + isBroadcastEnabled: false, + isVariableEnabled: false, + variableName: 'svc', + }), + ]; + + expect(names(filters, { referencedVariableNames: ['svc'] })).toEqual([]); + }); + + it('blocks a tile a broadcast reaches', () => { + const unscoped = filter({ minSelections: 1 }); + const scoped = filter({ + minSelections: 1, + appliesToSourceIds: ['logs'], + }); + + expect(names([unscoped], { sourceId: 'traces' })).toEqual(['Service']); + expect(names([scoped], { sourceId: 'logs' })).toEqual(['Service']); + expect(names([scoped], { sourceId: 'traces' })).toEqual([]); + expect(names([scoped], {})).toEqual([]); + }); + + it('preserves filter order across the two scopes', () => { + const filters = [ + staticFilter({ + id: 'a', + name: 'A', + variableName: 'a', + minSelections: 1, + }), + filter({ + id: 'b', + name: 'B', + minSelections: 1, + isGlobalRequirement: true, + }), + staticFilter({ + id: 'c', + name: 'C', + variableName: 'c', + minSelections: 1, + }), + ]; + + expect(names(filters, { referencedVariableNames: ['c', 'a'] })).toEqual([ + 'A', + 'B', + 'C', + ]); + }); + + // Both modes at once is what "Applies to sources" plus a variable looks + // like, and either path on its own is enough to block. + it('blocks through either path when a filter does both', () => { + const filters = [ + filter({ + minSelections: 1, + isVariableEnabled: true, + variableName: 'svc', + appliesToSourceIds: ['logs'], + }), + ]; + + expect(names(filters, { sourceId: 'logs' })).toEqual(['Service']); + expect( + names(filters, { + sourceId: 'traces', + referencedVariableNames: ['svc'], + }), + ).toEqual(['Service']); + expect( + names(filters, { + sourceId: 'traces', + referencedVariableNames: ['other'], + }), + ).toEqual([]); + }); + + // A PromQL tile is handed no filters, and a raw-SQL tile can drop them, so + // sharing a source with a broadcast does not mean the tile reads it. + it('does not block on broadcast when the tile ignores broadcast filters', () => { + const filters = [ + filter({ id: 'a', minSelections: 1 }), + filter({ + id: 'b', + name: 'Global', + minSelections: 1, + isGlobalRequirement: true, + }), + staticFilter({ id: 'c', minSelections: 1 }), + ]; + + expect( + names(filters, { + sourceId: 'logs', + referencedVariableNames: ['env'], + consumesBroadcastFilters: false, + }), + ).toEqual(['Global', 'Environment']); + }); + + it('returns nothing for an empty list', () => { + expect(names([], { sourceId: 'logs' })).toEqual([]); + }); + }); }); diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index 08bffec854..b853a2ff3c 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -1,5 +1,6 @@ import { deriveVariableName, + doesFilterApplyToSource, FilterState, filterStateToPredicate, filtersToQuery, @@ -11,6 +12,8 @@ import { getPendingFilterValuesVariables, hasFilterEffect, isFilterBroadcastEnabled, + isFilterGlobalRequirement, + isFilterRequired, isFilterVariableEnabled, isQueryExpressionFilter, isRenderablePinnedFilter, @@ -1661,6 +1664,95 @@ describe('filters', () => { }); }); + describe('isFilterRequired', () => { + it('treats a missing or zero minimum as not required', () => { + expect(isFilterRequired({})).toBe(false); + expect(isFilterRequired({ minSelections: undefined })).toBe(false); + expect(isFilterRequired({ minSelections: 0 })).toBe(false); + }); + + it('treats a null minimum as not required', () => { + expect( + isFilterRequired({ + minSelections: null, + } as unknown as DashboardFilter), + ).toBe(false); + }); + + it('holds for a minimum of one', () => { + expect(isFilterRequired({ minSelections: 1 })).toBe(true); + }); + }); + + describe('isFilterGlobalRequirement', () => { + it('treats a missing flag as covering only the tiles that read the filter', () => { + expect(isFilterGlobalRequirement({})).toBe(false); + expect( + isFilterGlobalRequirement({ isGlobalRequirement: undefined }), + ).toBe(false); + }); + + it('respects an explicit flag', () => { + expect(isFilterGlobalRequirement({ isGlobalRequirement: true })).toBe( + true, + ); + expect(isFilterGlobalRequirement({ isGlobalRequirement: false })).toBe( + false, + ); + }); + }); + + describe('doesFilterApplyToSource', () => { + const filter = ( + overrides: Partial = {}, + ): QueryExpressionDashboardFilter => ({ + id: 'f1', + type: 'QUERY_EXPRESSION', + name: 'Service', + expression: 'ServiceName', + source: 'logs', + ...overrides, + }); + + it('reaches every tile when the scope is empty', () => { + for (const scoped of [ + filter(), + filter({ appliesToSourceIds: [] }), + filter({ appliesToSourceIds: undefined }), + ]) { + expect(doesFilterApplyToSource(scoped, 'traces')).toBe(true); + expect(doesFilterApplyToSource(scoped, undefined)).toBe(true); + } + }); + + it('reaches only the scoped sources', () => { + const scoped = filter({ appliesToSourceIds: ['logs'] }); + + expect(doesFilterApplyToSource(scoped, 'logs')).toBe(true); + expect(doesFilterApplyToSource(scoped, 'traces')).toBe(false); + expect(doesFilterApplyToSource(scoped, undefined)).toBe(false); + }); + + it('reaches nothing when broadcasting is off', () => { + expect( + doesFilterApplyToSource(filter({ isBroadcastEnabled: false }), 'logs'), + ).toBe(false); + expect( + doesFilterApplyToSource( + { + id: 'f2', + type: 'STATIC_LIST', + name: 'Environment', + options: ['prod'], + isBroadcastEnabled: false, + isVariableEnabled: true, + }, + 'logs', + ), + ).toBe(false); + }); + }); + describe('hasFilterEffect', () => { it('holds for a filter that predates both flags', () => { expect(hasFilterEffect({})).toBe(true); diff --git a/packages/common-utils/src/dashboardFilterValues.ts b/packages/common-utils/src/dashboardFilterValues.ts index f099537ff0..dcbdb6bf7d 100644 --- a/packages/common-utils/src/dashboardFilterValues.ts +++ b/packages/common-utils/src/dashboardFilterValues.ts @@ -1,8 +1,12 @@ import { + doesFilterApplyToSource, FilterState, filtersToQuery, + getDashboardVariableFilters, getFilterExpression, getFilterVariableName, + isFilterGlobalRequirement, + isFilterRequired, isFilterVariableEnabled, parseQuery, } from '@/filters'; @@ -158,3 +162,51 @@ export function resolveFilterSelection( if (expression == null) return undefined; return new Map(Object.entries(parsed.byExpression)).get(expression); } + +/** The required filters that have nothing selected, in filter order. */ +export function getUnsatisfiedRequiredFilters< + T extends { id: string; minSelections?: number }, +>( + filters: T[] | undefined, + selectionByFilterId: ReadonlyMap, +): T[] { + return (filters ?? []).filter( + filter => + isFilterRequired(filter) && + (selectionByFilterId.get(filter.id)?.included.size ?? 0) === 0, + ); +} + +/** + * The subset of `unsatisfiedRequiredFilters` that blocks the given tile. + * + * A tile is blocked if any of the following conditions hold: + * 1. The filter has isGlobalRequirement set. + * 2. The tile references the filter's variable. + * 3. The filter applies via broadcast to the tile's source, and the tile + * applies broadcast conditions (`consumesBroadcastFilters`). + */ +export function getBlockingRequiredFilters( + unsatisfiedRequiredFilters: DashboardFilter[], + tile: { + sourceId?: string; + referencedVariableNames?: readonly string[]; + /** Whether the tile's query applies broadcast filter conditions. */ + consumesBroadcastFilters: boolean; + }, +): DashboardFilter[] { + const referencedNamesSet = new Set(tile.referencedVariableNames ?? []); + const referencedFilterIds = new Set( + getDashboardVariableFilters(unsatisfiedRequiredFilters) + .filter(({ name }) => referencedNamesSet.has(name)) + .map(({ filter }) => filter.id), + ); + + return unsatisfiedRequiredFilters.filter( + filter => + isFilterGlobalRequirement(filter) || + referencedFilterIds.has(filter.id) || + (tile.consumesBroadcastFilters && + doesFilterApplyToSource(filter, tile.sourceId)), + ); +} diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index bb4af3919d..1f377675bd 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -847,6 +847,24 @@ export function isFilterVariableEnabled(filter: { return filter.isVariableEnabled === true; } +/** + * Whether a filter must have at least one value selected before the tiles it + * covers will load. + */ +export function isFilterRequired(filter: { minSelections?: number }): boolean { + return (filter.minSelections ?? 0) > 0; +} + +/** + * Whether the given required filter blocks every tile on the dashboard, + * rather than only the tiles that read it. + */ +export function isFilterGlobalRequirement(filter: { + isGlobalRequirement?: boolean; +}): boolean { + return !!filter.isGlobalRequirement; +} + /** The discriminant every dashboard-filter shape carries. */ export type DashboardFilterKind = DashboardFilter['type']; @@ -898,6 +916,22 @@ export function getFilterBroadcastTarget( }; } +/** + * Whether a filter broadcasts its selected value onto a tile whose source is + * `sourceId`. A broadcasting filter with no `appliesToSourceIds` reaches every + * tile, including one with no source of its own. + */ +export function doesFilterApplyToSource( + filter: DashboardFilter, + sourceId: string | undefined, +): boolean { + const target = getFilterBroadcastTarget(filter); + if (!target) return false; + const appliesTo = target.appliesToSourceIds; + if (!appliesTo || appliesTo.length === 0) return true; + return !!sourceId && appliesTo.includes(sourceId); +} + /** * Whether a filter does anything at all with the value it collects — broadcast * it as a condition, expose it as `$variableName`, or both. diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 806b574b30..b0373d7572 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -1936,6 +1936,18 @@ const dashboardFilterBaseSchema = z.object({ .max(DASHBOARD_VARIABLE_NAME_MAX_LENGTH) .regex(DASHBOARD_VARIABLE_NAME_PATTERN_ANCHORED) .optional(), + /** + * How many values must be selected before tiles will load. `isGlobalRequirement` + * decides which tiles are blocked. Only 0 and 1 are currently allowed. + * undefined and 0 both imply no minimum selection requirement. + */ + minSelections: z.number().int().min(0).max(1).optional(), + /** + * Whether an unsatisfied requirement blocks every tile on the dashboard, + * rather than only the tiles that read this filter (via variable or broadcast). + * Ignored unless `minSelections` marks the filter required. + */ + isGlobalRequirement: z.boolean().optional(), }); /**