fix(editors): select editor should call save only once

- in some use case we want to make sure to call the save() method before destroying it (if it wasn't already called) but we need to make sure that save() wasn't already called. So the save method could be called in 2 places, via `onClose` event or via the `destroy` method, either or we shouldn't call `save` more than once

examples/webpack-demo-vanilla-bundle/src/examples/example12.ts

@@ -352,7 +352,7 @@ export class Example12 {
       {
         id: 'action', name: 'Action', field: 'action', width: 70, minWidth: 70, maxWidth: 70,
         excludeFromExport: true,
-        formatter: () => `<div class="button-style margin-auto" style="width: 35px; margin-top: -1px;"><span class="mdi mdi-chevron-down mdi-22px color-primary"></span></div>`,
+        formatter: () => `<div class="button-style margin-auto" style="width: 35px; margin-top: -1px;"><span class="mdi mdi-dots-vertical mdi-22px color-primary"></span></div>`,
         cellMenu: {
           hideCloseButton: false,
           commandTitle: 'Commands',
@@ -554,6 +554,7 @@ export class Example12 {
   handleOnCellChange(event) {
     const args = event && event.detail && event.detail.args;
     const dataContext = args && args.item;
+    console.log('cell change', args)
 
     // when the field "completed" changes to false, we also need to blank out the "finish" date
     if (dataContext && !dataContext.completed) {

packages/common/src/editors/__tests__/selectEditor.spec.ts

@@ -539,10 +539,25 @@ describe('SelectEditor', () => {
         editor.loadValue(mockItemData);
         editor.destroy();
 
+        expect(saveSpy).toHaveBeenCalledTimes(1);
         expect(saveSpy).toHaveBeenCalledWith(true);
         expect(lockSpy).toHaveBeenCalled();
       });
 
+      it('should call "save" only once even when both "onClose" and "destroy" are called', () => {
+        mockItemData = { id: 1, gender: 'male', isActive: true };
+        gridOptionMock.autoCommitEdit = true;
+
+        editor = new SelectEditor(editorArguments, true);
+        const saveSpy = jest.spyOn(editor, 'save');
+
+        editor.loadValue(mockItemData);
+        editor.editorDomElement.multipleSelect('close');
+        editor.destroy();
+
+        expect(saveSpy).toHaveBeenCalledTimes(1);
+      });
+
       it('should not call "commitCurrentEdit" when "hasAutoCommitEdit" is disabled', () => {
         mockItemData = { id: 1, gender: 'male', isActive: true };
         gridOptionMock.autoCommitEdit = false;

packages/common/src/editors/selectEditor.ts

@@ -37,7 +37,7 @@ export class SelectEditor implements Editor {
 
   // flag to signal that the editor is destroying itself, helps prevent
   // commit changes from being called twice and erroring
-  protected _isDisposing = false;
+  protected _isDisposingOrCallingSave = false;
 
   /** Collection Service */
   protected _collectionService!: CollectionService;
@@ -129,7 +129,8 @@ export class SelectEditor implements Editor {
         if (compositeEditorOptions) {
           this.handleChangeOnCompositeEditor(compositeEditorOptions);
         } else {
-          this.save();
+          this._isDisposingOrCallingSave = true;
+          this.save(this.hasAutoCommitEdit);
         }
       },
     };
@@ -403,12 +404,12 @@ export class SelectEditor implements Editor {
   destroy() {
     // when autoCommitEdit is enabled, we might end up leaving an editor without it being saved, if so do call a save before destroying
     // this mainly happens doing a blur or focusing on another cell in the grid (it won't come here if we click outside the grid, in the body)
-    if (this.$editorElm && this.hasAutoCommitEdit && this.isValueChanged() && !this._isDisposing && !this.isCompositeEditor) {
-      this._isDisposing = true; // change destroying flag to avoid infinite loop
+    if (this.$editorElm && this.hasAutoCommitEdit && this.isValueChanged() && !this._isDisposingOrCallingSave && !this.isCompositeEditor) {
+      this._isDisposingOrCallingSave = true; // change destroying flag to avoid infinite loop
       this.save(true);
     }
 
-    this._isDisposing = true;
+    this._isDisposingOrCallingSave = true;
     if (this.$editorElm && typeof this.$editorElm.multipleSelect === 'function') {
       this.$editorElm.multipleSelect('destroy');
       this.$editorElm.remove();
@@ -548,7 +549,7 @@ export class SelectEditor implements Editor {
     const validation = this.validate();
     const isValid = validation?.valid ?? false;
 
-    if ((!this._isDisposing || forceCommitCurrentEdit) && this.hasAutoCommitEdit && isValid) {
+    if ((!this._isDisposingOrCallingSave || forceCommitCurrentEdit) && this.hasAutoCommitEdit && isValid) {
       // do not use args.commitChanges() as this sets the focus to the next row.
       // also the select list will stay shown when clicking off the grid
       this.grid.getEditorLock().commitCurrentEdit();