diff --git a/.betterer.eslint.config.js b/.betterer.eslint.config.js index 1be44601a3c..349ee612837 100644 --- a/.betterer.eslint.config.js +++ b/.betterer.eslint.config.js @@ -113,6 +113,7 @@ module.exports = [ ignores: ['public/app/plugins/**', '**/*.story.tsx', '**/*.{test,spec}.{ts,tsx}', '**/__mocks__/', 'public/test'], rules: { '@grafana/no-untranslated-strings': 'error', + '@grafana/no-translation-top-level': 'error', }, }, ]; diff --git a/.betterer.results b/.betterer.results index d5ac5c018d3..1e0cc343e3b 100644 --- a/.betterer.results +++ b/.betterer.results @@ -549,6 +549,11 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] ], + "packages/grafana-ui/src/components/DataSourceSettings/DataSourceHttpSettings.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"] + ], "packages/grafana-ui/src/components/DataSourceSettings/types.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"] @@ -3044,7 +3049,8 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"] ], "public/app/features/connections/tabs/ConnectData/Search/Search.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"] ], "public/app/features/connections/tabs/ConnectData/Search/index.tsx:5381": [ [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"] @@ -4568,9 +4574,18 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] ], + "public/app/features/explore/RichHistory/RichHistorySettingsTab.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "3"] + ], "public/app/features/explore/RichHistory/RichHistoryStarredTab.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], + "public/app/features/explore/ShortLinkButtonMenu.tsx:5381": [ + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"] + ], "public/app/features/explore/SupplementaryResultError.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], @@ -4929,8 +4944,14 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "4"] ], "public/app/features/inspector/InspectJSONTab.tsx:5381": [ - [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] + [0, 0, 0, "Do not use the t() function outside of a component or function", "0"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "1"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "2"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "3"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "4"], + [0, 0, 0, "Do not use the t() function outside of a component or function", "5"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "6"], + [0, 0, 0, "No untranslated strings. Wrap text with ", "7"] ], "public/app/features/inspector/InspectStatsTab.tsx:5381": [ [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"] diff --git a/.vale.ini b/.vale.ini index cf4c71392dc..37e764e6d0f 100644 --- a/.vale.ini +++ b/.vale.ini @@ -1,5 +1,5 @@ MinAlertLevel = warning -[*] +[*.md] BasedOnStyles = Grafana TokenIgnores = (+?), \*\*[^\n]+\*\* diff --git a/docs/sources/alerting/_index.md b/docs/sources/alerting/_index.md index 601ae34e4e1..8362360d188 100644 --- a/docs/sources/alerting/_index.md +++ b/docs/sources/alerting/_index.md @@ -25,7 +25,7 @@ cards: items: - title: Introduction href: ./fundamentals/ - description: Learn more about the fundamentals and available features that help you create, manage, and respond to alerts; and improve your team’s ability to resolve issues quickly. + description: Learn more about the fundamentals and available features that help you create, manage, and respond to alerts; and improve your team's ability to resolve issues quickly. height: 24 - title: Configure alert rules href: ./alerting-rules/ @@ -55,7 +55,8 @@ Monitor your incoming metrics data or log entries and set up your Grafana Alerti In this way, you eliminate the need for manual monitoring and provide a first line of defense against system outages or changes that could turn into major incidents. -Using Grafana Alerting, you create queries and expressions from multiple data sources — no matter where your data is stored — giving you the flexibility to combine your data and alert on your metrics and logs in new and unique ways. You can then create, manage, and take action on your alerts from a single, consolidated view, and improve your team’s ability to identify and resolve issues quickly. +Using Grafana Alerting, you create queries and expressions from multiple data sources, no matter where you store your data, giving you the flexibility to combine your data and alert on your metrics and logs in new and unique ways. +You can then create, manage, and take action on your alerts from a single, consolidated view, and improve your team's ability to identify and resolve issues quickly. ## Explore diff --git a/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md b/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md index 41c9150b52c..3b30819b328 100644 --- a/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md +++ b/docs/sources/alerting/alerting-rules/create-grafana-managed-rule.md @@ -118,15 +118,17 @@ refs: # Configure Grafana-managed alert rules -Grafana-managed rules can query data from multiple data sources in a single alert rule. They are the most flexible [alert rule type](ref:alert-rules). You can also add expressions to transform your data, set alert conditions, and images in alert notifications. +Grafana-managed rules can query data from multiple data sources in a single alert rule. +They're the most flexible [alert rule type](ref:alert-rules). +You can also add expressions to transform your data, set alert conditions, and images in alert notifications. -{{% admonition type="note" %}} +{{< admonition type="note" >}} In Grafana Cloud, the number of Grafana-managed alert rules you can create depends on your Grafana Cloud plan. - Free Forever plan: You can create up to 100 free alert rules, with each alert rule having a maximum of 1000 alert instances. - All paid plans (Pro and Advanced): They have a soft limit of 2000 alert rules and support unlimited alert instances. To increase the limit, open a support ticket from the [Cloud portal](/docs/grafana-cloud/account-management/support/). -{{% /admonition %}} +{{< /admonition >}} To create or edit Grafana-managed alert rules, follow the instructions below. For a practical example, check out our [tutorial on getting started with Grafana alerting](http://grafana.com/tutorials/alerting-get-started/). @@ -144,9 +146,11 @@ Only users with **Edit** permissions for the folder storing the rules can edit o You can use default or advanced options for Grafana-managed alert rule creation. The default options streamline rule creation with a cleaner header and a single query and condition. For more complex rules, use advanced options to add multiple queries and expressions. -You can toggle between the two options. Once you have created an alert rule, the system defaults to your previous choice for the next alert rule. +You can toggle between the two options. +After you have created an alert rule, the system defaults to your previous choice for the next alert rule. -Switching from advanced to default may result in queries and expressions that cannot be converted. In this case, a warning message asks if you want to continue to reset to default settings. +Switching from advanced to default may result in queries and expressions that can't be converted. +In this case, a warning message asks if you want to continue to reset to default settings. {{< docs/shared lookup="alerts/configure-alert-rule-name.md" source="grafana" version="" >}} @@ -169,9 +173,10 @@ You can toggle between **Default** and **Advanced** options. If the [Default vs. {{< collapse title="Advanced options" >}} 1. Select a data source. -1. From the **Options** dropdown, specify a [time range](ref:time-units-and-relative-ranges). +1. From the **Options** drop-down menu, specify a [time range](ref:time-units-and-relative-ranges). - Note that Grafana Alerting only supports fixed relative time ranges, for example, `now-24hr: now`. It does not support absolute time ranges: `2021-12-02 00:00:00 to 2021-12-05 23:59:592` or semi-relative time ranges: `now/d to: now`. + Note that Grafana Alerting only supports fixed relative time ranges, for example, `now-24hr: now`. + It doesn't support absolute time ranges: `2021-12-02 00:00:00 to 2021-12-05 23:59:592` or semi-relative time ranges: `now/d to: now`. 1. Add a query. @@ -222,7 +227,8 @@ To do this, you need to make sure that your alert rule is in the right evaluatio The pending period is the period in which an alert rule can be in breach of the condition until it fires. - Once a condition is met, the alert goes into the **Pending** state. If the condition remains active for the duration specified, the alert transitions to the **Firing** state, else it reverts to the **Normal** state. + After a condition is met, the alert goes into the **Pending** state. + If the condition remains active for the duration specified, the alert transitions to the **Firing** state, else it reverts to the **Normal** state. 1. Turn on pause alert notifications, if required. @@ -253,7 +259,7 @@ Complete the following steps to set up notifications. 1. Choose this option to select an existing [contact point](ref:contact-points). - All notifications for this alert rule are sent to this contact point automatically and notification policies are not used. + All notifications for this alert rule are sent to this contact point automatically and notification policies aren't used. 1. You can also optionally select a mute timing as well as groupings and timings to define when not to send notifications. @@ -261,7 +267,8 @@ Complete the following steps to set up notifications. 1. Choose this option to use the [notification policy tree](ref:notification-policies) to handle alert notifications. - All notifications for this alert rule are managed by the notification policy tree, which routes alerts based on their labels. If an alert does not match a specific policy, the default notification policy applies, ensuring all alerts are handled. + All notifications for this alert rule are managed by the notification policy tree, which routes alerts based on their labels. + If an alert doesn't match a specific policy, the default notification policy applies, ensuring all alerts are handled. 1. Preview your alert instance routing set up. diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index f40ad6a1766..7ceb0dc3460 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -226,6 +226,7 @@ Experimental features might be changed or removed without prior notice. | `datasourceConnectionsTab` | Shows defined connections for a data source in the plugins detail page | | `newLogsPanel` | Enables the new logs panel in Explore | | `pluginsCDNSyncLoader` | Load plugins from CDN synchronously | +| `assetSriChecks` | Enables SRI checks for Grafana JavaScript assets | ## Development feature toggles diff --git a/package.json b/package.json index 1c9393c4bd9..3708b2b49ac 100644 --- a/package.json +++ b/package.json @@ -248,6 +248,7 @@ "webpack-livereload-plugin": "3.0.2", "webpack-manifest-plugin": "5.0.0", "webpack-merge": "6.0.1", + "webpack-subresource-integrity": "^5.2.0-rc.1", "webpackbar": "^7.0.0", "yaml": "^2.0.0", "yargs": "^17.5.1" diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 87c5ff7c0d1..7e3ac0d6dba 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -254,4 +254,5 @@ export interface FeatureToggles { alertingRuleVersionHistoryRestore?: boolean; newShareReportDrawer?: boolean; rendererDisableAppPluginsPreload?: boolean; + assetSriChecks?: boolean; } diff --git a/packages/grafana-eslint-rules/README.md b/packages/grafana-eslint-rules/README.md index 2f73d46a529..e57e2ec09a1 100644 --- a/packages/grafana-eslint-rules/README.md +++ b/packages/grafana-eslint-rules/README.md @@ -156,7 +156,18 @@ const SearchTitle = ({ term }) => ( #### How to translate props or attributes -Right now, we only check if a string is wrapped up by the `Trans` tag. We currently do not apply this rule to props, attributes or similar, but we also ask for them to be translated with the `t()` function. +This rule checks if a string is wrapped up by the `Trans` tag, or if certain props contain untranslated strings. +We ask for such props to be translated with the `t()` function. + +The below props are checked for untranslated strings: + +- `label` +- `description` +- `placeholder` +- `aria-label` +- `title` +- `text` +- `tooltip` ```tsx // Bad ❌ @@ -168,3 +179,50 @@ return ; ``` Check more info about how translations work in Grafana in [Internationalization.md](https://github.com/grafana/grafana/blob/main/contribute/internationalization.md) + +### `no-translation-top-level` + +Ensure that `t()` translation method is not used at the top level of a file, outside of a component of method. +This is to prevent calling the translation method before it's been instantiated. + +This does not cause an error if a file is lazily loaded, but refactors can cause errors, and it can cause problems in tests. +Fix the + +```tsx +// Bad ❌ +const someTranslatedText = t('some.key', 'Some text'); +const SomeComponent = () => { + return
; +}; + +// Good ✅ +const SomeComponent = () => { + const someTranslatedText = t('some.key', 'Some text'); + return
; +}; + +// Bad ❌ +const someConfigThatHasToBeShared = [{ foo: t('some.key', 'Some text') }]; +const SomeComponent = () => { + return ( +
+ {someConfigThatHasToBeShared.map((cfg) => { + return
{cfg.foo}
; + })} +
+ ); +}; + +// Good ✅ +const someConfigThatHasToBeShared = () => [{ foo: t('some.key', 'Some text') }]; +const SomeComponent = () => { + const configs = someConfigThatHasToBeShared(); + return ( +
+ {configs.map((cfg) => { + return
{cfg.foo}
; + })} +
+ ); +}; +``` diff --git a/packages/grafana-eslint-rules/index.cjs b/packages/grafana-eslint-rules/index.cjs index 21c3bfa739e..11be6258993 100644 --- a/packages/grafana-eslint-rules/index.cjs +++ b/packages/grafana-eslint-rules/index.cjs @@ -2,6 +2,7 @@ const noAriaLabelSelectors = require('./rules/no-aria-label-e2e-selectors.cjs'); const noBorderRadiusLiteral = require('./rules/no-border-radius-literal.cjs'); const noUnreducedMotion = require('./rules/no-unreduced-motion.cjs'); const noUntranslatedStrings = require('./rules/no-untranslated-strings.cjs'); +const noTranslationTopLevel = require('./rules/no-translation-top-level.cjs'); const themeTokenUsage = require('./rules/theme-token-usage.cjs'); module.exports = { @@ -11,5 +12,6 @@ module.exports = { 'no-border-radius-literal': noBorderRadiusLiteral, 'theme-token-usage': themeTokenUsage, 'no-untranslated-strings': noUntranslatedStrings, + 'no-translation-top-level': noTranslationTopLevel, }, }; diff --git a/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs b/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs new file mode 100644 index 00000000000..2290a980cf1 --- /dev/null +++ b/packages/grafana-eslint-rules/rules/no-translation-top-level.cjs @@ -0,0 +1,59 @@ +// @ts-check +const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/utils'); + +/** + * @typedef {import('@typescript-eslint/utils').TSESTree.Node} Node + * @typedef {import('@typescript-eslint/utils').TSESLint.RuleContext} RuleContext + */ + +const createRule = ESLintUtils.RuleCreator( + (name) => `https://github.com/grafana/grafana/blob/main/packages/grafana-eslint-rules/README.md#${name}` +); + +/** + * @param {RuleContext} context + * @param {Node} node + * @returns {boolean} + */ +const isInFunction = (context, node) => { + const ancestors = context.sourceCode.getAncestors(node); + return ancestors.some((anc) => { + return [ + AST_NODE_TYPES.ArrowFunctionExpression, + AST_NODE_TYPES.FunctionDeclaration, + AST_NODE_TYPES.FunctionExpression, + AST_NODE_TYPES.ClassDeclaration, + ].includes(anc.type); + }); +}; + +const noTranslationTopLevel = createRule({ + create(context) { + return { + CallExpression(node) { + if (node.callee.type === AST_NODE_TYPES.Identifier && node.callee.name === 't') { + if (!isInFunction(context, node)) { + context.report({ + node, + messageId: 'noMethodOutsideComponent', + }); + } + } + }, + }; + }, + name: 'no-translation-top-level', + meta: { + type: 'suggestion', + docs: { + description: 'Do not use translation functions outside of components', + }, + messages: { + noMethodOutsideComponent: 'Do not use the t() function outside of a component or function', + }, + schema: [], + }, + defaultOptions: [], +}); + +module.exports = noTranslationTopLevel; diff --git a/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js b/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js new file mode 100644 index 00000000000..88326b500d4 --- /dev/null +++ b/packages/grafana-eslint-rules/tests/no-translation-top-level.test.js @@ -0,0 +1,58 @@ +import { RuleTester } from 'eslint'; + +import noTranslationTopLevel from '../rules/no-translation-top-level.cjs'; + +RuleTester.setDefaultConfig({ + languageOptions: { + ecmaVersion: 2018, + sourceType: 'module', + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + }, +}); + +const expectedErrorMessage = 'Do not use the t() function outside of a component or function'; + +const ruleTester = new RuleTester(); + +ruleTester.run('eslint no-translation-top-level', noTranslationTopLevel, { + valid: [ + { + code: ` +function Component() { + return
{t('some.key', 'Some text')}
; +} + `, + }, + { + code: `const foo = () => t('some.key', 'Some text');`, + }, + { + code: `const foo = ttt('some.key', 'Some text');`, + }, + { + code: `class Component { +render() { + return t('some.key', 'Some text'); +} +}`, + }, + ], + invalid: [ + { + code: `const thing = t('some.key', 'Some text');`, + errors: [{ message: expectedErrorMessage }], + }, + { + code: `const things = [t('some.key', 'Some text')];`, + errors: [{ message: expectedErrorMessage }], + }, + { + code: `const objectThings = [{foo: t('some.key', 'Some text')}];`, + errors: [{ message: expectedErrorMessage }], + }, + ], +}); diff --git a/packages/grafana-schema/src/schema/dashboard/v2alpha0/examples.ts b/packages/grafana-schema/src/schema/dashboard/v2alpha0/examples.ts index c12f1cc73f7..fe92defe024 100644 --- a/packages/grafana-schema/src/schema/dashboard/v2alpha0/examples.ts +++ b/packages/grafana-schema/src/schema/dashboard/v2alpha0/examples.ts @@ -18,6 +18,18 @@ export const handyTestingSchema: DashboardV2Spec = { timezone: 'UTC', to: 'now', weekStart: 'monday', + quickRanges: [ + { + display: 'Last 6 hours', + from: 'now-6h', + to: 'now', + }, + { + display: 'Last 3 days', + from: 'now-3d', + to: 'now', + }, + ], }, annotations: [ { diff --git a/pkg/services/featuremgmt/codeowners.go b/pkg/services/featuremgmt/codeowners.go index 13520e2a02b..a201b5e4b23 100644 --- a/pkg/services/featuremgmt/codeowners.go +++ b/pkg/services/featuremgmt/codeowners.go @@ -13,6 +13,7 @@ const ( grafanaBackendServicesSquad codeowner = "@grafana/grafana-backend-services-squad" grafanaSearchAndStorageSquad codeowner = "@grafana/search-and-storage" grafanaPluginsPlatformSquad codeowner = "@grafana/plugins-platform-backend" + grafanaFrontendOpsWG codeowner = "@grafana/frontend-ops" grafanaAsCodeSquad codeowner = "@grafana/grafana-as-code" identityAccessTeam codeowner = "@grafana/identity-access-team" grafanaObservabilityLogsSquad codeowner = "@grafana/observability-logs" diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index f3b1da31b5c..27da9533003 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1775,6 +1775,13 @@ var ( HideFromDocs: true, FrontendOnly: true, }, + { + Name: "assetSriChecks", + Description: "Enables SRI checks for Grafana JavaScript assets", + Stage: FeatureStageExperimental, + Owner: grafanaFrontendOpsWG, + FrontendOnly: true, + }, } ) diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index fc0b0c8dfb7..ace09d69f56 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -235,3 +235,4 @@ alertingJiraIntegration,experimental,@grafana/alerting-squad,false,false,true alertingRuleVersionHistoryRestore,GA,@grafana/alerting-squad,false,false,true newShareReportDrawer,experimental,@grafana/sharing-squad,false,false,false rendererDisableAppPluginsPreload,experimental,@grafana/sharing-squad,false,false,true +assetSriChecks,experimental,@grafana/frontend-ops,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 56c3a7d53da..d651c7290f6 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -950,4 +950,8 @@ const ( // FlagRendererDisableAppPluginsPreload // Disable pre-loading app plugins when the request is coming from the renderer FlagRendererDisableAppPluginsPreload = "rendererDisableAppPluginsPreload" + + // FlagAssetSriChecks + // Enables SRI checks for Grafana JavaScript assets + FlagAssetSriChecks = "assetSriChecks" ) diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 3f5045123d9..b093cb5dd21 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -598,6 +598,22 @@ "codeowner": "@grafana/grafana-frontend-platform" } }, + { + "metadata": { + "name": "assetSriChecks", + "resourceVersion": "1739984409734", + "creationTimestamp": "2025-02-19T15:56:59Z", + "annotations": { + "grafana.app/updatedTimestamp": "2025-02-19 17:00:09.734088 +0000 UTC" + } + }, + "spec": { + "description": "Enables SRI checks for Grafana JavaScript assets", + "stage": "experimental", + "codeowner": "@grafana/frontend-ops", + "frontend": true + } + }, { "metadata": { "name": "authAPIAccessTokenAuth", diff --git a/public/app/features/alerting/unified/api/featureDiscoveryApi.ts b/public/app/features/alerting/unified/api/featureDiscoveryApi.ts index a0d40e6c874..b6d8c812163 100644 --- a/public/app/features/alerting/unified/api/featureDiscoveryApi.ts +++ b/public/app/features/alerting/unified/api/featureDiscoveryApi.ts @@ -40,7 +40,7 @@ export const featureDiscoveryApi = alertingApi.injectEndpoints({ queryFn: async (rulesSourceIdentifier) => { const dataSourceUID = getDataSourceUID(rulesSourceIdentifier); if (!dataSourceUID) { - return { error: new Error(`Unable to find data source for ${rulesSourceIdentifier}`) }; + return { error: new Error(`Unable to find data source for ${JSON.stringify(rulesSourceIdentifier)}`) }; } if (dataSourceUID === GrafanaRulesSourceSymbol) { diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx index f8e85ba76d6..36d2d6f06d1 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.test.tsx @@ -10,6 +10,7 @@ import { AccessControlAction, FolderDTO } from 'app/types'; import { setupMswServer } from '../mockApi'; import { mockDataSource, mockFolder, mockRulerAlertingRule, mockRulerGrafanaRule } from '../mocks'; import { setupDataSources } from '../testSetup/datasources'; +import { GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource'; import { useFolder } from './useFolder'; import { useIsRuleEditable } from './useIsRuleEditable'; @@ -56,7 +57,9 @@ describe('useIsRuleEditable', () => { const wrapper = getProviderWrapper(); - const { result } = renderHook(() => useIsRuleEditable('grafana', mockRulerGrafanaRule()), { wrapper }); + const { result } = renderHook(() => useIsRuleEditable(GRAFANA_RULES_SOURCE_NAME, mockRulerGrafanaRule()), { + wrapper, + }); await waitFor(() => expect(result.current.loading).toBe(false)); expect(result.current.isRemovable).toBe(true); diff --git a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts index 50ca91b8188..017e008c02c 100644 --- a/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts +++ b/public/app/features/alerting/unified/hooks/useIsRuleEditable.ts @@ -3,7 +3,6 @@ import { RulerRuleDTO } from 'app/types/unified-alerting-dto'; import { featureDiscoveryApi } from '../api/featureDiscoveryApi'; import { getRulesPermissions } from '../utils/access-control'; -import { getDatasourceAPIUid } from '../utils/datasource'; import { isGrafanaRulerRule } from '../utils/rules'; import { useFolder } from './useFolder'; @@ -13,11 +12,16 @@ interface ResultBag { isEditable?: boolean; isRemovable?: boolean; loading: boolean; + error?: unknown; } export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): ResultBag { - const { currentData: dsFeatures, isLoading } = featureDiscoveryApi.endpoints.discoverDsFeatures.useQuery({ - uid: getDatasourceAPIUid(rulesSourceName), + const { + currentData: dsFeatures, + isLoading, + error, + } = featureDiscoveryApi.endpoints.discoverDsFeatures.useQuery({ + rulesSourceName, }); const folderUID = rule && isGrafanaRulerRule(rule) ? rule.grafana_alert.namespace_uid : undefined; @@ -25,6 +29,17 @@ export function useIsRuleEditable(rulesSourceName: string, rule?: RulerRuleDTO): const rulePermission = getRulesPermissions(rulesSourceName); const { folder, loading } = useFolder(folderUID); + // handle discovery and data source errors + if (error) { + return { + isEditable: false, + isRemovable: false, + loading: false, + isRulerAvailable: false, + error, + }; + } + if (!rule) { return { isEditable: false, isRemovable: false, loading: false }; } diff --git a/public/app/features/alerting/unified/mocks.ts b/public/app/features/alerting/unified/mocks.ts index 353a9531b9d..8e2701e0e04 100644 --- a/public/app/features/alerting/unified/mocks.ts +++ b/public/app/features/alerting/unified/mocks.ts @@ -57,6 +57,7 @@ import { import { DashboardSearchItem, DashboardSearchItemType } from '../../search/types'; import { SimpleConditionIdentifier } from './components/rule-editor/query-and-alert-condition/SimpleCondition'; +import { GRAFANA_RULES_SOURCE_NAME } from './utils/datasource'; import { parsePromQLStyleMatcherLooseSafe } from './utils/matchers'; let nextDataSourceId = 1; @@ -686,7 +687,7 @@ export function getGrafanaRule(override?: Partial, rulerOverride?: namespace: { groups: [], name: 'Grafana', - rulesSource: 'grafana', + rulesSource: GRAFANA_RULES_SOURCE_NAME, }, rulerRule: mockGrafanaRulerRule(rulerOverride), ...override, diff --git a/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx b/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx index 00641db1eda..3ca54fdd9aa 100644 --- a/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx +++ b/public/app/features/alerting/unified/rule-editor/ExistingRuleEditor.tsx @@ -17,17 +17,30 @@ interface ExistingRuleEditorProps { } export function ExistingRuleEditor({ identifier, prefill }: ExistingRuleEditorProps) { - const { - loading: loadingAlertRule, - result: ruleWithLocation, - error, - } = useRuleWithLocation({ ruleIdentifier: identifier }); const [queryParams] = useQueryParams(); const isManualRestore = Boolean(queryParams.isManualRestore); - const ruleSourceName = ruleId.ruleIdentifierToRuleSourceName(identifier); + const { + loading: loadingAlertRule, + result: ruleWithLocation, + error: fetchRuleError, + } = useRuleWithLocation({ ruleIdentifier: identifier }); - const { isEditable, loading: loadingEditable } = useIsRuleEditable(ruleSourceName, ruleWithLocation?.rule); + const ruleSourceName = ruleId.ruleIdentifierToRuleSourceName(identifier); + const { + isEditable, + loading: loadingEditable, + error: errorEditable, + } = useIsRuleEditable(ruleSourceName, ruleWithLocation?.rule); + + // error handling for fetching rule and rule RBAC + if (fetchRuleError || errorEditable) { + return ( + + {stringifyErrorLike(errorEditable ?? fetchRuleError)} + + ); + } const loading = loadingAlertRule || loadingEditable; @@ -35,14 +48,6 @@ export function ExistingRuleEditor({ identifier, prefill }: ExistingRuleEditorPr return ; } - if (error) { - return ( - - {stringifyErrorLike(error)} - - ); - } - if (!ruleWithLocation && !loading) { return Sorry! This rule does not exist.; } diff --git a/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx b/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx index 1781ab75d2b..8f6538f60b7 100644 --- a/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx +++ b/public/app/features/alerting/unified/rule-editor/RuleEditorExisting.test.tsx @@ -4,7 +4,6 @@ import { render, screen } from 'test/test-utils'; import { contextSrv } from 'app/core/services/context_srv'; import { setFolderResponse } from 'app/features/alerting/unified/mocks/server/configure'; -import { MIMIR_DATASOURCE_UID } from 'app/features/alerting/unified/mocks/server/constants'; import { captureRequests } from 'app/features/alerting/unified/mocks/server/events'; import { DashboardSearchItemType } from 'app/features/search/types'; import { AccessControlAction } from 'app/types'; @@ -12,6 +11,7 @@ import { AccessControlAction } from 'app/types'; import { setupMswServer } from '../mockApi'; import { grantUserPermissions, mockDataSource, mockFolder } from '../mocks'; import { grafanaRulerRule } from '../mocks/grafanaRulerApi'; +import { MIMIR_DATASOURCE_UID } from '../mocks/server/constants'; import { setupDataSources } from '../testSetup/datasources'; import { Annotation } from '../utils/constants'; @@ -151,3 +151,15 @@ describe('RuleEditor grafana managed rules', () => { expect(postBody.interval).toBe('12m'); }); }); + +describe('Data source managed rules', () => { + beforeEach(() => { + jest.clearAllMocks(); + grantUserPermissions([AccessControlAction.AlertingRuleExternalRead, AccessControlAction.AlertingRuleExternalWrite]); + }); + + it('should show an error if the data source does not exist', async () => { + renderRuleEditor('cri%24grafana-cloudd%24delete me%24delete me 3%24recording_rule_delete_2%24-476183141'); + expect(await screen.findByText(/unable to find data source/i)).toBeInTheDocument(); + }); +}); diff --git a/public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx b/public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx index 558d2294899..3a4a0972a05 100644 --- a/public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx +++ b/public/app/features/dashboard-scene/scene/layout-tabs/TabItem.tsx @@ -1,16 +1,15 @@ -import { ReactNode } from 'react'; - import { SceneObjectState, SceneObjectBase, sceneGraph, VariableDependencyConfig, SceneObject } from '@grafana/scenes'; import { t } from 'app/core/internationalization'; import { OptionsPaneCategoryDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneCategoryDescriptor'; +import { getDefaultVizPanel } from '../../utils/utils'; import { ResponsiveGridLayoutManager } from '../layout-responsive-grid/ResponsiveGridLayoutManager'; import { BulkActionElement } from '../types/BulkActionElement'; import { DashboardLayoutManager } from '../types/DashboardLayoutManager'; import { EditableDashboardElement, EditableDashboardElementInfo } from '../types/EditableDashboardElement'; import { LayoutParent } from '../types/LayoutParent'; -import { getEditOptions, renderActions } from './TabItemEditor'; +import { getEditOptions } from './TabItemEditor'; import { TabItemRenderer } from './TabItemRenderer'; import { TabItems } from './TabItems'; import { TabsLayoutManager } from './TabsLayoutManager'; @@ -56,14 +55,6 @@ export class TabItem return getEditOptions(this); } - public renderActions(): ReactNode { - return renderActions(this); - } - - public getParentLayout(): TabsLayoutManager { - return sceneGraph.getAncestor(this, TabsLayoutManager); - } - public onDelete() { const layout = sceneGraph.getAncestor(this, TabsLayoutManager); layout.removeTab(this); @@ -73,7 +64,43 @@ export class TabItem return new TabItems(items.filter((item) => item instanceof TabItem)); } + public onAddPanel(panel = getDefaultVizPanel()) { + this.getLayout().addPanel(panel); + } + + public onAddTabBefore() { + this._getParentLayout().addTabBefore(this); + } + + public onAddTabAfter() { + this._getParentLayout().addTabAfter(this); + } + + public onMoveLeft() { + this._getParentLayout().moveTabLeft(this); + } + + public onMoveRight() { + this._getParentLayout().moveTabRight(this); + } + + public isFirstTab(): boolean { + return this._getParentLayout().isFirstTab(this); + } + + public isLastTab(): boolean { + return this._getParentLayout().isLastTab(this); + } + public onChangeTitle(title: string) { this.setState({ title }); } + + public getParentLayout(): TabsLayoutManager { + return this._getParentLayout(); + } + + private _getParentLayout(): TabsLayoutManager { + return sceneGraph.getAncestor(this, TabsLayoutManager); + } } diff --git a/public/app/features/dashboard-scene/scene/layout-tabs/TabItemEditor.tsx b/public/app/features/dashboard-scene/scene/layout-tabs/TabItemEditor.tsx index 7d0e8c70b8c..2769620060e 100644 --- a/public/app/features/dashboard-scene/scene/layout-tabs/TabItemEditor.tsx +++ b/public/app/features/dashboard-scene/scene/layout-tabs/TabItemEditor.tsx @@ -1,10 +1,11 @@ -import { ReactNode, useMemo } from 'react'; +import { useMemo } from 'react'; -import { Button, Input } from '@grafana/ui'; +import { Input } from '@grafana/ui'; import { t } from 'app/core/internationalization'; import { OptionsPaneCategoryDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneCategoryDescriptor'; import { OptionsPaneItemDescriptor } from 'app/features/dashboard/components/PanelEditor/OptionsPaneItemDescriptor'; +import { EditPaneHeader } from '../../edit-pane/EditPaneHeader'; import { useLayoutCategory } from '../layouts-shared/DashboardLayoutSelector'; import { TabItem } from './TabItem'; @@ -12,9 +13,12 @@ import { TabItem } from './TabItem'; export function getEditOptions(model: TabItem): OptionsPaneCategoryDescriptor[] { const tabOptions = useMemo(() => { return new OptionsPaneCategoryDescriptor({ - title: t('dashboard.tabs-layout.tab-options.title', 'Tab options'), + title: '', id: 'tab-options', - isOpenDefault: true, + isOpenable: false, + renderTitle: () => ( + model.onDelete()} /> + ), }).addItem( new OptionsPaneItemDescriptor({ title: t('dashboard.tabs-layout.tab-options.title-option', 'Title'), @@ -29,15 +33,6 @@ export function getEditOptions(model: TabItem): OptionsPaneCategoryDescriptor[] return [tabOptions, layoutOptions]; } -export function renderActions(tab: TabItem): ReactNode { - return ( - <> -