-
Notifications
You must be signed in to change notification settings - Fork 668
DYN-6329 DYN-6396 Pm publishpackage packagecontents #14496
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
DYN-6329 DYN-6396 Pm publishpackage packagecontents #14496
Conversation
- now all colors are controlled by the COLORS object in confing.js
- correctly creates minimum depth folder structure for the files added
- updated browser to only show root items
- added pages for the wizard-like experience when publishing a package
- using Dispatcher fixes the async tree creation
- tests added to assert correct functionality of collecting files and folders
- updated files and folders icons - now only allows LibraryNode checkboxes for Assembly dll files
- started the structure for preview build
- added tests for the core methods of PreviewPackageBuild
- changed `RemoveItem` method to account for folder items being removed - fixed a bug where removing an Assembly file would generate an error. Assembly files can also be added to 'additional items'
- UI added to allow users to add/remove items from current Package selection
- ui tweaks
- preview contents now correctly display based on user choice (retain folder or not) - fixed a bug where CustomDefinitions would read as root item - Pages are disabled when not displayed, in order to stop handling of tasks that affect other Pages
- now sorts browser alphabetically - fixed disabled behavior
- assemblies now will show up with their file path, but still get picked up from their assembly resource on disk
- we need the ability to display custom nodes as file with file paths during package creation. CustomDefinition does not have the attributes to address that, so we are adding a new 'preview' item type to server the purpose
- added tests for delete item - fixed an issue where removing all items would not result in cleaning the RootContent items - added detailed description for the customTreeView_SelectedItemChanged method
- fixed checkbox behavior to be consistent when interacting with the rest of the controls
- clearing data and ui after publishing - clearing data and ui with Cancel button
- finished the main flow between the pages
- publish local retaining folder structure
- now warns the user of losing changes if using cancel or navigating away
- also clear custom definition filepaths
|
@dnenov The dynamo icon is added in multiple locations under the test folder. Any reason to do that? Better to reuse only 1 icon. |
- added a detailed comment describing the new PublishRetainFolderStructure method
- removed duplicate icon.png files / replaced with empty text files instead - updated tests that were failing since last changes
I removed the icons, by replacing them with empty text files for the purposes of the Tests. I also had to rework a few tests that were now failing because of recent changes. |
| InitializeComponent(); | ||
|
|
||
| this.DataContextChanged += PublishFinishPage_DataContextChanged; | ||
| this.Tag = "Publish Finished Page"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this to the resources, so that it can be localized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just an internal Tag, it's not meant to be shown on UI, I don't think we should localize Tags.
- not sure how the namespaces got wiped with the previous commit
- the tests were failing because another test folder was starting with a preceding alphabet letter
| this.Package = null; | ||
| this.CustomNodeDefinitions = new List<CustomNodeDefinition>(); | ||
| RaisePropertyChanged("PackageContents"); | ||
| //System.Windows.Application.Current.Dispatcher.Invoke(() => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove commented code if not using.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I will do that in the other PR.
| SubmitCommand.RaiseCanExecuteChanged(); | ||
| PublishLocallyCommand.RaiseCanExecuteChanged(); | ||
| }); | ||
| // Can we try commenting out the can execute? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this commented code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was hoping we discuss this before committing to this change.
|
@dnenov Can you address the last 2 comments in the other package manager PR that you have open. Merging this for now as all tests have passed and want to get some manual testing on the package upload functionality. |
|
|
||
| /// <summary> | ||
| /// Build a new version of the package and upload retaining folder structure | ||
| /// TODO: Should that be a separate method or an override? Break API ok? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dnenov @reddyashish there is still a TODO in here.
| { | ||
| var packageUploadHandle = new PackageUploadHandle(PackageUploadBuilder.NewRequestBody(package)); | ||
|
|
||
| Task.Factory.StartNew(() => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is the recommended style any longer.
This reverts commit 803d8e1.
…ynamoDS#14496)" (DynamoDS#14620)" This reverts commit 2a6d173.



Purpose
Complete rehaul of the Package Manager publishing package experience. WIP, a lot of items are still moving on this PR!
UI Flow
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
(FILL ME IN) Brief description of the fix / enhancement. Mandatory section
Reviewers
(FILL ME IN) Reviewer 1 (If possible, assign the Reviewer for the PR)
(FILL ME IN, optional) Any additional notes to reviewers or testers.
FYIs
(FILL ME IN, Optional) Names of anyone else you wish to be notified of