Pm - retainfolderstructure rework

[dnenov]

trafficstars

Purpose

Based on https://github.com/DynamoDS/Dynamo/pull/15314, this PR completes the changes to the retain folder structure logic. Here we are making sure that the 'preview' of the package produces the same result.

The change focuses around the idea that we have 2 main file/folder structures.

• single root folder
• multiple root folders

Single root folder

The problem here was that when using a single folder structure, we would nest the origin into the newly created package folder. This was causing confusion for authors that wanted to keep their original folder structure 'just as it is'.

Multiple root folders

In this scenario, we have 2 or more 'roots' - this can happen for various reasons, for example if we load files from 2 separate drive letters (C:\ and D:). Here we have no other option but to nest all of the roots into the newly created package folder.

Legacy code review - delete dyf files

Added a test to highlight an existing behavior where Dynamo will delete all Custom Nodes during the build process. This side effect is unexpected and not communicated to the user, which could lead to issues. BuildPackageDirectory_DeletesOriginalDyfFiles()

• Upon first inspection, after commenting out the logic removing the dyfs when building, there isn't anything immediately obvious as a negative consequence. Furthermore, we do have a guardrails in the code to prevent from publishing (and loading) custom nodes already under package control:

private IEnumerable<string> GetAllUnqualifiedFiles()

@QilongTang @zeusongit @Amoursol

Known limitations - skip empty folders (should we keep or do we want to change?)

Currently, we only 'care' about files. As a consequence, we will skip empty folders.

Declarations

Check these if you believe they are true

• [x] The codebase is in a better state after this PR
• [x] Is documented according to the standards
• [x] The level of testing this PR includes is appropriate
• [x] User facing strings, if any, are extracted into *.resx files
• [x] All tests pass using the self-service CI.
• [x] Snapshot of UI changes, if any.
• [x] Changes to the API follow Semantic Versioning and are documented in the API Changes document.
• [x] This PR modifies some build requirements and the readme is updated
• [x] This PR contains no files larger than 50 MB

Release Notes

• changes to the PackageDirectoryBuilder to pass on the 'roots' information so the build logic can correctly dispatch the files/folders matching the preview
• fixed an issue where the 'custom nodes' will be displayed in the root folder as well as in their corresponding folder(s) (@zeusongit)
• adjusted test to follow the changes

Reviewers

@QilongTang @zeusongit @reddyashish

FYIs

@StudioLE

[zeusongit]

From our discussion in slack, lets remove the code that deletes dyf files from source location.

[github-actions[bot]]

UI Smoke Tests

Test: success. 11 passed, 0 failed. TestComplete Test Result Workflow Run: UI Smoke Tests Check: UI Smoke Tests - net8.0

[StudioLE]

Have I been included as an FYI in error?

[QilongTang]

Have I been included as an FYI in error?

I am guessing this is meant to FYI: @Amoursol but when typing Sol you are the first to appear. Corrected

[dnenov]

Have I been included as an FYI in error?

Apologies, I must have looped you in by mistake!

[zeusongit]

Hi @dnenov , I noticed one more possible improvement. When a user clicks on "Add Directory", but hits cancel on the opened file dialog the package window moved forward to the next step with an empty view. Can we stay on the same view when user cancels upload?

[dnenov]

Hi @dnenov , I noticed one more possible improvement. When a user clicks on "Add Directory", but hits cancel on the opened file dialog the package window moved forward to the next step with an empty view. Can we stay on the same view when user cancels upload?

Ooh, ok! I know what you mean @zeusongit! Would you mind, would it be OK if I targeted this with another PR? It is a tricky one, and I would not like to slow this PR down if possible. From memory, the flow at the moment forces you to go to the next page. Or, in other words, we navigate away from this page the moment we hit the 'load' button, and we check the result 'on the other side'. This improvement would mean moving the logic over to the page, so we can first check the dialog result and then navigate away.

[zeusongit]

Hi @dnenov , I noticed one more possible improvement. When a user clicks on "Add Directory", but hits cancel on the opened file dialog the package window moved forward to the next step with an empty view. Can we stay on the same view when user cancels upload?

Ooh, ok! I know what you mean @zeusongit! Would you mind, would it be OK if I targeted this with another PR? It is a tricky one, and I would not like to slow this PR down if possible. From memory, the flow at the moment forces you to go to the next page. Or, in other words, we navigate away from this page the moment we hit the 'load' button, and we check the result 'on the other side'. This improvement would mean moving the logic over to the page, so we can first check the dialog result and then navigate away.

Sounds good 👍🏽

[zeusongit]

Test failures: DynamoPythonTests.PythonEngineSelectorTests.CanCopydAndPasteAndUndoPythonEngine DynamoCoreWpfTests.PackageManager.PackageManagerUITests.AssertGetExistingRootItemViewModelReturnsCorrectItem DynamoCoreWpfTests.PackageManager.PackageManagerUITests.AssertPreviewPackageRetainFolderStructureEqualsPublishLocalPackageResultsForNestedFolders DynamoCoreWpfTests.PackageManager.PackageManagerUITests.AssertPublishNewPackageVersion_SuccessfulForLoadedPackages DynamoCoreWpfTests.PackageManager.PackageManagerUITests.PackageManagerLoadManuallyUnloadedBuiltIn DynamoCoreWpfTests.PackageManager.PackageManagerUITests.PackageManagerNoDuplicatesOnPathIsRemovedThenAdded DynamoCoreWpfTests.PackageManager.PackageManagerUITests.PackageManagerUninstallCommand Dynamo.PackageManager.Tests.PackageLoaderTests.DisablingBuiltinPackagesCorrectlyDisablesLoading Dynamo.PackageManager.Tests.PackageLoaderTests.DisablingCustomPackagePathsCorrectlyDisablesLoading Dynamo.PackageManager.Tests.PackageLoaderTests.PackageInBuiltinPackageLocationIsLoaded

[dnenov]

I fixed the failing tests which this PR was responsible for. I am trying to figure out why there is a failing test which indicates that there are packages installed under the test environment.

[dnenov]

It looks like everything is passing now.