Skip to content
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

unblock main thread when fetching user votes #14793

Merged
merged 5 commits into from
Feb 12, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ internal static bool ShowTermsOfUseDialog(bool forPublishing, string additionalT
var executingAssemblyPathName = Assembly.GetExecutingAssembly().Location;
var rootModuleDirectory = Path.GetDirectoryName(executingAssemblyPathName);
var touFilePath = Path.Combine(rootModuleDirectory, "TermsOfUse.rtf");

var termsOfUseView = new TermsOfUseView(touFilePath);
termsOfUseView.Closed += TermsOfUseView_Closed;
isTermsOfUseCreated = true;
Expand Down Expand Up @@ -251,12 +251,12 @@ public string Username
public ICommand ToggleLoginStateCommand { get; private set; }

/// <summary>
/// Contains all votes the user has been submitted.
/// Contains all votes the user has submitted.
/// Will allow the user to vote for a package they have not upvoted before
/// </summary>
private List<string> Uservotes { get; set; }

internal PackageManagerClientViewModel(DynamoViewModel dynamoViewModel, PackageManagerClient model )
internal PackageManagerClientViewModel(DynamoViewModel dynamoViewModel, PackageManagerClient model)
{
this.DynamoViewModel = dynamoViewModel;
this.AuthenticationManager = dynamoViewModel.Model.AuthenticationManager;
Expand All @@ -273,11 +273,6 @@ internal PackageManagerClientViewModel(DynamoViewModel dynamoViewModel, PackageM
RaisePropertyChanged("Username");
};
}

if (AuthenticationManager.LoginState.Equals(LoginState.LoggedIn) && !dynamoViewModel.Model.NoNetworkMode)
{
Uservotes = Model.UserVotes();
}
}

private void ToggleLoginState()
Expand Down Expand Up @@ -328,7 +323,7 @@ public void PublishCurrentWorkspace(object m)
return;
}
}

MessageBoxService.Show(
ViewModelOwner,
Resources.MessageSelectSymbolNotFound,
Expand Down Expand Up @@ -422,8 +417,8 @@ public void PublishSelectedNodes(object m)
}
MessageBoxService.Show(
ViewModelOwner,
Resources.MessageGettingNodeError,
Resources.SelectionErrorMessageBoxTitle,
Resources.MessageGettingNodeError,
Resources.SelectionErrorMessageBoxTitle,
MessageBoxButton.OK, MessageBoxImage.Question);
}

