DYN-6411 update version check #14583
Conversation
- removed the ability to update package version if not the creator of the package - ui updates as per latest Figma design
src/DynamoCoreWpf/ViewModels/PackageManager/PackageViewModel.cs
Outdated
Show resolved
Hide resolved
|
|
||
| if (_pubPkgView.IsLoaded && IsLoaded) _pubPkgView.Owner = this; | ||
| // setting the owner to the packageManagerWindow will centralize promts originating from the Package Manager | ||
| dynamoViewModel.Owner = packageManagerWindow; |
There was a problem hiding this comment.
This line does not look correct to me... did I miss anything?
There was a problem hiding this comment.
The way I used it and the way I understand it is, whichever more major View 'takes control', gets ownership of this property. The same thing happens in the PreferencesView:
The Owner property is used when utilizing the MessageBoxService:
public static MessageBoxResult Show(Window owner,string msg, string title, MessageBoxButton button, MessageBoxImage img)
So when we launch message boxes from inside the DynamoViewModel, we are centralizing them around that Owner.
Another thing it does is to keep the reference of the current window high up in the hierarchy. For example, in one instance, we are calling the UnninstallCommand which is part of the PackageViewModel from deep inside the PacakgeManagerView. The PackageViewModel and the Uninstall method do not have a reference to the PackageManagerView, though, but they do have a reference to the DynamoViewModel. So when prompting the user, we are setting the Owner of the DynamoViewModel, which is the PackageManagerView, so the prompt is correctly centralized.
I did not do a good job of cleaning up this logic, the logic exists from before. Maybe I did not fully understand it myself, this is my current working logic of it.
There was a problem hiding this comment.
After checking out the code, I think I understand what you are saying. I believe the Owner property was created so DynamoViewModel could have a reference to the view as a convenient way to raise message box with the Owner correctly passed in. But it is not meant to be served as the property indicating current Window, because I do not see any logic to reset that Owner property back to DynamoView. I can foresee a bug that after Preferences panel opened and dragged out of the screen, then closed, new message boxes using such owner will be out of DynamoView.
But thank you for pointing this out, I do think we provided bad examples in code so it's good for this PR but I will file a follow up task to address that.
There was a problem hiding this comment.
I understand. If you want me, I can try to trace all the moments where we are setting using message boxes and try to remove that need by setting the Onwer directly somehow.
- now_searchPkgsView.Closed subscribes and unsubscribes explicitly - if _searchPkgsView has been created, we will unsubscribe inside the DynamoView WindowClosed event
- updated the correct PackageManagerView to be targeted by the changes
|
@dnenov 2 tests failing here: |
- the 2 failing tests were referring to the outdated PublishPackageView - now replaced with the PackageManagerView which contains the publish package ui
Purpose
This is a PR fixing an issue where anyone could update a version of a package regardless of their ownership of the package. This issue has been flagged here https://jira.autodesk.com/browse/DYN-6411.
Also updates some of the UI elements as per the latest Figma design.
UI Changes
Publish Version...is disabled if not the package owner.Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Reviewers
@avidit
@reddyashish
FYIs
@Amoursol