Fixed settings issues with deleted build definitions (#31)

* Server settings edit mode now closes when saving
* Improved handling when selecting build definitions.
* Imporved handling of deleted build definitions.
* Improved logging and handling of unsuccessful requests
* Fixed some issues with handling of edit mode in server settings
* fix possible stackoverflow exception

BuildsAppReborn.Access.UI/Base/ViewModel/ProviderViewModelBase.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.ComponentModel.Composition;
 using System.Linq;
 using BuildsAppReborn.Contracts;
@@ -14,9 +15,19 @@ internal abstract class ProviderViewModelBase : ViewModelBase, IBuildProviderVie
     {
         public abstract String DisplayName { get; protected set; }
 
+        public Boolean IsInEditMode
+        {
+            get { return this.isInEditMode; }
+            set
+            {
+                this.isInEditMode = value;
+                OnPropertyChanged();
+            }
+        }
+
         public BuildMonitorSettings MonitorSettings { get; private set; }
 
-        public abstract IEnumerable<IBuildDefinition> SelectedBuildDefinitions { get; }
+        public abstract ObservableCollection<IBuildDefinition> SelectedBuildDefinitions { get; }
 
         public Boolean SupportsDefaultCredentials => BuildProviderMetaData.SupportedAuthenticationModes.HasFlag(AuthenticationModes.Default);
 
@@ -57,6 +68,10 @@ public virtual void OnImportsSatisfied()
             }
         }
 
+        public virtual void Save()
+        {
+        }
+
         protected IBuildProvider BuildProvider { get; private set; }
 
         protected IBuildProviderMetadata BuildProviderMetaData { get; private set; }
@@ -69,6 +84,8 @@ protected virtual void OnInitialized()
 #pragma warning disable 649
         private Lazy<IBuildProvider, IBuildProviderMetadata>[] buildProviders;
 
+        private Boolean isInEditMode;
+
 #pragma warning restore 649
     }
 }

BuildsAppReborn.Access.UI/Base/ViewModel/SubViewModels/BuildDefinitionViewModel.cs

@@ -18,6 +18,16 @@ public BuildDefinitionViewModel(IBuildDefinition buildDefinition)
 
         public IBuildDefinition BuildDefinition { get; }
 
+        public Boolean IsDeleted
+        {
+            get { return this.isDeleted; }
+            set
+            {
+                this.isDeleted = value;
+                OnPropertyChanged();
+            }
+        }
+
         public Boolean IsSelected
         {
             get { return this.isSelected; }
@@ -29,6 +39,7 @@ public Boolean IsSelected
         }
 
         public String Name => BuildDefinition.Name;
+        private Boolean isDeleted;
 
         private Boolean isSelected;
     }

BuildsAppReborn.Access.UI/BuildsAppReborn.Access.UI.csproj

@@ -40,6 +40,7 @@
     <Reference Include="System" />
     <Reference Include="System.ComponentModel.Composition" />
     <Reference Include="System.Core" />
+    <Reference Include="System.Net.Http" />
     <Reference Include="System.Xaml">
       <RequiredTargetFramework>4.0</RequiredTargetFramework>
     </Reference>

BuildsAppReborn.Access.UI/TFS/ViewModel/TfsBuildProviderViewModelBase.cs

@@ -1,9 +1,9 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.ObjectModel;
-using System.ComponentModel;
 using System.Linq;
 using System.Net;
+using System.Net.Http;
 using System.Threading.Tasks;
 using System.Windows;
 using BuildsAppReborn.Access.UI.ViewModel.SubViewModels;
@@ -78,7 +78,7 @@ public String ProjectUrl
             }
         }
 
