Issue/5868 right sidebar #5930

matborowczyk · 2024-08-30T10:01:02Z

Description

Short description here *

closes Add ticket reference here

Self Check:

Strike through any lines that are not applicable (~~line~~) then check the box

Attached issue to pull request
Changelog entry
Code is clear and sufficiently documented
Sufficient test cases (reproduces the bug/tests the requested feature)
Correct, in line with design
End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )

…68-right-sidebar

….0 (PR #5938) Bumps [eslint-plugin-testing-library](https://github.com/testing-library/eslint-plugin-testing-library) from 6.2.2 to 6.3.0. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/testing-library/eslint-plugin-testing-library/releases">eslint-plugin-testing-library's releases</a>.</em></p> <blockquote> <h2>v6.3.0</h2> <h1><a href="https://github.com/testing-library/eslint-plugin-testing-library/compare/v6.2.2...v6.3.0">6.3.0</a> (2024-08-11)</h1> <h3>Features</h3> <ul> <li>add support for flat config (<a href="https://redirect.github.com/testing-library/eslint-plugin-testing-library/issues/923">#923</a>) (<a href="https://github.com/testing-library/eslint-plugin-testing-library/commit/4dc7caa86b880d7b762a5df23f4ec736a548b502">4dc7caa</a>)</li> </ul> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/testing-library/eslint-plugin-testing-library/commit/4dc7caa86b880d7b762a5df23f4ec736a548b502"><code>4dc7caa</code></a> feat: add support for flat config (<a href="https://redirect.github.com/testing-library/eslint-plugin-testing-library/issues/923">#923</a>)</li> <li><a href="https://github.com/testing-library/eslint-plugin-testing-library/commit/e04a94092cd5a6399c9d3b605766f9ed0eb56570"><code>e04a940</code></a> docs(contributing): fix broken link (<a href="https://redirect.github.com/testing-library/eslint-plugin-testing-library/issues/911">#911</a>)</li> <li><a href="https://github.com/testing-library/eslint-plugin-testing-library/commit/c975ced3c17ddb8808332422891e69a2fa3f126e"><code>c975ced</code></a> docs: add Chamion as a contributor for code, and test (<a href="https://redirect.github.com/testing-library/eslint-plugin-testing-library/issues/896">#896</a>)</li> <li>See full diff in <a href="https://github.com/testing-library/eslint-plugin-testing-library/compare/v6.2.2...v6.3.0">compare view</a></li> </ul> </details> <br /> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=eslint-plugin-testing-library&package-manager=npm_and_yarn&previous-version=6.2.2&new-version=6.3.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details>

…g related Inventories

…68-right-sidebar

…to issue/5868-right-sidebar

LukasStordeur · 2024-09-10T07:32:25Z

src/Data/Managers/V2/GetInstanceWithRelations/useGetInstanceWithRelations.ts

@@ -28,6 +33,7 @@ interface GetInstanceWithRelationsHook {
 */
 export const useGetInstanceWithRelations = (
  instanceId: string,
+  serviceModel: ServiceModel | undefined,


if you can have the value as undefined, it means it is optional, I prefer having the '?'

LukasStordeur · 2024-09-10T07:32:51Z

src/Data/Managers/V2/GetInstanceWithRelations/useGetInstanceWithRelations.ts

@@ -61,6 +67,63 @@ export const useGetInstanceWithRelations = (
    return await response.json();
  };

+  const getAllRelations = (


LukasStordeur · 2024-09-10T07:33:00Z

src/Data/Managers/V2/GetInstanceWithRelations/useGetInstanceWithRelations.ts

+    return [...relations, ...nestedRelations];
+  };
+
+  const getAllEmbeddedEntities = (


docstrings
And by the name you gave, I'd expect the full items, not an array of strings, so maybe rename it to contain the keyword 'name'

LukasStordeur · 2024-09-10T07:33:07Z

src/Data/Managers/V2/GetInstanceWithRelations/useGetInstanceWithRelations.ts

+    return [...embedded, ...nestedEmbedded];
+  };
+
+  const findAllRelatedIds = (


docstrings
also, maybe try to be consistent, just like the method before, this would be getAllRelatedIds.

Semantically, a getter typically wouldn't return undefined, whereas a find method potentially could.

LukasStordeur · 2024-09-10T07:33:50Z

src/Data/Managers/V2/GetInstanceWithRelations/useGetInstanceWithRelations.ts

+    embeddedNames: string[],
+  ): string[] => {
+    const relationIDs = relationNames.map(
+      (relationName) => (attributes[relationName] as string) || "",


why the empty string?

After a thought I changed it to filter out (possible) undefined, but casting has to stay as we can't assert for certain that ids will be string from the interfaces as the attributes are of type unknown

LukasStordeur · 2024-09-10T08:51:22Z

src/UI/Components/Diagram/actions.ts

+  holderName = "",
+): ServiceEntityBlock {
+  //Create shape for Entity
+  const instanceAsTable = new ServiceEntityBlock().setName(serviceModel.name);


That may be personal, but I don't like chaining a method and then directly setting something to the result of that method like this. I'd rather have it in two separate statements. Or pass the name in as an argument so the constructor of your class can assign it on creation

LukasStordeur · 2024-09-10T08:53:04Z

src/UI/Components/Diagram/actions.ts

+    instanceAsTable,
+    serviceModel.key_attributes || [],
+    attributes,
+    true,


I'd prefer no anonymous arguments, especially when there are multiple boolean arguments in the list

LukasStordeur · 2024-09-10T08:54:10Z

src/UI/Components/Diagram/actions.ts

+  const attributes =
+    serviceInstance.candidate_attributes ||
+    serviceInstance.active_attributes ||
+    undefined;


why undefined here, and not an empty object like in the other file?
Also, shouldn't active have priority over candidate?

LukasStordeur · 2024-09-10T08:55:47Z

src/UI/Components/Diagram/actions.ts

  );

+  //if instance is not main, we need to apply it's stencil name to the shape to later disable it's adequate blueprint in the sidebar


Suggested change

//if instance is not main, we need to apply it's stencil name to the shape to later disable it's adequate blueprint in the sidebar

// If the instance is not main, we need to apply its stencil name to the shape to later disable its adequate stencil in the sidebar

LukasStordeur · 2024-09-10T08:59:33Z

src/UI/Components/Diagram/actions.ts

+              const relatedCell = allCells.find((cell) => cell.id === key);
+
+              if (relatedCell) {
+                const relation = serviceModel.inter_service_relations?.find(


you're relying too much on the safe operator, needing it is typically a code-smell unless you're doing Angular. It usually means your not defensive enough in your coding and bypassing typescript

add right sidebar

0f622e3

matborowczyk changed the base branch from master to issue/composer-v2 August 30, 2024 10:01

matborowczyk self-assigned this Aug 30, 2024

matborowczyk and others added 9 commits September 2, 2024 17:02

Merge branch 'master' of github.com:inmanta/web-console into issue/58…

6b95c8e

…68-right-sidebar

e2e + test fix + improvements

04c3484

e2e fix

cb1ee3b

test fix

c6c187a

tsc fix

bd49654

add rendering of parents from the children perspective + block editin…

8bf5c32

…g related Inventories

Merge branch 'master' of github.com:inmanta/web-console into issue/58…

3bcf106

…68-right-sidebar

Merge branch 'issue/composer-v2' of github.com:inmanta/web-console in…

e6c6f02

…to issue/5868-right-sidebar

LukasStordeur reviewed Sep 10, 2024

View reviewed changes

matborowczyk added 3 commits September 10, 2024 17:34

e2e + tests + fixes

96225d6

test fix

12fc477

fix

4b1b27f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue/5868 right sidebar #5930

Issue/5868 right sidebar #5930

matborowczyk commented Aug 30, 2024

LukasStordeur Sep 10, 2024

LukasStordeur Sep 10, 2024

LukasStordeur Sep 10, 2024

LukasStordeur Sep 10, 2024

LukasStordeur Sep 10, 2024

matborowczyk Sep 19, 2024

LukasStordeur Sep 10, 2024

LukasStordeur Sep 10, 2024

LukasStordeur Sep 10, 2024

LukasStordeur Sep 10, 2024

LukasStordeur Sep 10, 2024

		);

		//if instance is not main, we need to apply it's stencil name to the shape to later disable it's adequate blueprint in the sidebar

	//if instance is not main, we need to apply it's stencil name to the shape to later disable it's adequate blueprint in the sidebar
	// If the instance is not main, we need to apply its stencil name to the shape to later disable its adequate stencil in the sidebar

Issue/5868 right sidebar #5930

Are you sure you want to change the base?

Issue/5868 right sidebar #5930

Conversation

matborowczyk commented Aug 30, 2024

Description

Self Check:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment