From ab97105c919fbbb43802f84b7657c5712bcba0e4 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Fri, 19 Sep 2025 16:14:01 +0200 Subject: [PATCH 1/3] Move logic for hotspots into another view model to avoid making ReportViewModel a god class. --- .../Hotspots/HotspotsReportViewModelTest.cs | 139 ++++++++++++++ .../ReportView/ReportViewModelTest.cs | 177 ++++-------------- .../Hotspots/HotspotsReportViewModel.cs | 71 +++++++ .../ReportView/ReportViewControl.xaml.cs | 8 +- .../ReportView/ReportViewModel.cs | 31 +-- .../ReportView/ReportViewToolWindow.cs | 5 +- 6 files changed, 259 insertions(+), 172 deletions(-) create mode 100644 src/IssueViz.Security.UnitTests/ReportView/Hotspots/HotspotsReportViewModelTest.cs create mode 100644 src/IssueViz.Security/ReportView/Hotspots/HotspotsReportViewModel.cs diff --git a/src/IssueViz.Security.UnitTests/ReportView/Hotspots/HotspotsReportViewModelTest.cs b/src/IssueViz.Security.UnitTests/ReportView/Hotspots/HotspotsReportViewModelTest.cs new file mode 100644 index 0000000000..433318ba63 --- /dev/null +++ b/src/IssueViz.Security.UnitTests/ReportView/Hotspots/HotspotsReportViewModelTest.cs @@ -0,0 +1,139 @@ +/* + * SonarLint for Visual Studio + * Copyright (C) 2016-2025 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using SonarLint.VisualStudio.Core.Analysis; +using SonarLint.VisualStudio.IssueVisualization.Models; +using SonarLint.VisualStudio.IssueVisualization.Security.Hotspots; +using SonarLint.VisualStudio.IssueVisualization.Security.IssuesStore; +using SonarLint.VisualStudio.IssueVisualization.Security.ReportView; +using SonarLint.VisualStudio.IssueVisualization.Security.ReportView.Hotspots; +using SonarLint.VisualStudio.TestInfrastructure; + +namespace SonarLint.VisualStudio.IssueVisualization.Security.UnitTests.ReportView.Hotspots; + +[TestClass] +public class HotspotsReportViewModelTest +{ + private ILocalHotspotsStore localHotspotsStore; + private HotspotsReportViewModel testSubject; + + [TestInitialize] + public void TestInitialize() + { + localHotspotsStore = Substitute.For(); + testSubject = new HotspotsReportViewModel(localHotspotsStore); + } + + [TestMethod] + public void MefCtor_CheckIsExported() => + MefTestHelpers.CheckTypeCanBeImported( + MefTestHelpers.CreateExport()); + + [TestMethod] + public void MefCtor_CheckIsSingleton() => MefTestHelpers.CheckIsSingletonMefComponent(); + + [TestMethod] + public void Constructor_SubscribesToIssuesChanged() => localHotspotsStore.Received().IssuesChanged += Arg.Any>(); + + [TestMethod] + public void Dispose_UnsubscribesFromIssuesChanged() + { + testSubject.Dispose(); + + localHotspotsStore.Received().IssuesChanged -= Arg.Any>(); + } + + [TestMethod] + public void GetHotspotsGroupViewModels_GroupsByFilePath() + { + var file1 = "file1.cs"; + var file2 = "file2.cs"; + MockHotspotsInStore(CreateMockedHotspot(file1), CreateMockedHotspot(file1), CreateMockedHotspot(file2)); + + var groups = testSubject.GetHotspotsGroupViewModels(); + + groups.Should().HaveCount(2); + groups.Select(g => g.Title).Should().Contain([file1, file2]); + groups.First(g => g.Title == file1).FilteredIssues.Should().HaveCount(2); + groups.First(g => g.Title == file2).FilteredIssues.Should().HaveCount(1); + } + + [TestMethod] + public void GetHotspotsGroupViewModels_TwoHotspotsInSameFile_CreatesOneGroupVmWithTwoIssues() + { + var path = "myFile.cs"; + var hotspot1 = CreateMockedHotspot(path); + var hotspot2 = CreateMockedHotspot(path); + MockHotspotsInStore(hotspot1, hotspot2); + + var groups = testSubject.GetHotspotsGroupViewModels(); + + groups.Should().HaveCount(1); + VerifyExpectedHotspotGroupViewModel(groups[0] as GroupFileViewModel, hotspot1, hotspot2); + } + + [TestMethod] + public void GetHotspotsGroupViewModels_TwoHotspotsInDifferentFiles_CreatesTwoGroupsWithOneIssueEach() + { + var hotspot1 = CreateMockedHotspot("myFile.cs"); + var hotspot2 = CreateMockedHotspot("myFile.js"); + MockHotspotsInStore(hotspot1, hotspot2); + + var groups = testSubject.GetHotspotsGroupViewModels(); + + groups.Should().HaveCount(2); + VerifyExpectedHotspotGroupViewModel(groups[0] as GroupFileViewModel, hotspot1); + VerifyExpectedHotspotGroupViewModel(groups[1] as GroupFileViewModel, hotspot2); + } + + [TestMethod] + public void HotspotsChanged_RaisedOnStoreIssuesChanged() + { + var raised = false; + testSubject.HotspotsChanged += (_, _) => raised = true; + + localHotspotsStore.IssuesChanged += Raise.Event>(null, null); + + raised.Should().BeTrue(); + } + + private static LocalHotspot CreateMockedHotspot(string filePath) + { + var analysisIssueVisualization = Substitute.For(); + var analysisIssueBase = Substitute.For(); + analysisIssueBase.PrimaryLocation.FilePath.Returns(filePath); + analysisIssueVisualization.Issue.Returns(analysisIssueBase); + + return new LocalHotspot(analysisIssueVisualization, default, default); + } + + private void MockHotspotsInStore(params LocalHotspot[] hotspots) => localHotspotsStore.GetAllLocalHotspots().Returns(hotspots); + + private static void VerifyExpectedHotspotGroupViewModel(GroupFileViewModel groupFileVm, params LocalHotspot[] expectedHotspots) + { + groupFileVm.Should().NotBeNull(); + groupFileVm.FilePath.Should().Be(expectedHotspots[0].Visualization.Issue.PrimaryLocation.FilePath); + groupFileVm.FilteredIssues.Should().HaveCount(expectedHotspots.Length); + foreach (var expectedHotspot in expectedHotspots) + { + groupFileVm.FilteredIssues.Should().ContainSingle(vm => ((HotspotViewModel)vm).LocalHotspot == expectedHotspot); + } + } +} diff --git a/src/IssueViz.Security.UnitTests/ReportView/ReportViewModelTest.cs b/src/IssueViz.Security.UnitTests/ReportView/ReportViewModelTest.cs index 09397f770f..e68a58c1a0 100644 --- a/src/IssueViz.Security.UnitTests/ReportView/ReportViewModelTest.cs +++ b/src/IssueViz.Security.UnitTests/ReportView/ReportViewModelTest.cs @@ -26,10 +26,7 @@ using SonarLint.VisualStudio.Core.Telemetry; using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.IssueVisualization.IssueVisualizationControl.ViewModels.Commands; -using SonarLint.VisualStudio.IssueVisualization.Models; using SonarLint.VisualStudio.IssueVisualization.Security.DependencyRisks; -using SonarLint.VisualStudio.IssueVisualization.Security.Hotspots; -using SonarLint.VisualStudio.IssueVisualization.Security.IssuesStore; using SonarLint.VisualStudio.IssueVisualization.Security.ReportView; using SonarLint.VisualStudio.IssueVisualization.Security.ReportView.Hotspots; using SonarLint.VisualStudio.TestInfrastructure; @@ -42,7 +39,7 @@ public class ReportViewModelTest private ReportViewModel testSubject; private IActiveSolutionBoundTracker activeSolutionBoundTracker; private IDependencyRisksStore dependencyRisksStore; - private ILocalHotspotsStore localHotspotsStore; + private IHotspotsReportViewModel hotspotsReportViewModel; private IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler; private IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler; private INavigateToRuleDescriptionCommand navigateToRuleDescriptionCommand; @@ -57,7 +54,7 @@ public void Initialize() { activeSolutionBoundTracker = Substitute.For(); dependencyRisksStore = Substitute.For(); - localHotspotsStore = Substitute.For(); + hotspotsReportViewModel = Substitute.For(); showDependencyRiskInBrowserHandler = Substitute.For(); changeDependencyRiskStatusHandler = Substitute.For(); navigateToRuleDescriptionCommand = Substitute.For(); @@ -66,6 +63,7 @@ public void Initialize() telemetryManager = Substitute.For(); threadHandling = Substitute.ForPartsOf(); eventHandler = Substitute.For(); + hotspotsReportViewModel.GetHotspotsGroupViewModels().Returns([]); testSubject = CreateTestSubject(); } @@ -76,7 +74,7 @@ public void Initialize() [TestMethod] public void Class_SubscribesToEvents() { - localHotspotsStore.Received(1).IssuesChanged += Arg.Any>(); + hotspotsReportViewModel.Received(1).HotspotsChanged += Arg.Any(); dependencyRisksStore.Received(1).DependencyRisksChanged += Arg.Any(); } @@ -99,47 +97,19 @@ public void Ctor_InitializesDependencyRisks() VerifyExpectedDependencyRiskGroupViewModel(testSubject.GroupViewModels[0] as GroupDependencyRiskViewModel, dependencyRisk); } - [TestMethod] - public void Ctor_TwoHotspotsInSameFile_CreatesOneGroupVmWithTwoIssues() - { - var path = "myFile.cs"; - var hotspot1 = CreateMockedHotspot(filePath: path); - var hotspot2 = CreateMockedHotspot(filePath: path); - MockHotspotsInStore(hotspot1, hotspot2); - - testSubject = CreateTestSubject(); - - testSubject.GroupViewModels.Should().HaveCount(1); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[0] as GroupFileViewModel, hotspot1, hotspot2); - } - - [TestMethod] - public void Ctor_TwoHotspotsInDifferentFiles_CreatesTwoGroupsWithOneIssueEach() - { - var hotspot1 = CreateMockedHotspot(filePath: "myFile.cs"); - var hotspot2 = CreateMockedHotspot(filePath: "myFile.js"); - MockHotspotsInStore(hotspot1, hotspot2); - - testSubject = CreateTestSubject(); - - testSubject.GroupViewModels.Should().HaveCount(2); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[0] as GroupFileViewModel, hotspot1); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[1] as GroupFileViewModel, hotspot2); - } - [TestMethod] public void Ctor_MixedIssuesTypes_CreatesGroupViewModelsCorrectly() { var dependencyRisk = CreateDependencyRisk(); MockRisksInStore(dependencyRisk); - var hotspot = CreateMockedHotspot(filePath: "myFile.cs"); - MockHotspotsInStore(hotspot); + var hotspotGroupViewModel = CreateMockedGroupViewModel(filePath: "myFile.cs"); + hotspotsReportViewModel.GetHotspotsGroupViewModels().Returns([hotspotGroupViewModel]); testSubject = CreateTestSubject(); testSubject.GroupViewModels.Should().HaveCount(2); VerifyExpectedDependencyRiskGroupViewModel(testSubject.GroupViewModels[0] as GroupDependencyRiskViewModel, dependencyRisk); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[1] as GroupFileViewModel, hotspot); + testSubject.GroupViewModels[1].Should().Be(hotspotGroupViewModel); } [TestMethod] @@ -159,7 +129,8 @@ public void Dispose_UnsubscribesFromEvents() testSubject.Dispose(); dependencyRisksStore.Received(1).DependencyRisksChanged -= Arg.Any(); - localHotspotsStore.Received(1).IssuesChanged -= Arg.Any>(); + hotspotsReportViewModel.Received(1).HotspotsChanged -= Arg.Any(); + hotspotsReportViewModel.Received(1).Dispose(); } [TestMethod] @@ -282,101 +253,33 @@ public void SelectedItem_SetToNull_DoesNotCallTelemetry() } [TestMethod] - public void HotspotsAddedInStore_ExistingFile_UpdatesExistingGroup() + public void HotspotsChanged_TwoGroups_UpdatesOnlyHotspotGroupViewModels() { - var initialHotspot = CreateMockedHotspot(filePath: "myFile.cs"); - var addedHotspot = CreateMockedHotspot(filePath: "myFile.cs"); - localHotspotsStore.GetAllLocalHotspots().Returns([initialHotspot], [initialHotspot, addedHotspot]); - testSubject = CreateTestSubject(); - - localHotspotsStore.IssuesChanged += Raise.EventWith(testSubject, new IssuesChangedEventArgs([], [])); - - testSubject.GroupViewModels.Should().HaveCount(1); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[0] as GroupFileViewModel, initialHotspot, addedHotspot); - } - - [TestMethod] - public void HotspotsAddedInStore_NewFile_CreatesNewGroup() - { - var initialHotspot = CreateMockedHotspot(filePath: "myFile.cs"); - var addedHotspot = CreateMockedHotspot(filePath: "otherFile.cs"); - localHotspotsStore.GetAllLocalHotspots().Returns([initialHotspot], [initialHotspot, addedHotspot]); - testSubject = CreateTestSubject(); - - localHotspotsStore.IssuesChanged += Raise.EventWith(testSubject, new IssuesChangedEventArgs([], [])); - - testSubject.GroupViewModels.Should().HaveCount(2); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[0] as GroupFileViewModel, initialHotspot); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[1] as GroupFileViewModel, addedHotspot); - } - - [TestMethod] - public void HotspotsAddedInStore_DoesNotUpdateDependencyRisks() - { - var initialHotspot = CreateMockedHotspot(filePath: "myFile.cs"); - var addedHotspot = CreateMockedHotspot(filePath: "otherFile.cs"); - localHotspotsStore.GetAllLocalHotspots().Returns([initialHotspot], [initialHotspot, addedHotspot]); - testSubject = CreateTestSubject(); + var group1 = CreateMockedGroupViewModel(filePath: "myFile.cs"); + var group2 = CreateMockedGroupViewModel(filePath: "myFile.cs"); + hotspotsReportViewModel.GetHotspotsGroupViewModels().Returns([group1, group2]); dependencyRisksStore.ClearReceivedCalls(); - localHotspotsStore.IssuesChanged += Raise.EventWith(testSubject, new IssuesChangedEventArgs([], [])); + hotspotsReportViewModel.HotspotsChanged += Raise.EventWith(testSubject, EventArgs.Empty); - dependencyRisksStore.DidNotReceive().GetAll(); + testSubject.GroupViewModels.Should().HaveCount(2); + testSubject.GroupViewModels.Should().Contain(group1); + testSubject.GroupViewModels.Should().Contain(group2); VerifyHasGroupsUpdated(); + dependencyRisksStore.DidNotReceive().GetAll(); } [TestMethod] - public void HotspotsRemovedFromStore_DeletedHotspotFromExistingFile_UpdatesExistingGroup() - { - var initialHotspot = CreateMockedHotspot(filePath: "myFile.cs"); - var initialHotspot2 = CreateMockedHotspot(filePath: "myFile.cs"); - localHotspotsStore.GetAllLocalHotspots().Returns([initialHotspot, initialHotspot2], [initialHotspot]); - testSubject = CreateTestSubject(); - - localHotspotsStore.IssuesChanged += Raise.EventWith(testSubject, new IssuesChangedEventArgs([], [])); - - testSubject.GroupViewModels.Should().HaveCount(1); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[0] as GroupFileViewModel, initialHotspot); - } - - [TestMethod] - public void HotspotsRemovedFromStore_DeletedSingleHotspotFromExistingFile_RemovesGroup() - { - var initialHotspot = CreateMockedHotspot(filePath: "myFile.cs"); - localHotspotsStore.GetAllLocalHotspots().Returns([initialHotspot], new LocalHotspot[] { }); - testSubject = CreateTestSubject(); - - localHotspotsStore.IssuesChanged += Raise.EventWith(testSubject, new IssuesChangedEventArgs([], [])); - - testSubject.GroupViewModels.Should().BeEmpty(); - } - - [TestMethod] - public void HotspotsRemovedFromStore_DeletedHotspotFromDifferentFile_UpdatesGroupCorrectly() - { - var initialHotspot = CreateMockedHotspot(filePath: "myFile.cs"); - var initialHotspot2 = CreateMockedHotspot(filePath: "otherFile.cs"); - localHotspotsStore.GetAllLocalHotspots().Returns([initialHotspot, initialHotspot2], [initialHotspot]); - testSubject = CreateTestSubject(); - - localHotspotsStore.IssuesChanged += Raise.EventWith(testSubject, new IssuesChangedEventArgs([], [])); - - testSubject.GroupViewModels.Should().HaveCount(1); - VerifyExpectedHotspotGroupViewModel(testSubject.GroupViewModels[0] as GroupFileViewModel, initialHotspot); - } - - [TestMethod] - public void HotspotsRemovedFromStore_DoesNotUpdateDependencyRisks() + public void HotspotsChanged_NoGroups_RaisesProperty() { - var initialHotspot = CreateMockedHotspot(filePath: "myFile.cs"); - localHotspotsStore.GetAllLocalHotspots().Returns([initialHotspot], []); - testSubject = CreateTestSubject(); + hotspotsReportViewModel.GetHotspotsGroupViewModels().Returns([]); dependencyRisksStore.ClearReceivedCalls(); - localHotspotsStore.IssuesChanged += Raise.EventWith(testSubject, new IssuesChangedEventArgs([], [])); + hotspotsReportViewModel.HotspotsChanged += Raise.EventWith(testSubject, EventArgs.Empty); - dependencyRisksStore.DidNotReceive().GetAll(); + testSubject.GroupViewModels.Should().BeEmpty(); VerifyHasGroupsUpdated(); + dependencyRisksStore.DidNotReceive().GetAll(); } [TestMethod] @@ -412,11 +315,11 @@ public void DependencyRisksAddedInStore_DoesNotUpdateHotspots() var addedRisk = CreateDependencyRisk(); dependencyRisksStore.GetAll().Returns([], [addedRisk]); testSubject = CreateTestSubject(); - dependencyRisksStore.ClearReceivedCalls(); + hotspotsReportViewModel.ClearReceivedCalls(); dependencyRisksStore.DependencyRisksChanged += Raise.Event(); - localHotspotsStore.DidNotReceive().GetAll(); + hotspotsReportViewModel.DidNotReceive().GetHotspotsGroupViewModels(); VerifyHasGroupsUpdated(); } @@ -452,11 +355,11 @@ public void DependencyRisksRemovedFromStore_DoesNotUpdateHotspots() var initialRisk = CreateDependencyRisk(); dependencyRisksStore.GetAll().Returns([initialRisk], new IDependencyRisk[] { }); testSubject = CreateTestSubject(); - dependencyRisksStore.ClearReceivedCalls(); + hotspotsReportViewModel.ClearReceivedCalls(); dependencyRisksStore.DependencyRisksChanged += Raise.Event(); - localHotspotsStore.DidNotReceive().GetAll(); + hotspotsReportViewModel.DidNotReceive().GetHotspotsGroupViewModels(); VerifyHasGroupsUpdated(); } @@ -505,11 +408,11 @@ private ReportViewModel CreateTestSubject() { var reportViewModel = new ReportViewModel(activeSolutionBoundTracker, dependencyRisksStore, - localHotspotsStore, showDependencyRiskInBrowserHandler, changeDependencyRiskStatusHandler, navigateToRuleDescriptionCommand, locationNavigator, + hotspotsReportViewModel, messageBox, telemetryManager, threadHandling); @@ -526,31 +429,15 @@ private static IDependencyRisk CreateDependencyRisk(Guid? id = null, bool isReso return risk; } - private static LocalHotspot CreateMockedHotspot(string filePath) + private static IGroupViewModel CreateMockedGroupViewModel(string filePath) { - var analysisIssueVisualization = Substitute.For(); - var analysisIssueBase = Substitute.For(); - analysisIssueBase.PrimaryLocation.FilePath.Returns(filePath); - analysisIssueVisualization.Issue.Returns(analysisIssueBase); - - return new LocalHotspot(analysisIssueVisualization, default, default); + var group = Substitute.For(); + group.Title.Returns(filePath); + return group; } private void MockRisksInStore(params IDependencyRisk[] dependencyRisks) => dependencyRisksStore.GetAll().Returns(dependencyRisks); - private void MockHotspotsInStore(params LocalHotspot[] hotspots) => localHotspotsStore.GetAllLocalHotspots().Returns(hotspots); - - private static void VerifyExpectedHotspotGroupViewModel(GroupFileViewModel groupFileVm, params LocalHotspot[] expectedHotspots) - { - groupFileVm.Should().NotBeNull(); - groupFileVm.FilePath.Should().Be(expectedHotspots[0].Visualization.Issue.PrimaryLocation.FilePath); - groupFileVm.FilteredIssues.Should().HaveCount(expectedHotspots.Length); - foreach (var expectedHotspot in expectedHotspots) - { - groupFileVm.FilteredIssues.Should().ContainSingle(vm => ((HotspotViewModel)vm).LocalHotspot == expectedHotspot); - } - } - private static void VerifyExpectedDependencyRiskGroupViewModel(GroupDependencyRiskViewModel dependencyRiskGroupVm, params IDependencyRisk[] expectedDependencyRisks) { dependencyRiskGroupVm.Should().NotBeNull(); diff --git a/src/IssueViz.Security/ReportView/Hotspots/HotspotsReportViewModel.cs b/src/IssueViz.Security/ReportView/Hotspots/HotspotsReportViewModel.cs new file mode 100644 index 0000000000..53b06311cc --- /dev/null +++ b/src/IssueViz.Security/ReportView/Hotspots/HotspotsReportViewModel.cs @@ -0,0 +1,71 @@ +/* + * SonarLint for Visual Studio + * Copyright (C) 2016-2025 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using System.Collections.ObjectModel; +using System.ComponentModel.Composition; +using SonarLint.VisualStudio.IssueVisualization.Security.Hotspots; +using SonarLint.VisualStudio.IssueVisualization.Security.IssuesStore; + +namespace SonarLint.VisualStudio.IssueVisualization.Security.ReportView.Hotspots; + +internal interface IHotspotsReportViewModel : IDisposable +{ + ObservableCollection GetHotspotsGroupViewModels(); + + event EventHandler HotspotsChanged; +} + +[Export(typeof(IHotspotsReportViewModel))] +[PartCreationPolicy(CreationPolicy.Shared)] +internal sealed class HotspotsReportViewModel : IHotspotsReportViewModel +{ + private readonly ILocalHotspotsStore hotspotsStore; + + [ImportingConstructor] + public HotspotsReportViewModel(ILocalHotspotsStore hotspotsStore) + { + this.hotspotsStore = hotspotsStore; + hotspotsStore.IssuesChanged += HotspotsStore_IssuesChanged; + } + + public void Dispose() => hotspotsStore.IssuesChanged -= HotspotsStore_IssuesChanged; + + public event EventHandler HotspotsChanged; + + public ObservableCollection GetHotspotsGroupViewModels() + { + var hotspots = hotspotsStore.GetAllLocalHotspots().Select(x => new HotspotViewModel(x)); + return GetGroupViewModel(hotspots); + } + + private static ObservableCollection GetGroupViewModel(IEnumerable issueViewModels) + { + var issuesByFileGrouping = issueViewModels.GroupBy(vm => vm.FilePath); + var groupViewModels = new ObservableCollection(); + foreach (var group in issuesByFileGrouping) + { + groupViewModels.Add(new GroupFileViewModel(group.Key, new ObservableCollection(group))); + } + + return groupViewModels; + } + + private void HotspotsStore_IssuesChanged(object sender, IssuesChangedEventArgs e) => HotspotsChanged?.Invoke(this, EventArgs.Empty); +} diff --git a/src/IssueViz.Security/ReportView/ReportViewControl.xaml.cs b/src/IssueViz.Security/ReportView/ReportViewControl.xaml.cs index 742afaace4..257901bf2a 100644 --- a/src/IssueViz.Security/ReportView/ReportViewControl.xaml.cs +++ b/src/IssueViz.Security/ReportView/ReportViewControl.xaml.cs @@ -31,7 +31,7 @@ using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.IssueVisualization.IssueVisualizationControl.ViewModels.Commands; using SonarLint.VisualStudio.IssueVisualization.Security.DependencyRisks; -using SonarLint.VisualStudio.IssueVisualization.Security.Hotspots; +using SonarLint.VisualStudio.IssueVisualization.Security.ReportView.Hotspots; using SonarLint.VisualStudio.IssueVisualization.Security.ReviewStatus; namespace SonarLint.VisualStudio.IssueVisualization.Security.ReportView; @@ -43,13 +43,14 @@ internal sealed partial class ReportViewControl : UserControl private readonly IBrowserService browserService; public ReportViewModel ReportViewModel { get; } + public IHotspotsReportViewModel HotspotsReportViewModel { get; } public IResourceFinder ResourceFinder { get; } = new ResourceFinder(); public ReportViewControl( IActiveSolutionBoundTracker activeSolutionBoundTracker, IBrowserService browserService, IDependencyRisksStore dependencyRisksStore, - ILocalHotspotsStore hotspotsStore, + IHotspotsReportViewModel hotspotsReportViewModel, IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler, IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler, INavigateToRuleDescriptionCommand navigateToRuleDescriptionCommand, @@ -60,13 +61,14 @@ public ReportViewControl( { this.activeSolutionBoundTracker = activeSolutionBoundTracker; this.browserService = browserService; + HotspotsReportViewModel = hotspotsReportViewModel; ReportViewModel = new ReportViewModel(activeSolutionBoundTracker, dependencyRisksStore, - hotspotsStore, showDependencyRiskInBrowserHandler, changeDependencyRiskStatusHandler, navigateToRuleDescriptionCommand, locationNavigator, + HotspotsReportViewModel, messageBox, telemetryManager, threadHandling); diff --git a/src/IssueViz.Security/ReportView/ReportViewModel.cs b/src/IssueViz.Security/ReportView/ReportViewModel.cs index 92ec698586..61e45ab257 100644 --- a/src/IssueViz.Security/ReportView/ReportViewModel.cs +++ b/src/IssueViz.Security/ReportView/ReportViewModel.cs @@ -30,8 +30,6 @@ using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.IssueVisualization.IssueVisualizationControl.ViewModels.Commands; using SonarLint.VisualStudio.IssueVisualization.Security.DependencyRisks; -using SonarLint.VisualStudio.IssueVisualization.Security.Hotspots; -using SonarLint.VisualStudio.IssueVisualization.Security.IssuesStore; using SonarLint.VisualStudio.IssueVisualization.Security.ReportView.Hotspots; namespace SonarLint.VisualStudio.IssueVisualization.Security.ReportView; @@ -40,34 +38,34 @@ internal class ReportViewModel : ServerViewModel { private readonly IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler; private readonly IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler; + private readonly IHotspotsReportViewModel hotspotsReportViewModel; private readonly IMessageBox messageBox; private readonly ITelemetryManager telemetryManager; private readonly IDependencyRisksStore dependencyRisksStore; - private readonly ILocalHotspotsStore hotspotsStore; private readonly object @lock = new(); private IIssueViewModel selectedItem; public ReportViewModel( IActiveSolutionBoundTracker activeSolutionBoundTracker, IDependencyRisksStore dependencyRisksStore, - ILocalHotspotsStore hotspotsStore, IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler, IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler, INavigateToRuleDescriptionCommand navigateToRuleDescriptionCommand, ILocationNavigator locationNavigator, + IHotspotsReportViewModel hotspotsReportViewModel, IMessageBox messageBox, ITelemetryManager telemetryManager, IThreadHandling threadHandling) : base(activeSolutionBoundTracker) { this.dependencyRisksStore = dependencyRisksStore; - this.hotspotsStore = hotspotsStore; this.showDependencyRiskInBrowserHandler = showDependencyRiskInBrowserHandler; this.changeDependencyRiskStatusHandler = changeDependencyRiskStatusHandler; + this.hotspotsReportViewModel = hotspotsReportViewModel; this.messageBox = messageBox; this.telemetryManager = telemetryManager; threadHandling.RunOnUIThread(() => { BindingOperations.EnableCollectionSynchronization(GroupViewModels, @lock); }); - hotspotsStore.IssuesChanged += HotspotsStore_IssuesChanged; + hotspotsReportViewModel.HotspotsChanged += HotspotsChanged; dependencyRisksStore.DependencyRisksChanged += DependencyRisksStore_DependencyRiskChanged; InitializeCommands(navigateToRuleDescriptionCommand, locationNavigator); @@ -114,7 +112,9 @@ public async Task ChangeStatusAsync(IDependencyRisk dependencyRisk, DependencyRi protected override void Dispose(bool disposing) { - hotspotsStore.IssuesChanged -= HotspotsStore_IssuesChanged; + hotspotsReportViewModel.HotspotsChanged -= HotspotsChanged; + hotspotsReportViewModel.Dispose(); + dependencyRisksStore.DependencyRisksChanged -= DependencyRisksStore_DependencyRiskChanged; foreach (var groupViewModel in GroupViewModels) { @@ -131,7 +131,7 @@ private void UpdateTelemetry(IIssueViewModel issueViewModel) } } - private void HotspotsStore_IssuesChanged(object sender, IssuesChangedEventArgs e) + private void HotspotsChanged(object sender, EventArgs e) { foreach (var groupViewModel in GroupViewModels.Where(vm => vm is not GroupDependencyRiskViewModel).ToList()) { @@ -169,24 +169,11 @@ private void InitializeDependencyRisks() private void InitializeHotspots() { - var hotspots = hotspotsStore.GetAllLocalHotspots().Select(x => new HotspotViewModel(x)); - var groups = GetGroupViewModel(hotspots); + var groups = hotspotsReportViewModel.GetHotspotsGroupViewModels(); groups.ToList().ForEach(g => GroupViewModels.Add(g)); RaisePropertyChanged(nameof(HasGroups)); } - private static ObservableCollection GetGroupViewModel(IEnumerable issueViewModels) - { - var issuesByFileGrouping = issueViewModels.GroupBy(vm => vm.FilePath); - var groupViewModels = new ObservableCollection(); - foreach (var group in issuesByFileGrouping) - { - groupViewModels.Add(new GroupFileViewModel(group.Key, new ObservableCollection(group))); - } - - return groupViewModels; - } - private void InitializeCommands( INavigateToRuleDescriptionCommand navigateToRuleDescriptionCommand, ILocationNavigator locationNavigator) diff --git a/src/IssueViz.Security/ReportView/ReportViewToolWindow.cs b/src/IssueViz.Security/ReportView/ReportViewToolWindow.cs index cab73b35ef..0993bd0445 100644 --- a/src/IssueViz.Security/ReportView/ReportViewToolWindow.cs +++ b/src/IssueViz.Security/ReportView/ReportViewToolWindow.cs @@ -28,7 +28,7 @@ using SonarLint.VisualStudio.IssueVisualization.Editor; using SonarLint.VisualStudio.IssueVisualization.IssueVisualizationControl.ViewModels.Commands; using SonarLint.VisualStudio.IssueVisualization.Security.DependencyRisks; -using SonarLint.VisualStudio.IssueVisualization.Security.Hotspots; +using SonarLint.VisualStudio.IssueVisualization.Security.ReportView.Hotspots; namespace SonarLint.VisualStudio.IssueVisualization.Security.ReportView; @@ -43,11 +43,12 @@ public ReportViewToolWindow(IServiceProvider serviceProvider) { var componentModel = serviceProvider.GetService(typeof(SComponentModel)) as IComponentModel; Caption = Resources.ReportViewToolWindowCaption; + var hotspotsReportViewModel = componentModel?.GetService(); Content = new ReportViewControl( componentModel?.GetService(), componentModel?.GetService(), componentModel?.GetService(), - componentModel?.GetService(), + hotspotsReportViewModel, componentModel?.GetService(), componentModel?.GetService(), componentModel?.GetService(), From aa48edc03658484c5ee578845ace4cb55da3c99e Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Fri, 19 Sep 2025 17:40:08 +0200 Subject: [PATCH 2/3] Move logic for dependency risks into another view model to avoid making ReportViewModel a god class. --- .../DependencyRisksReportViewModelTest.cs | 163 ++++++++++++++++++ .../ReportView/ReportViewModelTest.cs | 60 +------ .../DependencyRisksReportViewModel.cs | 95 ++++++++++ .../ReportView/ReportViewControl.xaml.cs | 17 +- .../ReportView/ReportViewModel.cs | 50 ++---- .../ReportView/ReportViewToolWindow.cs | 8 +- 6 files changed, 278 insertions(+), 115 deletions(-) create mode 100644 src/IssueViz.Security.UnitTests/DependencyRisks/DependencyRisksReportViewModelTest.cs create mode 100644 src/IssueViz.Security/DependencyRisks/DependencyRisksReportViewModel.cs diff --git a/src/IssueViz.Security.UnitTests/DependencyRisks/DependencyRisksReportViewModelTest.cs b/src/IssueViz.Security.UnitTests/DependencyRisks/DependencyRisksReportViewModelTest.cs new file mode 100644 index 0000000000..c3510a9710 --- /dev/null +++ b/src/IssueViz.Security.UnitTests/DependencyRisks/DependencyRisksReportViewModelTest.cs @@ -0,0 +1,163 @@ +using System.Windows; +using SonarLint.VisualStudio.Core; +using SonarLint.VisualStudio.Core.Analysis; +using SonarLint.VisualStudio.IssueVisualization.Security.DependencyRisks; +using SonarLint.VisualStudio.TestInfrastructure; + +namespace SonarLint.VisualStudio.IssueVisualization.Security.UnitTests.DependencyRisks; + +[TestClass] +public class DependencyRisksReportViewModelTest +{ + private IDependencyRisksStore dependencyRisksStore; + private IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler; + private IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler; + private IMessageBox messageBox; + private DependencyRisksReportViewModel testSubject; + + [TestInitialize] + public void Initialize() + { + dependencyRisksStore = Substitute.For(); + showDependencyRiskInBrowserHandler = Substitute.For(); + changeDependencyRiskStatusHandler = Substitute.For(); + messageBox = Substitute.For(); + + testSubject = new DependencyRisksReportViewModel(dependencyRisksStore, showDependencyRiskInBrowserHandler, changeDependencyRiskStatusHandler, messageBox); + } + + [TestMethod] + public void MefCtor_CheckIsExported() => + MefTestHelpers.CheckTypeCanBeImported( + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport() + ); + + [TestMethod] + public void MefCtor_CheckIsSingleton() => MefTestHelpers.CheckIsSingletonMefComponent(); + + [TestMethod] + public void Constructor_SubscribesToIssuesChanged() => dependencyRisksStore.Received().DependencyRisksChanged += Arg.Any(); + + [TestMethod] + public void Dispose_UnsubscribesFromIssuesChanged() + { + testSubject.Dispose(); + + dependencyRisksStore.Received().DependencyRisksChanged -= Arg.Any(); + } + + [TestMethod] + public void ShowDependencyRiskInBrowser_CallsHandler() + { + var riskId = Guid.NewGuid(); + var dependencyRisk = CreateDependencyRisk(riskId); + + testSubject.ShowDependencyRiskInBrowser(dependencyRisk); + + showDependencyRiskInBrowserHandler.Received(1).ShowInBrowser(riskId); + } + + [TestMethod] + public async Task ChangeDependencyRiskStatusAsync_CallsHandler_Success() + { + var riskId = Guid.NewGuid(); + var dependencyRisk = CreateDependencyRisk(riskId); + var transition = DependencyRiskTransition.Accept; + var comment = "test comment"; + changeDependencyRiskStatusHandler.ChangeStatusAsync(riskId, transition, comment).Returns(true); + + await testSubject.ChangeDependencyRiskStatusAsync(dependencyRisk, transition, comment); + + await changeDependencyRiskStatusHandler.Received(1).ChangeStatusAsync(riskId, transition, comment); + messageBox.DidNotReceiveWithAnyArgs().Show(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [TestMethod] + public async Task ChangeDependencyRiskStatusAsync_CallsHandler_Failure_ShowsMessageBox() + { + var riskId = Guid.NewGuid(); + var dependencyRisk = CreateDependencyRisk(riskId); + const DependencyRiskTransition transition = DependencyRiskTransition.Accept; + const string comment = "test comment"; + changeDependencyRiskStatusHandler.ChangeStatusAsync(riskId, transition, comment).Returns(false); + + await testSubject.ChangeDependencyRiskStatusAsync(dependencyRisk, transition, comment); + + await changeDependencyRiskStatusHandler.Received(1).ChangeStatusAsync(riskId, transition, comment); + messageBox.Received(1).Show(Resources.DependencyRiskStatusChangeFailedTitle, Resources.DependencyRiskStatusChangeError, MessageBoxButton.OK, MessageBoxImage.Error); + } + + [TestMethod] + public async Task ChangeDependencyRiskStatusAsync_NullTransition_DoesNotCallHandler_ShowsMessageBox() + { + var dependencyRisk = CreateDependencyRisk(); + DependencyRiskTransition? transition = null; + const string comment = "test comment"; + + await testSubject.ChangeDependencyRiskStatusAsync(dependencyRisk, transition, comment); + + await changeDependencyRiskStatusHandler.DidNotReceiveWithAnyArgs().ChangeStatusAsync(Arg.Any(), Arg.Any(), Arg.Any()); + messageBox.Received(1).Show(Resources.DependencyRiskStatusChangeFailedTitle, Resources.DependencyRiskNullTransitionError, MessageBoxButton.OK, MessageBoxImage.Error); + } + + [TestMethod] + public void GetDependencyRisksGroup_ReturnsGroup_WhenNotFixedRisksExist() + { + var risk1 = CreateDependencyRisk(); + var risk2 = CreateDependencyRisk(isFixed: true); + var risk3 = CreateDependencyRisk(); + dependencyRisksStore.GetAll().Returns([risk1, risk2, risk3]); + + var group = testSubject.GetDependencyRisksGroup(); + + group.Should().NotBeNull(); + group.FilteredIssues.Should().HaveCount(2); + group.FilteredIssues.Should().AllBeOfType(); + group.FilteredIssues.Cast().Select(vm => vm.DependencyRisk).Should().Contain([risk1, risk3]); + } + + [TestMethod] + public void GetDependencyRisksGroup_ReturnsNull_WhenAllRisksAreFixed() + { + var fixedRisk = CreateDependencyRisk(isFixed: true); + var fixedRisk2 = CreateDependencyRisk(isFixed: true); + dependencyRisksStore.GetAll().Returns([fixedRisk, fixedRisk2]); + + var group = testSubject.GetDependencyRisksGroup(); + + group.Should().BeNull(); + } + + [TestMethod] + public void GetDependencyRisksGroup_ReturnsNull_WhenNoRisks() + { + dependencyRisksStore.GetAll().Returns([]); + + var group = testSubject.GetDependencyRisksGroup(); + + group.Should().BeNull(); + } + + [TestMethod] + public void DependencyRisksChanged_RaisedOnStoreIssuesChanged() + { + var raised = false; + testSubject.DependencyRisksChanged += (_, _) => raised = true; + + dependencyRisksStore.DependencyRisksChanged += Raise.Event(null, null); + + raised.Should().BeTrue(); + } + + private static IDependencyRisk CreateDependencyRisk(Guid? id = null, bool isFixed = false) + { + var risk = Substitute.For(); + risk.Id.Returns(id ?? Guid.NewGuid()); + risk.Transitions.Returns([]); + risk.Status.Returns(isFixed ? DependencyRiskStatus.Fixed : DependencyRiskStatus.Open); + return risk; + } +} diff --git a/src/IssueViz.Security.UnitTests/ReportView/ReportViewModelTest.cs b/src/IssueViz.Security.UnitTests/ReportView/ReportViewModelTest.cs index e68a58c1a0..6ce107bf7a 100644 --- a/src/IssueViz.Security.UnitTests/ReportView/ReportViewModelTest.cs +++ b/src/IssueViz.Security.UnitTests/ReportView/ReportViewModelTest.cs @@ -19,7 +19,6 @@ */ using System.ComponentModel; -using System.Windows; using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Core.Analysis; using SonarLint.VisualStudio.Core.Binding; @@ -133,60 +132,6 @@ public void Dispose_UnsubscribesFromEvents() hotspotsReportViewModel.Received(1).Dispose(); } - [TestMethod] - public void ShowInBrowser_CallsHandler() - { - var riskId = Guid.NewGuid(); - var dependencyRisk = CreateDependencyRisk(riskId); - - testSubject.ShowInBrowser(dependencyRisk); - - showDependencyRiskInBrowserHandler.Received(1).ShowInBrowser(riskId); - } - - [TestMethod] - public async Task ChangeStatusAsync_CallsHandler_Success() - { - var riskId = Guid.NewGuid(); - var dependencyRisk = CreateDependencyRisk(riskId); - var transition = DependencyRiskTransition.Accept; - var comment = "test comment"; - changeDependencyRiskStatusHandler.ChangeStatusAsync(riskId, transition, comment).Returns(true); - - await testSubject.ChangeStatusAsync(dependencyRisk, transition, comment); - - await changeDependencyRiskStatusHandler.Received(1).ChangeStatusAsync(riskId, transition, comment); - messageBox.DidNotReceiveWithAnyArgs().Show(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); - } - - [TestMethod] - public async Task ChangeStatusAsync_CallsHandler_Failure_ShowsMessageBox() - { - var riskId = Guid.NewGuid(); - var dependencyRisk = CreateDependencyRisk(riskId); - const DependencyRiskTransition transition = DependencyRiskTransition.Accept; - const string comment = "test comment"; - changeDependencyRiskStatusHandler.ChangeStatusAsync(riskId, transition, comment).Returns(false); - - await testSubject.ChangeStatusAsync(dependencyRisk, transition, comment); - - await changeDependencyRiskStatusHandler.Received(1).ChangeStatusAsync(riskId, transition, comment); - messageBox.Received(1).Show(Resources.DependencyRiskStatusChangeFailedTitle, Resources.DependencyRiskStatusChangeError, MessageBoxButton.OK, MessageBoxImage.Error); - } - - [TestMethod] - public async Task ChangeStatusAsync_NullTransition_DoesNotCallHandler_ShowsMessageBox() - { - var dependencyRisk = CreateDependencyRisk(); - DependencyRiskTransition? transition = null; - const string comment = "test comment"; - - await testSubject.ChangeStatusAsync(dependencyRisk, transition, comment); - - await changeDependencyRiskStatusHandler.DidNotReceiveWithAnyArgs().ChangeStatusAsync(Arg.Any(), Arg.Any(), Arg.Any()); - messageBox.Received(1).Show(Resources.DependencyRiskStatusChangeFailedTitle, Resources.DependencyRiskNullTransitionError, MessageBoxButton.OK, MessageBoxImage.Error); - } - [TestMethod] public void SelectedItem_Initially_IsNull() => testSubject.SelectedItem.Should().BeNull(); @@ -407,13 +352,10 @@ public void NavigateToLocationCommand_NavigatesToLocation() private ReportViewModel CreateTestSubject() { var reportViewModel = new ReportViewModel(activeSolutionBoundTracker, - dependencyRisksStore, - showDependencyRiskInBrowserHandler, - changeDependencyRiskStatusHandler, navigateToRuleDescriptionCommand, locationNavigator, hotspotsReportViewModel, - messageBox, + new DependencyRisksReportViewModel(dependencyRisksStore, showDependencyRiskInBrowserHandler, changeDependencyRiskStatusHandler, messageBox), telemetryManager, threadHandling); reportViewModel.PropertyChanged += eventHandler; diff --git a/src/IssueViz.Security/DependencyRisks/DependencyRisksReportViewModel.cs b/src/IssueViz.Security/DependencyRisks/DependencyRisksReportViewModel.cs new file mode 100644 index 0000000000..8cf9c240de --- /dev/null +++ b/src/IssueViz.Security/DependencyRisks/DependencyRisksReportViewModel.cs @@ -0,0 +1,95 @@ +/* + * SonarLint for Visual Studio + * Copyright (C) 2016-2025 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +using System.ComponentModel.Composition; +using System.Windows; +using SonarLint.VisualStudio.Core; +using SonarLint.VisualStudio.Core.Analysis; +using SonarLint.VisualStudio.IssueVisualization.Security.ReportView; + +namespace SonarLint.VisualStudio.IssueVisualization.Security.DependencyRisks; + +public interface IDependencyRisksReportViewModel : IDisposable +{ + IGroupViewModel GetDependencyRisksGroup(); + + Task ChangeDependencyRiskStatusAsync(IDependencyRisk dependencyRisk, DependencyRiskTransition? selectedTransition, string getNormalizedComment); + + void ShowDependencyRiskInBrowser(IDependencyRisk dependencyRisk); + + event EventHandler DependencyRisksChanged; +} + +[Export(typeof(IDependencyRisksReportViewModel))] +[PartCreationPolicy(CreationPolicy.Shared)] +internal sealed class DependencyRisksReportViewModel : IDependencyRisksReportViewModel +{ + private readonly IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler; + private readonly IDependencyRisksStore dependencyRisksStore; + private readonly IMessageBox messageBox; + private readonly IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler; + + [ImportingConstructor] + public DependencyRisksReportViewModel( + IDependencyRisksStore dependencyRisksStore, + IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler, + IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler, + IMessageBox messageBox) + { + this.dependencyRisksStore = dependencyRisksStore; + this.showDependencyRiskInBrowserHandler = showDependencyRiskInBrowserHandler; + this.changeDependencyRiskStatusHandler = changeDependencyRiskStatusHandler; + this.messageBox = messageBox; + dependencyRisksStore.DependencyRisksChanged += DependencyRisksStore_DependencyRiskChanged; + } + + public void Dispose() => dependencyRisksStore.DependencyRisksChanged -= DependencyRisksStore_DependencyRiskChanged; + + public event EventHandler DependencyRisksChanged; + + public IGroupViewModel GetDependencyRisksGroup() + { + var groupDependencyRisk = new GroupDependencyRiskViewModel(dependencyRisksStore); + groupDependencyRisk.InitializeRisks(); + return groupDependencyRisk.FilteredIssues.Any() ? groupDependencyRisk : null; + } + + public async Task ChangeDependencyRiskStatusAsync(IDependencyRisk dependencyRisk, DependencyRiskTransition? selectedTransition, string getNormalizedComment) + { + if (selectedTransition is not { } transition) + { + ShowFailureMessage(Resources.DependencyRiskNullTransitionError); + return; + } + + var result = await changeDependencyRiskStatusHandler.ChangeStatusAsync(dependencyRisk.Id, transition, getNormalizedComment); + + if (!result) + { + ShowFailureMessage(Resources.DependencyRiskStatusChangeError); + } + } + + public void ShowDependencyRiskInBrowser(IDependencyRisk dependencyRisk) => showDependencyRiskInBrowserHandler.ShowInBrowser(dependencyRisk.Id); + + private void ShowFailureMessage(string errorMessage) => messageBox.Show(Resources.DependencyRiskStatusChangeFailedTitle, errorMessage, MessageBoxButton.OK, MessageBoxImage.Error); + + private void DependencyRisksStore_DependencyRiskChanged(object sender, EventArgs e) => DependencyRisksChanged?.Invoke(null, EventArgs.Empty); +} diff --git a/src/IssueViz.Security/ReportView/ReportViewControl.xaml.cs b/src/IssueViz.Security/ReportView/ReportViewControl.xaml.cs index 257901bf2a..0ec34fe86c 100644 --- a/src/IssueViz.Security/ReportView/ReportViewControl.xaml.cs +++ b/src/IssueViz.Security/ReportView/ReportViewControl.xaml.cs @@ -44,32 +44,28 @@ internal sealed partial class ReportViewControl : UserControl public ReportViewModel ReportViewModel { get; } public IHotspotsReportViewModel HotspotsReportViewModel { get; } + public IDependencyRisksReportViewModel DependencyRisksReportViewModel { get; } public IResourceFinder ResourceFinder { get; } = new ResourceFinder(); public ReportViewControl( IActiveSolutionBoundTracker activeSolutionBoundTracker, IBrowserService browserService, - IDependencyRisksStore dependencyRisksStore, IHotspotsReportViewModel hotspotsReportViewModel, - IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler, - IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler, + IDependencyRisksReportViewModel dependencyRisksReportViewModel, INavigateToRuleDescriptionCommand navigateToRuleDescriptionCommand, ILocationNavigator locationNavigator, - IMessageBox messageBox, ITelemetryManager telemetryManager, IThreadHandling threadHandling) { this.activeSolutionBoundTracker = activeSolutionBoundTracker; this.browserService = browserService; HotspotsReportViewModel = hotspotsReportViewModel; + DependencyRisksReportViewModel = dependencyRisksReportViewModel; ReportViewModel = new ReportViewModel(activeSolutionBoundTracker, - dependencyRisksStore, - showDependencyRiskInBrowserHandler, - changeDependencyRiskStatusHandler, navigateToRuleDescriptionCommand, locationNavigator, HotspotsReportViewModel, - messageBox, + DependencyRisksReportViewModel, telemetryManager, threadHandling); InitializeComponent(); @@ -114,7 +110,7 @@ private void ViewDependencyRiskInBrowser_OnClick(object sender, RoutedEventArgs return; } - ReportViewModel.ShowInBrowser(selectedDependencyRiskViewModel.DependencyRisk); + DependencyRisksReportViewModel.ShowDependencyRiskInBrowser(selectedDependencyRiskViewModel.DependencyRisk); } private void DependencyRiskContextMenu_OnLoaded(object sender, RoutedEventArgs e) @@ -145,7 +141,8 @@ private async void ChangeScaStatusMenuItem_OnClick(object sender, RoutedEventArg var dialog = new ChangeStatusWindow(changeStatusViewModel, browserService, activeSolutionBoundTracker); if (dialog.ShowDialog(Application.Current.MainWindow) is true) { - await ReportViewModel.ChangeStatusAsync(selectedDependencyRiskViewModel.DependencyRisk, changeStatusViewModel.GetSelectedTransition(), changeStatusViewModel.GetNormalizedComment()); + await DependencyRisksReportViewModel.ChangeDependencyRiskStatusAsync(selectedDependencyRiskViewModel.DependencyRisk, changeStatusViewModel.GetSelectedTransition(), + changeStatusViewModel.GetNormalizedComment()); } } diff --git a/src/IssueViz.Security/ReportView/ReportViewModel.cs b/src/IssueViz.Security/ReportView/ReportViewModel.cs index 61e45ab257..787b5d67b3 100644 --- a/src/IssueViz.Security/ReportView/ReportViewModel.cs +++ b/src/IssueViz.Security/ReportView/ReportViewModel.cs @@ -19,12 +19,10 @@ */ using System.Collections.ObjectModel; -using System.Windows; using System.Windows.Data; using System.Windows.Input; using Microsoft.VisualStudio.PlatformUI; using SonarLint.VisualStudio.Core; -using SonarLint.VisualStudio.Core.Analysis; using SonarLint.VisualStudio.Core.Binding; using SonarLint.VisualStudio.Core.Telemetry; using SonarLint.VisualStudio.IssueVisualization.Editor; @@ -36,37 +34,28 @@ namespace SonarLint.VisualStudio.IssueVisualization.Security.ReportView; internal class ReportViewModel : ServerViewModel { - private readonly IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler; - private readonly IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler; private readonly IHotspotsReportViewModel hotspotsReportViewModel; - private readonly IMessageBox messageBox; + private readonly IDependencyRisksReportViewModel dependencyRisksReportViewModel; private readonly ITelemetryManager telemetryManager; - private readonly IDependencyRisksStore dependencyRisksStore; private readonly object @lock = new(); private IIssueViewModel selectedItem; public ReportViewModel( IActiveSolutionBoundTracker activeSolutionBoundTracker, - IDependencyRisksStore dependencyRisksStore, - IShowDependencyRiskInBrowserHandler showDependencyRiskInBrowserHandler, - IChangeDependencyRiskStatusHandler changeDependencyRiskStatusHandler, INavigateToRuleDescriptionCommand navigateToRuleDescriptionCommand, ILocationNavigator locationNavigator, IHotspotsReportViewModel hotspotsReportViewModel, - IMessageBox messageBox, + IDependencyRisksReportViewModel dependencyRisksReportViewModel, ITelemetryManager telemetryManager, IThreadHandling threadHandling) : base(activeSolutionBoundTracker) { - this.dependencyRisksStore = dependencyRisksStore; - this.showDependencyRiskInBrowserHandler = showDependencyRiskInBrowserHandler; - this.changeDependencyRiskStatusHandler = changeDependencyRiskStatusHandler; this.hotspotsReportViewModel = hotspotsReportViewModel; - this.messageBox = messageBox; + this.dependencyRisksReportViewModel = dependencyRisksReportViewModel; this.telemetryManager = telemetryManager; threadHandling.RunOnUIThread(() => { BindingOperations.EnableCollectionSynchronization(GroupViewModels, @lock); }); hotspotsReportViewModel.HotspotsChanged += HotspotsChanged; - dependencyRisksStore.DependencyRisksChanged += DependencyRisksStore_DependencyRiskChanged; + dependencyRisksReportViewModel.DependencyRisksChanged += DependencyRisksChanged; InitializeCommands(navigateToRuleDescriptionCommand, locationNavigator); InitializeViewModels(); @@ -90,32 +79,14 @@ public IIssueViewModel SelectedItem } } - public async Task ChangeStatusAsync(IDependencyRisk dependencyRisk, DependencyRiskTransition? selectedTransition, string getNormalizedComment) - { - if (selectedTransition is not { } transition) - { - ShowFailureMessage(Resources.DependencyRiskNullTransitionError); - return; - } - - var result = await changeDependencyRiskStatusHandler.ChangeStatusAsync(dependencyRisk.Id, transition, getNormalizedComment); - - if (!result) - { - ShowFailureMessage(Resources.DependencyRiskStatusChangeError); - } - } - - private void ShowFailureMessage(string errorMessage) => messageBox.Show(Resources.DependencyRiskStatusChangeFailedTitle, errorMessage, MessageBoxButton.OK, MessageBoxImage.Error); - - public void ShowInBrowser(IDependencyRisk dependencyRisk) => showDependencyRiskInBrowserHandler.ShowInBrowser(dependencyRisk.Id); - protected override void Dispose(bool disposing) { hotspotsReportViewModel.HotspotsChanged -= HotspotsChanged; hotspotsReportViewModel.Dispose(); - dependencyRisksStore.DependencyRisksChanged -= DependencyRisksStore_DependencyRiskChanged; + dependencyRisksReportViewModel.DependencyRisksChanged -= DependencyRisksChanged; + dependencyRisksReportViewModel.Dispose(); + foreach (var groupViewModel in GroupViewModels) { groupViewModel.Dispose(); @@ -140,7 +111,7 @@ private void HotspotsChanged(object sender, EventArgs e) InitializeHotspots(); } - private void DependencyRisksStore_DependencyRiskChanged(object sender, EventArgs e) + private void DependencyRisksChanged(object sender, EventArgs e) { if (GroupViewModels.SingleOrDefault(vm => vm is GroupDependencyRiskViewModel) is { } groupDependencyRiskViewModel) { @@ -158,9 +129,8 @@ private void InitializeViewModels() private void InitializeDependencyRisks() { - var groupDependencyRisk = new GroupDependencyRiskViewModel(dependencyRisksStore); - groupDependencyRisk.InitializeRisks(); - if (groupDependencyRisk.FilteredIssues.Any()) + var groupDependencyRisk = dependencyRisksReportViewModel.GetDependencyRisksGroup(); + if (groupDependencyRisk != null) { GroupViewModels.Add(groupDependencyRisk); } diff --git a/src/IssueViz.Security/ReportView/ReportViewToolWindow.cs b/src/IssueViz.Security/ReportView/ReportViewToolWindow.cs index 0993bd0445..79ff5bd46c 100644 --- a/src/IssueViz.Security/ReportView/ReportViewToolWindow.cs +++ b/src/IssueViz.Security/ReportView/ReportViewToolWindow.cs @@ -43,17 +43,13 @@ public ReportViewToolWindow(IServiceProvider serviceProvider) { var componentModel = serviceProvider.GetService(typeof(SComponentModel)) as IComponentModel; Caption = Resources.ReportViewToolWindowCaption; - var hotspotsReportViewModel = componentModel?.GetService(); Content = new ReportViewControl( componentModel?.GetService(), componentModel?.GetService(), - componentModel?.GetService(), - hotspotsReportViewModel, - componentModel?.GetService(), - componentModel?.GetService(), + componentModel?.GetService(), + componentModel?.GetService(), componentModel?.GetService(), componentModel?.GetService(), - componentModel?.GetService(), componentModel?.GetService(), componentModel?.GetService() ); From 77beb40100970ea532e8773f83807af081f0cdc1 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Fri, 19 Sep 2025 18:06:47 +0200 Subject: [PATCH 3/3] SLVS-2573 Fix QG --- .../ReportView/Hotspots/HotspotsReportViewModelTest.cs | 4 ++-- .../DependencyRisks/DependencyRisksReportViewModel.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/IssueViz.Security.UnitTests/ReportView/Hotspots/HotspotsReportViewModelTest.cs b/src/IssueViz.Security.UnitTests/ReportView/Hotspots/HotspotsReportViewModelTest.cs index 433318ba63..5f21316d68 100644 --- a/src/IssueViz.Security.UnitTests/ReportView/Hotspots/HotspotsReportViewModelTest.cs +++ b/src/IssueViz.Security.UnitTests/ReportView/Hotspots/HotspotsReportViewModelTest.cs @@ -72,7 +72,7 @@ public void GetHotspotsGroupViewModels_GroupsByFilePath() groups.Should().HaveCount(2); groups.Select(g => g.Title).Should().Contain([file1, file2]); groups.First(g => g.Title == file1).FilteredIssues.Should().HaveCount(2); - groups.First(g => g.Title == file2).FilteredIssues.Should().HaveCount(1); + groups.First(g => g.Title == file2).FilteredIssues.Should().ContainSingle(); } [TestMethod] @@ -85,7 +85,7 @@ public void GetHotspotsGroupViewModels_TwoHotspotsInSameFile_CreatesOneGroupVmWi var groups = testSubject.GetHotspotsGroupViewModels(); - groups.Should().HaveCount(1); + groups.Should().ContainSingle(); VerifyExpectedHotspotGroupViewModel(groups[0] as GroupFileViewModel, hotspot1, hotspot2); } diff --git a/src/IssueViz.Security/DependencyRisks/DependencyRisksReportViewModel.cs b/src/IssueViz.Security/DependencyRisks/DependencyRisksReportViewModel.cs index 8cf9c240de..e0cce675f0 100644 --- a/src/IssueViz.Security/DependencyRisks/DependencyRisksReportViewModel.cs +++ b/src/IssueViz.Security/DependencyRisks/DependencyRisksReportViewModel.cs @@ -91,5 +91,5 @@ public async Task ChangeDependencyRiskStatusAsync(IDependencyRisk dependencyRisk private void ShowFailureMessage(string errorMessage) => messageBox.Show(Resources.DependencyRiskStatusChangeFailedTitle, errorMessage, MessageBoxButton.OK, MessageBoxImage.Error); - private void DependencyRisksStore_DependencyRiskChanged(object sender, EventArgs e) => DependencyRisksChanged?.Invoke(null, EventArgs.Empty); + private void DependencyRisksStore_DependencyRiskChanged(object sender, EventArgs e) => DependencyRisksChanged?.Invoke(this, EventArgs.Empty); }