Skip to content
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.

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

[IMPROVE][Performance] Solves problems regarding settings editing performance on Administration #17916

Merged
merged 29 commits into from
Jun 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
0cb8baa
Simplify settings comparison
tassoevan Jun 13, 2020
c27e142
Remove useReducer
tassoevan Jun 13, 2020
153b652
Split privileged setting editing hooks
tassoevan Jun 13, 2020
6f3fc84
Replace stateRef with maps for settings
tassoevan Jun 14, 2020
6007bf6
Change PrivilegedSettingsProvider initialization
tassoevan Jun 15, 2020
a4b130f
Add useQuery hook
tassoevan Jun 15, 2020
65a8ae7
Add small refactoring
tassoevan Jun 15, 2020
7f0015e
Subscribe to edit privileged settings
tassoevan Jun 15, 2020
0372b6b
Subscribe to a setting or a section of a settings group
tassoevan Jun 15, 2020
c2e507c
Handle state only from collections
tassoevan Jun 16, 2020
ca683eb
Refine some types
tassoevan Jun 16, 2020
95d3607
Fix some reactivity issues
tassoevan Jun 16, 2020
87428a0
Add eslint rules for React hooks
tassoevan Jun 16, 2020
58769d6
Move cached collections for settings
tassoevan Jun 16, 2020
d54f152
Prepare to unify all contexts for settings
tassoevan Jun 16, 2020
b9398ae
Merge branch 'develop' into perf/privileged-settings
tassoevan Jun 16, 2020
d5306a1
Use custom queries to fetch settings
tassoevan Jun 16, 2020
7686112
Move hook to AdminSidebar
tassoevan Jun 16, 2020
53ba377
Change setting update hooks
tassoevan Jun 17, 2020
33cb20c
Augment Tracker.nonreactive
tassoevan Jun 17, 2020
6375114
Use a single SettingsProvider for public and private settings
tassoevan Jun 17, 2020
9dcb446
Rename PrivilegedSettingsContext as EditableSettingsContext
tassoevan Jun 17, 2020
cf5b0e0
Merge branch 'develop' into perf/privileged-settings
tassoevan Jun 17, 2020
d55667b
Change setting editing actions
tassoevan Jun 17, 2020
d6b95a2
Change group actions on editing settings
tassoevan Jun 17, 2020
2a12dfe
Remove exceeding hooks from EditingSettingsContext
tassoevan Jun 17, 2020
59a34d8
Merge branch 'develop' of github.com:RocketChat/Rocket.Chat into perf…
tassoevan Jun 17, 2020
398d9f5
Merge branch 'develop' of github.com:RocketChat/Rocket.Chat into perf…
tassoevan Jun 18, 2020
fb6ca09
Fix some broken JSX
tassoevan Jun 18, 2020
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
13 changes: 3 additions & 10 deletions app/settings/client/lib/settings.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,13 @@
import { Meteor } from 'meteor/meteor';
import { ReactiveDict } from 'meteor/reactive-dict';

import { CachedCollection } from '../../../ui-cached-collection';
import { PublicSettingsCachedCollection } from '../../../../client/lib/settings/PublicSettingsCachedCollection';
import { SettingsBase, SettingValue } from '../../lib/settings';

const cachedCollection = new CachedCollection({
name: 'public-settings',
eventType: 'onAll',
userRelated: false,
listenChangesForLoggedUsersOnly: true,
});

