Skip to content

Commit

Permalink
SLVS-1561 Close notifications when solution is closed (#5781)
Browse files Browse the repository at this point in the history
  • Loading branch information
vnaskos-sonar authored and georgii-borovinskikh-sonarsource committed Nov 5, 2024
1 parent a9b49c2 commit 39a6438
Show file tree
Hide file tree
Showing 10 changed files with 96 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public void Show_GeneratesCorrectNotificationStructure()
notificationActions[1].CommandText.Should().Be(BindingStrings.SharedBindingSuggestionInfoOptionText);
notificationActions[1].ShouldDismissAfterAction.Should().BeFalse();
notificationActions[2].Should().BeSameAs(doNotShowAgainMock.Object);
notification.CloseOnSolutionClose.Should().Be(true);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ public void Show(ServerType serverType, Action onConnectHandler)
var notification = new Notification(
id: string.Format(IdTemplate, solutionInfoProvider.GetSolutionName()),
message: string.Format(BindingStrings.SharedBindingSuggestionMainText, serverType),
actions: new INotificationAction[]
{
actions:
[
new NotificationAction(BindingStrings.SharedBindingSuggestionConnectOptionText, _ => onConnectHandler(), true),
new NotificationAction(BindingStrings.SharedBindingSuggestionInfoOptionText, _ => OnLearnMore(), false),
doNotShowAgainNotificationAction
});
]);

notificationService.ShowNotification(notification);
}
Expand Down
56 changes: 49 additions & 7 deletions src/Core.UnitTests/Notifications/NotificationServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public class NotificationServiceTests
{
private IInfoBarManager infoBarManager;
private IDisabledNotificationsStorage disabledNotificationsStorage;
private IActiveSolutionTracker activeSolutionTracker;
private IThreadHandling threadHandling;
private TestLogger logger;
private NotificationService testSubject;
Expand All @@ -38,9 +39,10 @@ public void TestInitialize()
{
infoBarManager = Substitute.For<IInfoBarManager>();
disabledNotificationsStorage = Substitute.For<IDisabledNotificationsStorage>();
activeSolutionTracker = Substitute.For<IActiveSolutionTracker>();
threadHandling = Substitute.For<IThreadHandling>();
logger = new TestLogger();
testSubject = new NotificationService(infoBarManager, disabledNotificationsStorage, threadHandling, logger);
testSubject = new NotificationService(infoBarManager, disabledNotificationsStorage, activeSolutionTracker, threadHandling, logger);

AllowRunningUiThreadActions();
}
Expand All @@ -51,6 +53,7 @@ public void MefCtor_CheckIsExported()
MefTestHelpers.CheckTypeCanBeImported<NotificationService, INotificationService>(
MefTestHelpers.CreateExport<IInfoBarManager>(),
MefTestHelpers.CreateExport<IDisabledNotificationsStorage>(),
MefTestHelpers.CreateExport<IActiveSolutionTracker>(),
MefTestHelpers.CreateExport<IThreadHandling>(),
MefTestHelpers.CreateExport<ILogger>());
}
Expand Down Expand Up @@ -105,7 +108,7 @@ public void ShowNotification_AttachesInfoBarToWindow_RunsOnUIThread()
Action showInfoBar = null;
var captureParamThreadHandling = Substitute.For<IThreadHandling>();
captureParamThreadHandling.RunOnUIThreadAsync(Arg.Do<Action>(arg => showInfoBar = arg));
var threadHandledTestSubject = new NotificationService(infoBarManager, disabledNotificationsStorage, captureParamThreadHandling, logger);
var threadHandledTestSubject = new NotificationService(infoBarManager, disabledNotificationsStorage, activeSolutionTracker, captureParamThreadHandling, logger);

// Trigger the ShowNotification without running the action to validate that the UI action is the one responsible for showing the notification
threadHandledTestSubject.ShowNotification(notification);
Expand Down Expand Up @@ -255,7 +258,38 @@ public void ShowNotification_WithToolWindowGuid_ShowOnToolWindow()