-        public override IEnumerable<IBuildDefinition> SelectedBuildDefinitions => new ObservableCollection<IBuildDefinition>(MonitorSettings.SelectedBuildDefinitions);
+        public override ObservableCollection<IBuildDefinition> SelectedBuildDefinitions => this.selectedBuildDefinitions;
 
         public Boolean ShowPersonalAccessTokenInput
         {
@@ -102,6 +102,26 @@ public String StatusText
 
         public override String Url => ProjectUrl;
 
+        public override void Save()
+        {
+            if (IsInEditMode)
+            {
+                base.Save();
+
+                MonitorSettings.SelectedBuildDefinitions.Clear();
+                var selected = BuildDefinitions.Where(a => a.IsSelected && !a.IsDeleted);
+                foreach (var viewModel in selected)
+                {
+                    MonitorSettings.SelectedBuildDefinitions.Add(viewModel.BuildDefinition);
+                }
+
+                IsInEditMode = false;
+
+                UpdateSelectedCollection();
+                BuildDefinitions.Clear();
+            }
+        }
+
         protected abstract String ProviderName { get; }
 
         protected override void OnInitialized()
@@ -115,6 +135,8 @@ protected override void OnInitialized()
 
             SetDisplayName(MonitorSettings.SelectedBuildDefinitions);
 
+            UpdateSelectedCollection();
+
             // ReSharper disable once ExplicitCallerInfoArgument
             OnPropertyChanged(nameof(ProjectUrl));
             ConnectCommand?.RaiseCanExecuteChanged();
@@ -137,28 +159,26 @@ private void ApplyBuildDefinitions(IEnumerable<IBuildDefinition> buildDefinition
         {
             BuildDefinitions.AddRange(buildDefinitions.Select(buildDefinition =>
             {
-                var selectedBuildDefinitions = MonitorSettings.SelectedBuildDefinitions;
-
                 var vm = new BuildDefinitionViewModel(buildDefinition);
 
-                var definition = selectedBuildDefinitions.SingleOrDefault(a => a.Id == vm.BuildDefinition.Id);
+                var definition = MonitorSettings.SelectedBuildDefinitions.SingleOrDefault(a => a.Id == vm.BuildDefinition.Id);
                 if (definition != null && this.buildDefinitionEqualityComparer.Equals(definition, vm.BuildDefinition))
                 {
                     vm.IsSelected = true;
                 }
 
-                vm.PropertyChanged += BuildDefinitionPropertyChanged;
-
                 return vm;
             }));
-        }
 
-        private void BuildDefinitionPropertyChanged(Object sender, PropertyChangedEventArgs e)
-        {
-            var vm = sender as BuildDefinitionViewModel;
-            if (vm != null && e.PropertyName == nameof(vm.IsSelected))
+            var obsoleteDefinitions = MonitorSettings.SelectedBuildDefinitions.Where(a => BuildDefinitions.All(x => x.BuildDefinition.Id != a.Id)).ToList();
+            foreach (var obsoleteDefinition in obsoleteDefinitions)
             {
-                UpdateSelectedBuildDefinitionList(vm);
+                var vm = new BuildDefinitionViewModel(obsoleteDefinition)
+                {
+                    IsDeleted = true,
+                    IsSelected = true
+                };
+                BuildDefinitions.Add(vm);
             }
         }
 
@@ -178,17 +198,9 @@ private async Task OnConnectAsync()
 
             try
             {
-                Application.Current.Dispatcher.Invoke(() =>
-                {
-                    foreach (var buildDefinitionViewModel in BuildDefinitions)
-                    {
-                        buildDefinitionViewModel.PropertyChanged -= BuildDefinitionPropertyChanged;
-                    }
+                Application.Current.Dispatcher.Invoke(() => { BuildDefinitions.Clear(); });
 
-                    BuildDefinitions.Clear();
-                });
-
-                var buildDefinitions = await Task.Run(() => BuildProvider.GetBuildDefinitionsAsync(MonitorSettings)).ConfigureAwait(false);
+                var buildDefinitions = await BuildProvider.GetBuildDefinitionsAsync(MonitorSettings).ConfigureAwait(false);
 
                 if (buildDefinitions.IsSuccessStatusCode)
                 {
@@ -198,17 +210,13 @@ private async Task OnConnectAsync()
                 }
                 else
                 {
-                    if (buildDefinitions.StatusCode == HttpStatusCode.Unauthorized)
-                    {
-                        ShowPersonalAccessTokenInput = true;
-                        StatusText = $"Authorization failed, please provide personal access token!";
-                    }
-                    else
-                    {
-                        StatusText = $"Error connecting. StatusCode: {buildDefinitions.StatusCode}";
-                    }
+                    AccessTokenInputWhenUnauthorized(buildDefinitions.StatusCode);
                 }
             }
+            catch (AdvancedHttpRequestException ex)
+            {
+                AccessTokenInputWhenUnauthorized(ex.StatusCode);
+            }
             catch (Exception ex)
             {
                 StatusText = ex.Message;
@@ -217,6 +225,19 @@ private async Task OnConnectAsync()
             IsConnecting = false;
         }
 
+        private void AccessTokenInputWhenUnauthorized(HttpStatusCode statusCode)
+        {
+            if (statusCode == HttpStatusCode.Unauthorized)
+            {
+                ShowPersonalAccessTokenInput = true;
+                StatusText = $"Authorization failed, please provide personal access token!";
+            }
+            else
+            {
+                StatusText = $"Error connecting. StatusCode: {statusCode}";
+            }
+        }
+
         private void SetDisplayName(IEnumerable<IBuildDefinition> buildDefinitions)
         {
             // this is okay as they are from the same project
@@ -232,29 +253,18 @@ private void SetDisplayName(IEnumerable<IBuildDefinition> buildDefinitions)
             }
         }
 
-        private void UpdateSelectedBuildDefinitionList(BuildDefinitionViewModel buildDefinitionViewModel)
+        private void UpdateSelectedCollection()
         {
-            if (buildDefinitionViewModel.IsSelected)
-            {
-                MonitorSettings.SelectedBuildDefinitions.Add(buildDefinitionViewModel.BuildDefinition);
-            }
-            else if (!buildDefinitionViewModel.IsSelected)
-            {
-                var currentItem = MonitorSettings.SelectedBuildDefinitions.SingleOrDefault(
-                    definition => this.buildDefinitionEqualityComparer.Equals(definition, buildDefinitionViewModel.BuildDefinition));
-
-                MonitorSettings.SelectedBuildDefinitions.Remove(currentItem);
-            }
-
-            // ReSharper disable once ExplicitCallerInfoArgument
-            OnPropertyChanged(nameof(SelectedBuildDefinitions));
+            this.selectedBuildDefinitions.Clear();
+            this.selectedBuildDefinitions.AddRange(MonitorSettings.SelectedBuildDefinitions);
         }
 
         private readonly IEqualityComparer<IBuildDefinition> buildDefinitionEqualityComparer;
 
         private String displayName;
 
         private Boolean isConnecting;
+        private readonly RangeObservableCollection<IBuildDefinition> selectedBuildDefinitions = new RangeObservableCollection<IBuildDefinition>();
 
         private Boolean showPersonalAccessTokenInput;
 

BuildsAppReborn.Access.UI/TFS/Views/TfsBuildProviderViewBase.xaml

@@ -114,7 +114,18 @@
                                 </Grid.ColumnDefinitions>
                                 <CheckBox VerticalAlignment="Center" IsChecked="{Binding IsSelected}" />
                                 <StackPanel Grid.Column="1" Margin="8 0 0 0">
-                                    <TextBlock FontWeight="Bold" Text="{Binding Name}" />
+                                    <TextBlock FontWeight="Bold" Text="{Binding Name}">
+                                        <TextBlock.Style>
+                                            <Style TargetType="TextBlock">
+                                                <Style.Triggers>
+                                                    <DataTrigger Binding="{Binding IsDeleted}" Value="True">
+                                                        <Setter Property="Foreground" Value="Red" />
+                                                        <Setter Property="ToolTip" Value="Build Definition is deleted on TFS. Will be removed when settings are saved." />
+                                                    </DataTrigger>
+                                                </Style.Triggers>
+                                            </Style>
+                                        </TextBlock.Style>
+                                    </TextBlock>
                                 </StackPanel>
                             </Grid>
                         </Border>

BuildsAppReborn.Access/BuildMonitor.cs

@@ -43,7 +43,7 @@ public BuildMonitor(LazyContainer<IBuildProvider, IBuildProviderMetadata> buildP
         public async Task BeginPollingBuildsAsync()
         {
             var builds = new List<IBuild>();
-            foreach (var pair in this.providerSettingsGroup)
+            foreach (var pair in this.providerSettingsGroup.ToList())
             {
                 foreach (var setting in pair.Value)
                 {