From f8a5beb1755d97953e2c5161a94da08a32d83f1f Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Fri, 28 Aug 2026 20:49:29 -0300 Subject: [PATCH] Deduplicate System Console config isDisabled dependencies (#38124) * Add ESLint rule to dedupe System Console config dependencies Introduce @mattermost/no-redundant-admin-config-deps so admin_definition settings that depend on a bool parent via stateIsFalse do not repeat that parent's config/state isDisabled checks, matching the dependency-tree approach discussed on #38024. Co-authored-by: Jesse Hallam * Remove redundant System Console isDisabled config checks Drop duplicated parent config/state conditions from SAML and OAuth DCR settings now covered by bool parent dependencies, as enforced by @mattermost/no-redundant-admin-config-deps. Co-authored-by: Jesse Hallam * Attribute redundant admin config deps to closest parent Build the full bool-parent dependency tree before reporting, then pick the most specific immediate parent that implies a condition so grandparent-first declaration order cannot mislabel the fix target. Co-authored-by: Jesse Hallam * Fix eslint-plugin lint violations in admin config deps rule Resolve no-nested-ternary, wrap-regex, no-unused-vars, no-underscore-dangle, and lines-around-comment issues so Web App CI check-lint passes on the new rule and its tests. Co-authored-by: Jesse Hallam * Harden admin config deps rule for regex keys and parent parse Keep distinct regex patterns from collapsing to the same canonicalize key, and skip non-JSON identifier parent args instead of throwing. Co-authored-by: mattermost-code --------- Co-authored-by: Cursor Agent Co-authored-by: Jesse Hallam Co-authored-by: mattermost-code --- .../admin_console/admin_definition.tsx | 10 +- webapp/platform/eslint-plugin/README.md | 48 ++ webapp/platform/eslint-plugin/configs/base.js | 1 + webapp/platform/eslint-plugin/package.json | 1 + webapp/platform/eslint-plugin/rules/index.js | 2 + .../rules/no-redundant-admin-config-deps.js | 439 ++++++++++++++++++ .../no-redundant-admin-config-deps.test.js | 339 ++++++++++++++ 7 files changed, 832 insertions(+), 8 deletions(-) create mode 100644 webapp/platform/eslint-plugin/rules/no-redundant-admin-config-deps.js create mode 100644 webapp/platform/eslint-plugin/rules/no-redundant-admin-config-deps.test.js diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index f8d311fd9de8..7ee4b0ed5ebd 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -177,6 +177,8 @@ const SAML_SETTINGS_CANONICAL_ALGORITHM_C14N11 = 'Canonical1.1'; // - type: which define the widget type. // - label (and label_default): which define the main text of the setting. // - isDisabled: a function which receive current config, the state of the page and the license. +// Prefer depending on a parent bool setting (it.stateIsFalse('Parent.Key')) over repeating that +// parent's config/state checks; @mattermost/no-redundant-admin-config-deps enforces this. // - isHidden: a function which receive current config, the state of the page and the license. // // Custom Widget (extends from Widget): @@ -4619,7 +4621,6 @@ const AdminDefinition: AdminDefinitionType = { it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)), it.configIsFalse('GuestAccountsSettings', 'Enable'), it.stateIsFalse('SamlSettings.EnableSyncWithLdap'), - it.stateIsFalse('SamlSettings.Enable'), ), }, { @@ -4641,7 +4642,6 @@ const AdminDefinition: AdminDefinitionType = { help_text_markdown: false, isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)), - it.stateIsFalse('SamlSettings.Enable'), it.stateIsFalse('SamlSettings.EnableSyncWithLdap'), ), }, @@ -4795,7 +4795,6 @@ const AdminDefinition: AdminDefinitionType = { remove_action: removePrivateSamlCertificate, isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)), - it.stateIsFalse('SamlSettings.Enable'), it.stateIsFalse('SamlSettings.Encrypt'), ), }, @@ -4813,7 +4812,6 @@ const AdminDefinition: AdminDefinitionType = { remove_action: removePublicSamlCertificate, isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)), - it.stateIsFalse('SamlSettings.Enable'), it.stateIsFalse('SamlSettings.Encrypt'), ), }, @@ -4835,7 +4833,6 @@ const AdminDefinition: AdminDefinitionType = { label: defineMessage({id: 'admin.saml.signatureAlgorithmTitle', defaultMessage: 'Signature Algorithm'}), isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)), - it.stateIsFalse('SamlSettings.Encrypt'), it.stateIsFalse('SamlSettings.SignRequest'), ), options: [ @@ -4879,7 +4876,6 @@ const AdminDefinition: AdminDefinitionType = { ], isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.SAML)), - it.stateIsFalse('SamlSettings.Encrypt'), it.stateIsFalse('SamlSettings.SignRequest'), ), }, @@ -4948,7 +4944,6 @@ const AdminDefinition: AdminDefinitionType = { isDisabled: it.any( it.not(it.isSystemAdmin), it.stateIsFalse('SamlSettings.EnableAdminAttribute'), - it.stateIsFalse('SamlSettings.Enable'), ), }, { @@ -6060,7 +6055,6 @@ const AdminDefinition: AdminDefinitionType = { placeholder: defineMessage({id: 'admin.oauth.dcrRedirectURIAllowlistPlaceholder', defaultMessage: 'E.g.: https://*.example.com/**, https://app.example.com/callback'}), isDisabled: it.any( it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.INTEGRATIONS.INTEGRATION_MANAGEMENT)), - it.stateIsFalse('ServiceSettings.EnableOAuthServiceProvider'), it.stateIsFalse('ServiceSettings.EnableDynamicClientRegistration'), ), isHidden: it.licensedForFeature('Cloud'), diff --git a/webapp/platform/eslint-plugin/README.md b/webapp/platform/eslint-plugin/README.md index 9ba7dfa09ea4..8f830f7d6a38 100644 --- a/webapp/platform/eslint-plugin/README.md +++ b/webapp/platform/eslint-plugin/README.md @@ -28,6 +28,54 @@ export function someAction() { } ``` +### no-redundant-admin-config-deps + +Enforces that System Console settings in `admin_definition` files do not repeat `isDisabled` config/state checks already implied by a parent setting they depend on. + +When setting B disables itself with `it.stateIsFalse('A')`, A is a bool setting, and A already includes condition C (for example `it.configIsTrue('ClusterSettings', 'Enable')`), repeating C on B is redundant and tends to drift. Keep the dependency on A and omit the duplicated checks. Inheritance only follows bool parents (disabled bools are forced false on save). Permission and license helpers are not treated as config dependencies. + +The rule builds the full bool-parent dependency tree before reporting. If a setting lists both a parent and a grandparent, redundant conditions are attributed to the closest (most specific) parent — not whichever ancestor appears first in `isDisabled`. + +Examples of **incorrect** code for this rule: +```javascript +{ + type: 'bool', + key: 'EmailSettings.EnableEmailBatching', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.SendEmailNotifications'), + it.configIsTrue('ClusterSettings', 'Enable'), + ), +}, +{ + type: 'number', + key: 'EmailSettings.EmailBatchingBufferSize', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.SendEmailNotifications'), + it.stateIsFalse('EmailSettings.EnableEmailBatching'), + it.configIsTrue('ClusterSettings', 'Enable'), + ), +}, +``` + +Examples of **correct** code for this rule: +```javascript +{ + type: 'bool', + key: 'EmailSettings.EnableEmailBatching', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.SendEmailNotifications'), + it.configIsTrue('ClusterSettings', 'Enable'), + ), +}, +{ + type: 'number', + key: 'EmailSettings.EmailBatchingBufferSize', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.EnableEmailBatching'), + ), +}, +``` + ### use-external-link Ensures that any link which opens a URL outside of Mattermost using `target="_blank"` uses the `ExternalLink` component. diff --git a/webapp/platform/eslint-plugin/configs/base.js b/webapp/platform/eslint-plugin/configs/base.js index afd7e8adcbbe..e9cf9b48024e 100644 --- a/webapp/platform/eslint-plugin/configs/base.js +++ b/webapp/platform/eslint-plugin/configs/base.js @@ -52,6 +52,7 @@ const base = { }, rules: { '@mattermost/no-dispatch-getstate': 2, + '@mattermost/no-redundant-admin-config-deps': 2, '@mattermost/use-external-link': 2, '@stylistic/array-bracket-spacing': [ 2, diff --git a/webapp/platform/eslint-plugin/package.json b/webapp/platform/eslint-plugin/package.json index 4984d13aec1d..ddbfb8966b89 100644 --- a/webapp/platform/eslint-plugin/package.json +++ b/webapp/platform/eslint-plugin/package.json @@ -41,6 +41,7 @@ "scripts": { "check": "eslint . --quiet", "fix": "eslint . --quiet --fix", + "test": "node --test rules/*.test.js", "clean": "rm -rf node_modules" } } diff --git a/webapp/platform/eslint-plugin/rules/index.js b/webapp/platform/eslint-plugin/rules/index.js index f7c527f68fe5..1939fb252ced 100644 --- a/webapp/platform/eslint-plugin/rules/index.js +++ b/webapp/platform/eslint-plugin/rules/index.js @@ -2,9 +2,11 @@ // See LICENSE.txt for license information. import noDispatchGetState from './no-dispatch-getstate.js'; +import noRedundantAdminConfigDeps from './no-redundant-admin-config-deps.js'; import useExternalLink from './use-external-link.js'; export default { 'no-dispatch-getstate': noDispatchGetState, + 'no-redundant-admin-config-deps': noRedundantAdminConfigDeps, 'use-external-link': useExternalLink, }; diff --git a/webapp/platform/eslint-plugin/rules/no-redundant-admin-config-deps.js b/webapp/platform/eslint-plugin/rules/no-redundant-admin-config-deps.js new file mode 100644 index 000000000000..9d7117c74e5a --- /dev/null +++ b/webapp/platform/eslint-plugin/rules/no-redundant-admin-config-deps.js @@ -0,0 +1,439 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +/** + * Detects redundant System Console isDisabled conditions that are already + * implied by a parent config setting dependency. + * + * When setting B declares `it.stateIsFalse('A')` (or equivalent) and A is a bool + * setting that already disables itself under condition C, repeating C on B is + * redundant and can drift when A's dependencies change. + * + * Inheritance only follows bool parents: disabled bools are forced false on + * save (see getSettingValue), so stateIsFalse(parent) covers the parent's + * config checks. File/text parents can remain truthy while disabled, so their + * checks are not treated as transitive. + * + * The dependency tree is built fully before reporting. When a setting lists + * both a parent and a grandparent, redundant conditions are attributed to the + * closest (most specific) bool parent — not whichever ancestor appears first + * in the isDisabled list. + * + * Only config/state helpers are considered (stateIsFalse/True/Equals/..., + * configIsFalse/True, …). Permission and license checks are left alone — + * non-bool children still need their own permission gates. + */ + +const CONFIG_HELPERS = new Set([ + 'stateIsFalse', + 'stateIsTrue', + 'stateEquals', + 'stateEqualsOrDefault', + 'stateMatches', + 'configIsFalse', + 'configIsTrue', + 'configContains', + 'clientConfigIsTrue', + 'clientConfigIsFalse', +]); + +function isItMemberCall(node, name) { + return ( + node?.type === 'CallExpression' && + node.callee?.type === 'MemberExpression' && + !node.callee.computed && + node.callee.object?.type === 'Identifier' && + node.callee.object.name === 'it' && + node.callee.property?.type === 'Identifier' && + (name === undefined || node.callee.property.name === name) + ); +} + +function literalValue(node) { + if (!node) { + return undefined; + } + if (node.type === 'Literal') { + return node.value; + } + if (node.type === 'TemplateLiteral' && node.expressions.length === 0) { + return node.quasis[0]?.value?.cooked; + } + return undefined; +} + +function serializeArg(node) { + // Regex literals must be keyed by pattern+flags. literalValue returns a + // RegExp whose JSON.stringify is "{}", which would collapse distinct patterns. + if (node?.type === 'Literal' && node.regex) { + return `re:${JSON.stringify(node.regex.pattern)}:${JSON.stringify(node.regex.flags)}`; + } + + const value = literalValue(node); + if (value === undefined) { + if (node?.type === 'Identifier') { + return `id:${node.name}`; + } + if (node?.type === 'MemberExpression') { + return `mem:${serializeArg(node.object)}.${node.property?.name ?? '?'}`; + } + if (node?.type === 'UnaryExpression' && node.operator === '!' && node.argument?.type === 'Literal') { + return `!${JSON.stringify(node.argument.value)}`; + } + return null; + } + return JSON.stringify(value); +} + +function canonicalize(node) { + if (!node) { + return null; + } + + if (isItMemberCall(node, 'not') && node.arguments.length === 1) { + const inner = canonicalize(node.arguments[0]); + return inner ? `not:${inner}` : null; + } + + if (!isItMemberCall(node) || node.arguments.length === 0) { + return null; + } + + const name = node.callee.property.name; + if (!CONFIG_HELPERS.has(name)) { + return null; + } + + const args = node.arguments.map(serializeArg); + if (args.some((a) => a === null)) { + return null; + } + + return `${name}:${args.join(',')}`; +} + +function collectTopLevelConditions(node, out) { + if (!node) { + return; + } + + if (isItMemberCall(node, 'any')) { + for (const arg of node.arguments) { + collectTopLevelConditions(arg, out); + } + return; + } + + const key = canonicalize(node); + if (key) { + out.push({key, node}); + } +} + +function getProperty(objectExpression, name) { + if (objectExpression?.type !== 'ObjectExpression') { + return null; + } + for (const prop of objectExpression.properties) { + if (prop.type !== 'Property' || prop.computed) { + continue; + } + const keyName = getPropertyKeyName(prop.key); + if (keyName === name) { + return prop; + } + } + return null; +} + +function getPropertyKeyName(keyNode) { + if (keyNode.type === 'Identifier') { + return keyNode.name; + } + if (keyNode.type === 'Literal') { + return keyNode.value; + } + return null; +} + +function getSettingKey(objectExpression) { + const keyProp = getProperty(objectExpression, 'key'); + if (!keyProp) { + return null; + } + return literalValue(keyProp.value); +} + +function getSettingType(objectExpression) { + const typeProp = getProperty(objectExpression, 'type'); + if (!typeProp) { + return null; + } + return literalValue(typeProp.value); +} + +function parseSerializedStringArg(serialized) { + try { + const value = JSON.parse(serialized); + return typeof value === 'string' ? value : undefined; + } catch { + return undefined; + } +} + +function dependencyParentKeys(conditions) { + const parents = []; + for (const {key} of conditions) { + // stateIsFalse:"Section.Setting" or not:stateIsTrue:"Section.Setting" + let match = (/^stateIsFalse:(.*)$/).exec(key); + if (match) { + const value = parseSerializedStringArg(match[1]); + if (value !== undefined) { + parents.push(value); + } + continue; + } + match = (/^not:stateIsTrue:(.*)$/).exec(key); + if (match) { + const value = parseSerializedStringArg(match[1]); + if (value !== undefined) { + parents.push(value); + } + } + } + return parents; +} + +function isBoolSetting(key, settingsByKey) { + const defs = settingsByKey.get(key) || []; + return defs.some((def) => def.settingType === 'bool'); +} + +/** + * Build the full bool-parent dependency tree once, then derive inherited + * condition sets. Reporting uses this snapshot so parent attribution does not + * depend on isDisabled declaration order. + * + * @returns {Map, + * boolParents: string[], + * inheritedConditions: Set, + * ancestors: Set, + * }>} + */ +function buildDependencyTree(settingsByKey) { + const tree = new Map(); + + for (const [key, defs] of settingsByKey) { + const directConditions = new Set(); + const parentKeys = []; + let isBool = false; + + for (const def of defs) { + if (def.settingType === 'bool') { + isBool = true; + } + for (const {key: conditionKey} of def.conditions) { + directConditions.add(conditionKey); + } + for (const parentKey of def.parentKeys) { + if (!parentKeys.includes(parentKey)) { + parentKeys.push(parentKey); + } + } + } + + tree.set(key, { + isBool, + directConditions, + boolParents: parentKeys.filter((parentKey) => isBoolSetting(parentKey, settingsByKey)), + inheritedConditions: new Set(), + ancestors: new Set(), + }); + } + + const walked = new Set(); + + function walk(settingKey, visiting) { + const node = tree.get(settingKey); + if (!node || walked.has(settingKey)) { + return node; + } + if (visiting.has(settingKey)) { + return node; + } + visiting.add(settingKey); + + const inheritedConditions = new Set(); + const ancestors = new Set(); + + for (const parentKey of node.boolParents) { + ancestors.add(parentKey); + const parentNode = walk(parentKey, visiting); + if (!parentNode) { + continue; + } + for (const conditionKey of parentNode.directConditions) { + inheritedConditions.add(conditionKey); + } + for (const conditionKey of parentNode.inheritedConditions) { + inheritedConditions.add(conditionKey); + } + for (const ancestorKey of parentNode.ancestors) { + ancestors.add(ancestorKey); + } + } + + node.inheritedConditions = inheritedConditions; + node.ancestors = ancestors; + walked.add(settingKey); + visiting.delete(settingKey); + return node; + } + + for (const key of tree.keys()) { + walk(key, new Set()); + } + + return tree; +} + +function nodeImpliesCondition(node, conditionKey) { + return node.directConditions.has(conditionKey) || node.inheritedConditions.has(conditionKey); +} + +/** + * Among immediate bool parents that imply the condition, pick the closest / + * most specific one: a parent that is a descendant of another candidate wins + * over that ancestor. Declaration order is ignored. + */ +function closestImplyingParent(settingKey, conditionKey, tree) { + const node = tree.get(settingKey); + if (!node) { + return null; + } + + const candidates = node.boolParents.filter((parentKey) => { + const parentNode = tree.get(parentKey); + return parentNode && nodeImpliesCondition(parentNode, conditionKey); + }); + + if (candidates.length === 0) { + return null; + } + + return candidates.reduce((best, candidate) => { + const bestNode = tree.get(best); + const candidateNode = tree.get(candidate); + + // Prefer the candidate that is under the current best (more specific). + if (candidateNode.ancestors.has(best)) { + return candidate; + } + + // Keep best when it is under the candidate. + if (bestNode.ancestors.has(candidate)) { + return best; + } + + // Disjoint parents that both imply the condition: stable tie-break by key. + return candidate < best ? candidate : best; + }); +} + +export default { + meta: { + type: 'problem', + docs: { + description: 'Disallow System Console isDisabled conditions already implied by a parent config dependency', + }, + schema: [], + messages: { + redundant: + "Redundant isDisabled condition '{{condition}}' on '{{setting}}' — already implied by dependency on '{{parent}}'. Depend on the parent setting and omit duplicated config checks.", + }, + }, + create(context) { + const filename = context.filename || context.getFilename(); + if (!(/admin_definition/).test(filename)) { + return {}; + } + + const settings = []; + + return { + ObjectExpression(node) { + const settingKey = getSettingKey(node); + if (typeof settingKey !== 'string' || !settingKey.includes('.')) { + return; + } + + const settingType = getSettingType(node); + if (typeof settingType !== 'string') { + return; + } + + const isDisabledProp = getProperty(node, 'isDisabled'); + if (!isDisabledProp) { + return; + } + + const conditions = []; + collectTopLevelConditions(isDisabledProp.value, conditions); + if (conditions.length === 0) { + return; + } + + settings.push({ + key: settingKey, + settingType, + conditions, + parentKeys: dependencyParentKeys(conditions), + node, + }); + }, + + 'Program:exit'() { + const settingsByKey = new Map(); + for (const setting of settings) { + if (!settingsByKey.has(setting.key)) { + settingsByKey.set(setting.key, []); + } + settingsByKey.get(setting.key).push(setting); + } + + // Build the complete tree before attributing any parent. + const tree = buildDependencyTree(settingsByKey); + + for (const setting of settings) { + const node = tree.get(setting.key); + if (!node || node.boolParents.length === 0 || node.inheritedConditions.size === 0) { + continue; + } + + for (const {key, node: conditionNode} of setting.conditions) { + if (!node.inheritedConditions.has(key)) { + continue; + } + + const implyingParent = closestImplyingParent(setting.key, key, tree); + if (!implyingParent) { + continue; + } + + context.report({ + node: conditionNode, + messageId: 'redundant', + data: { + condition: key, + setting: setting.key, + parent: implyingParent, + }, + }); + } + } + }, + }; + }, +}; diff --git a/webapp/platform/eslint-plugin/rules/no-redundant-admin-config-deps.test.js b/webapp/platform/eslint-plugin/rules/no-redundant-admin-config-deps.test.js new file mode 100644 index 000000000000..23c756d481f3 --- /dev/null +++ b/webapp/platform/eslint-plugin/rules/no-redundant-admin-config-deps.test.js @@ -0,0 +1,339 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {RuleTester} from 'eslint'; + +import rule from './no-redundant-admin-config-deps.js'; + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: 'module', + }, +}); + +const filename = 'admin_definition.tsx'; + +ruleTester.run('no-redundant-admin-config-deps', rule, { + valid: [ + { + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'EmailSettings.EnableEmailBatching', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.SendEmailNotifications'), + it.configIsTrue('ClusterSettings', 'Enable'), + ), + }, + { + type: 'number', + key: 'EmailSettings.EmailBatchingBufferSize', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.EnableEmailBatching'), + ), + }, + ], + }; + `, + }, + { + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'ServiceSettings.EnableOAuthServiceProvider', + isDisabled: it.not(it.userHasWritePermissionOnResource('integrations')), + }, + { + type: 'bool', + key: 'ServiceSettings.EnableDynamicClientRegistration', + isDisabled: it.any( + it.not(it.userHasWritePermissionOnResource('integrations')), + it.stateIsFalse('ServiceSettings.EnableOAuthServiceProvider'), + ), + }, + { + type: 'text', + key: 'ServiceSettings.DCRRedirectURIAllowlist', + isDisabled: it.any( + it.not(it.userHasWritePermissionOnResource('integrations')), + it.stateIsFalse('ServiceSettings.EnableDynamicClientRegistration'), + ), + }, + ], + }; + `, + }, + { + + // Non-bool parents (fileupload) do not imply their isDisabled conditions, + // so repeating Encrypt alongside PrivateKeyFile is intentional. + + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'SamlSettings.Encrypt', + isDisabled: it.stateIsFalse('SamlSettings.Enable'), + }, + { + type: 'fileupload', + key: 'SamlSettings.PrivateKeyFile', + isDisabled: it.stateIsFalse('SamlSettings.Encrypt'), + }, + { + type: 'bool', + key: 'SamlSettings.SignRequest', + isDisabled: it.any( + it.stateIsFalse('SamlSettings.Encrypt'), + it.stateIsFalse('SamlSettings.PrivateKeyFile'), + ), + }, + ], + }; + `, + }, + { + + // Distinct regex patterns on the same helper+key are not the same condition. + + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'SupportSettings.Enable', + isDisabled: it.stateMatches('SupportSettings.ReportAProblemType', /link/), + }, + { + type: 'text', + key: 'SupportSettings.Mail', + isDisabled: it.any( + it.stateIsFalse('SupportSettings.Enable'), + it.stateMatches('SupportSettings.ReportAProblemType', /email/), + ), + }, + ], + }; + `, + }, + { + + // Identifier parent keys must not throw during canonicalize/JSON.parse. + + filename, + code: ` + const PARENT_KEY = 'ServiceSettings.EnableOAuthServiceProvider'; + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'ServiceSettings.EnableDynamicClientRegistration', + isDisabled: it.stateIsFalse(PARENT_KEY), + }, + { + type: 'text', + key: 'ServiceSettings.DCRRedirectURIAllowlist', + isDisabled: it.stateIsFalse('ServiceSettings.EnableDynamicClientRegistration'), + }, + ], + }; + `, + }, + { + + // Non-admin_definition files are ignored + + filename: 'other_file.tsx', + code: ` + const x = { + type: 'number', + key: 'EmailSettings.EmailBatchingBufferSize', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.SendEmailNotifications'), + it.stateIsFalse('EmailSettings.EnableEmailBatching'), + ), + }; + const y = { + type: 'bool', + key: 'EmailSettings.EnableEmailBatching', + isDisabled: it.stateIsFalse('EmailSettings.SendEmailNotifications'), + }; + `, + }, + ], + invalid: [ + { + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'EmailSettings.EnableEmailBatching', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.SendEmailNotifications'), + it.configIsTrue('ClusterSettings', 'Enable'), + it.configIsFalse('ServiceSettings', 'SiteURL'), + ), + }, + { + type: 'number', + key: 'EmailSettings.EmailBatchingBufferSize', + isDisabled: it.any( + it.stateIsFalse('EmailSettings.SendEmailNotifications'), + it.stateIsFalse('EmailSettings.EnableEmailBatching'), + it.configIsTrue('ClusterSettings', 'Enable'), + it.configIsFalse('ServiceSettings', 'SiteURL'), + ), + }, + ], + }; + `, + errors: [ + {messageId: 'redundant', data: {condition: 'stateIsFalse:"EmailSettings.SendEmailNotifications"', setting: 'EmailSettings.EmailBatchingBufferSize', parent: 'EmailSettings.EnableEmailBatching'}}, + {messageId: 'redundant', data: {condition: 'configIsTrue:"ClusterSettings","Enable"', setting: 'EmailSettings.EmailBatchingBufferSize', parent: 'EmailSettings.EnableEmailBatching'}}, + {messageId: 'redundant', data: {condition: 'configIsFalse:"ServiceSettings","SiteURL"', setting: 'EmailSettings.EmailBatchingBufferSize', parent: 'EmailSettings.EnableEmailBatching'}}, + ], + }, + { + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'ServiceSettings.EnableOAuthServiceProvider', + isDisabled: it.not(it.userHasWritePermissionOnResource('integrations')), + }, + { + type: 'bool', + key: 'ServiceSettings.EnableDynamicClientRegistration', + isDisabled: it.any( + it.not(it.userHasWritePermissionOnResource('integrations')), + it.stateIsFalse('ServiceSettings.EnableOAuthServiceProvider'), + ), + }, + { + type: 'text', + key: 'ServiceSettings.DCRRedirectURIAllowlist', + isDisabled: it.any( + it.not(it.userHasWritePermissionOnResource('integrations')), + it.stateIsFalse('ServiceSettings.EnableOAuthServiceProvider'), + it.stateIsFalse('ServiceSettings.EnableDynamicClientRegistration'), + ), + }, + ], + }; + `, + errors: [ + {messageId: 'redundant', data: {condition: 'stateIsFalse:"ServiceSettings.EnableOAuthServiceProvider"', setting: 'ServiceSettings.DCRRedirectURIAllowlist', parent: 'ServiceSettings.EnableDynamicClientRegistration'}}, + ], + }, + { + + // Bool parent Encrypt implies Enable; PrivateKeyFile should not repeat Enable + + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'SamlSettings.Encrypt', + isDisabled: it.stateIsFalse('SamlSettings.Enable'), + }, + { + type: 'fileupload', + key: 'SamlSettings.PrivateKeyFile', + isDisabled: it.any( + it.stateIsFalse('SamlSettings.Enable'), + it.stateIsFalse('SamlSettings.Encrypt'), + ), + }, + ], + }; + `, + errors: [ + {messageId: 'redundant', data: {condition: 'stateIsFalse:"SamlSettings.Enable"', setting: 'SamlSettings.PrivateKeyFile', parent: 'SamlSettings.Encrypt'}}, + ], + }, + { + + // Grandparent listed before parent: still attribute to the closest parent (Mid), + // not Root, after the full tree is built. + + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'FeatureSettings.Root', + isDisabled: it.configIsTrue('ClusterSettings', 'Enable'), + }, + { + type: 'bool', + key: 'FeatureSettings.Mid', + isDisabled: it.any( + it.stateIsFalse('FeatureSettings.Root'), + it.configIsTrue('ClusterSettings', 'Enable'), + ), + }, + { + type: 'number', + key: 'FeatureSettings.Leaf', + isDisabled: it.any( + it.stateIsFalse('FeatureSettings.Root'), + it.stateIsFalse('FeatureSettings.Mid'), + it.configIsTrue('ClusterSettings', 'Enable'), + ), + }, + ], + }; + `, + errors: [ + {messageId: 'redundant', data: {condition: 'configIsTrue:"ClusterSettings","Enable"', setting: 'FeatureSettings.Mid', parent: 'FeatureSettings.Root'}}, + {messageId: 'redundant', data: {condition: 'stateIsFalse:"FeatureSettings.Root"', setting: 'FeatureSettings.Leaf', parent: 'FeatureSettings.Mid'}}, + {messageId: 'redundant', data: {condition: 'configIsTrue:"ClusterSettings","Enable"', setting: 'FeatureSettings.Leaf', parent: 'FeatureSettings.Mid'}}, + ], + }, + { + + // Same regex on parent and child is redundant; distinct patterns are not. + + filename, + code: ` + const AdminDefinition = { + settings: [ + { + type: 'bool', + key: 'SupportSettings.Enable', + isDisabled: it.stateMatches('SupportSettings.ReportAProblemType', /link/), + }, + { + type: 'text', + key: 'SupportSettings.Mail', + isDisabled: it.any( + it.stateIsFalse('SupportSettings.Enable'), + it.stateMatches('SupportSettings.ReportAProblemType', /link/), + ), + }, + ], + }; + `, + errors: [ + {messageId: 'redundant', data: {condition: 'stateMatches:"SupportSettings.ReportAProblemType",re:"link":""', setting: 'SupportSettings.Mail', parent: 'SupportSettings.Enable'}}, + ], + }, + ], +});