Skip to content
Snippets Groups Projects
Unverified Commit 279b3721 authored by Mahatthana (Kelvin) Nomsawadi's avatar Mahatthana (Kelvin) Nomsawadi Committed by GitHub
Browse files

Add ESLint rule for Metabase strings (#38553)

* Fix case where Metabase links are render inside template strings

* Add `no-literal-metabase-strings` ESLint rule

* Fix all Metabase string errors

* Address review: Fix the rule

The rule was checking if we have imported the selector
`getApplicationName` then ignore all Metabase strings. This is different
than `no-unconditional-metabase-links-render` because in that rule, when
fixed the Documentation URLs are still in the file, but in this rule,
when fixed, there should be no Metabase strings left in the file.

* Fix errors from the new lint rule
parent 985ef5d0
No related branches found
No related tags found
No related merge requests found
Showing
with 297 additions and 7 deletions
......@@ -161,13 +161,23 @@
{
"files": ["*.js", "*.jsx", "*.ts", "*.tsx"],
"rules": {
"no-unconditional-metabase-links-render": "error"
"no-unconditional-metabase-links-render": "error",
"no-literal-metabase-strings": "error"
}
},
{
"files": ["*.unit.spec.*", "frontend/src/metabase/admin/**/*"],
"files": [
"*.unit.spec.*",
"frontend/src/metabase/admin/**/*",
"frontend/src/metabase/setup/**/*",
"frontend/lint/**/*",
"*.stories.*",
"e2e/**/*",
"**/tests/*"
],
"rules": {
"no-unconditional-metabase-links-render": "off"
"no-unconditional-metabase-links-render": "off",
"no-literal-metabase-strings": "off"
}
},
{
......
......@@ -74,12 +74,15 @@ export const ImpersonationModalView = ({
const impersonationUsesUsers = database.engine === "redshift";
const modalTitle = impersonationUsesUsers
? t`Map a Metabase user attribute to database users`
? // eslint-disable-next-line no-literal-metabase-strings -- Metabase settings
t`Map a Metabase user attribute to database users`
: t`Map a user attribute to database roles`;
const modalMessage = impersonationUsesUsers
? t`When the person runs a query (including native queries), Metabase will impersonate the privileges of the database user you associate with the user attribute.`
: t`When the person runs a query (including native queries), Metabase will impersonate the privileges of the database role you associate with the user attribute.`;
? // eslint-disable-next-line no-literal-metabase-strings -- Metabase settings
t`When the person runs a query (including native queries), Metabase will impersonate the privileges of the database user you associate with the user attribute.`
: // eslint-disable-next-line no-literal-metabase-strings -- Metabase settings
t`When the person runs a query (including native queries), Metabase will impersonate the privileges of the database role you associate with the user attribute.`;
return (
<ImpersonationModalViewRoot>
......
......@@ -16,8 +16,10 @@ export const ImpersonationWarning = ({
const databaseUser = database.details.user;
const isRedshift = database.engine === "redshift";
// eslint-disable-next-line no-literal-metabase-strings -- Metabase settings
const emptyText = t`Make sure the main database credential has access to everything different user groups may need access to. It's what Metabase uses to sync table information.`;
// eslint-disable-next-line no-literal-metabase-strings -- Metabase settings
const redshiftWarning = jt`You’re connecting Metabase to the ${(
<BoldCode key="2" size={13}>
{database.name}
......@@ -32,6 +34,7 @@ export const ImpersonationWarning = ({
</BoldCode>
)} must be a superuser in Redshift.`;
// eslint-disable-next-line no-literal-metabase-strings -- Metabase settings
const regularWarning = jt`${(
<BoldCode key="1" size={13}>
{databaseUser}
......
......@@ -6,6 +6,7 @@ import { Icon } from "metabase/ui";
const OpenInMetabase = ({ ...props }) => (
<Link {...props} className="link flex align-center" target="_blank">
<Icon name="external" className="mr1" />
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
{t`Open in Metabase`}
</Link>
);
......
......@@ -42,6 +42,7 @@ const UnsubscribeUserForm = ({ user, onUnsubscribe, onClose }) => {
{t`This will delete any dashboard subscriptions or alerts ${user.common_name} has created, and remove them as a recipient from any other subscriptions or alerts.`}
</ModalMessage>
<ModalMessage>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
{t`This does not affect email distribution lists that are managed outside of Metabase.`}
</ModalMessage>
</ModalContent>
......
......@@ -39,6 +39,7 @@ const DeprecationSection = () => {
key="link"
to={`/collection/${auditCollection.id}`}
>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
{t`Metabase Analytics Collection`}
</Link>
)}
......
......@@ -89,6 +89,7 @@ const SettingsSAMLForm = ({ elements = [], settingValues = {}, onSubmit }) => {
</SAMLFormCaption>
<SAMLFormSection>
<h3 className="mb0">{t`Configure your identity provider (IdP)`}</h3>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
<p className="mb4 mt1 text-medium">{t`Your identity provider will need the following info about Metabase.`}</p>
<FormTextInput
......@@ -123,7 +124,9 @@ const SettingsSAMLForm = ({ elements = [], settingValues = {}, onSubmit }) => {
</SAMLFormSection>
<SAMLFormSection>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
<h3 className="mb0">{t`Tell Metabase about your identity provider`}</h3>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
<p className="mb4 mt1 text-medium">{t`Metabase will need the following info about your provider.`}</p>
<Stack gap="md">
<FormTextInput
......@@ -173,6 +176,7 @@ const SettingsSAMLForm = ({ elements = [], settingValues = {}, onSubmit }) => {
<SAMLFormSection wide>
<h3 className="mb0">{t`Synchronize group membership with your SSO`}</h3>
<p className="mb4 mt1 text-medium">
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
{t`To enable this, you'll need to create mappings to tell Metabase which group(s) your users should
be added to based on the SSO group they're in.`}
</p>
......
......@@ -46,6 +46,7 @@ const DatabaseCacheTimeField = () => {
const DatabaseCacheTimeDescription = (): JSX.Element => {
return (
<div>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
{jt`How long to keep question results. By default, Metabase will use the value you supply on the ${(
<Link key="link" to="/admin/settings/caching">
{t`cache settings page`}
......
......@@ -32,6 +32,7 @@ export function CollectionInstanceAnalyticsIcon({
<Icon
{...iconProps}
name={collectionType.icon}
// eslint-disable-next-line no-literal-metabase-strings -- Metabase analytics
tooltip={t`This is a read-only Metabase Analytics ${collectionIconTooltipNameMap[entity]}.`}
data-testid="instance-analytics-collection-marker"
/>
......
......@@ -30,6 +30,7 @@ export const EmbeddingAppSameSiteCookieDescription = () => {
return (
<Stack spacing="sm">
{shouldDisplayNote && <AuthorizedOriginsNote />}
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
<Text>{t`Determines whether or not cookies are allowed to be sent on cross-site requests. You’ll likely need to change this to None if your embedding application is hosted under a different domain than Metabase. Otherwise, leave it set to Lax, as it's more secure.`}</Text>
<Text>{jt`If you set this to None, you'll have to use HTTPS (unless you're just embedding locally), or browsers will reject the request. ${(
<ExternalLink key="learn-more" href={docsUrl}>
......
......@@ -45,6 +45,7 @@ if (hasPremiumFeature("embedding")) {
{
value: "strict",
name: t`Strict (not recommended)`,
// eslint-disable-next-line no-literal-metabase-strings -- Metabase settings
description: t`Never allows cookies to be sent on a cross-site request. Warning: this will prevent users from following external links to Metabase.`,
},
{
......
......@@ -7,8 +7,10 @@ export const SettingsCloudStoreLink = () => {
return (
<div>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
<Description>{t`Manage your Cloud account, including billing preferences and technical settings about this instance in your Metabase Store account.`}</Description>
<Link href={url}>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
{t`Go to the Metabase Store`}
<LinkIcon name="external" />
</Link>
......
......@@ -10,8 +10,10 @@ export const BillingGoToStore = () => {
return (
<>
<SectionHeader>{t`Billing`}</SectionHeader>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
<Text color="text-md">{t`Manage your Cloud account, including billing preferences, in your Metabase Store account.`}</Text>
<StoreButtonLink href={url}>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
{t`Go to the Metabase Store`}
</StoreButtonLink>
</>
......
......@@ -5,6 +5,7 @@ import { SettingsApi, StoreApi } from "metabase/services";
export const LICENSE_ACCEPTED_URL_HASH = "#activated";
const INVALID_TOKEN_ERROR = t`This token doesn't seem to be valid. Double-check it, then contact support if you think it should be working.`;
// eslint-disable-next-line no-literal-metabase-strings -- Metabase settings
const UNABLE_TO_VALIDATE_TOKEN = t`We're having trouble validating your token. Please double-check that your instance can connect to Metabase's servers.`;
export type TokenStatus = {
......
......@@ -18,6 +18,7 @@ export const getLogoUrl = (state: EnterpriseState) =>
export const getLoadingMessage = (state: EnterpriseState) =>
LOADING_MESSAGE_BY_SETTING[getSetting(state, "loading-message")];
// eslint-disable-next-line no-literal-metabase-strings -- This is a Metabase string we want to keep. It's used for comparison.
const DEFAULT_APPLICATION_NAME = "Metabase";
export const getIsWhiteLabeling = (state: EnterpriseState) =>
getApplicationName(state) !== DEFAULT_APPLICATION_NAME;
......
......@@ -58,6 +58,7 @@ export const HelpLinkSettings = ({
<Stack>
<Radio.Group value={helpLinkSetting} onChange={handleRadioChange}>
<Stack>
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
<Radio label={t`Link to Metabase help`} value="metabase" />
<Radio label={t`Hide it`} value="hidden" />
<Radio label={t`Go to a custom destination...`} value="custom" />
......
......@@ -2,6 +2,7 @@ import { t, jt } from "ttag";
import { Anchor, Popover, Stack, Text } from "metabase/ui";
export function MetabaseLinksToggleDescription() {
// eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */
return jt`Control the visibility of links to Metabase documentation and Metabase references in your instance. ${(
<Popover key="popover" position="top-start">
<Popover.Target>
......@@ -12,6 +13,7 @@ export function MetabaseLinksToggleDescription() {
<Popover.Dropdown>
<Stack p="md" spacing="sm" maw={420}>
<Text size="sm">
{/* eslint-disable-next-line no-literal-metabase-strings -- Metabase settings */}
{t`This affects all links in the product experience (outside of the admin panel) that point to Metabase.com URLs.`}
</Text>
<Text size="sm">
......
/**
* ------------------------------------------------------------------------------
* Rule Definition
* ------------------------------------------------------------------------------
*/
const ADD_COMMENT_MESSAGE =
'add comment to indicate the reason why this rule needs to be disabled.\nExample: "// eslint-disable-next-line no-literal-metabase-strings -- This string only shows for admins."';
const ERROR_MESSAGE =
"Metabase string must not be used directly.\n\nPlease import `getApplicationName` selector from `metabase/selectors/whitelabel` and use it to render the application name.\n\nOr " +
ADD_COMMENT_MESSAGE;
const LITERAL_METABASE_STRING_REGEX = /Metabase/;
// eslint-disable-next-line import/no-commonjs
module.exports = {
meta: {
type: "problem",
docs: {
description:
"Ensure that Metabase string literals are not used so whitelabeled names are used instead",
},
schema: [], // no options
},
create(context) {
return {
Literal(node) {
if (
typeof node.value !== "string" ||
["ExportNamedDeclaration", "ImportDeclaration"].includes(
node.parent.type,
)
) {
return;
}
if (LITERAL_METABASE_STRING_REGEX.exec(node.value)) {
context.report({
node,
message: ERROR_MESSAGE,
});
}
},
TemplateLiteral(node) {
const quasis = node.quasis;
quasis.forEach(quasi => {
if (LITERAL_METABASE_STRING_REGEX.exec(quasi.value.raw)) {
context.report({
node,
message: ERROR_MESSAGE,
});
}
});
},
JSXText(node) {
if (LITERAL_METABASE_STRING_REGEX.exec(node.value)) {
context.report({
node,
message: ERROR_MESSAGE,
});
}
},
Program() {
const comments = context.getSourceCode().getAllComments();
const ESLINT_DISABLE_BLOCK_REGEX =
/eslint-disable\s+no-literal-metabase-strings/;
const ESLINT_DISABLE_LINE_REGEX =
/eslint-disable-next-line\s+no-literal-metabase-strings/;
const ALLOWED_ESLINT_DISABLE_LINE_REGEX =
/eslint-disable-next-line\s+no-literal-metabase-strings -- \w+/;
comments.forEach(comment => {
if (ESLINT_DISABLE_BLOCK_REGEX.exec(comment.value)) {
const { start, end } = comment.loc;
context.report({
loc: {
start: { line: start.line - 1, column: start.column },
end: { line: end.line - 1, column: end.column },
},
message: "Please use inline disable with comments instead.",
});
}
if (
ESLINT_DISABLE_LINE_REGEX.exec(comment.value) &&
!ALLOWED_ESLINT_DISABLE_LINE_REGEX.exec(comment.value)
) {
context.report({
loc: comment.loc,
message: `Please ${ADD_COMMENT_MESSAGE}`,
});
}
});
},
};
},
};
......@@ -172,7 +172,10 @@ module.exports = {
TemplateLiteral(node) {
const quasis = node.quasis;
quasis.forEach(quasi => {
if (LITERAL_METABASE_URL_REGEX.exec(quasi.value.raw)) {
if (
LITERAL_METABASE_URL_REGEX.exec(quasi.value.raw) &&
!isGetShowMetabaseLinksSelectorImported
) {
context.report({
node,
message: ERROR_MESSAGE,
......
import { RuleTester } from "eslint";
import noLiteralMetabaseString from "../eslint-rules/no-literal-metabase-strings";
const ruleTester = new RuleTester({
parserOptions: {
ecmaVersion: 2015,
sourceType: "module",
ecmaFeatures: { jsx: true },
},
});
const VALID_CASES = [
{
// "Metabase in import sources"
code: `
import OpenInMetabase from "../components/OpenInMetabase";`,
},
{
// "Metabase in reexports"
code: `
export { MetabaseLinksToggleWidget } from "./MetabaseLinksToggleWidget";`,
},
{
// "No Metabase string",
code: `
const label = "some string"`,
},
{
// "Detect disabled rule next line",
code: `
function MyComponent() {
// eslint-disable-next-line no-literal-metabase-strings -- In admin settings
return <div>Metabase store {"interpolation"} something else</div>;
}`,
},
];
const INVALID_CASES = [
{
name: "Detect in literal strings",
code: `
const label = "Metabase blabla"`,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in literal strings",
code: `
function MyComponent() {
return <AnotherComponent label="Hello Metabase" />;
}`,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in literal strings",
code: `
import { getApplicationName } from 'metabase/selectors/whitelabel';
const label = "Metabase blabla"`,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in literal strings",
code: `
import { getApplicationName } from 'metabase/selectors/whitelabel';
function MyComponent() {
return <AnotherComponent label="Hello Metabase" />;
}`,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in template strings",
code: `
const label = t\`Metabase blabla\``,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in template strings",
code: `
function MyComponent() {
return <AnotherComponent label={t\`Hello Metabase\`} />;
}`,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in template strings",
code: `
import { getApplicationName } from 'metabase/selectors/whitelabel';
const label = t\`Metabase blabla\``,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in template strings",
code: `
import { getApplicationName } from 'metabase/selectors/whitelabel';
function MyComponent() {
return <AnotherComponent label={t\`Hello Metabase\`} />;
}`,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in JSX tags",
code: `
function MyComponent() {
return <div>Metabase store {"interpolation"} something else</div>;
}`,
error: /Metabase string must not be used directly./,
},
{
name: "Detect in JSX tags",
code: `
import { getApplicationName } from 'metabase/selectors/whitelabel';
function MyComponent() {
return <div>Metabase store {"interpolation"} something else</div>;
}`,
error: /Metabase string must not be used directly./,
},
{
name: "Detect disabled rule next line",
code: `
function MyComponent() {
// eslint-disable-next-line no-literal-metabase-strings
return <div>Metabase store {"interpolation"} something else</div>;
}`,
error:
/Please add comment to indicate the reason why this rule needs to be disabled./,
},
{
name: "Detect disabled rule block",
code: `
/* eslint-disable no-literal-metabase-strings */
function MyComponent() {
return <div>Metabase store {"interpolation"} something else</div>;
}`,
error: "Please use inline disable with comments instead.",
},
];
ruleTester.run("no-literal-metabase-strings", noLiteralMetabaseString, {
valid: VALID_CASES,
invalid: INVALID_CASES.map(invalidCase => {
return {
code: invalidCase.code,
errors: [
{
message: invalidCase.error,
},
],
};
}),
});
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment