Skip to content

feat(useSetting): sync values with LS and store in api #3111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat(useSetting): sync values with LS and store in api #3111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7f5c82e
feat(useSetting); sync values with LS and store in api
artemmufazalov Nov 19, 2025
cbfadb0
fix: copilot review
artemmufazalov Nov 19, 2025
b5eeee2
fix: failed test
artemmufazalov Nov 19, 2025
29a9e3d
fix: log errors
artemmufazalov Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/containers/Tenant/Diagnostics/TopQueries/QueriesTableWithDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ const b = cn('kv-top-queries');
interface SimpleTableWithDrawerProps {
columns: Column<KeyValueRow>[];
data: KeyValueRow[];
loading?: boolean;
isFetching?: boolean;
isLoading?: boolean;
onRowClick?: (
row: KeyValueRow | null,
index?: number,
Expand All @@ -39,7 +40,8 @@ interface SimpleTableWithDrawerProps {
export function QueriesTableWithDrawer({
columns,
data,
loading,
isFetching,
isLoading,
onRowClick,
columnsWidthLSKey,
emptyDataMessage,
Expand Down Expand Up @@ -104,7 +106,8 @@ export function QueriesTableWithDrawer({
columnsWidthLSKey={columnsWidthLSKey}
columns={columns}
data={data}
isFetching={loading}
isFetching={isFetching}
isLoading={isLoading}
settings={tableSettings}
onRowClick={handleRowClick}
rowClassName={(row) => b('row', {active: isEqual(row, selectedRow)})}
Expand Down
5 changes: 3 additions & 2 deletions src/containers/Tenant/Diagnostics/TopQueries/RunningQueriesData.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,12 @@ export const RunningQueriesData = ({
</TableWithControlsLayout.Controls>

{error ? <ResponseError error={parseQueryErrorToString(error)} /> : null}
<TableWithControlsLayout.Table loading={isLoading}>
<TableWithControlsLayout.Table>
<QueriesTableWithDrawer
columns={columnsToShow}
data={rows || []}
loading={isFetching && currentData === undefined}
isFetching={isFetching && currentData === undefined}
isLoading={isLoading}
columnsWidthLSKey={RUNNING_QUERIES_COLUMNS_WIDTH_LS_KEY}
emptyDataMessage={i18n('no-data')}
sortOrder={tableSort}
Expand Down
5 changes: 3 additions & 2 deletions src/containers/Tenant/Diagnostics/TopQueries/TopQueriesData.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,12 @@ export const TopQueriesData = ({
</TableWithControlsLayout.Controls>

{error ? <ResponseError error={parseQueryErrorToString(error)} /> : null}
<TableWithControlsLayout.Table loading={isLoading}>
<TableWithControlsLayout.Table>
<QueriesTableWithDrawer
columns={columnsToShow}
data={rows || []}
loading={isFetching && currentData === undefined}
isFetching={isFetching && currentData === undefined}
isLoading={isLoading}
columnsWidthLSKey={TOP_QUERIES_COLUMNS_WIDTH_LS_KEY}
emptyDataMessage={i18n('no-data')}
sortOrder={tableSort}
Expand Down
2 changes: 1 addition & 1 deletion src/services/api/metaSettings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class MetaSettingsAPI extends BaseMetaAPI {
preventBatching,
}: GetSingleSettingParams & {preventBatching?: boolean}) {
if (preventBatching) {
return this.get<Setting>(this.getPath('/meta/user_settings'), {name, user});
return this.get<Setting | undefined>(this.getPath('/meta/user_settings'), {name, user});
}

return new Promise<Setting>((resolve, reject) => {
Expand Down
176 changes: 138 additions & 38 deletions src/store/reducers/settings/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,107 +6,202 @@ import type {
SetSingleSettingParams,
Setting,
} from '../../../types/api/settings';
import type {AppDispatch} from '../../defaultStore';
import type {AppDispatch, RootState} from '../../defaultStore';
import {api} from '../api';

import {SETTINGS_OPTIONS} from './constants';
import {getSettingValue, setSettingValue} from './settings';
import {
getSettingDefault,
parseSettingValue,
readSettingValueFromLS,
setSettingValueToLS,
shouldSyncSettingToLS,
stringifySettingValue,
} from './utils';

const invalidParamsError =
'Missing required parameters (name, user) or MetaSettings API is not available';

export const settingsApi = api.injectEndpoints({
endpoints: (builder) => ({
getSingleSetting: builder.query({
queryFn: async ({name, user}: GetSingleSettingParams, baseApi) => {
getSingleSetting: builder.query<Setting | undefined, Partial<GetSingleSettingParams>>({
queryFn: async ({name, user}) => {
try {
if (!window.api.metaSettings) {
throw new Error('MetaSettings API is not available');
// In this case localStorage should be used for settings
// Actual value will be loaded to store in onQueryStarted
if (!name || !user || !window.api.metaSettings) {
throw new Error(invalidParamsError);
}

const data = await window.api.metaSettings.getSingleSetting({
name,
user,
// Directly access options here to avoid them in cache key
preventBatching: SETTINGS_OPTIONS[name]?.preventBatching,
});

const dispatch = baseApi.dispatch as AppDispatch;

// Try to sync local value if there is no backend value
syncLocalValueToMetaIfNoData(data, dispatch);

return {data};
} catch (error) {
console.error('Cannot get setting:', error);
return {error};
}
},
async onQueryStarted(args, {dispatch, queryFulfilled}) {
const {name, user} = args;

if (!name) {
return;
}

const shouldUseLocalSettings = !user || !window.api.metaSettings;

const defaultValue = getSettingDefault(name);

// Preload value from LS or default to store
if (shouldUseLocalSettings || shouldSyncSettingToLS(name)) {
const savedValue = readSettingValueFromLS(name);
const value = savedValue ?? defaultValue;

dispatch(setSettingValue(name, value));
} else {
dispatch(setSettingValue(name, defaultValue));
}

// Do not process query result when only LS is used
// There is not actual api call in this case
if (shouldUseLocalSettings) {
return;
}

try {
const {data} = await queryFulfilled;

// Load api value to store if present
// In case local storage should be used
// query will finish with an error and this code will not run
const parsedValue = parseSettingValue(data?.value);

if (isNil(data?.value)) {
// Pass setting params without value if data is empty
const newValue = data ?? {name, user};

// Try to sync local value if there is no backend value
syncLocalValueToMetaIfNoData(newValue, dispatch);
Copy link
Collaborator

@astandrik astandrik Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible that null/undefined on backend is intentional?
i mean we removed some setting for example

but LS has some old value (in another browser for example) and pushes this setting to backend?

Copy link
Member Author

@artemmufazalov artemmufazalov Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a current limitation, yes. We cannot have null or undefined as backend value, there should be at least something, otherwise defined localStorage value will be loaded to backend. Without this it's unclear, when we should sync local value to backend

} else {
dispatch(setSettingValue(name, parsedValue));

if (shouldSyncSettingToLS(name)) {
setSettingValueToLS(name, data.value);
}
}
} catch {
// In case of an error there is no value to sync
// LS or default value was loaded to store
}
},
}),
setSingleSetting: builder.mutation({
queryFn: async (params: SetSingleSettingParams) => {
queryFn: async ({
name,
user,
value,
}: Partial<Omit<SetSingleSettingParams, 'value'>> & {value: unknown}) => {
try {
if (!window.api.metaSettings) {
throw new Error('MetaSettings API is not available');
if (!name || !user || !window.api.metaSettings) {
throw new Error(invalidParamsError);
}

const data = await window.api.metaSettings.setSingleSetting(params);
const data = await window.api.metaSettings.setSingleSetting({
name,
user,
value: stringifySettingValue(value),
});

if (data.status !== 'SUCCESS') {
throw new Error('Setting status is not SUCCESS');
}

return {data};
} catch (error) {
console.error('Cannot update setting:', error);
return {error};
}
},
async onQueryStarted(args, {dispatch, queryFulfilled}) {
async onQueryStarted(args, {dispatch, queryFulfilled, getState}) {
const {name, user, value} = args;

// Optimistically update existing cache entry
const patchResult = dispatch(
settingsApi.util.updateQueryData('getSingleSetting', {name, user}, (draft) => {
return {...draft, name, user, value};
}),
);
if (!name) {
return;
}

// Extract previous value to revert to it if set is not successful
const previousSettingValue = getSettingValue(getState() as RootState, name);

// Optimistically update store
dispatch(setSettingValue(name, value));

const shouldUseLocalSettings = !user || !window.api.metaSettings;

// If local storage settings should be used
// Update LS and return early
if (shouldUseLocalSettings) {
setSettingValueToLS(name, value);
return;
}

try {
await queryFulfilled;

// If mutation is successful, we can store new value in LS
if (shouldSyncSettingToLS(name)) {
setSettingValueToLS(name, value);
}
} catch {
patchResult.undo();
// Set previous value to store in case of error
dispatch(setSettingValue(name, previousSettingValue));
}
},
}),
getSettings: builder.query({
queryFn: async ({name, user}: GetSettingsParams, baseApi) => {
queryFn: async ({name, user}: Partial<GetSettingsParams>, baseApi) => {
try {
if (!window.api.metaSettings) {
throw new Error('MetaSettings API is not available');
if (!window.api.metaSettings || !name || !user) {
throw new Error(invalidParamsError);
}
const data = await window.api.metaSettings.getSettings({name, user});

const patches: Promise<void>[] = [];
const patches: Promise<unknown>[] = [];
const dispatch = baseApi.dispatch as AppDispatch;

// Upsert received data in getSingleSetting cache
// Upsert received data in getSingleSetting cache to prevent further redundant requests
name.forEach((settingName) => {
const settingData = data[settingName] ?? {};

const cacheEntryParams: GetSingleSettingParams = {
name: settingName,
user,
};
const newValue = {name: settingName, user, value: settingData?.value};
const newSetting = {name: settingName, user, value: settingData?.value};

const patch = dispatch(
settingsApi.util.upsertQueryData(
'getSingleSetting',
cacheEntryParams,
newValue,
newSetting,
),
).then(() => {
);
if (isNil(newSetting.value)) {
// Try to sync local value if there is no backend value
// Do it after upsert if finished to ensure proper values update order
// 1. New entry added to cache with nil value
// 2. Positive entry update - local storage value replace nil in cache
// 3.1. Set is successful, local value in cache
// 3.2. Set is not successful, cache value reverted to previous nil
syncLocalValueToMetaIfNoData(settingData, dispatch);
});
syncLocalValueToMetaIfNoData(newSetting, dispatch);
} else {
const parsedValue = parseSettingValue(newSetting.value);
dispatch(setSettingValue(settingName, parsedValue));

if (shouldSyncSettingToLS(settingName)) {
setSettingValueToLS(settingName, newSetting.value);
}
}

patches.push(patch);
});
Expand All @@ -116,6 +211,7 @@ export const settingsApi = api.injectEndpoints({

return {data};
} catch (error) {
console.error('Cannot get settings:', error);
Copy link
Collaborator

@astandrik astandrik Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be that error is "Cannot get setting" - but the real situation is that one of patches failed ?

return {error};
}
},
Expand All @@ -124,7 +220,11 @@ export const settingsApi = api.injectEndpoints({
overrideExisting: 'throw',
});

function syncLocalValueToMetaIfNoData(params: Setting, dispatch: AppDispatch) {
function syncLocalValueToMetaIfNoData(params: Partial<Setting>, dispatch: AppDispatch) {
if (!params.name) {
return;
}

const localValue = localStorage.getItem(params.name);
Copy link
Collaborator

@astandrik astandrik Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use localStorage directly here but there are already functions for this like

readSettingValueFromLS / shouldSyncSettingToLS

these functions as I may suppose were created to make work with LS somehow guarded


if (isNil(params.value) && !isNil(localValue)) {
Expand Down
2 changes: 1 addition & 1 deletion src/store/reducers/settings/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export const DEFAULT_USER_SETTINGS = {
[SETTING_KEYS.ACL_SYNTAX]: AclSyntax.YdbShort,
} as const satisfies Record<SettingKey, unknown>;

export const SETTINGS_OPTIONS: Record<string, SettingOptions> = {
export const SETTINGS_OPTIONS: Record<string, SettingOptions | undefined> = {
[SETTING_KEYS.THEME]: {
preventBatching: true,
},
Expand Down
Loading
Loading