Improve Instance Composer code readability and remove duplicated code…

… (Issue #5443, PR #5456) # Description * Short description here * closes #5443 # Self Check: Strike through any lines that are not applicable (`~~line~~`) then check the box - [x] Attached issue to pull request - [x] Changelog entry - [x] Code is clear and sufficiently documented - [x] Sufficient test cases (reproduces the bug/tests the requested feature) - [x] Correct, in line with design - [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )
inmanta · Jan 23, 2024 · 89afc8f · 89afc8f
1 parent 0236675
commit 89afc8f
Show file tree

Hide file tree

Showing 11 changed files with 164 additions and 160 deletions.
diff --git a/changelogs/unreleased/5443-instance-composer-refactor.yml b/changelogs/unreleased/5443-instance-composer-refactor.yml
@@ -0,0 +1,6 @@
+description: "Improve Instance Composer code readability and remove duplicated code"
+issue-nr: 5443
+change-type: patch
+destination-branches: [master, iso7]
+sections:
+  minor-improvement: "{{description}}"
diff --git a/cypress/e2e/scenario-8-instance-composer.cy.js b/cypress/e2e/scenario-8-instance-composer.cy.js
@@ -154,7 +154,7 @@ if (Cypress.env("edition") === "iso") {
         .trigger("mousedown")
         .trigger("mousemove", {
           clientX: 700,
-          clientY: 300,
+          clientY: 400,
         })
         .trigger("mouseup");
 
@@ -211,7 +211,7 @@ if (Cypress.env("edition") === "iso") {
       cy.get(".canvas").should("be.visible");
       cy.get('[data-type="app.ServiceEntityBlock"]').should("be.visible");
       cy.get('[joint-selector="headerLabel"]')
-        .contains("embedded-entity-service-extra")
+        .contains("embedded-entity-")
         .should("exist");
       cy.get('[joint-selector="headerLabel"]')
         .contains("ro_meta")
@@ -656,7 +656,7 @@ if (Cypress.env("edition") === "iso") {
         .trigger("mouseover")
         .trigger("mousedown")
         .trigger("mousemove", {
-          clientX: 900,
+          clientX: 1000,
           clientY: 300,
         })
         .trigger("mouseup");

diff --git a/src/UI/Components/Diagram/Canvas.tsx b/src/UI/Components/Diagram/Canvas.tsx
@@ -158,22 +158,27 @@ const Canvas = ({
     setDiagramHandlers(actions);
     if (instance) {
       const isMainInstance = true;
-      const cells = actions.addInstance(instance, services, isMainInstance);
-      const newInstances = new Map();
-      cells.forEach((cell) => {
-        if (cell.type === "app.ServiceEntityBlock") {
-          newInstances.set(cell.id, {
-            instance_id: cell.id,
-            service_entity: cell.entityName,
-            config: {},
-            action: null,
-            attributes: cell.instanceAttributes,
-            embeddedTo: cell.embeddedTo,
-            relatedTo: cell.relatedTo,
-          });
-        }
-      });
-      setInstancesToSend(newInstances);
+      try {
+        const cells = actions.addInstance(instance, services, isMainInstance);
+        const newInstances = new Map();
+        cells.forEach((cell) => {
+          if (cell.type === "app.ServiceEntityBlock") {
+            newInstances.set(cell.id, {
+              instance_id: cell.id,
+              service_entity: cell.entityName,
+              config: {},
+              action: null,
+              attributes: cell.instanceAttributes,
+              embeddedTo: cell.embeddedTo,
+              relatedTo: cell.relatedTo,
+            });
+          }
+        });
+        setInstancesToSend(newInstances);
+      } catch (error) {
+        setAlertType(AlertVariant.danger);
+        setAlertMessage(String(error));
+      }
     }
 
     return () => {

diff --git a/src/UI/Components/Diagram/actions.ts b/src/UI/Components/Diagram/actions.ts
@@ -35,7 +35,16 @@ export function showLinkTools(
     target.id as dia.Cell.ID,
   ) as ServiceEntityBlock;
 
-  const check = (cellOne: ServiceEntityBlock, cellTwo: ServiceEntityBlock) => {
+  /**
+   * checks if the connection between cells can be deleted thus if we should hide linkTool
+   * @param cellOne ServiceEntityBlock
+   * @param cellTwo ServiceEntityBlock
+   * @returns boolean
+   */
+  const shouldHideLinkTool = (
+    cellOne: ServiceEntityBlock,
+    cellTwo: ServiceEntityBlock,
+  ) => {
     const nameOne = cellOne.getName();
     const nameTwo = cellTwo.getName();
 
@@ -56,7 +65,10 @@ export function showLinkTools(
     return false;
   };
 
-  if (check(sourceCell, targetCell) || check(targetCell, sourceCell)) {
+  if (
+    shouldHideLinkTool(sourceCell, targetCell) ||
+    shouldHideLinkTool(targetCell, sourceCell)
+  ) {
     return;
   }
 
@@ -172,7 +184,10 @@ export function appendInstance(
   const serviceInstance = serviceWithReferences.instance.data;
   const serviceInstanceModel = services.find(
     (model) => model.name === serviceInstance.service_entity,
-  ) as ServiceModel;
+  );
+  if (!serviceInstanceModel) {
+    throw Error(words("inventory.instanceComposer.errorMessage"));
+  }
   const instanceAsTable = new ServiceEntityBlock().setName(
     serviceInstance.service_entity,
   );
@@ -186,7 +201,7 @@ export function appendInstance(
   }
 
   //check for any presentable attributes, where candidate attrs have priority, if there is a set, then append them to  JointJS shape and try to display and connect embedded entities
-  if (serviceInstance.candidate_attributes !== null) {
+  if (serviceInstance.candidate_attributes) {
     handleAttributes(
       graph,
       paper,
@@ -196,18 +211,19 @@ export function appendInstance(
       "candidate",
       instanceToConnectRelation,
     );
-  } else {
+  } else if (serviceInstance.active_attributes) {
     handleAttributes(
       graph,
       paper,
       instanceAsTable,
       serviceInstanceModel,
-      serviceInstance.active_attributes as InstanceAttributeModel,
+      serviceInstance.active_attributes,
       "active",
       instanceToConnectRelation,
     );
   }
 
+  //map through relatedInstances and either append them or connect to them
   serviceWithReferences.relatedInstances.forEach((relatedInstance) => {
     const isInstanceMain = false;
     const cellAdded = graph.getCell(relatedInstance.instance.data.id);
@@ -300,90 +316,35 @@ export function appendEmbeddedEntity(
   paper: dia.Paper,
   graph: dia.Graph,
   embeddedEntity: EmbeddedEntity,
-  entityAttributes: InstanceAttributeModel,
+  entityAttributes: InstanceAttributeModel | InstanceAttributeModel[],
   embeddedTo: string | null,
   holderName: string,
   instanceToConnectRelation?: ServiceEntityBlock,
   presentedAttr?: "candidate" | "active",
   isBlockedFromEditing?: boolean,
 ): ServiceEntityBlock[] {
   //Create shape for Entity
-  const flatAttributes = embeddedEntity.attributes.map(
-    (attribute) => attribute.name,
-  );
 
-  if (Array.isArray(entityAttributes)) {
-    const createdInstances: ServiceEntityBlock[] = [];
-
-    (entityAttributes as InstanceAttributeModel[]).map((entityInstance) => {
-      const instanceAsTable = new ServiceEntityBlock()
-        .setTabColor("embedded")
-        .setName(embeddedEntity.name);
-
-      appendColumns(instanceAsTable, flatAttributes, entityInstance);
-      instanceAsTable.set("isEmbedded", true);
-      instanceAsTable.set("holderName", holderName);
-      instanceAsTable.set("embeddedTo", embeddedTo);
-      instanceAsTable.set("isInEditMode", true);
-      instanceAsTable.set("isBlockedFromEditing", isBlockedFromEditing);
-      //add to graph
-      instanceAsTable.addTo(graph);
-
-      createdInstances.push(instanceAsTable);
-      //iterate through embedded entities to create and connect them
-      embeddedEntity.embedded_entities.forEach((entity) => {
-        //we are basing iteration on service Model, if there is no value in the instance, skip that part
-        if (!entityInstance[entity.name]) {
-          return;
-        }
-        const appendedEntities = appendEmbeddedEntity(
-          paper,
-          graph,
-          entity,
-          entityInstance[entity.name] as InstanceAttributeModel,
-          instanceAsTable.id as string,
-          embeddedEntity.name,
-          instanceToConnectRelation,
-          presentedAttr,
-          isBlockedFromEditing,
-        );
-        appendedEntities.map((appendedEntities) => {
-          handleInfoIcon(appendedEntities, presentedAttr);
-        });
-        connectEntities(
-          graph,
-          instanceAsTable,
-          appendedEntities,
-          isBlockedFromEditing,
-        );
-      });
-
-      embeddedEntity.inter_service_relations?.map((relation) => {
-        const relationId = entityAttributes[relation.name] as relationId;
-        if (relationId) {
-          instanceAsTable.addRelation(relationId, relation.name);
-          if (
-            instanceToConnectRelation &&
-            relationId === instanceToConnectRelation.id
-          ) {
-            connectEntities(
-              graph,
-              instanceAsTable,
-              [instanceToConnectRelation],
-              isBlockedFromEditing,
-            );
-          }
-        }
-      });
-    });
+  /**
+   *  Create single Embedded Entity, and handle setting all of the essential data and append it and it's eventual children to the graph.
+   * Then connect it with it's eventual children and other entities that have inter-service relation to this Entity
+   *
+   * @param entityInstance instance of entity Attributes
+   * @returns ServiceEntityBlock
+   */
+  function appendSingleEntity(
+    entityInstance: InstanceAttributeModel,
+  ): ServiceEntityBlock {
+    const flatAttributes = embeddedEntity.attributes.map(
+      (attribute) => attribute.name,
+    );
 
-    return createdInstances;
-  } else {
+    //Create shape for Entity
     const instanceAsTable = new ServiceEntityBlock()
       .setTabColor("embedded")
       .setName(embeddedEntity.name);
 
-    appendColumns(instanceAsTable, flatAttributes, entityAttributes);
+    appendColumns(instanceAsTable, flatAttributes, entityInstance);
     instanceAsTable.set("isEmbedded", true);
     instanceAsTable.set("holderName", holderName);
     instanceAsTable.set("embeddedTo", embeddedTo);
@@ -399,7 +360,7 @@ export function appendEmbeddedEntity(
         paper,
         graph,
         entity,
-        entityAttributes[entity.name] as InstanceAttributeModel,
+        entityInstance[entity.name] as InstanceAttributeModel,
         instanceAsTable.id as string,
         entity.name,
         instanceToConnectRelation,
@@ -418,7 +379,7 @@ export function appendEmbeddedEntity(
     });
 
     embeddedEntity.inter_service_relations?.map((relation) => {
-      const relationId = entityAttributes[relation.name] as relationId;
+      const relationId = entityInstance[relation.name] as relationId;
       if (relationId) {
         instanceAsTable.addRelation(relationId, relation.name);
         if (
@@ -434,7 +395,13 @@ export function appendEmbeddedEntity(
         }
       }
     });
-    return [instanceAsTable];
+    return instanceAsTable;
+  }
+
+  if (Array.isArray(entityAttributes)) {
+    return entityAttributes.map((entity) => appendSingleEntity(entity));
+  } else {
+    return [appendSingleEntity(entityAttributes)];
   }
 }
 
@@ -606,9 +573,11 @@ function handleAttributes(
       presentedAttr,
       !serviceModel.strict_modifier_enforcement,
     );
+
     appendedEntities.map((entity) => {
       handleInfoIcon(entity, presentedAttr);
     });
+
     connectEntities(
       graph,
       instanceAsTable,

diff --git a/src/UI/Components/Diagram/components/FormModal.tsx b/src/UI/Components/Diagram/components/FormModal.tsx
@@ -256,7 +256,7 @@ const FormModal = ({
             onOpenChange={(isOpen) => setIsSelectOpen(isOpen)}
             isOpen={isSelectOpen}
             onSelect={(_evt, value) => {
-              onEntityChosen(value as string, possibleForms);
+              onEntityChosen(String(value), possibleForms);
             }}
           >
             {possibleForms.map(({ key, value }) => (