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.

Sign up for GitHub

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

Jump to bottom

Add banner to all pages #8579

Merged
merged 11 commits into from
May 18, 2021
Merged

Add banner to all pages #8579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6546ecb
banner to all pages
May 14, 2021
547d80e
remove the banner flag from home display view model
May 14, 2021
6961f9d
fix unit test
May 14, 2021
69b16eb
remove space line
May 14, 2021
ce42d2c
fix name
May 17, 2021
848199a
rename
May 17, 2021
ccfc30c
fix unit test
May 17, 2021
173953c
fix unit test
May 17, 2021
311fa69
add unit test
May 17, 2021
29625a5
fix unit test
May 17, 2021
41f450a
fix unit test
May 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/NuGetGallery/Controllers/AppController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public abstract partial class AppController
: Controller
{
private ICookieExpirationService _cookieExpirationService;
private IFeatureFlagService _featureFlagService;

private IOwinContext _overrideContext;

Expand All @@ -41,11 +42,20 @@ public void SetCookieExpirationService(ICookieExpirationService cookieExpiration
_cookieExpirationService = cookieExpirationService;
}

/// <summary>
/// This method is used for the unit test.
/// </summary>
public void SetFeatureFlagsService(IFeatureFlagService featureFlagService)
{
_featureFlagService = featureFlagService;
}

protected AppController()
{
NuGetContext = new NuGetContext(this);

_cookieExpirationService = GetService<ICookieExpirationService>();
_featureFlagService = GetService<IFeatureFlagService>();
}

protected internal virtual T GetService<T>()
Expand Down Expand Up @@ -117,6 +127,7 @@ protected override void OnActionExecuting(ActionExecutingContext filterContext)
protected override void OnActionExecuted(ActionExecutedContext filterContext)
{
SetCookieCompliance(filterContext);
SetBannerFeatureFlag();

base.OnActionExecuted(filterContext);
}
Expand All @@ -135,5 +146,17 @@ private void SetCookieCompliance(ActionExecutedContext filterContext)
ViewBag.CanWriteAnalyticsCookies = true;
}
}

private void SetBannerFeatureFlag()
{
if (_featureFlagService.IsDisplayBannerEnabled())
{
ViewBag.DisplayBanner = true;
}
else
{
ViewBag.DisplayBanner = false;
}
}
}
}
3 changes: 1 addition & 2 deletions src/NuGetGallery/Controllers/PagesController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,8 @@ public virtual ActionResult Home()
var externalIdentityList = ClaimsExtensions.GetExternalCredentialIdentityList(identity);
var showEnable2FAModal = _featureFlagService.IsShowEnable2FADialogEnabled();
var getFeedbackOnModalDismiss = _featureFlagService.IsGet2FADismissFeedbackEnabled();
var displayBanner = _featureFlagService.IsDisplayBannerEnabled();

return View(new GalleryHomeViewModel(showTransformModal, transformIntoOrganization, showEnable2FAModal, getFeedbackOnModalDismiss, displayBanner, externalIdentityList));
return View(new GalleryHomeViewModel(showTransformModal, transformIntoOrganization, showEnable2FAModal, getFeedbackOnModalDismiss, externalIdentityList));
}

[HttpGet]
Expand Down
5 changes: 0 additions & 5 deletions src/NuGetGallery/ViewModels/GalleryHomeViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,11 @@ public class GalleryHomeViewModel

public bool GetFeedbackOnModalDismissFeatureEnabled { get; set; }

public bool DisplayBannerFeatureEnabled { get; set; }

public GalleryHomeViewModel() : this(
showTransformModal: false,
transformIntoOrganization: false,
showEnable2FAModalFeatureEnabled: false,
getFeedbackOnModalDismiss: false,
displayBanner: false,
identity: null)
{ }

