Skip to content

Commit 5000811

Browse files
committed
Debounce auto save for rapid config changes
Prevent excessive save requests when dragging sliders with saveOnSlide enabled. The first change saves immediately to prevent changing behavior for regular updates. Subsequent rapid changes are batched into a single trailing save that fires once changes stop. REDMINE-21191
1 parent cca77a9 commit 5000811

2 files changed

Lines changed: 124 additions & 6 deletions

File tree

package/spec/editor/models/mixins/configurationContainer-spec.js

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,91 @@ describe('configurationContainer', () => {
102102
expect(model.save).toHaveBeenCalled();
103103
});
104104

105+
it('debounces auto save for rapid changes', () => {
106+
jest.useFakeTimers();
107+
const Model = Backbone.Model.extend({
108+
mixins: [configurationContainer({autoSave: true})],
109+
});
110+
const model = new Model({id: 5, configuration: {some: 'value'}});
111+
model.save = jest.fn();
112+
113+
model.configuration.set('some', 'a');
114+
model.configuration.set('some', 'b');
115+
model.configuration.set('some', 'c');
116+
117+
expect(model.save).toHaveBeenCalledTimes(1);
118+
119+
jest.runAllTimers();
120+
121+
expect(model.save).toHaveBeenCalledTimes(2);
122+
jest.useRealTimers();
123+
});
124+
125+
it('resets trailing save delay on each new change', () => {
126+
jest.useFakeTimers();
127+
const Model = Backbone.Model.extend({
128+
mixins: [configurationContainer({autoSave: true})],
129+
});
130+
const model = new Model({id: 5, configuration: {some: 'value'}});
131+
model.save = jest.fn();
132+
133+
model.configuration.set('some', 'a');
134+
expect(model.save).toHaveBeenCalledTimes(1);
135+
136+
jest.advanceTimersByTime(300);
137+
model.configuration.set('some', 'b');
138+
139+
jest.advanceTimersByTime(300);
140+
expect(model.save).toHaveBeenCalledTimes(1);
141+
142+
jest.advanceTimersByTime(200);
143+
expect(model.save).toHaveBeenCalledTimes(2);
144+
jest.useRealTimers();
145+
});
146+
147+
it('does not fire trailing save for single change', () => {
148+
jest.useFakeTimers();
149+
const Model = Backbone.Model.extend({
150+
mixins: [configurationContainer({autoSave: true})],
151+
});
152+
const model = new Model({id: 5, configuration: {some: 'value'}});
153+
model.save = jest.fn();
154+
155+
model.configuration.set('some', 'other value');
156+
157+
expect(model.save).toHaveBeenCalledTimes(1);
158+
159+
jest.runAllTimers();
160+
161+
expect(model.save).toHaveBeenCalledTimes(1);
162+
jest.useRealTimers();
163+
});
164+
165+
it('does not fire trailing save for destroyed model', () => {
166+
jest.useFakeTimers();
167+
const Model = Backbone.Model.extend({
168+
mixins: [
169+
configurationContainer({autoSave: true}),
170+
delayedDestroying
171+
],
172+
});
173+
const model = new Model({id: 5, configuration: {some: 'value'}}, {urlRoot: '/models'});
174+
model.save = jest.fn();
175+
176+
model.configuration.set('some', 'a');
177+
model.configuration.set('some', 'b');
178+
179+
expect(model.save).toHaveBeenCalledTimes(1);
180+
181+
model.destroyWithDelay();
182+
testContext.requests[0].respond(204, { 'Content-Type': 'application/json' }, '');
183+
184+
jest.runAllTimers();
185+
186+
expect(model.save).toHaveBeenCalledTimes(1);
187+
jest.useRealTimers();
188+
});
189+
105190
it('does not auto save new model', () => {
106191
const Model = Backbone.Model.extend({
107192
mixins: [configurationContainer({autoSave: true})],

package/src/editor/models/mixins/configurationContainer.js

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,21 @@ export function configurationContainer({configurationModel, autoSave, includeAtt
4444
this.configuration = new configurationModel(this.get('configuration'));
4545
this.configuration.parent = this;
4646

47-
this.listenTo(this.configuration, 'change', function(model, options) {
48-
if (!this.isNew() &&
49-
(!this.isDestroying || !this.isDestroying()) &&
50-
(!this.isDestroyed || !this.isDestroyed()) &&
51-
autoSave &&
52-
options.autoSave !== false) {
47+
const canSave = () =>
48+
!this.isNew() &&
49+
(!this.isDestroying || !this.isDestroying()) &&
50+
(!this.isDestroyed || !this.isDestroyed());
51+
52+
const debouncedSave = leadingTrailingDebounce(() => {
53+
if (canSave()) {
5354
this.save();
5455
}
56+
}, 500);
57+
58+
this.listenTo(this.configuration, 'change', function(model, options) {
59+
if (canSave() && autoSave && options.autoSave !== false) {
60+
debouncedSave();
61+
}
5562

5663
this.trigger('change:configuration', this, undefined, options);
5764

@@ -81,3 +88,29 @@ export function configurationContainer({configurationModel, autoSave, includeAtt
8188
}
8289
};
8390
}
91+
92+
function leadingTrailingDebounce(fn, delay) {
93+
let timer = null;
94+
let pendingTrailing = false;
95+
96+
function debounced() {
97+
if (timer === null) {
98+
fn();
99+
}
100+
else {
101+
pendingTrailing = true;
102+
clearTimeout(timer);
103+
}
104+
105+
timer = setTimeout(() => {
106+
timer = null;
107+
108+
if (pendingTrailing) {
109+
pendingTrailing = false;
110+
fn();
111+
}
112+
}, delay);
113+
}
114+
115+
return debounced;
116+
}

0 commit comments

Comments
 (0)