Skip to content

Commit 3b0dd5d

Browse files
authored
Bug: multiple changes in quick succession on new entry cause exception (#1758)
* When an entry is created, wait for it, before successive saves * Make sure we don't diff anything with a deleted entry. * Fix race conditions when switching entries before saved
1 parent 8665061 commit 3b0dd5d

2 files changed

Lines changed: 72 additions & 63 deletions

File tree

src/angular-app/languageforge/lexicon/editor/editor.component.ts

Lines changed: 71 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ import {
3232
import { LexiconProject } from '../shared/model/lexicon-project.model';
3333
import { LexOptionList } from '../shared/model/option-list.model';
3434
import { FieldControl } from './field/field-control.model';
35-
import {OfflineCacheUtilsService} from '../../../bellows/core/offline/offline-cache-utils.service';
35+
import { OfflineCacheUtilsService } from '../../../bellows/core/offline/offline-cache-utils.service';
36+
import { IPromise } from 'angular';
3637

3738
class Show {
3839
more: () => void;
@@ -77,6 +78,7 @@ export class LexiconEditorController implements angular.IController {
7778

7879
private pristineEntry: LexEntry = new LexEntry();
7980
private warnOfUnsavedEditsId: string;
81+
private saving$: IPromise<void>;
8082

8183
static $inject = ['$filter', '$interval',
8284
'$q', '$scope',
@@ -156,7 +158,7 @@ export class LexiconEditorController implements angular.IController {
156158
this.saveCurrentEntry();
157159
}
158160
// destroy listeners when leaving editor page
159-
angular.element(window).unbind('keyup', (e: Event) => {});
161+
angular.element(window).unbind('keyup', (e: Event) => { });
160162
};
161163

162164
this.show.entryListModifiers = !(this.$window.localStorage.getItem('viewFilter') == null ||
@@ -223,7 +225,7 @@ export class LexiconEditorController implements angular.IController {
223225
$onDestroy(): void {
224226
this.cancelAutoSaveTimer();
225227
this.saveCurrentEntry();
226-
angular.element(window).unbind('keydown', (e: Event) => {});
228+
angular.element(window).unbind('keydown', (e: Event) => { });
227229
}
228230

229231
navigateToLiftImport(): void {
@@ -353,11 +355,16 @@ export class LexiconEditorController implements angular.IController {
353355
return diffs && diffs.length && diffs.some((diff) => diff.kind === 'A');
354356
}
355357

356-
saveCurrentEntry = (doSetEntry: boolean = false, successCallback: () => void = () => { },
358+
saveCurrentEntry = async (doSetEntry: boolean = false, successCallback: () => void = () => { },
357359
failCallback: (reason?: any) => void = () => { }) => {
360+
const isNewEntry = LexiconEditorController.entryIsNew(this.currentEntry);
361+
if (isNewEntry) {
362+
// We have to wait for the initial save to complete so that we have
363+
await this.saving$;
364+
}
365+
358366
// `doSetEntry` is mainly used for when the save button is pressed, that is when the user is saving the current
359367
// entry and is NOT going to a different entry (as is the case with editing another entry.
360-
let isNewEntry = false;
361368
let newEntryTempId: string;
362369

363370
if (this.hasUnsavedChanges() && this.lecRights.canEditEntry()) {
@@ -367,8 +374,7 @@ export class LexiconEditorController implements angular.IController {
367374
this.currentEntry = LexiconEditorController.normalizeStrings(this.currentEntry);
368375
this.control.currentEntry = this.currentEntry;
369376
const entryToSave = angular.copy(this.currentEntry);
370-
if (LexiconEditorController.entryIsNew(entryToSave)) {
371-
isNewEntry = true;
377+
if (isNewEntry) {
372378
newEntryTempId = entryToSave.id;
373379
entryToSave.id = ''; // send empty id to indicate "create new"
374380
}
@@ -379,18 +385,20 @@ export class LexiconEditorController implements angular.IController {
379385
id: entryForUpdate.id,
380386
_update_deep_diff: diff(LexiconEditorController.normalizeStrings(pristineEntryForDiffing), entryForDiffing)
381387
};
388+
382389
let entryOrDiff = isNewEntry ? entryForUpdate : diffForUpdate;
383390
if (!isNewEntry && this.hasArrayChange(diffForUpdate._update_deep_diff)) {
384391
// Updates involving adding or deleting any array item cannot be delta updates due to MongoDB limitations
385392
entryOrDiff = entryForUpdate;
386393
}
387394

388-
return this.$q.all({
389-
entry: this.lexService.update(entryOrDiff),
390-
isSR: this.sendReceive.isSendReceiveProject()
391-
}).then(data => {
392-
const entry = data.entry.data;
393-
if (!entry && data.isSR) {
395+
try {
396+
const { result: { data: entry }, isSR } = await this.$q.all({
397+
result: this.lexService.update(entryOrDiff),
398+
isSR: this.sendReceive.isSendReceiveProject()
399+
});
400+
401+
if (!entry && isSR) {
394402
this.warnOfUnsavedEdits(entryToSave);
395403
this.sendReceive.startSyncStatusTimer();
396404
}
@@ -429,41 +437,40 @@ export class LexiconEditorController implements angular.IController {
429437
}
430438

431439
// refresh data will add the new entry to the entries list
432-
this.editorService.refreshEditorData().then(() => {
433-
this.activityService.markRefreshRequired();
434-
if (entry && isNewEntry) {
435-
this.setCurrentEntry(this.entries[this.editorService.getIndexInList(entry.id, this.entries)]);
436-
this.editorService.removeEntryFromLists(newEntryTempId);
437-
438-
if (doSetEntry) {
439-
this.$state.go('.', {
440-
entryId: entry.id,
441-
}, { notify: false });
442-
443-
this.scrollListToEntry(entry.id, 'top');
444-
}
440+
await this.editorService.refreshEditorData();
441+
this.activityService.markRefreshRequired();
442+
if (entry && isNewEntry) {
443+
this.setCurrentEntry(this.entries[this.editorService.getIndexInList(entry.id, this.entries)]);
444+
this.editorService.removeEntryFromLists(newEntryTempId);
445+
446+
if (doSetEntry) {
447+
this.$state.go('.', {
448+
entryId: entry.id,
449+
}, { notify: false });
450+
451+
this.scrollListToEntry(entry.id, 'top');
445452
}
446-
});
447453

448-
this.saveStatus = 'saved';
449-
successCallback();
450-
}).catch(reason => {
454+
this.saveStatus = 'saved';
455+
successCallback();
456+
}
457+
} catch (reason) {
451458
this.saveStatus = 'unsaved';
452459
failCallback(reason);
453-
});
460+
}
454461
} else {
455462
successCallback();
456463
}
457464
}
458465

459-
editEntryAndScroll(id: string): void {
460-
this.editEntry(id);
466+
async editEntryAndScroll(id: string): Promise<void> {
467+
await this.editEntry(id);
461468
this.scrollListToEntry(id);
462469
}
463470

464-
editEntry(id: string): void {
471+
async editEntry(id: string): Promise<void> {
465472
if (this.currentEntry.id !== id) {
466-
this.saveCurrentEntry();
473+
await this.saveCurrentEntry();
467474
this.setCurrentEntry(this.entries[this.editorService.getIndexInList(id, this.entries)]);
468475
// noinspection JSIgnoredPromiseFromCall - comments will load in the background
469476
this.commentService.loadEntryComments(id);
@@ -476,10 +483,10 @@ export class LexiconEditorController implements angular.IController {
476483
this.goToEntry(id);
477484
}
478485

479-
gotoToEntry(index: number, isValid: boolean) {
486+
async gotoToEntry(index: number, isValid: boolean): Promise<void> {
480487
if (isValid) {
481488
let id = this.editorService.getIdInFilteredList(Number(index));
482-
this.editEntryAndScroll(id);
489+
await this.editEntryAndScroll(id);
483490
}
484491
}
485492

@@ -495,9 +502,9 @@ export class LexiconEditorController implements angular.IController {
495502
return i >= 0 && i < this.visibleEntries.length;
496503
}
497504

498-
skipToEntry(distance: number): void {
505+
async skipToEntry(distance: number): Promise<void> {
499506
const i = this.editorService.getIndexInList(this.currentEntry.id, this.visibleEntries) + distance;
500-
this.editEntry(this.visibleEntries[i].id);
507+
await this.editEntry(this.visibleEntries[i].id);
501508
this.scrollListToEntry(this.visibleEntries[i].id);
502509
}
503510

@@ -518,30 +525,32 @@ export class LexiconEditorController implements angular.IController {
518525
});
519526
}
520527

521-
deleteEntry = (entry: LexEntry): void => {
528+
deleteEntry = async (entry: LexEntry): Promise<void> => {
522529
const deleteMsg = 'Are you sure you want to delete the entry <b>\'' +
523530
LexiconUtilityService.getLexeme(this.lecConfig, this.lecConfig.entry, entry) + '\'</b>?';
524-
this.modal.showModalSimple('Delete Entry', deleteMsg, 'Cancel', 'Delete Entry').then(() => {
525-
let iShowList = this.editorService.getIndexInList(entry.id, this.visibleEntries);
526-
this.editorService.removeEntryFromLists(entry.id);
527-
if (this.entries.length > 0) {
528-
if (iShowList !== 0) {
529-
iShowList--;
530-
}
531-
this.editEntryAndScroll(this.visibleEntries[iShowList].id);
532-
} else {
533-
this.returnToList();
534-
}
531+
await this.modal.showModalSimple('Delete Entry', deleteMsg, 'Cancel', 'Delete Entry');
535532

536-
if (!LexiconEditorController.entryIsNew(entry)) {
537-
this.sendReceive.setStateUnsynced();
538-
this.lexService.remove(entry.id, () => {
539-
this.editorService.refreshEditorData();
540-
});
533+
let iShowList = this.editorService.getIndexInList(entry.id, this.visibleEntries);
534+
this.editorService.removeEntryFromLists(entry.id);
535+
if (this.entries.length > 0) {
536+
if (iShowList !== 0) {
537+
iShowList--;
541538
}
539+
this.currentEntry = new LexEntry();
540+
this.pristineEntry = new LexEntry();
541+
await this.editEntryAndScroll(this.visibleEntries[iShowList].id);
542+
} else {
543+
this.returnToList();
544+
}
542545

543-
this.hideRightPanel();
544-
}, () => { });
546+
if (!LexiconEditorController.entryIsNew(entry)) {
547+
this.sendReceive.setStateUnsynced();
548+
this.lexService.remove(entry.id, () => {
549+
this.editorService.refreshEditorData();
550+
});
551+
}
552+
553+
this.hideRightPanel();
545554
}
546555

547556
makeValidModelRecursive = (config: LexConfigField, data: any = {}, stopAtNodes: string | string[] = []): any => {
@@ -969,7 +978,7 @@ export class LexiconEditorController implements angular.IController {
969978
}
970979
}
971980

972-
private evaluateStateFromURL(): void {
981+
private async evaluateStateFromURL(): Promise<void> {
973982
this.editorService.loadEditorData().then(async () => {
974983
if (this.$state.is("editor.entry")) {
975984

@@ -982,7 +991,7 @@ export class LexiconEditorController implements angular.IController {
982991

983992
// see if there is a most-recently viewed entry in the cache
984993
await this.offlineCacheUtils.getProjectMruEntryData().then(data => {
985-
if(data && data.mruEntryId && this.editorService.getIndexInList(data.mruEntryId, this.entries) != null){
994+
if (data && data.mruEntryId && this.editorService.getIndexInList(data.mruEntryId, this.entries) != null) {
986995
entryId = data.mruEntryId;
987996
}
988997

@@ -992,7 +1001,7 @@ export class LexiconEditorController implements angular.IController {
9921001
}
9931002
});
9941003
}
995-
this.editEntryAndScroll(entryId);
1004+
await this.editEntryAndScroll(entryId);
9961005
} else {
9971006
// there are no entries, go to the list view
9981007
this.$state.go('editor.list');
@@ -1285,7 +1294,7 @@ export class LexiconEditorController implements angular.IController {
12851294
}
12861295

12871296
private static syncListEntryWithCurrentEntry(elementId: string, alignment: string = 'center'): void {
1288-
const element = document.querySelector(elementId);
1297+
const element = document.querySelector(elementId);
12891298
const block = alignment !== 'top' ? 'center' : 'start';
12901299

12911300
// https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView

src/angular-app/languageforge/lexicon/editor/field/field-control.model.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ export class FieldControl {
1414
commentContext: { contextGuid: string };
1515
config: LexiconConfig;
1616
currentEntry: LexEntry;
17-
deleteEntry: (currentEntry: LexEntry) => void;
17+
deleteEntry: (currentEntry: LexEntry) => Promise<void>;
1818
getContextParts: (contextGuid: string) => any;
1919
getNewComment?: () => LexComment;
2020
hideRightPanel: () => void;

0 commit comments

Comments
 (0)