AssertNotificationWasShown(attachedNotification, aToolWindowId);
}

[TestMethod]
public void ActiveSolutionChangedEvent_WhenNoActiveNotification_NotificationNotClosed()
{
activeSolutionTracker.ActiveSolutionChanged += Raise.Event<EventHandler<ActiveSolutionChangedEventArgs>>(this, new ActiveSolutionChangedEventArgs(false));

infoBarManager.DidNotReceive().CloseInfoBar(Arg.Any<IInfoBar>());
}

[TestMethod]
public void ActiveSolutionChangedEvent_WhenSolutionClosedAndCloseOnSolutionClose_NotificationCloses()
{
var attachedNotification = ShowNotification(closeOnSolutionClose: true);

activeSolutionTracker.ActiveSolutionChanged += Raise.Event<EventHandler<ActiveSolutionChangedEventArgs>>(this, new ActiveSolutionChangedEventArgs(false));

AssertNotificationClosed(attachedNotification);
}

[TestMethod]
[DataRow(false, false)]
[DataRow(true, false)]
[DataRow(true, true)]
public void ActiveSolutionChangedEvent_WhenSolutionOpenedOrNotCloseOnSolutionClose_NotificationDoesNotClose(bool isSolutionOpen, bool closeOnSolutionClose)
{
var attachedNotification = ShowNotification(closeOnSolutionClose: closeOnSolutionClose);

activeSolutionTracker.ActiveSolutionChanged += Raise.Event<EventHandler<ActiveSolutionChangedEventArgs>>(this, new ActiveSolutionChangedEventArgs(isSolutionOpen));

AssertNotificationNotClosed(attachedNotification);
}

[TestMethod]
public void Dispose_ClosesNotification()
{
Expand All @@ -266,6 +300,14 @@ public void Dispose_ClosesNotification()
AssertNotificationClosed(attachedNotification);
}

[TestMethod]
public void Dispose_UnsubscribesFromActiveSolutionChangeEvents()
{
testSubject.Dispose();

activeSolutionTracker.Received(1).ActiveSolutionChanged -= Arg.Any<EventHandler<ActiveSolutionChangedEventArgs>>();
}

[TestMethod]
public void ShowNotification_WhenNonCriticalException_ExceptionCaught()
{
Expand Down Expand Up @@ -321,7 +363,7 @@ public void CloseNotification_ClosesInfoBar_RunsOnUIThread()
Action runThreadAction = null;
var captureParamThreadHandling = Substitute.For<IThreadHandling>();
captureParamThreadHandling.RunOnUIThreadAsync(Arg.Do<Action>(arg => runThreadAction = arg));
var threadHandledTestSubject = new NotificationService(infoBarManager, disabledNotificationsStorage, captureParamThreadHandling, logger);
var threadHandledTestSubject = new NotificationService(infoBarManager, disabledNotificationsStorage, activeSolutionTracker, captureParamThreadHandling, logger);

// Show a notification to be able to close it
threadHandledTestSubject.ShowNotification(notification);
Expand Down Expand Up @@ -368,18 +410,18 @@ private void AllowRunningUiThreadActions()
threadHandling.RunOnUIThreadAsync(Arg.Do<Action>(arg => arg()));
}

private AttachedNotification ShowNotification(string id = null, bool oncePerSession = true, bool disabled = false, params INotificationAction[] actions)
private AttachedNotification ShowNotification(string id = null, bool oncePerSession = true, bool disabled = false, bool closeOnSolutionClose = true, params INotificationAction[] actions)
{
var notification = CreateNotification(id, oncePerSession, disabled, actions);
var notification = CreateNotification(id, oncePerSession, disabled, closeOnSolutionClose, actions);
var attachedNotification = AttachNotification(notification);
testSubject.ShowNotification(notification);
return attachedNotification;
}