Expand All @@ -31,14 +28,12 @@ public GalleryHomeViewModel(
bool transformIntoOrganization,
bool showEnable2FAModalFeatureEnabled,
bool getFeedbackOnModalDismiss,
bool displayBanner,
string identity = null)
{
ShowTransformModal = showTransformModal;
TransformIntoOrganization = transformIntoOrganization;
ShowEnable2FAModalFeatureEnabled = showEnable2FAModalFeatureEnabled;
GetFeedbackOnModalDismissFeatureEnabled = getFeedbackOnModalDismiss;
DisplayBannerFeatureEnabled = displayBanner;
Identity = identity;
}
}
Expand Down
1 change: 0 additions & 1 deletion src/NuGetGallery/Views/Pages/Home.cshtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
ViewBag.ShowSearchInNavbar = false;
ViewBag.AutofocusSearch = true;
ViewBag.HasJumbotron = true;
ViewBag.DisplayBanner = Model.DisplayBannerFeatureEnabled;
var AskUserToEnable2FA = TempData.ContainsKey("AskUserToEnable2FA")
&& Convert.ToBoolean(TempData["AskUserToEnable2FA"].ToString())
&& Model.ShowEnable2FAModalFeatureEnabled;
Expand Down
32 changes: 32 additions & 0 deletions tests/NuGetGallery.Facts/Controllers/AppControllerFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ public void SetViewBagGivenHttpContextItemsWithCanWriteAnalyticsCookies(bool can
{
// Arrange
var cookieExpirationService = GetMock<ICookieExpirationService>();
var featureFlagsService = GetMock<IFeatureFlagService>();
cookieExpirationService.Setup(e => e.ExpireAnalyticsCookies(It.IsAny<HttpContextBase>()));
featureFlagsService.Setup(e => e.IsDisplayBannerEnabled()).Returns(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to have a separate test for the banner flag.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to verify the banner flag in these two tests for cookies, if we have another test to cover it.


var httpContext = new Mock<HttpContextBase>();
var items = new Dictionary<string, bool>
Expand All @@ -82,12 +84,14 @@ public void SetViewBagGivenHttpContextItemsWithCanWriteAnalyticsCookies(bool can

var controller = GetController<TestableAppController>();
controller.SetCookieExpirationService(cookieExpirationService.Object);
controller.SetFeatureFlagsService(featureFlagsService.Object);

// Act
InvokeOnActionExecutedMethod(controller.ControllerContext, httpContext.Object, controller);

// Assert
Assert.Equal(canWriteAnalyticsCookies, controller.ViewBag.CanWriteAnalyticsCookies);
Assert.Equal(false, controller.ViewBag.DisplayBanner);
if (canWriteAnalyticsCookies)
{
cookieExpirationService.Verify(e => e.ExpireAnalyticsCookies(It.IsAny<HttpContextBase>()), Times.Never);
Expand All @@ -103,14 +107,17 @@ public void SetViewBagGivenHttpContextItemsWithNullCanWriteAnalyticsCookies()
{
// Arrange
var cookieExpirationService = GetMock<ICookieExpirationService>();
var featureFlagsService = GetMock<IFeatureFlagService>();
cookieExpirationService.Setup(e => e.ExpireAnalyticsCookies(It.IsAny<HttpContextBase>()));
featureFlagsService.Setup(e => e.IsDisplayBannerEnabled()).Returns(false);

var httpContext = new Mock<HttpContextBase>();
var items = new Dictionary<string, bool>();
httpContext.Setup(c => c.Items).Returns(items);

var controller = GetController<TestableAppController>();
controller.SetCookieExpirationService(cookieExpirationService.Object);
controller.SetFeatureFlagsService(featureFlagsService.Object);

// Act
InvokeOnActionExecutedMethod(controller.ControllerContext, httpContext.Object, controller);
Expand All @@ -120,6 +127,31 @@ public void SetViewBagGivenHttpContextItemsWithNullCanWriteAnalyticsCookies()
cookieExpirationService.Verify(e => e.ExpireAnalyticsCookies(It.IsAny<HttpContextBase>()), Times.Once);
}

[Theory]
[InlineData(true)]
[InlineData(false)]
public void SetViewBagWithBannerWhenIsDisplayBannerEnabled(bool isDisplayBannerEnabled) {
// Arrange
var cookieExpirationService = GetMock<ICookieExpirationService>();
var featureFlagsService = GetMock<IFeatureFlagService>();
cookieExpirationService.Setup(e => e.ExpireAnalyticsCookies(It.IsAny<HttpContextBase>()));
Copy link
Contributor

@zhhyu zhhyu May 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

You can also remove this setup and verification because we test the banner flag here.

Copy link
Contributor Author

@lyndaidaii lyndaidaii May 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

it's private method. I don't think we could remove the set up. let me know if I am wrong.

featureFlagsService.Setup(e => e.IsDisplayBannerEnabled()).Returns(isDisplayBannerEnabled);

var httpContext = new Mock<HttpContextBase>();
var items = new Dictionary<string, bool>();
httpContext.Setup(c => c.Items).Returns(items);

var controller = GetController<TestableAppController>();
controller.SetCookieExpirationService(cookieExpirationService.Object);
controller.SetFeatureFlagsService(featureFlagsService.Object);

// Act
InvokeOnActionExecutedMethod(controller.ControllerContext, httpContext.Object, controller);

// Assert
Assert.Equal(isDisplayBannerEnabled, controller.ViewBag.DisplayBanner);
}

private void InvokeOnActionExecutedMethod(ControllerContext controllerContext, HttpContextBase httpContext, AppController controller)
{
var actionExecutedContext = new ActionExecutedContext(controllerContext,
Expand Down
1 change: 1 addition & 0 deletions tests/NuGetGallery.Facts/Controllers/StatisticsControllerFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,7 @@ public async Task IgnoresUserCultureAndReturnsStatisticsAsNumbers()

var controller = CreateController(aggregateStatsService, request);
controller.SetCookieExpirationService(Mock.Of<ICookieExpirationService>());
controller.SetFeatureFlagsService(Mock.Of<IFeatureFlagService>());

// Act

Expand Down