| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 666
DYN-9645: finishing package publish fix #16601
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-9645: finishing package publish fix #16601
Conversation
- race condition was preventing packages to be correctly displayed as successfully published - removing ClearAllEntries from running as part of the publishing routine resolves the issue - correctly uses ClearAllEntries only after the user interacts with the UI after the package is published
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.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9645
benglin
left a comment
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.
Thanks for addressing this, @dnenov! The fix makes perfect sense and should prevent the race condition. Just one small adjustment and weβll be all set!
| }); | ||
| OnPublishSuccess(); | ||
| // Don't clear entries on success - user needs to see the success state | ||
| // Clearing will happen when user clicks Done/Reset from the success 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.
Nice -- this looks like a logical change, since clearing triggers a lot of property change events, and thereβs no real urgency to clear the contents right away.
| } | ||
|
|
||
| if (Name.Length <= 0 && !PackageContents.Any()) | ||
| if (Name.Length <= 0 && !PackageContents.Any() && !PreviewPackageContents.Any()) |
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 think the addition of PreviewPackageContents wasnβt intended to fix the issue, but rather for completeness?
That said, it wasnβt immediately clear to me what PreviewPackageContents was for, so I did some Cursoring to find out. After some back-and-forth, hereβs what I found -- if you could integrate this into the code, it might help future readers (and AI agents) understand it more easily. Thanks!
/// <summary>
/// Stores the raw files/folders the user has added for package publishing. Items represent
/// files in their current disk locations, NOT their final locations in the published package.
/// This collection is modified when users add files via `ShowAddFileDialogAndAddCommand`,
/// add directories via `SelectDirectoryAndAddFilesRecursivelyCommand`, or remove items via
/// `RemoveItemCommand`.
/// </summary>
public ObservableCollection<PackageItemRootViewModel> PackageContents { get; set; }
/// <summary>
/// Preview of the final package directory structure before publishing. Shows how files in
/// PackageContents will be organized in the published package. It reorganizes the files
/// into the standard Dynamo package folders (`bin/`, `dyf/`, `extra/`, `doc/`, `pkg.json`)
/// if `RetainFolderStructureOverride == false`, or preserves the user's existing folder
/// structure from PackageContents if `RetainFolderStructureOverride == true`.
/// </summary>
public ObservableCollection<PackageItemRootViewModel> PreviewPackageContents { get; set; }- added detailed property descriptions for PackageContents and PreviewPackageContents
β¦nenov/Dynamo into DYN-9645-pm-publish-dyf-ui-fix
benglin
left a comment
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.
Thanks for accommodating the requests, @dnenov! LGTM! π
Purpose
Problem
Packages published successfully but showed validation error instead of success page.
Root Cause
ClearAllEntries() was called immediately after OnPublishSuccess(), clearing form data before the frontend received the success notification, causing validation to fail.
Declarations
Check these if you believe they are true
Release Notes
Reviewers
@zeusongit
@benglin
FYIs