#5949 - Delete of micromolecules bonds works wrong (or doesn't work)

- reworked bonds/atoms deletion logic - moved post execution logic from renderer to command
epam · Dec 11, 2024 · 0fc2679 · 0fc2679
1 parent 5009d97
commit 0fc2679
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 25 deletions.
diff --git a/packages/ketcher-core/src/application/editor/EditorHistory.ts b/packages/ketcher-core/src/application/editor/EditorHistory.ts
@@ -76,7 +76,6 @@ export class EditorHistory {
 
     const lastCommand = this.historyStack[this.historyPointer];
     lastCommand.execute(this.editor.renderersContainer);
-    lastCommand.executeAfterAllOperations(this.editor.renderersContainer);
     this.historyPointer++;
   }
 

diff --git a/packages/ketcher-core/src/application/editor/operations/coreAtom/atom.ts b/packages/ketcher-core/src/application/editor/operations/coreAtom/atom.ts
@@ -84,6 +84,7 @@ function deleteAtomFromMoleculeStruct(atom: Atom) {
 export class AtomAddOperation implements Operation {
   public atom: Atom;
   private deletedMoleculeStructItems?: DeletedMoleculeStructItems;
+  public priority = 2;
 
   constructor(
     public addAtomChangeModel: (atom?: Atom) => Atom,
@@ -92,9 +93,8 @@ export class AtomAddOperation implements Operation {
     this.atom = this.addAtomChangeModel();
   }
 
-  public execute(renderersManager: RenderersManager) {
+  public execute() {
     this.atom = this.addAtomChangeModel(this.atom);
-    renderersManager.addAtom(this.atom);
 
     if (this.deletedMoleculeStructItems) {
       addAtomToMoleculeStruct(
@@ -105,28 +105,35 @@ export class AtomAddOperation implements Operation {
     }
   }
 
-  public invert(renderersManager: RenderersManager) {
+  public invert() {
     if (this.atom) {
       this.deleteAtomChangeModel(this.atom);
-      renderersManager.deleteAtom(this.atom);
     }
 
     this.deletedMoleculeStructItems = deleteAtomFromMoleculeStruct(this.atom);
   }
+
+  public executeAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.addAtom(this.atom);
+  }
+
+  public invertAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.deleteAtom(this.atom);
+  }
 }
 
 export class AtomDeleteOperation implements Operation {
   private deletedMoleculeStructItems?: DeletedMoleculeStructItems;
+  public priority = 2;
 
   constructor(
     public atom: Atom,
     public deleteAtomChangeModel: () => void,
     public addAtomChangeModel: (atom?: Atom) => Atom,
   ) {}
 
-  public execute(renderersManager: RenderersManager) {
+  public execute() {
     this.deleteAtomChangeModel();
-    renderersManager.deleteAtom(this.atom);
 
     this.deletedMoleculeStructItems = deleteAtomFromMoleculeStruct(this.atom);
   }
@@ -146,4 +153,8 @@ export class AtomDeleteOperation implements Operation {
   public invertAfterAllOperations(renderersManager: RenderersManager) {
     renderersManager.addAtom(this.atom);
   }
+
+  public executeAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.deleteAtom(this.atom);
+  }
 }
diff --git a/packages/ketcher-core/src/application/editor/operations/coreBond/bond.ts b/packages/ketcher-core/src/application/editor/operations/coreBond/bond.ts
@@ -42,44 +42,51 @@ function deleteBondFromMoleculeStruct(bond: Bond) {
 export class BondAddOperation implements Operation {
   public bond: Bond;
   private bondInMoleculeStruct?: MicromoleculesBond;
+  public priority = 1;
   constructor(
     public addBondChangeModel: (bond?: Bond) => Bond,
     public deleteBondChangeModel: (bond: Bond) => void,
   ) {
     this.bond = this.addBondChangeModel();
   }
 
-  public execute(renderersManager: RenderersManager) {
+  public execute() {
     this.bond = this.addBondChangeModel(this.bond);
-    renderersManager.addBond(this.bond);
 
     if (this.bondInMoleculeStruct) {
       addBondToMoleculeStruct(this.bond, this.bondInMoleculeStruct);
     }
   }
 
-  public invert(renderersManager: RenderersManager) {
+  public invert() {
     if (this.bond) {
       this.deleteBondChangeModel(this.bond);
-      renderersManager.deleteBond(this.bond);
     }
 
     this.bondInMoleculeStruct = deleteBondFromMoleculeStruct(this.bond);
   }
+
+  public executeAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.addBond(this.bond);
+  }
+
+  public invertAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.deleteBond(this.bond);
+  }
 }
 
 export class BondDeleteOperation implements Operation {
   private bondInMoleculeStruct?: MicromoleculesBond;
+  public priority = 1;
 
   constructor(
     public bond: Bond,
     public deleteBondChangeModel: (bond: Bond) => void,
     public addBondChangeModel: (bond: Bond) => Bond,
   ) {}
 
-  public execute(renderersManager: RenderersManager) {
+  public execute() {
     this.deleteBondChangeModel(this.bond);
-    renderersManager.deleteBond(this.bond);
 
     this.bondInMoleculeStruct = deleteBondFromMoleculeStruct(this.bond);
   }
@@ -92,6 +99,10 @@ export class BondDeleteOperation implements Operation {
     }
   }
 
+  public executeAfterAllOperations(renderersManager: RenderersManager) {
+    renderersManager.deleteBond(this.bond);
+  }
+
   public invertAfterAllOperations(renderersManager: RenderersManager) {
     renderersManager.addBond(this.bond);
   }

diff --git a/packages/ketcher-core/src/application/editor/operations/drawingEntity/index.ts b/packages/ketcher-core/src/application/editor/operations/drawingEntity/index.ts
@@ -36,15 +36,8 @@ export class DrawingEntityMoveOperation implements Operation {
       : this.moveDrawingEntityChangeModel();
   }
 
-  public invert(renderersManager: RenderersManager) {
+  public invert() {
     this.invertMoveDrawingEntityChangeModel();
-
-    if (this.drawingEntity instanceof BaseBond) {
-      renderersManager.redrawDrawingEntity(this.drawingEntity);
-    } else {
-      renderersManager.moveDrawingEntity(this.drawingEntity);
-    }
-
     this.wasInverted = true;
   }
 
@@ -59,6 +52,14 @@ export class DrawingEntityMoveOperation implements Operation {
       renderersManager.moveDrawingEntity(this.drawingEntity);
     }
   }
+
+  public invertAfterAllOperations(renderersManager: RenderersManager) {
+    if (this.drawingEntity instanceof BaseBond) {
+      renderersManager.redrawDrawingEntity(this.drawingEntity);
+    } else {
+      renderersManager.moveDrawingEntity(this.drawingEntity);
+    }
+  }
 }
 
 export class DrawingEntityRedrawOperation implements Operation {

diff --git a/packages/ketcher-core/src/application/render/renderers/RenderersManager.ts b/packages/ketcher-core/src/application/render/renderers/RenderersManager.ts
@@ -445,8 +445,6 @@ export class RenderersManager {
   public update(modelChanges?: Command) {
     this.reinitializeViewModel();
     modelChanges?.execute(this);
-    modelChanges?.executeAfterAllOperations(this);
-
     this.runPostRenderMethods();
     notifyRenderComplete();
   }

diff --git a/packages/ketcher-core/src/domain/entities/Command.ts b/packages/ketcher-core/src/domain/entities/Command.ts
@@ -42,6 +42,8 @@ export class Command {
     this.operations.forEach((operation) =>
       operation.execute(renderersManagers),
     );
+    renderersManagers.reinitializeViewModel();
+    this.executeAfterAllOperations(renderersManagers);
     renderersManagers.runPostRenderMethods();
   }
 

diff --git a/packages/ketcher-core/src/domain/entities/DrawingEntitiesManager.ts b/packages/ketcher-core/src/domain/entities/DrawingEntitiesManager.ts
@@ -1784,9 +1784,9 @@ export class DrawingEntitiesManager {
       const bondAddCommand = targetDrawingEntitiesManager.addBond(
         newFirstAtom,
         newSecondAtom,
-        bond.bondIdInMicroMode,
         bond.type,
         bond.stereo,
+        bond.bondIdInMicroMode,
       );
       const addedBond = bondAddCommand.operations[0].bond as Bond;
 
@@ -2222,6 +2222,12 @@ export class DrawingEntitiesManager {
 
         command.merge(this.deleteBond(bond));
       });
+
+      this.monomerToAtomBonds.forEach((monomerToAtomBond) => {
+        if (monomerToAtomBond.atom === atom && !monomerToAtomBond.selected) {
+          command.merge(this.deleteMonomerToAtomBond(monomerToAtomBond));
+        }
+      });
     }
 
     return command;
@@ -2299,9 +2305,9 @@ export class DrawingEntitiesManager {
           this.addBondChangeModel(
             bond.firstAtom,
             bond.secondAtom,
-            bond.bondIdInMicroMode,
             bond.type,
             bond.stereo,
+            bond.bondIdInMicroMode,
             bond,
           ),
       ),