Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add highlighting for loose inter-service relations #6020

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelogs/unreleased/5998-loose-relations.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
description: Add highlighting for not connected inter-service relations elements on the canvas in the Instance Composer
issue-nr: 5998
change-type: patch
destination-branches: [master]
sections:
minor-improvement: "{{description}}"
6 changes: 6 additions & 0 deletions cypress/e2e/scenario-8-instance-composer.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,9 @@ if (Cypress.env("edition") === "iso") {
})
.trigger("mouseup");

//highlighted loose element should be visible
cy.get(".joint-loose_element-highlight").should("be.visible");

cy.get('[data-type="app.ServiceEntityBlock"')
.contains("many-defaults")
.click();
Expand All @@ -470,6 +473,9 @@ if (Cypress.env("edition") === "iso") {
})
.trigger("mouseup");

//highlighted loose element should be removed
cy.get(".joint-loose_element-highlight").should("not.exist");

cy.get('[data-type="Link"]').should("have.length", 3);

//add another parent instance to the canvas and connect it to the embedded instance
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface InstanceWithRelations {
instance: ServiceInstanceModel;
interServiceRelations: ServiceInstanceModel[];
coordinates?: string;
referenced_by?: boolean;
}

/**
Expand All @@ -29,13 +30,15 @@ interface GetInstanceWithRelationsHook {
* React Query hook to fetch an instance with its related instances from the API. The related instances are all instances connected with given instance by inter-service relation, both, as a parent and as a child.
* @param {string} id - The ID of the instance to fetch.
* @param {string} environment - The environment in which we are looking for instances.
* @param {ServiceModel} serviceModel - The service Model of the instance (optional as it can be undefined at the init of the component that use the hook)
* @param {ServiceModel} [serviceModel] - The service Model of the instance (optional as it can be undefined at the init of the component that use the hook)
* @param {boolean} [referenced_by] - A flag indicating if we should fetch instances that reference our main instance - defaults to false.
* @returns {GetInstanceWithRelationsHook} An object containing a custom hook to fetch the instance with its related instances.
*/
export const useGetInstanceWithRelations = (
instanceId: string,
environment: string,
serviceModel?: ServiceModel,
referenced_by: boolean = false,
): GetInstanceWithRelationsHook => {
//extracted headers to avoid breaking rules of Hooks
const { createHeaders, handleErrors } = useFetchHelpers();
Expand All @@ -57,7 +60,7 @@ export const useGetInstanceWithRelations = (
): Promise<{ data: ServiceInstanceModel }> => {
//we use this endpoint instead /lsm/v1/service_inventory/{service_entity}/{service_id} because referenced_by property includes only ids, without information about service_entity for given ids
const response = await fetch(
`${baseUrl}/lsm/v1/service_inventory?service_id=${id}&include_deployment_progress=false&exclude_read_only_attributes=false&include_referenced_by=true&include_metadata=true`,
`${baseUrl}/lsm/v1/service_inventory?service_id=${id}&include_deployment_progress=false&exclude_read_only_attributes=false&include_referenced_by=${referenced_by}&include_metadata=true`,
{
headers,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const ComposerEditorProvider: React.FC<Props> = ({
instance,
environment,
mainService,
!editable, //if editable is true, we don't fetch referenced_by instances as they should be displayed to keep it aligned with the regular instance form
).useOneTime();

const relatedInventoriesQuery = useGetInventoryList(
Expand Down
15 changes: 7 additions & 8 deletions src/UI/Components/Diagram/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
ServiceInstanceModelWithTargetStates,
ServiceModel,
} from "@/Core";

import { Service, ServiceInstance } from "@/Test";
import {
a as InstanceAttributesA,
Expand Down Expand Up @@ -1782,22 +1781,22 @@ describe("showLinkTools", () => {
graph.addCell(link);
const linkView = paper.findViewByModel(link) as dia.LinkView;

return { graph, linkView, connectionRules };
return { paper, graph, linkView, connectionRules };
};

it("adds tools to the link when instances aren't in EditMode and there is no rule with rw modifier", () => {
const isParentInEditMode = false;
const isChildInEditMode = false;
const modifier = "rw+";
const { graph, linkView, connectionRules } = setup(
const { paper, graph, linkView, connectionRules } = setup(
isParentInEditMode,
isChildInEditMode,
modifier,
);

expect(linkView.hasTools()).toBeFalsy();

showLinkTools(graph, linkView, connectionRules);
showLinkTools(paper, graph, linkView, connectionRules);

expect(linkView.hasTools()).toBeTruthy();
});
Expand All @@ -1806,15 +1805,15 @@ describe("showLinkTools", () => {
const isParentInEditMode = true;
const isChildInEditMode = false;
const modifier = "rw";
const { graph, linkView, connectionRules } = setup(
const { paper, graph, linkView, connectionRules } = setup(
isParentInEditMode,
isChildInEditMode,
modifier,
);

expect(linkView.hasTools()).toBeFalsy();

showLinkTools(graph, linkView, connectionRules);
showLinkTools(paper, graph, linkView, connectionRules);

expect(linkView.hasTools()).toBeTruthy();
});
Expand All @@ -1823,15 +1822,15 @@ describe("showLinkTools", () => {
const isParentInEditMode = false;
const isChildInEditMode = true;
const modifier = "rw";
const { graph, linkView, connectionRules } = setup(
const { paper, graph, linkView, connectionRules } = setup(
isParentInEditMode,
isChildInEditMode,
modifier,
);

expect(linkView.hasTools()).toBeFalsy();

showLinkTools(graph, linkView, connectionRules);
showLinkTools(paper, graph, linkView, connectionRules);
expect(linkView.hasTools()).toBeFalsy();
});
});
82 changes: 53 additions & 29 deletions src/UI/Components/Diagram/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -793,13 +793,15 @@ export const updateServiceOrderItems = (
* https://resources.jointjs.com/docs/jointjs/v3.6/joint.html#dia.LinkView
* https://resources.jointjs.com/docs/jointjs/v3.6/joint.html#linkTools
*
* @param {dia.Paper} paper - JointJS paper object
* @param {dia.Graph} graph JointJS graph object
* @param {dia.LinkView} linkView - The view for the joint.dia.Link model.
* @param {ConnectionRules} connectionRules - The rules for the connections between entities.
*
* @returns {void}
*/
export function showLinkTools(
paper: dia.Paper,
graph: dia.Graph,
linkView: dia.LinkView,
connectionRules: ConnectionRules,
Expand Down Expand Up @@ -890,36 +892,9 @@ export function showLinkTools(
target.id as dia.Cell.ID,
) as ServiceEntityBlock;

/**
* Function that remove any data in this connection between cells
* @param {ServiceEntityBlock} elementCell cell that we checking
* @param {ServiceEntityBlock} disconnectingCell cell that is being connected to elementCell
* @returns {void}
*/
const removeConnectionData = (
elementCell: ServiceEntityBlock,
disconnectingCell: ServiceEntityBlock,
): void => {
const elementRelations = elementCell.getRelations();

// resolve any possible relation connections between cells
if (
elementRelations &&
elementRelations.has(String(disconnectingCell.id))
) {
elementCell.removeRelation(String(disconnectingCell.id));

document.dispatchEvent(
new CustomEvent("updateServiceOrderItems", {
detail: { cell: sourceCell, actions: ActionEnum.UPDATE },
}),
);
}
};

//as the connection between two cells is bidirectional we need attempt to remove data from both cells
removeConnectionData(sourceCell, targetCell);
removeConnectionData(targetCell, sourceCell);
removeConnectionData(paper, graph, sourceCell, targetCell);
removeConnectionData(paper, graph, targetCell, sourceCell);

model.remove({ ui: true, tool: toolView.cid });
},
Expand All @@ -929,3 +904,52 @@ export function showLinkTools(

linkView.addTools(tools);
}

/**
* Function that remove data about niter-service relation between cells
* @param {dia.Paper} paper JointJS paper object
* @param {dia.Graph} graph JointJS graph object
* @param {ServiceEntityBlock} elementCell cell that we checking
* @param {ServiceEntityBlock} disconnectingCell cell that is being connected to elementCell
* @returns {void}
*/
const removeConnectionData = (
paper: dia.Paper,
graph: dia.Graph,
elementCell: ServiceEntityBlock,
disconnectingCell: ServiceEntityBlock,
): void => {
const elementRelations = elementCell.getRelations();

// resolve any possible relation connections between cells
if (elementRelations && elementRelations.has(String(disconnectingCell.id))) {
elementCell.removeRelation(String(disconnectingCell.id));

//get all connected entities to inter-service relation cell (the current relation that will be removed is included here)
const disconnectingCellNeighbors = graph.getNeighbors(disconnectingCell);

//the inters-service relataion cell can be also embedded entity, so attempt to get id of the holder
const embeddedToID = disconnectingCell.get("embeddedTo");

//filter out every cell that is either embedded entity of to the disconnectingCell or is core or the disconnectingCell is embedded to it
const interServiceRelations = disconnectingCellNeighbors.filter(
(cell) =>
!(cell.get("embeddedTo") === disconnectingCell.id) ||
!(embeddedToID === cell.id),
);

//all left are the inter-service relation connections, if there is more than one then we should highlight the cell
if (interServiceRelations.length === 1) {
toggleLooseElement(
paper.findViewByModel(disconnectingCell),
EmbeddedEventEnum.ADD,
);
}

document.dispatchEvent(
new CustomEvent("updateServiceOrderItems", {
detail: { cell: elementCell, actions: ActionEnum.UPDATE },
}),
);
}
};
11 changes: 6 additions & 5 deletions src/UI/Components/Diagram/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,14 @@ export function diagramInit(

//trigger highlighter when user drag element from stencil
graph.on("add", function (cell) {
//if it's a link, we don't want to assert any highlighting, if the canvas is not editable then there shouldn't be any highlighting in the first place as there are no loose elements by default and no way to add them
if (cell.get("type") === "Link" || !editable) {
return;
}

const paperRepresentation = paper.findViewByModel(cell);

if (
cell.get("isEmbedded") &&
!cell.get("embeddedTo") &&
paperRepresentation
) {
if (!cell.get("isCore") && paperRepresentation) {
toggleLooseElement(paperRepresentation, EmbeddedEventEnum.ADD);
}
});
Expand Down
6 changes: 5 additions & 1 deletion src/UI/Components/Diagram/paper/paper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ export class ComposerPaper {
return;
}

showLinkTools(graph, linkView, connectionRules);
showLinkTools(this.paper, graph, linkView, connectionRules);
});

//Event that is triggered when user leaves the link. It's used to remove the link tools and the link labels for connected cells
Expand Down Expand Up @@ -283,6 +283,10 @@ export class ComposerPaper {
detail: { cell: sourceCell, action: ActionEnum.UPDATE },
}),
);
toggleLooseElement(
this.paper.findViewByModel(connectingCell),
EmbeddedEventEnum.REMOVE,
);
}
}

Expand Down