Skip to content

Commit

Permalink
feat: accept many settings changes at once
Browse files Browse the repository at this point in the history
  • Loading branch information
mbondyra committed Dec 20, 2019
1 parent b5ca47f commit 0c993ee
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 40 deletions.
12 changes: 5 additions & 7 deletions src/core/public/ui_settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export interface IUiSettingsClient {
* server fails then a updateErrors$ will be notified and the setting will be
* reverted to its value before `set()` was called.
*/
set: (key: string, value: any) => Promise<boolean>;
set: (value: any) => Promise<boolean>;

/**
* Overrides the default value for a setting in this specific browser tab. If the page
Expand Down Expand Up @@ -102,21 +102,19 @@ export interface IUiSettingsClient {

/**
* Returns an Observable that notifies subscribers of each update to the uiSettings,
* including the key, newValue, and oldValue of the setting that changed.
* including the incomingChanges, and oldValues of the settings that changed.
*/
getUpdate$: <T = any>() => Observable<{
key: string;
newValue: T;
incomingChanges: T;
oldValue: T;
}>;

/**
* Returns an Observable that notifies subscribers of each update to the uiSettings,
* including the key, newValue, and oldValue of the setting that changed.
* including the incomingChanges, and oldValues of the settings that changed.
*/
getSaved$: <T = any>() => Observable<{
key: string;
newValue: T;
incomingChanges: T;
oldValue: T;
}>;

Expand Down
9 changes: 6 additions & 3 deletions src/core/public/ui_settings/ui_settings_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ interface Changes {
callback(error?: Error, response?: UiSettingsApiResponse): void;
}

export interface IncomingChanges {
[key: string]: any;
}

const NOOP_CHANGES = {
values: {},
callback: () => {
Expand All @@ -54,14 +58,13 @@ export class UiSettingsApi {
* already in progress it will wait until the previous request is complete
* before sending the next request
*/
public batchSet(key: string, value: any) {
public batchSet(incomingChanges: IncomingChanges) {
return new Promise<UiSettingsApiResponse>((resolve, reject) => {
const prev = this.pendingChanges || NOOP_CHANGES;

this.pendingChanges = {
values: {
...prev.values,
[key]: value,
...incomingChanges,
},

callback(error, resp) {
Expand Down
60 changes: 34 additions & 26 deletions src/core/public/ui_settings/ui_settings_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { filter, map } from 'rxjs/operators';
import { UiSettingsParams, UserProvidedValues } from 'src/core/server/types';
import { IUiSettingsClient, UiSettingsState } from './types';

import { UiSettingsApi } from './ui_settings_api';
import { UiSettingsApi, IncomingChanges } from './ui_settings_api';

interface UiSettingsClientParams {
api: UiSettingsApi;
Expand All @@ -34,8 +34,8 @@ interface UiSettingsClientParams {
}

export class UiSettingsClient implements IUiSettingsClient {
private readonly update$ = new Subject<{ key: string; newValue: any; oldValue: any }>();
private readonly saved$ = new Subject<{ key: string; newValue: any; oldValue: any }>();
private readonly update$ = new Subject<{ incomingChanges: any; oldValue: any }>();
private readonly saved$ = new Subject<{ incomingChanges: any; oldValue: any }>();
private readonly updateErrors$ = new Subject<Error>();

private readonly api: UiSettingsApi;
Expand Down Expand Up @@ -97,18 +97,18 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
return concat(
defer(() => of(this.get(key, defaultOverride))),
this.update$.pipe(
filter(update => update.key === key),
filter(update => update.incomingChanges[key] === key),
map(() => this.get(key, defaultOverride))
)
);
}

async set(key: string, value: any) {
return await this.update(key, value);
async set(change: IncomingChanges) {
return await this.update(change);
}

async remove(key: string) {
return await this.update(key, null);
return await this.update({ [key]: null });
}

isDeclared(key: string) {
Expand Down Expand Up @@ -145,8 +145,8 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r

// don't broadcast change if userValue was already overriding the default
if (this.cache[key].userValue == null) {
this.update$.next({ key, newValue: newDefault, oldValue: prevDefault });
this.saved$.next({ key, newValue: newDefault, oldValue: prevDefault });
this.update$.next({ incomingChanges: newDefault, oldValue: prevDefault });
this.saved$.next({ incomingChanges: newDefault, oldValue: prevDefault });
}
}

Expand All @@ -170,29 +170,35 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
}
}

private async update(key: string, newVal: any): Promise<boolean> {
this.assertUpdateAllowed(key);
private async update(incomingChanges: IncomingChanges): Promise<boolean> {
const entries = Object.entries(incomingChanges);
const initialValues: IncomingChanges = {};

const declared = this.isDeclared(key);
const defaults = this.defaults;
entries.forEach(([key, newVal]) => {
this.assertUpdateAllowed(key);

const oldVal = declared ? this.cache[key].userValue : undefined;
const declared = this.isDeclared(key);

const unchanged = oldVal === newVal;
if (unchanged) {
return true;
}
const oldVal = declared ? this.cache[key].userValue : undefined;

const unchanged = oldVal === newVal;
if (unchanged) {
return true;
}

const initialVal = declared ? this.get(key) : undefined;
this.setLocally(key, newVal);
initialValues[key] = declared ? this.get(key) : undefined;
this.setLocally(key, newVal);
});

try {
const { settings } = await this.api.batchSet(key, newVal);
this.cache = defaultsDeep({}, defaults, settings);
this.saved$.next({ key, newValue: newVal, oldValue: initialVal });
const { settings } = await this.api.batchSet(incomingChanges);
this.cache = defaultsDeep({}, this.defaults, settings);
this.saved$.next({ incomingChanges, oldValue: initialValues });
return true;
} catch (error) {
this.setLocally(key, initialVal);
Object.entries(initialValues).forEach(([key, initialVal]) =>
this.setLocally(key, initialVal)
);
this.updateErrors$.next(error);
return false;
}
Expand All @@ -217,7 +223,9 @@ You can use \`IUiSettingsClient.get("${key}", defaultValue)\`, which will just r
this.cache[key].userValue = newValue;
}
}

this.update$.next({ key, newValue, oldValue });
this.update$.next({
incomingChanges: { [key]: newValue },
oldValue,
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ export class AdvancedSettings extends Component {
}

saveConfig = (name, value) => {
return this.props.config.set(name, value);
const config = {
[name]: value,
};
return this.props.config.set(config);
};

clearConfig = name => {
Expand Down
11 changes: 8 additions & 3 deletions src/legacy/ui/public/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,14 @@ module.service(`config`, function($rootScope, Promise) {
// modify remove() to use angular Promises
this.remove = key => Promise.resolve(uiSettings.remove(key));

// modify set() to use angular Promises and angular.toJson()
this.set = (key, value) =>
Promise.resolve(uiSettings.set(key, isPlainObject(value) ? angular.toJson(value) : value));
// modify set() to use angular Promises
this.set = change => {
const processedChange = {};
Object.entries(change).forEach(([key, value]) => {
processedChange[key] = isPlainObject(value) ? angular.toJson(value) : value;
});
return Promise.resolve(uiSettings.set(processedChange));
};

//////////////////////////////
//* angular specific methods *
Expand Down

0 comments on commit 0c993ee

Please sign in to comment.