regression: Admin settings with `enableQuery` and `displayQuery` do not react to changes (#35928)

pull/35948/head
gabriellsh 8 months ago committed by GitHub
parent bfdf17e5a2
commit 29264b8620
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 61
      apps/meteor/client/views/admin/EditableSettingsContext.spec.tsx
  2. 34
      apps/meteor/client/views/admin/EditableSettingsContext.ts
  3. 31
      apps/meteor/client/views/admin/settings/EditableSettingsProvider.tsx
  4. 9
      apps/meteor/client/views/admin/settings/Setting/Setting.tsx

@ -0,0 +1,61 @@
import { mockAppRoot as _mockAppRoot } from '@rocket.chat/mock-providers';
import { renderHook } from '@testing-library/react';
import { useEditableSettingVisibilityQuery } from './EditableSettingsContext';
import EditableSettingsProvider from './settings/EditableSettingsProvider';
const mockAppRoot = () => _mockAppRoot().wrap((children) => <EditableSettingsProvider>{children}</EditableSettingsProvider>);
describe('useEditableSettingVisibilityQuery', () => {
it('should return true when no query is provided', () => {
const { result } = renderHook(() => useEditableSettingVisibilityQuery(), {
wrapper: mockAppRoot().build(),
});
expect(result.current).toBe(true);
});
it('should handle settings with a query', () => {
const { result } = renderHook(() => useEditableSettingVisibilityQuery({ _id: 'setting1', value: true }), {
wrapper: mockAppRoot().withSetting('setting1', true).build(),
});
expect(result.current).toBe(true);
});
it('should handle multiple conditions in enableQuery', () => {
const { result } = renderHook(
() =>
useEditableSettingVisibilityQuery([
{ _id: 'setting5', value: true },
{ _id: 'setting6', value: true },
]),
{
wrapper: mockAppRoot().withSetting('setting5', true).withSetting('setting6', true).build(),
},
);
expect(result.current).toBe(true);
const { result: result2 } = renderHook(
() =>
useEditableSettingVisibilityQuery([
{ _id: 'setting5', value: true },
{ _id: 'setting6', value: true },
]),
{
wrapper: mockAppRoot().withSetting('setting5', true).withSetting('setting6', false).build(),
},
);
expect(result2.current).toBe(false);
});
it('should handle string queries', () => {
const { result } = renderHook(() => useEditableSettingVisibilityQuery(JSON.stringify({ _id: 'setting7', value: true })), {
wrapper: mockAppRoot().withSetting('setting7', true).build(),
});
expect(result.current).toBe(true);
});
});

@ -1,4 +1,5 @@
import type { ISetting } from '@rocket.chat/core-typings';
import { createFilterFromQuery } from '@rocket.chat/mongo-adapter';
import { createContext, useContext } from 'react';
import { create, type StoreApi, type UseBoundStore } from 'zustand';
import { useShallow } from 'zustand/shallow';
@ -21,6 +22,28 @@ export const compareSettings = (a: EditableSetting, b: EditableSetting): number
return i18nLabel;
};
export const performSettingQuery = (
query:
| string
| {
_id: string;
value: unknown;
}
| {
_id: string;
value: unknown;
}[]
| undefined,
settings: ISetting[],
) => {
if (!query) {
return true;
}
const queries = [].concat(typeof query === 'string' ? JSON.parse(query) : query);
return queries.every((query) => settings.some(createFilterFromQuery(query)));
};
type EditableSettingsContextQuery =
| {
group: ISetting['_id'];
@ -125,3 +148,14 @@ export const useEditableSettingsDispatch = (): ((changes: Partial<EditableSettin
const { useEditableSettingsStore } = useContext(EditableSettingsContext);
return useEditableSettingsStore((state) => state.mutate);
};
export const useEditableSettingVisibilityQuery = (query?: ISetting['enableQuery'] | ISetting['displayQuery']): boolean => {
const { useEditableSettingsStore } = useContext(EditableSettingsContext);
return useEditableSettingsStore((state) => {
if (!query) {
return true;
}
return performSettingQuery(query, state.state);
});
};

@ -1,37 +1,14 @@
import type { ISetting } from '@rocket.chat/core-typings';
import { createFilterFromQuery } from '@rocket.chat/mongo-adapter';
import { useSettings } from '@rocket.chat/ui-contexts';
import type { ReactNode } from 'react';
import { useEffect, useMemo, useState } from 'react';
import { create } from 'zustand';
import type { EditableSetting, IEditableSettingsState } from '../EditableSettingsContext';
import { EditableSettingsContext } from '../EditableSettingsContext';
import { EditableSettingsContext, performSettingQuery } from '../EditableSettingsContext';
const defaultOmit: Array<ISetting['_id']> = ['Cloud_Workspace_AirGapped_Restrictions_Remaining_Days'];
const performSettingQuery = (
query:
| string
| {
_id: string;
value: unknown;
}
| {
_id: string;
value: unknown;
}[]
| undefined,
settings: ISetting[],
) => {
if (!query) {
return true;
}
const queries = [].concat(typeof query === 'string' ? JSON.parse(query) : query);
return queries.every((query) => settings.some(createFilterFromQuery(query)));
};
type EditableSettingsProviderProps = {
children?: ReactNode;
};
@ -48,6 +25,8 @@ const EditableSettingsProvider = ({ children }: EditableSettingsProviderProps) =
(persisted): EditableSetting => ({
...persisted,
changed: false,
// TODO: This might not be needed anymore due to implementation of useEditableSettingVisibilityQuery
// This was left here to avoid unexpected breaking changes
disabled: persisted.blocked || !performSettingQuery(persisted.enableQuery, persistedSettings),
invisible: !performSettingQuery(persisted.displayQuery, persistedSettings),
}),
@ -62,6 +41,8 @@ const EditableSettingsProvider = ({ children }: EditableSettingsProviderProps) =
...state.find(({ _id }) => _id === persisted._id),
...persisted,
changed: false,
// TODO: This might not be needed anymore due to implementation of useEditableSettingVisibilityQuery
// This was left here to avoid unexpected breaking changes
disabled: persisted.blocked || !performSettingQuery(persisted.enableQuery, state),
invisible: !performSettingQuery(persisted.displayQuery, state),
}),
@ -85,8 +66,6 @@ const EditableSettingsProvider = ({ children }: EditableSettingsProviderProps) =
return {
...current,
...change,
disabled: persisted.blocked || !performSettingQuery(persisted.enableQuery, state),
invisible: !performSettingQuery(persisted.displayQuery, state),
};
}),
}));

