Backmerge: #2949 Attachment points should be within S-Group brackets (#…

…2977)

ketcher-autotests/tests/R-group-tool/r-group-fragment-tool.spec.ts

@@ -17,6 +17,7 @@ import {
   openFileAndAddToCanvas,
   SelectTool,
   pressButton,
+  clickOnAtom,
 } from '@utils';
 
 async function openRGroupModalForTopAtom(page: Page) {
@@ -31,6 +32,7 @@ async function openRGroupModalForTopAtom(page: Page) {
 }
 
 const rGroupFromFile = 'R8';
+const atomIndex = 3;
 async function selectRGroups(page: Page, rGroups: string[]) {
   await selectNestedTool(page, RgroupTool.R_GROUP_FRAGMENT);
   await page.getByText(rGroupFromFile).click();
@@ -45,7 +47,7 @@ async function selectRGroup(page: Page, rgroup: string) {
 }
 
 async function clickModalButton(page: Page, button: 'Apply' | 'Cancel') {
-  await page.locator(`input[type="button"][value="${button}"]`);
+  await page.locator(`input[type="button"][value="${button}"]`).click();
 }
 
 test.describe('Open Ketcher', () => {
@@ -121,6 +123,32 @@ test.describe('Open Ketcher', () => {
     await clickModalButton(page, 'Apply');
   });
 
+  test('Brackets rendering for whole r-group structure', async ({ page }) => {
+    await openFileAndAddToCanvas('simple-chain.ket', page);
+    await selectNestedTool(page, RgroupTool.R_GROUP_FRAGMENT);
+    await clickOnAtom(page, 'C', atomIndex);
+    await page.getByText(rGroupFromFile).click();
+    await clickModalButton(page, 'Apply');
+  });
+
+  test('Brackets rendering for whole r-group structure even with attachment points', async ({
+    page,
+  }) => {
+    await openFileAndAddToCanvas('simple-chain.ket', page);
+    await selectNestedTool(page, RgroupTool.ATTACHMENT_POINTS);
+    await clickOnAtom(page, 'C', atomIndex);
+    await page.getByLabel(AttachmentPoint.PRIMARY).check();
+    await clickModalButton(page, 'Apply');
+    await selectNestedTool(
+      page,
+      RgroupTool.R_GROUP_FRAGMENT,
+      RgroupTool.ATTACHMENT_POINTS,
+    );
+    await clickOnAtom(page, 'C', atomIndex);
+    await page.getByText(rGroupFromFile).click();
+    await clickModalButton(page, 'Apply');
+  });
+
   test('Remove R-Group member from R-Group', async ({ page }) => {
     /* Test case: EPMLSOPKET-1589
       Description: Remove R-Group member from R-Group. File used for test - R-fragment-structure.mol

ketcher-autotests/tests/Structure-Creating-&-Editing/Actions-With-Structures/S-Group-Tool/Multiple-S-Group-Tool/multiple-s-group-tool.spec.ts

@@ -20,6 +20,9 @@ import {
   fillFieldByLabel,
   screenshotBetweenUndoRedo,
   saveToFile,
+  RgroupTool,
+  selectNestedTool,
+  AttachmentPoint,
 } from '@utils';
 import { getMolfile } from '@utils/formats';
 
@@ -70,7 +73,7 @@ test.describe('Multiple S-Group tool', () => {
     await selectMultipleGroup(page, 'Data', 'Multiple group', '88');
   });
 
-  test('Brackets rendering for whole structure', async ({ page }) => {
+  test('Brackets rendering for whole s-group structure', async ({ page }) => {
     /*
       Test case: EPMLSOPKET-1506
       Description: The brackets are rendered correctly around whole structure
@@ -81,6 +84,19 @@ test.describe('Multiple S-Group tool', () => {
     await selectMultipleGroup(page, 'Data', 'Multiple group', '88');
   });
 
+  test('Brackets rendering for whole s-group structure even with attachment points', async ({
+    page,
+  }) => {
+    await openFileAndAddToCanvas('simple-chain.ket', page);
+    await selectNestedTool(page, RgroupTool.ATTACHMENT_POINTS);
+    await clickOnAtom(page, 'C', 3);
+    await page.getByLabel(AttachmentPoint.PRIMARY).check();
+    await pressButton(page, 'Apply');
+    await page.keyboard.press('Control+a');
+    await selectLeftPanelButton(LeftPanelButton.S_Group, page);
+    await selectMultipleGroup(page, 'Data', 'Multiple group', '88');
+  });
+
   test('Edit S-Group', async ({ page }) => {
     /*
       Test case: EPMLSOPKET-1520

ketcher-autotests/tests/utils/canvas/tools/selectNestedTool/helpers.ts

@@ -21,10 +21,12 @@ export const openTool = async (
   page: Page,
   defaultToolId: string,
   currentType: string,
+  lastType?: string,
 ) => {
   // Selection tool is preselected by default so we do not need to click 2 times
   const defaultTool = await page.getByTestId(defaultToolId);
-  if (currentType !== TYPE_SELECT) {
+  lastType = lastType || TYPE_SELECT;
+  if (currentType !== lastType) {
     await defaultTool.click();
     await delay(DELAY_IN_SECONDS.ONE);
   }

ketcher-autotests/tests/utils/canvas/tools/selectNestedTool/selectNestedTool.ts

@@ -9,22 +9,25 @@ import { getToolType, openTool } from './helpers';
  *
  * @param page - playwright page object
  * @param toolElementId - tuple [presses: number, domElementId: string]
+ * @param currentToolId - current selected-group tool
  * presses - specifies the number of times to press Tab key
  * domElementId - id of the element to search
  */
 export const selectNestedTool = async (
   page: Page,
   toolElementId: [presses: number, domElementId: string],
+  currentToolId?: [presses: number, domElementId: string],
 ): Promise<void> => {
   const toolType = getToolType(toolElementId[1]);
 
   const toolTypeValues = Object.values(toolType);
 
   if (toolType && toolTypeValues.includes(toolElementId)) {
-    const defaultToolId = toolTypeValues[0][1];
+    const defaultToolId =
+      (currentToolId && currentToolId[1]) || toolTypeValues[0][1];
     const currentType = toolElementId[1].split('-')[0];
-
-    await openTool(page, defaultToolId, currentType);
+    const lastType = currentToolId && currentToolId[1].split('-')[0];
+    await openTool(page, defaultToolId, currentType, lastType);
 
     const numberOfPresses = toolElementId[0];
 

packages/ketcher-core/__tests__/application/editor/actions/bond.test.ts

@@ -1,6 +1,6 @@
 import * as utils from 'application/editor/actions/utils';
 
-import { restruct, singleBond } from './data';
+import { restruct, singleBond } from '../../../mock-data';
 import { fromBondAddition } from 'application/editor/actions';
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any

packages/ketcher-core/__tests__/application/render/restruct/rergroup.test.ts

@@ -0,0 +1,19 @@
+import { ReRGroup, ReStruct } from 'application/render/restruct';
+import { restruct } from '../../../mock-data';
+import { RGroup } from 'domain/entities';
+import { mock, mockFn } from 'jest-mock-extended';
+import { Render } from 'src';
+
+describe('rergroup should calculate R-Group bounding box correctly', () => {
+  it('should calculate R-Group attachments points bounding box', () => {
+    const render = mock<Render>();
+    render.ctab = { ...restruct.molecule } as unknown as ReStruct;
+    const rGroup = new RGroup();
+    rGroup.frags.add(0);
+    const rerGroup = new ReRGroup(rGroup);
+    rerGroup.getAtoms = mockFn().mockReturnValue(restruct.molecule.atoms);
+    const attachmentsSpy = jest.spyOn(render.ctab, 'getAttachmentsPointsVBox');
+    rerGroup.calcBBox(render);
+    expect(attachmentsSpy).toHaveBeenCalled();
+  });
+});

packages/ketcher-core/__tests__/domain/entities/sgroup.test.ts

@@ -0,0 +1,17 @@
+import { ReStruct } from 'application/render/restruct';
+import { restruct } from '../../mock-data';
+import { SGroup } from 'domain/entities';
+import { mock } from 'jest-mock-extended';
+import { Render } from 'src';
+
+describe('sgroup should calculate S-Group bounding box correctly', () => {
+  it('should calculate S-Group attachments points bounding box', () => {
+    const render = mock<Render>();
+    render.ctab = { ...restruct.molecule } as unknown as ReStruct;
+    const sGroup = new SGroup('MUL');
+    sGroup.atoms = [0, 1, 2, 3, 4];
+    const attachmentsSpy = jest.spyOn(render.ctab, 'getAttachmentsPointsVBox');
+    SGroup.bracketPos(sGroup, restruct.molecule, {}, undefined, render);
+    expect(attachmentsSpy).toHaveBeenCalled();
+  });
+});

packages/ketcher-core/__tests__/application/editor/actions/data.ts → packages/ketcher-core/__tests__/mock-data.ts

@@ -1,7 +1,8 @@
 /* eslint-disable @typescript-eslint/no-empty-function */
 import { ReAtom, ReBond } from 'application/render';
 
-import { Pool } from 'domain/entities';
+import { Box2Abs, Pool, Vec2 } from 'domain/entities';
+import { mockFn } from 'jest-mock-extended';
 
 const mockAtoms = [
   {
@@ -634,6 +635,18 @@ const mockFrags = [
     stereoFlagPosition: undefined,
     enhancedStereoFlag: 'ABS',
     updateStereoFlag() {},
+    calcBBox: mockFn().mockReturnValue({
+      p0: {
+        x: 4,
+        y: 6,
+        z: 0,
+      },
+      p1: {
+        x: 6,
+        y: 8,
+        z: 0,
+      },
+    }),
   },
 ];
 const frags = new Map();
@@ -680,11 +693,26 @@ const molecule = {
   bondInitHalfBonds() {},
   atomAddNeighbor() {},
   setImplicitHydrogen() {},
+  getAttachmentsPointsVBox: mockFn().mockReturnValue(
+    new Box2Abs(
+      new Vec2({
+        x: 6,
+        y: 7,
+        z: 0,
+      }),
+      new Vec2({
+        x: 7,
+        y: 9,
+        z: 0,
+      }),
+    ),
+  ),
 };
 
 export const restruct = {
   atoms: new Map(),
   bonds: new Map(),
+  frags,
   molecule,
   connectedComponents: new Set(),
   render: {

packages/ketcher-core/src/application/render/restruct/rergroup.js

@@ -67,11 +67,6 @@ class ReRGroup extends ReObject {
           : fragBox;
       }
     });
-
-    rGroupBoundingBox = rGroupBoundingBox
-      ? rGroupBoundingBox.extend(BORDER_EXT, BORDER_EXT)
-      : rGroupBoundingBox;
-
     let attachmentPointsVBox = render.ctab.getAttachmentsPointsVBox(
       this.getAtoms(render),
     );

packages/ketcher-core/src/domain/entities/sgroup.ts

@@ -459,6 +459,8 @@ export class SGroup {
     remol?: ReStruct,
     render?,
   ): void {
+    const BORDER_EXT = new Vec2(0.05 * 3, 0.05 * 3);
+    const PADDING_VECTOR = new Vec2(0.2, 0.4);
     const atoms = sGroup.atoms;
     const crossBonds = crossBondsPerAtom
       ? Object.values(crossBondsPerAtom).flat()
@@ -482,7 +484,6 @@ export class SGroup {
     };
     atoms.forEach((aid) => {
       const atom = getAtom(aid);
-      const ext = new Vec2(0.05 * 3, 0.05 * 3);
       let position;
       let structBoundingBox;
       if ('getVBoxObj' in atom && render) {
@@ -491,7 +492,7 @@ export class SGroup {
         position = new Vec2(atom.pp);
         structBoundingBox = new Box2Abs(position, position);
       }
-      contentBoxes.push(structBoundingBox.extend(ext, ext));
+      contentBoxes.push(structBoundingBox.extend(BORDER_EXT, BORDER_EXT));
     });
     contentBoxes.forEach((bba) => {
       let bbb: Box2Abs | null = null;
@@ -504,8 +505,18 @@ export class SGroup {
       });
       braketBox = !braketBox ? bbb : Box2Abs.union(braketBox, bbb!);
     });
-    const vext = new Vec2(0.2, 0.4);
-    if (braketBox) braketBox = (braketBox as Box2Abs).extend(vext, vext);
+    if (!render) render = window.ketcher!.editor.render;
+    let attachmentPointsVBox = render.ctab.getAttachmentsPointsVBox(atoms);
+    attachmentPointsVBox = attachmentPointsVBox
+      ? attachmentPointsVBox.extend(BORDER_EXT, BORDER_EXT)
+      : attachmentPointsVBox;
+
+    braketBox =
+      attachmentPointsVBox && braketBox
+        ? Box2Abs.union(attachmentPointsVBox, braketBox)
+        : braketBox;
+    if (braketBox)
+      braketBox = (braketBox as Box2Abs).extend(PADDING_VECTOR, PADDING_VECTOR);
     sGroup.bracketBox = braketBox;
   }