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

[Environments] Auto reload after Extensions page visit; selectable error text #3589

Merged
merged 11 commits into from
Aug 29, 2024
9 changes: 8 additions & 1 deletion common/Environments/Helpers/ComputeSystemHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices.WindowsRuntime;
using System.Threading.Tasks;
using DevHome.Common.Environments.Models;
Expand Down Expand Up @@ -88,7 +89,13 @@ public static async Task<List<CardProperty>> GetComputeSystemCardPropertiesAsync
try
{
var currentProperties = await computeSystem.GetComputeSystemPropertiesAsync(string.Empty);
return GetComputeSystemCardProperties(currentProperties, packageFullName);

// Remove properties with empty values
var filteredProperties = currentProperties
huzaifa-d marked this conversation as resolved.
Show resolved Hide resolved
.Where(property => property?.Value != null && !string.IsNullOrEmpty(property.Value.ToString()))
.ToList();

return GetComputeSystemCardProperties(filteredProperties, packageFullName);
}
catch (Exception ex)
{
Expand Down
15 changes: 2 additions & 13 deletions common/Environments/Scripts/HyperVSetupScript.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,10 @@ 6. The user is already in the Hyper-V Admin group and the Hyper-V Feature is alr
$adminGroupResult = [OperationStatus]::OperationNotRun
$hyperVGroupSid = 'S-1-5-32-578'

# Check the security token the user logged on with contains the Hyper-V Administrators group SID (S-1-5-32-578). This can only be updated,
# once the user logs off and on again. Even if we add the user to the group later on in the script.
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe you should be able to pass the users sid by doing string formating to replace values within the string. I believe Nick does something similar for the virtualization management page. We'd put a place holder for the user name within the string. Then within the code once we get the whole scripts string, we can replace the placeholder with the users sid

This: https://github.com/microsoft/devhome/blob/a9f6a7361318fb9cf7a05147e6e780c84277ddbd/common/Scripts/ModifyWindowsOptionalFeatures.cs#L218C16-L218C36

is replaced with This:

var scriptString = Script.Replace("FEATURE_STRING_INPUT", featuresString);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not understanding the suggestion? And what's the advantage?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this was a suggestion sorry. It was for if you wanted to pass in the users S-I-D of the user since you removed lines 42. which had

[System.Security.Principal.WindowsIdentity]::GetCurrent().Groups.Value

I was thinking there might be a way to pass in something like a SID or username so we can get an object that would tell us if the user is in the group or not. But if we really don't need these lines then its all good to ignore.

$foundSecurityTokenString = [System.Security.Principal.WindowsIdentity]::GetCurrent().Groups.Value | Where-Object { $_ -eq $hyperVGroupSid }
$doesUserSecurityTokenContainHyperAdminGroup = $foundSecurityTokenString -eq $hyperVGroupSid

# Check if the Hyper-V feature is enabled
$featureState = Get-WindowsOptionalFeature -FeatureName 'Microsoft-Hyper-V' -Online | Select-Object -ExpandProperty State
$featureEnabled = $featureState -eq 'Enabled'

if ($doesUserSecurityTokenContainHyperAdminGroup -and $featureEnabled)
{
# User already in Admin group and feature already enabled
exit 6
}

# Enable the Hyper-V feature if it is not already enabled
if (-not $featureEnabled)
{
Expand Down Expand Up @@ -119,8 +108,8 @@ exit 5
}
# If both operations have not been run at this point, then user is already in the Hyper-V admin group and the Hyper-V feature is enabled.
# This could happen if the script runs the first time without the user being in the group, while Hyper-V is enabled but the user doesn't
# log off/on again or reboot. The second time we run the script there would be no work to be done. Since the actual token of the user
huzaifa-d marked this conversation as resolved.
Show resolved Hide resolved
# doesn't update until they log off, the $doesUserSecurityTokenContainHyperAdminGroup variable above will still remain false, which is
# log off/on again or reboot. The second time we run the script there would be no work to be done. Since the actual token of the user
# doesn't update until they log off, the $doesUserSecurityTokenContainHyperAdminGroup variable above will still remain false, which is
# how we ended up here.
elseif ($featureEnablementResult -eq [OperationStatus]::OperationNotRun -and $adminGroupResult -eq [OperationStatus]::OperationNotRun)
{
Expand Down
41 changes: 28 additions & 13 deletions common/Extensions/StackedNotificationsBehaviorExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,42 @@ public static void ShowWithWindowExtension(
var dispatcherQueue = Application.Current.GetService<DispatcherQueue>();

dispatcherQueue?.EnqueueAsync(() =>
{
{
var notificationToShow = new Notification
{
Title = title,
Message = message,
Severity = severity,
};
};

// Create a stack panel to hold the message and button
// A custom control is needed to allow text selection in the message
var stackPanel = new StackPanel
{
Orientation = Orientation.Vertical,
huzaifa-d marked this conversation as resolved.
Show resolved Hide resolved
Margin = new Thickness(0, -15, 0, 20),
Spacing = 10,
};

stackPanel.Children.Add(new TextBlock
{
Text = message,
TextWrapping = TextWrapping.WrapWholeWords,
IsTextSelectionEnabled = true,
});

if (command != null)
{
notificationToShow.ActionButton = new Button
{
Content = buttonContent,
Command = command,
};

{
// Make the command parameter the notification so RelayCommands can reference the notification in case they need
// to close it within the command.
notificationToShow.ActionButton.CommandParameter = notificationToShow;
}

stackPanel.Children.Add(new Button
{
Content = buttonContent,
Command = command,
CommandParameter = notificationToShow,
});
}

notificationToShow.Content = stackPanel;
behavior.Show(notificationToShow);
});
}
Expand Down
3 changes: 3 additions & 0 deletions common/Services/IExtensionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Windows.Foundation;

namespace DevHome.Common.Services;

Expand All @@ -21,6 +22,8 @@ public interface IExtensionService

public event EventHandler OnExtensionsChanged;

public event TypedEventHandler<IExtensionService, IExtensionWrapper> ExtensionToggled;
huzaifa-d marked this conversation as resolved.
Show resolved Hide resolved

public void EnableExtension(string extensionUniqueId);

public void DisableExtension(string extensionUniqueId);
Expand Down
13 changes: 9 additions & 4 deletions src/Services/ExtensionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Serilog;
using Windows.ApplicationModel;
using Windows.ApplicationModel.AppExtensions;
using Windows.Foundation;
using Windows.Foundation.Collections;
using static DevHome.Common.Helpers.ManagementInfrastructureHelper;

Expand All @@ -23,6 +24,8 @@ public class ExtensionService : IExtensionService, IDisposable

public event EventHandler OnExtensionsChanged = (_, _) => { };

public event TypedEventHandler<IExtensionService, IExtensionWrapper> ExtensionToggled = (_, _) => { };

private static readonly PackageCatalog _catalog = PackageCatalog.OpenForCurrentUser();
private static readonly object _lock = new();
private readonly SemaphoreSlim _getInstalledExtensionsLock = new(1, 1);
Expand Down Expand Up @@ -369,14 +372,16 @@ private List<string> GetCreateInstanceList(IPropertySet activationPropSet)

public void EnableExtension(string extensionUniqueId)
{
var extension = _installedExtensions.Where(extension => extension.ExtensionUniqueId.Equals(extensionUniqueId, StringComparison.Ordinal));
_enabledExtensions.Add(extension.First());
var extension = _installedExtensions.Where(extension => extension.ExtensionUniqueId.Equals(extensionUniqueId, StringComparison.Ordinal)).First();
ExtensionToggled.Invoke(this, extension);
_enabledExtensions.Add(extension);
}

public void DisableExtension(string extensionUniqueId)
{
var extension = _enabledExtensions.Where(extension => extension.ExtensionUniqueId.Equals(extensionUniqueId, StringComparison.Ordinal));
_enabledExtensions.Remove(extension.First());
var extension = _enabledExtensions.Where(extension => extension.ExtensionUniqueId.Equals(extensionUniqueId, StringComparison.Ordinal)).First();
ExtensionToggled.Invoke(this, extension);
_enabledExtensions.Remove(extension);
}

/// <inheritdoc cref="IExtensionService.DisableExtensionIfWindowsFeatureNotAvailable(IExtensionWrapper)"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ private async void SetPropertiesAsync()
}

var properties = await ComputeSystemHelpers.GetComputeSystemCardPropertiesAsync(ComputeSystem!, PackageFullName);

if (!ComputeSystemHelpers.RemoveAllItemsAndReplace(Properties, properties))
{
Properties = new(properties);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Threading.Tasks;
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
using CommunityToolkit.Mvvm.Messaging;
huzaifa-d marked this conversation as resolved.
Show resolved Hide resolved
using CommunityToolkit.WinUI;
using CommunityToolkit.WinUI.Behaviors;
using CommunityToolkit.WinUI.Collections;
Expand Down Expand Up @@ -48,6 +49,8 @@ public partial class LandingPageViewModel : ObservableObject, IDisposable

private bool _wasSyncButtonClicked;

private bool _extensionsToggled;

private string _selectedProvider = string.Empty;

public bool IsLoading { get; set; }
Expand Down Expand Up @@ -98,6 +101,7 @@ private enum SortOptions
public LandingPageViewModel(
INavigationService navigationService,
IComputeSystemManager manager,
IExtensionService extensionService,
Window mainWindow)
{
_computeSystemManager = manager;
Expand All @@ -111,6 +115,15 @@ public LandingPageViewModel(
_lastSyncTime = _stringResource.GetLocalized("MomentsAgo");
ComputeSystemCardsView = new AdvancedCollectionView(ComputeSystemCards);
ComputeSystemCardsView.SortDescriptions.Add(new SortDescription("IsCardCreating", SortDirection.Descending));
extensionService.ExtensionToggled += OnExtensionToggled;
}

private void OnExtensionToggled(IExtensionService sender, IExtensionWrapper extension)
{
if (extension.HasProviderType(ProviderType.ComputeSystem))
{
_extensionsToggled = true;
}
}

public void Initialize(StackedNotificationsBehavior notificationQueue)
Expand Down Expand Up @@ -216,7 +229,7 @@ private async Task RunSyncTimmer()
/// <summary>
/// Main entry point for loading the view model.
/// </summary>
public async Task LoadModelAsync(bool useDebugValues = false)
public async Task LoadModelAsync()
{
lock (_lock)
{
Expand All @@ -228,8 +241,9 @@ public async Task LoadModelAsync(bool useDebugValues = false)
// If the page has already loaded once, then we don't need to re-load the compute systems as that can take a while.
// The user can click the sync button to refresh the compute systems. However, there may be new operations that have started
// since the last time the page was loaded. So we need to add those to the view model quickly.
// But if the user toggled extensions, we need to reload the page to show refreshed data.
SetupCreateComputeSystemOperationForUI();
if (HasPageLoadedForTheFirstTime && !_wasSyncButtonClicked)
if (HasPageLoadedForTheFirstTime && !_wasSyncButtonClicked && !_extensionsToggled)
{
return;
}
Expand Down Expand Up @@ -269,6 +283,7 @@ public async Task LoadModelAsync(bool useDebugValues = false)
{
IsLoading = false;
HasPageLoadedForTheFirstTime = true;
_extensionsToggled = false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ public LandingPage()

private async void OnLoaded(object sender, RoutedEventArgs e)
{
await ViewModel.LoadModelAsync(false);
await ViewModel.LoadModelAsync();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
using System.Threading.Tasks;
using CommunityToolkit.Mvvm.ComponentModel;
using CommunityToolkit.Mvvm.Input;
using CommunityToolkit.Mvvm.Messaging;
huzaifa-d marked this conversation as resolved.
Show resolved Hide resolved
using CommunityToolkit.WinUI;
using DevHome.Common.Environments.Models;
using DevHome.Common.Extensions;
using DevHome.Common.Services;
using Microsoft.UI.Dispatching;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ public partial class ComputeSystemCardViewModel : ObservableObject
private bool _isSelected;

[ObservableProperty]
private string _computeSystemTitle;
private string _computeSystemTitle;

[ObservableProperty]
private string _computeSystemAlternativeTitle;

[ObservableProperty]
private string _computeSystemProviderDisplayName;
Expand All @@ -68,7 +71,8 @@ public ComputeSystemCardViewModel(ComputeSystemCache computeSystem, IComputeSyst
{
_dispatcherQueue = dispatcherQueue;
_computeSystemManager = manager;
ComputeSystemTitle = computeSystem.DisplayName.Value;
ComputeSystemTitle = computeSystem.DisplayName.Value;
ComputeSystemAlternativeTitle = computeSystem.SupplementalDisplayName.Value;
ComputeSystem = computeSystem;
ComputeSystem.StateChanged += _computeSystemManager.OnComputeSystemStateChanged;
_computeSystemManager.ComputeSystemStateChanged += OnComputeSystemStateChanged;
Expand Down Expand Up @@ -100,6 +104,7 @@ private async Task RefreshOperationDataAsync()
private async Task UpdatePropertiesAsync()
{
var properties = await ComputeSystemHelpers.GetComputeSystemCardPropertiesAsync(ComputeSystem, _packageFullName);

lock (_lock)
{
if (!ComputeSystemHelpers.RemoveAllItemsAndReplace(ComputeSystemProperties, properties))
Expand Down
Loading
Loading