class Settings extends SettingsBase {
cachedCollection = cachedCollection
cachedCollection = PublicSettingsCachedCollection.get()

collection = cachedCollection.collection;
collection = PublicSettingsCachedCollection.get().collection;

dict = new ReactiveDict<any>('settings');

Expand Down
6 changes: 3 additions & 3 deletions client/admin/AdministrationRouter.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import React, { lazy, useMemo, Suspense } from 'react';

import SettingsProvider from '../providers/SettingsProvider';
import AdministrationLayout from './AdministrationLayout';
import PrivilegedSettingsProvider from './PrivilegedSettingsProvider';
import PageSkeleton from './PageSkeleton';

function AdministrationRouter({ lazyRouteComponent, ...props }) {
const LazyRouteComponent = useMemo(() => lazy(lazyRouteComponent), [lazyRouteComponent]);
return <AdministrationLayout>
<PrivilegedSettingsProvider>
<SettingsProvider privileged>
<Suspense fallback={<PageSkeleton />}>
<LazyRouteComponent {...props} />
</Suspense>
</PrivilegedSettingsProvider>
</SettingsProvider>
</AdministrationLayout>;
}

Expand Down
217 changes: 0 additions & 217 deletions client/admin/PrivilegedSettingsProvider.js

This file was deleted.

87 changes: 78 additions & 9 deletions client/admin/settings/GroupPage.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,80 @@
import { Accordion, Box, Button, ButtonGroup, Skeleton } from '@rocket.chat/fuselage';
import React, { useMemo } from 'react';
import { useMutableCallback } from '@rocket.chat/fuselage-hooks';
import React, { useMemo, memo } from 'react';

import Page from '../../components/basic/Page';
import { useTranslation } from '../../contexts/TranslationContext';
import { useEditableSettingsDispatch, useEditableSettings } from '../../contexts/EditableSettingsContext';
import { useSettingsDispatch, useSettings } from '../../contexts/SettingsContext';
import { useToastMessageDispatch } from '../../contexts/ToastMessagesContext';
import { useTranslation, useLoadLanguage } from '../../contexts/TranslationContext';
import { useUser } from '../../contexts/UserContext';
import { Section } from './Section';

const style = { margin: '0 auto', width: '100%', maxWidth: '590px' };
export function GroupPage({ children, headerButtons, save, cancel, _id, i18nLabel, i18nDescription, changed }) {
function GroupPage({ children, headerButtons, _id, i18nLabel, i18nDescription }) {
const changedEditableSettings = useEditableSettings(useMemo(() => ({
group: _id,
changed: true,
}), [_id]));

const originalSettings = useSettings(useMemo(() => ({
_id: changedEditableSettings.map(({ _id }) => _id),
}), [changedEditableSettings]));

const dispatch = useSettingsDispatch();

const dispatchToastMessage = useToastMessageDispatch();
const t = useTranslation();
const loadLanguage = useLoadLanguage();
const user = useUser();

const save = useMutableCallback(async () => {
const changes = changedEditableSettings
.map(({ _id, value, editor }) => ({ _id, value, editor }));

if (changes.length === 0) {
return;
}

try {
await dispatch(changes);

if (changes.some(({ _id }) => _id === 'Language')) {
const lng = user?.language
|| changes.filter(({ _id }) => _id === 'Language').shift()?.value
|| 'en';

await loadLanguage(lng);
dispatchToastMessage({ type: 'success', message: t('Settings_updated', { lng }) });
return;
}

dispatchToastMessage({ type: 'success', message: t('Settings_updated') });
} catch (error) {
dispatchToastMessage({ type: 'error', message: error });
}
});

const dispatchToEditing = useEditableSettingsDispatch();

const cancel = useMutableCallback(() => {
dispatchToEditing(
changedEditableSettings
.map(({ _id }) => originalSettings.find((setting) => setting._id === _id))
.map((setting) => {
if (!setting) {
return;
}

return {
_id: setting._id,
value: setting.value,
editor: setting.editor,
changed: false,
};
})
.filter(Boolean),
);
});

const handleSubmit = (event) => {
event.preventDefault();
Expand All @@ -34,11 +101,11 @@ export function GroupPage({ children, headerButtons, save, cancel, _id, i18nLabe
return <Page is='form' action='#' method='post' onSubmit={handleSubmit}>
<Page.Header title={t(i18nLabel)}>
<ButtonGroup>
{changed && <Button danger primary type='reset' onClick={handleCancelClick}>{t('Cancel')}</Button>}
{changedEditableSettings.length > 0 && <Button danger primary type='reset' onClick={handleCancelClick}>{t('Cancel')}</Button>}
<Button
children={t('Save_changes')}
className='save'
disabled={!changed}
disabled={changedEditableSettings.length === 0}
primary
type='submit'
onClick={handleSaveClick}
Expand All @@ -48,7 +115,7 @@ export function GroupPage({ children, headerButtons, save, cancel, _id, i18nLabe
</Page.Header>

<Page.ScrollableContentWithShadow>
<Box style={style}>
<Box marginBlock='none' marginInline='auto' width='full' maxWidth='x580'>
{t.has(i18nDescription) && <Box is='p' color='hint' fontScale='p1'>{t(i18nDescription)}</Box>}

<Accordion className='page-settings'>
Expand All @@ -59,7 +126,7 @@ export function GroupPage({ children, headerButtons, save, cancel, _id, i18nLabe
</Page>;
}

export function GroupPageSkeleton() {
function GroupPageSkeleton() {
const t = useTranslation();

return <Page>
Expand Down Expand Up @@ -89,4 +156,6 @@ export function GroupPageSkeleton() {
</Page>;
}

GroupPage.Skeleton = GroupPageSkeleton;
export default Object.assign(memo(GroupPage), {
Skeleton: GroupPageSkeleton,
});
2 changes: 1 addition & 1 deletion client/admin/settings/GroupPage.stories.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';

import { GroupPage } from './GroupPage';
import GroupPage from './GroupPage';

export default {
title: 'admin/settings/GroupPage',
Expand Down
Loading