Expand Down Expand Up @@ -458,7 +453,7 @@ private void ShowNodePublishInfo(ICollection<Tuple<CustomNodeInfo, CustomNodeDef

if (pkg != null)
{
var m = Dynamo.Wpf.Utilities.MessageBoxService.Show(ViewModelOwner,
var m = Dynamo.Wpf.Utilities.MessageBoxService.Show(ViewModelOwner,
String.Format(Resources.MessageSubmitSameNamePackage,
DynamoViewModel.BrandingResourceProvider.ProductName, pkg.Name),
Resources.PackageWarningMessageBoxTitle,
Expand All @@ -482,6 +477,8 @@ public List<PackageManagerSearchElement> ListAll()
{
CachedPackageList = new List<PackageManagerSearchElement>();

// Calls to Model.UserVotes and Model.ListAll might take a long time to run (so do not use them syncronously in the UI thread)
Copy link
Contributor

@pinzart90 pinzart90 Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public List<PackageManagerSearchElement> ListAll() is called from an async method upstream
It is not going to block the UI thread anymore.

We do need a clear way to mark these methods (that they are Task based or async) . This will be part of another task

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what async method calls ListAll()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Task<IEnumerable<PackageManagerSearchElementViewModel>>.Factory.StartNew(RefreshAndSearch).ContinueWith((t) =>

Uservotes = this.Model.UserVotes();
foreach (var header in Model.ListAll())
pinzart90 marked this conversation as resolved.
Show resolved Hide resolved
{
var ele = new PackageManagerSearchElement(header);
Expand All @@ -492,7 +489,7 @@ public List<PackageManagerSearchElement> ListAll()
ele.HasUpvote = Uservotes.Contains(header._id);
}

CachedPackageList.Add( ele );
CachedPackageList.Add(ele);
}

return CachedPackageList;
Expand Down Expand Up @@ -556,7 +553,7 @@ public void DownloadAndInstallPackage(IPackageInfo packageInfo, string downloadP
MessageBoxImage.Error);
return;
}

ExecutePackageDownload(packageInfo.Name, version, downloadPath);
}

Expand All @@ -572,8 +569,8 @@ private string JoinPackageNames(IEnumerable<Package> pkgs, string seperator = ",
/// <param name="duplicatePackage">local package found to be duplicate of one being downloaded</param>
/// <param name="dependencyConflicts">List of packages that are in conflict with the dependencies of the package version to be downloaded (does not include the main package)</param>
/// <returns>True if the User opted to continue with the download operation. False otherwise</returns>
private bool WarnAboutDuplicatePackageConflicts(PackageVersion package,
Package duplicatePackage,
private bool WarnAboutDuplicatePackageConflicts(PackageVersion package,
Package duplicatePackage,
List<Package> dependencyConflicts)
{
var packageToDownload = $"{package.name} {package.version}";
Expand Down Expand Up @@ -631,9 +628,9 @@ private bool WarnAboutDuplicatePackageConflicts(PackageVersion package,
{
dialogResult = MessageBoxService.Show(
String.Format(Resources.MessageAlreadyInstallDynamo,
DynamoViewModel.BrandingResourceProvider.ProductName, dupPkg, packageToDownload),
DynamoViewModel.BrandingResourceProvider.ProductName, dupPkg, packageToDownload),
Resources.PackagesInUseConflictMessageBoxTitle,
MessageBoxButton.OKCancel, new string[] { Resources.UninstallLoadedPackage, Resources.GenericTaskDialogOptionCancel },
MessageBoxButton.OKCancel, new string[] { Resources.UninstallLoadedPackage, Resources.GenericTaskDialogOptionCancel },
MessageBoxImage.Exclamation);

if (dialogResult != MessageBoxResult.OK)
Expand Down Expand Up @@ -673,7 +670,7 @@ private bool WarnAboutDuplicatePackageConflicts(PackageVersion package,

immediateUninstalls.Add(pkg);
}

if (builtinPackages.Any())
{
// Conflicts with builtin packages
Expand All @@ -684,20 +681,20 @@ private bool WarnAboutDuplicatePackageConflicts(PackageVersion package,
Resources.BuiltInPackageConflictMessageBoxTitle,
MessageBoxButton.OKCancel, MessageBoxImage.Exclamation);

if(dialogResult == MessageBoxResult.Cancel || dialogResult == MessageBoxResult.None) return false;
if (dialogResult == MessageBoxResult.Cancel || dialogResult == MessageBoxResult.None) return false;
}

if (uninstallRequiringUserModifications.Any())
{
var conflictingPkgs = JoinPackageNames(uninstallRequiringUserModifications);
var message = string.Format(Resources.MessageForceInstallOrUninstallToContinue, packageToDownload, conflictingPkgs,
DynamoViewModel.BrandingResourceProvider.ProductName);

var dialogResult = MessageBoxService.Show(ViewModelOwner, message,
Resources.PackagesInUseConflictMessageBoxTitle,
MessageBoxButton.YesNo, MessageBoxImage.Exclamation);

if(dialogResult == MessageBoxResult.No || dialogResult == MessageBoxResult.None) return false;
if (dialogResult == MessageBoxResult.No || dialogResult == MessageBoxResult.None) return false;
}

var settings = DynamoViewModel.Model.PreferenceSettings;
Expand Down Expand Up @@ -941,7 +938,7 @@ internal async void ExecutePackageDownload(string name, PackageVersion package,
}
}
}

/// <summary>
/// This method downloads the package represented by the PackageDownloadHandle,
///
Expand All @@ -957,7 +954,7 @@ internal async void ExecutePackageDownload(string name, PackageVersion package,
Downloads.Add(packageDownloadHandle);

packageDownloadHandle.DownloadState = PackageDownloadHandle.State.Downloading;

return Task.Factory.StartNew(() =>
{
// Attempt to download package
Expand Down
Loading