W-17561980 Outputs DOT Code for DisplayDependencies Command

[MimiRaiSalesforce]

@W-17561980@

Summary

• Created nodes and edges according to the DependencyGraphJson. Nodes contain additional information including Package Name, Version Number, 04t ID. Edges contain a 04t source string and 04t target string.
• Call the produce() function to output nodes and edges in DOT code format and included output according to verbose and edge direction flag

[MimiRaiSalesforce]

We originally decided on using this error message if the DependencyGraphJson field was null from the Package2VersionCreateRequest table. However, I decided to make this error more general to also occur if values in the json are invalid (ex. cannot find a 04t listed in the JSON and therefore cannot query correct package name, version, etc. information)

[MimiRaiSalesforce]

I decided to store the string SubscriberPackageVersionIds (04t...) into the DependencyGraphEdge instead of DependencyGraphNodes because writing the dot code for the edges only requires the 04t... in order to match to the id of a node in the dot code (node_04t...).

[luop90]

nit: rename to resolvePackageCreateRequestId as you want a 08c, not a 0H3 or 033. The latter are what I think of when I hear "package id".

[MimiRaiSalesforce]

I renamed to resolvePackageCreateRequestId()
While at it, I also decided to rename the instance variables packageId and resolvedPackageId to userPackageVersionId and resolvedPackageVersionId

[luop90]

IMO both of these bits (87-93 and 96-101) should both be extracted into their own functions.

A "Clean Code" paradigm I subscribe to that functions help with self-documenting code in addition to code reuse. Thus, if you ever find yourself doing:

// Comment describing what the code below does
<code>

you should refactor this into:

def wellNamedFunctionDescribingTheCode:
    <code>

[MimiRaiSalesforce]

I separated the functions to query05iFrom04t() and query08cFrom05i.

[luop90]

Another good place to break up this function into smaller functions, maybe combining the last two into one since they're similar.

E.g., something like:

verifyCreateRequestExistsOnDevHub()
verifyTransitiveDependenciesCalculated()

[MimiRaiSalesforce]

I created verifyCreateRequesIdExistsOnDevHub() and verifyTransitiveDependenciesCalculated().

[luop90]

Maybe split this class into its own file?

[MimiRaiSalesforce]

I decided to leave the classes all together for the following reasons:

• I decided to follow a similar structure to the packageAncestry class which contained multiple classes in the same file.
• The majority of the files in this package directory directly correspond to an sf cli command and followed a packageCommandName.ts naming structure. The files that deviated from this structure (ex. subscriberPackageVersion.ts, versionNumber.ts) contained classes that were used in multiple other classes. Splitting the other classes in this file (eg. DependencyDotProducer and PackageDependencyNode) would add unnecessary files while only being used within the PackageVersionDependency.

TLDR: DependencyDotProducer and PackageDependencyNode are used very closely and only with PackageVersionDependency and should remain together for cleanliness.

[luop90]

Here's a less obvious but still a good place to think of creating a separate function. Something like:

throwErrorOnInvalidRecord()

or

verifyNoFieldsAreNull()

[MimiRaiSalesforce]

I created the throwErrorOnInvalidRecord() function.

[luop90]

Comment left behind?

[MimiRaiSalesforce]

Deleted it 😬

[luop90]

These lines of code are identical to the test below it, with the sole exception of:

      expect.fail('Expected TransitiveDependenciesRequiredError to be thrown');

vs

      expect.fail('Expected InvalidDependencyGraphError to be thrown');

There should be some way to remove the duplicate code. Since there's only two cases, a function (w/ a parameter for the error message name) is probably easiest

[MimiRaiSalesforce]

The difference between the two test cases is also that the values connectionStub resolves CalcTransitiveDependencies and DependencyGraphJson to. While I code create an additional function and pass in the parameters of these values and the expected errors, I don't think it would simplify the tests cases that significantly if only 2 tests use this function.

[luop90]

I don't love all this copious boilerplate just to setup a test, especially with it being repeated almost verbatim in the next test.

Trying to think of a more concise way to represent these, what I've currently got is you create a new object to represent this data and then store them in an array, e.g.

[
    new MockedDependency("TestPackage1", "1.0.0.0"),
    new MockedDependency("DependencyPackage1", "2.1.0.0"),
    new MockedDependency("DependencyPackage2", "1.2.3.0")
]

and then you have a helper function/class process these to create the versionIds in order, setup the stubs, and ideally also generate the asserts for you.

This is something Cursor should be really good at. See what I did in Package2DependencyGraphTest as an example, as I had the same issue: lots of boiler plate which a custom test library (written via Gemini) helped me compress.

[MimiRaiSalesforce]

This helped a lot! There were 3 main resolutions which I simplified to the functions: resolveDependencyGraphJsonCall(), resolveVersionBeingBuiltNodeCall(), and resolveDependencyNodeCall()

Due to the multiple checks to validate the 04t ID, calculateTransitiveDependencies, and DependencyGraphJson, resolveDependencyGraphJsonCall() is called multiple times. The best resolution I could think of to prevent writing the same call multiple times was to pass in an array of callIndices.