private INotification CreateNotification(string id = null, bool oncePerSession = true, bool disabled = false, params INotificationAction[] actions)
private INotification CreateNotification(string id = null, bool oncePerSession = true, bool disabled = false, bool closeOnSolutionClose = true, params INotificationAction[] actions)
{
var notificationId = id ?? Guid.NewGuid().ToString();
var notification = new Notification(notificationId, notificationId, actions, oncePerSession);
var notification = new Notification(notificationId, notificationId, actions, oncePerSession, closeOnSolutionClose);

disabledNotificationsStorage.IsNotificationDisabled(notificationId).Returns(disabled);

Expand Down
27 changes: 24 additions & 3 deletions src/Core/Notifications/INotificationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ public interface INotificationService : IDisposable
internal sealed class NotificationService : INotificationService
{
private static readonly Guid MainWindowId = Guid.Empty;

private readonly IInfoBarManager infoBarManager;
private readonly IDisabledNotificationsStorage notificationsStorage;
private readonly IActiveSolutionTracker activeSolutionTracker;
private readonly IThreadHandling threadHandling;
private readonly ILogger logger;

Expand All @@ -56,17 +57,22 @@ internal sealed class NotificationService : INotificationService
private AttachedNotification activeNotification;

[ImportingConstructor]
public NotificationService(IInfoBarManager infoBarManager,
public NotificationService(
IInfoBarManager infoBarManager,
IDisabledNotificationsStorage notificationsStorage,
IActiveSolutionTracker activeSolutionTracker,
IThreadHandling threadHandling,
ILogger logger)
{
this.infoBarManager = infoBarManager;
this.notificationsStorage = notificationsStorage;
this.activeSolutionTracker = activeSolutionTracker;
this.threadHandling = threadHandling;
this.logger = logger;

this.activeSolutionTracker.ActiveSolutionChanged += OnActiveSolutionChanged;
}

public void ShowNotification(INotification notification)
{
ShowNotification(notification, MainWindowId);
Expand Down Expand Up @@ -190,10 +196,25 @@ private AttachedNotification AttachInfoBar(INotification notification, Guid tool

return new AttachedNotification(notification, infoBar);
}

private void OnActiveSolutionChanged(object sender, ActiveSolutionChangedEventArgs e)
{
if (activeNotification == null)
{
return;
}

if (activeNotification.Notification.CloseOnSolutionClose && !e.IsSolutionOpen)
{
CloseNotification();
}
}

public void Dispose()
{
CloseNotification();

activeSolutionTracker.ActiveSolutionChanged -= OnActiveSolutionChanged;
}

private sealed record AttachedNotification(INotification Notification, IInfoBar InfoBar);
Expand Down
8 changes: 4 additions & 4 deletions src/Core/Notifications/Notification.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System;
using System.Collections.Generic;

namespace SonarLint.VisualStudio.Core.Notifications
{
public interface INotification
Expand All @@ -30,21 +27,24 @@ public interface INotification
IEnumerable<INotificationAction> Actions { get; }

bool ShowOncePerSession { get; }
bool CloseOnSolutionClose { get; }
}

public class Notification : INotification
{
public Notification(string id, string message, IEnumerable<INotificationAction> actions, bool showOncePerSession = true)
public Notification(string id, string message, IEnumerable<INotificationAction> actions, bool showOncePerSession = true, bool closeOnSolutionClose = true)
{
Id = id ?? throw new ArgumentNullException(nameof(id));
Message = message ?? throw new ArgumentNullException(nameof(message));
Actions = actions ?? throw new ArgumentNullException(nameof(actions));
ShowOncePerSession = showOncePerSession;
CloseOnSolutionClose = closeOnSolutionClose;
}

public string Id { get; }
public string Message { get; }
public IEnumerable<INotificationAction> Actions { get; }
public bool ShowOncePerSession { get; }
public bool CloseOnSolutionClose { get; }
}
}
2 changes: 0 additions & 2 deletions src/Integration/MefServices/SharedBindingSuggestionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@
*/

using System.ComponentModel.Composition;
using System.Windows;
using SonarLint.VisualStudio.ConnectedMode.Binding.Suggestion;
using SonarLint.VisualStudio.ConnectedMode.Shared;
using SonarLint.VisualStudio.ConnectedMode.UI;
using SonarLint.VisualStudio.ConnectedMode.UI.ManageBinding;
using SonarLint.VisualStudio.Core;
using SonarLint.VisualStudio.Core.Binding;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using SonarLint.VisualStudio.Core;
using SonarLint.VisualStudio.Core.Notifications;
using SonarLint.VisualStudio.SLCore.NodeJS.Notifications;
using Language = SonarLint.VisualStudio.SLCore.Common.Models.Language;

namespace SonarLint.VisualStudio.SLCore.UnitTests.NodeJS.Notifications
{
Expand All @@ -37,11 +38,11 @@ public void MefCtor_CheckIsExported()
}

[DataTestMethod]
[DataRow(SLCore.Common.Models.Language.JS, "321", "123", "123")]
[DataRow(SLCore.Common.Models.Language.CSS, "99.00.11", "9876.5432", "9876.5432")]
[DataRow(SLCore.Common.Models.Language.JS, "321", null, "Not found")]
[DataRow(SLCore.Common.Models.Language.TS, "5.4.3.2.1", null, "Not found")]
public void Show_ShowsCorrectMessageAndNotificationId(SLCore.Common.Models.Language language, string expectedVersion, string actualVersion, string displayActualVersion)
[DataRow(Language.JS, "321", "123", "123")]
[DataRow(Language.CSS, "99.00.11", "9876.5432", "9876.5432")]
[DataRow(Language.JS, "321", null, "Not found")]
[DataRow(Language.TS, "5.4.3.2.1", null, "Not found")]
public void Show_ShowsCorrectMessageAndNotificationId(Language language, string expectedVersion, string actualVersion, string displayActualVersion)
{
INotification createdNotification = null;
var notificationService = CreateNotificationService(n => createdNotification = n);
Expand All @@ -60,6 +61,7 @@ public void Show_ShowsCorrectMessageAndNotificationId(SLCore.Common.Models.Langu
createdNotification.Id.Should().Be("sonarlint.nodejs.min.version.not.found");
createdNotification.Message.Should().Be($"SonarLint: {language} analysis failed. Could not find a Node.js runtime (required: >={expectedVersion}, actual: {displayActualVersion}) on your computer.");
createdNotification.Actions.Count().Should().Be(2);
createdNotification.CloseOnSolutionClose.Should().Be(false);
}

[TestMethod]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Linq;
using SonarLint.VisualStudio.Core.Notifications;
using SonarLint.VisualStudio.SLCore.Notification;

Expand Down Expand Up @@ -59,6 +58,7 @@ public void Show_CreatesNotificationAndCallsShow()
notification.Id.Should().Be("sonarlint.sloop.restart.failed");
notification.Message.Should().Be("SonarLint background service failed to start");
notification.Actions.Should().HaveCount(1);
notification.CloseOnSolutionClose.Should().Be(false);

notification.Actions.First().Action(notification);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ public void Show(string languageName, string minVersion, string currentVersion =
notificationService.ShowNotification(new VisualStudio.Core.Notifications.Notification(
id: NotificationId,
message: string.Format(NotificationStrings.NotificationUnsupportedNode, languageName, minVersion, currentVersion ?? NotificationStrings.NotificationNoneVersion),
actions: new INotificationAction[]
{
actions:
[
new NotificationAction(NotificationStrings.NotificationShowMoreInfoAction, _ => ShowMoreInfo(), false),
doNotShowAgainNotificationAction
}));
],
closeOnSolutionClose: false)
);
}

private void ShowMoreInfo()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System;
using System.ComponentModel.Composition;
using SonarLint.VisualStudio.Core.Notifications;

Expand Down Expand Up @@ -52,7 +51,8 @@ public void Show(Action act)
{
new NotificationAction(SLCoreStrings.SloopRestartFailedNotificationService_Restart, _ => act(), true)
},
showOncePerSession: false
showOncePerSession: false,
closeOnSolutionClose: false
);

notificationService.ShowNotification(notification);
Expand Down

0 comments on commit 39a6438

Please sign in to comment.