@ -10,7 +10,7 @@ import { useTranslation } from 'react-i18next';
import MemoizedSetting from './MemoizedSetting';
import MarkdownText from '../../../../components/MarkdownText';
import { useEditableSetting, useEditableSettingsDispatch } from '../../EditableSettingsContext';
import { useEditableSetting, useEditableSettingsDispatch, useEditableSettingVisibilityQuery } from '../../EditableSettingsContext';
import { useHasSettingModule } from '../hooks/useHasSettingModule';
type SettingProps = {
@ -96,7 +96,10 @@ function Setting({ className = undefined, settingId, sectionChanged }: SettingPr
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [setting.value, (setting as ISettingColor).editor, update, persistedSetting]);
const { _id, disabled, readonly, type, packageValue, i18nLabel, i18nDescription, alert, invisible } = setting;
const { _id, readonly, type, packageValue, i18nLabel, i18nDescription, alert } = setting;
const disabled = !useEditableSettingVisibilityQuery(persistedSetting.enableQuery);
const invisible = !useEditableSettingVisibilityQuery(persistedSetting.displayQuery);
const labelText = (i18n.exists(i18nLabel) && t(i18nLabel)) || (i18n.exists(_id) && t(_id)) || i18nLabel || _id;
@ -162,7 +165,7 @@ function Setting({ className = undefined, settingId, sectionChanged }: SettingPr
showUpgradeButton={showUpgradeButton}
sectionChanged={sectionChanged}
{...setting}
disabled={setting.disabled || shouldDisableEnterprise}
disabled={disabled || shouldDisableEnterprise}
value={value}
editor={editor}
hasResetButton={hasResetButton}

Loading…
Cancel
Save