Make it possible to unsubscribe from package pushed notifications

Amends NuGet#2961

By popular request, making it possible to unsubscribe from package
pushed notifications separately.

src/NuGetGallery.Core/Entities/User.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Linq;
 using System.Collections.Generic;
+using System.ComponentModel;
 using System.ComponentModel.DataAnnotations;
 
 namespace NuGetGallery
@@ -37,6 +38,9 @@ public User(string username)
         public virtual ICollection<Role> Roles { get; set; }
         public bool EmailAllowed { get; set; }
 
+        [DefaultValue(true)]
+        public bool NotifyPackagePushed { get; set; }
+
         public bool Confirmed
         {
             get { return !String.IsNullOrEmpty(EmailAddress); }

src/NuGetGallery/App_Start/Routes.cs

@@ -233,16 +233,11 @@ public static void RegisterUIRoutes(RouteCollection routes)
                 RouteName.ConfirmAccount,
                 "account/confirm/{username}/{token}",
                 new { controller = "Users", action = "Confirm" });
-
-            routes.MapRoute(
-                RouteName.SubscribeToEmails,
-                "account/subscribe",
-                new { controller = "Users", action = "ChangeEmailSubscription", subscribe = true });
-
+
             routes.MapRoute(
-                RouteName.UnsubscribeFromEmails,
-                "account/unsubscribe",
-                new { controller = "Users", action = "ChangeEmailSubscription", subscribe = false });
+                RouteName.ChangeEmailSubscription,
+                "account/subscription/change",
+                new { controller = "Users", action = "ChangeEmailSubscription" });
 
             routes.MapRoute(
                 RouteName.Account,

src/NuGetGallery/Authentication/AuthenticationService.cs

@@ -143,6 +143,7 @@ public virtual async Task<AuthenticatedUser> Register(string username, string em
                 EmailAllowed = true,
                 UnconfirmedEmailAddress = emailAddress,
                 EmailConfirmationToken = CryptographyService.GenerateToken(),
+                NotifyPackagePushed = true,
                 CreatedUtc = DateTime.UtcNow
             };
 

src/NuGetGallery/Controllers/UsersController.cs

@@ -89,15 +89,18 @@ public virtual ActionResult Account()
         [Authorize]
         [HttpPost]
         [ValidateAntiForgeryToken]
-        public virtual async Task<ActionResult> ChangeEmailSubscription(bool subscribe)
+        public virtual async Task<ActionResult> ChangeEmailSubscription(bool? emailAllowed, bool? notifyPackagePushed)
         {
             var user = GetCurrentUser();
             if (user == null)
             {
                 return HttpNotFound();
             }
+
+            await UserService.ChangeEmailSubscriptionAsync(user, 
+                emailAllowed.HasValue && emailAllowed.Value, 
+                notifyPackagePushed.HasValue && notifyPackagePushed.Value);
 
-            await UserService.ChangeEmailSubscriptionAsync(user, subscribe);
             TempData["Message"] = Strings.EmailPreferencesUpdated;
             return RedirectToAction("Account");
         }

src/NuGetGallery/Migrations/201604061724388_NotifyPackagePushed.cs

@@ -0,0 +1,18 @@
+namespace NuGetGallery.Migrations
+{
+    using System;
+    using System.Data.Entity.Migrations;
+
+    public partial class NotifyPackagePushed : DbMigration
+    {
+        public override void Up()
+        {
+            AddColumn("dbo.Users", "NotifyPackagePushed", c => c.Boolean(nullable: false, defaultValue: true));
+        }
+
+        public override void Down()
+        {
+            DropColumn("dbo.Users", "NotifyPackagePushed");
+        }
+    }
+}

src/NuGetGallery/NuGetGallery.csproj

@@ -695,6 +695,10 @@
     <Compile Include="Migrations\201603151731262_AddIndexForPackageDeletes.Designer.cs">
       <DependentUpon>201603151731262_AddIndexForPackageDeletes.cs</DependentUpon>
     </Compile>
+    <Compile Include="Migrations\201604061724388_NotifyPackagePushed.cs" />
+    <Compile Include="Migrations\201604061724388_NotifyPackagePushed.Designer.cs">
+      <DependentUpon>201604061724388_NotifyPackagePushed.cs</DependentUpon>
+    </Compile>
     <Compile Include="OData\Conventions\CompositeODataKeyHelper.cs" />
     <Compile Include="OData\Conventions\EntitySetPropertyRoutingConvention.cs" />
     <Compile Include="OData\ODataServiceVersionHeaderPropagatingBatchHandler.cs" />
@@ -1772,6 +1776,9 @@
     <EmbeddedResource Include="Migrations\201603151731262_AddIndexForPackageDeletes.resx">
       <DependentUpon>201603151731262_AddIndexForPackageDeletes.cs</DependentUpon>
     </EmbeddedResource>
+    <EmbeddedResource Include="Migrations\201604061724388_NotifyPackagePushed.resx">
+      <DependentUpon>201604061724388_NotifyPackagePushed.cs</DependentUpon>
+    </EmbeddedResource>
     <EmbeddedResource Include="Strings.resx">
       <Generator>PublicResXFileCodeGenerator</Generator>
       <LastGenOutput>Strings.Designer.cs</LastGenOutput>

src/NuGetGallery/RouteNames.cs

@@ -60,8 +60,7 @@ public static class RouteName
         public const string RemoveCredential = "RemoveCredential";
         public const string RemovePassword = "RemovePassword";
         public const string ConfirmAccount = "ConfirmAccount";
-        public const string SubscribeToEmails = "SubscribeToEmails";
-        public const string UnsubscribeFromEmails = "UnsubscribeFromEmails";
+        public const string ChangeEmailSubscription = "ChangeEmailSubscription";
         public const string Error500 = "Error500";
         public const string Error404 = "Error404";
         public const string Status = "Status";

src/NuGetGallery/Services/IUserService.cs

@@ -8,7 +8,7 @@ namespace NuGetGallery
 {
     public interface IUserService
     {
-        Task ChangeEmailSubscriptionAsync(User user, bool emailAllowed);
+        Task ChangeEmailSubscriptionAsync(User user, bool emailAllowed, bool notifyPackagePushed);
 
         User FindByEmailAddress(string emailAddress);
 

src/NuGetGallery/Services/MessageService.cs

@@ -488,7 +488,7 @@ [change your email notification settings]({5}).
                 mailMessage.Body = body;
                 mailMessage.From = Config.GalleryOwner;
 
-                AddOwnersToMailMessage(package.PackageRegistration, mailMessage);
+                AddOwnersSubscribedToPackagePushedNotification(package.PackageRegistration, mailMessage);
 
                 if (mailMessage.To.Any())
                 {
@@ -504,5 +504,13 @@ private static void AddOwnersToMailMessage(PackageRegistration packageRegistrati
                 mailMessage.To.Add(owner.ToMailAddress());
             }
         }
+
+        private static void AddOwnersSubscribedToPackagePushedNotification(PackageRegistration packageRegistration, MailMessage mailMessage)
+        {
+            foreach (var owner in packageRegistration.Owners.Where(o => o.NotifyPackagePushed))
+            {
+                mailMessage.To.Add(owner.ToMailAddress());
+            }
+        }
     }
 }

src/NuGetGallery/Services/UserService.cs

@@ -9,6 +9,7 @@
 using NuGetGallery.Configuration;
 using NuGetGallery.Auditing;
 using System.Threading.Tasks;
+using NuGetGallery.Migrations;
 
 namespace NuGetGallery
 {
@@ -34,14 +35,15 @@ public UserService(
             Auditing = auditing;
         }
 
-        public async Task ChangeEmailSubscriptionAsync(User user, bool emailAllowed)
+        public async Task ChangeEmailSubscriptionAsync(User user, bool emailAllowed, bool notifyPackagePushed)
         {
             if (user == null)
             {
                 throw new ArgumentNullException(nameof(user));
             }
 
             user.EmailAllowed = emailAllowed;
+            user.NotifyPackagePushed = notifyPackagePushed;
             await UserRepository.CommitChangesAsync();
         }
 

src/NuGetGallery/ViewModels/AccountViewModel.cs

@@ -44,7 +44,7 @@ public class ChangePasswordViewModel
         [AllowHtml]
         public string NewPassword { get; set; }
     }
-
+    
     public class CredentialViewModel
     {
         public string Type { get; set; }

src/NuGetGallery/Views/Users/Account.cshtml

@@ -212,35 +212,45 @@
     **************************@
     @AccordianBar(
         groupName: "editprofile",
-        title: (CurrentUser.EmailAllowed ? "Subscribed to Email Notifications" : "Not subscribed to Email Notifications (users can't contact you)"),
-        enabled: CurrentUser.EmailAllowed,
+        title: "Email Notifications",
+        enabled: true,
         actions: @<text>
-            @item.ExpandLink("More Info", "Less Info")
-            @using (Html.BeginForm(actionName: "ChangeEmailSubscription", controllerName: "Users", routeValues: new { subscribe = !CurrentUser.EmailAllowed }, method: FormMethod.Post, htmlAttributes: new { @class = "form-inline" }))
+            @(CurrentUser.EmailAllowed ? "Users can contact you" : "Users can not contact you")
+            | @(CurrentUser.NotifyPackagePushed ? "Receiving package push notifications" : "Not receiving package push notifications")
+            @item.ExpandButton("Change", "Cancel")
+        </text>,
+        content: @<text>
+            @using (Html.BeginRouteForm(routeName: RouteName.ChangeEmailSubscription, method: FormMethod.Post, htmlAttributes: new { @class = "form-inline" }))
             {
                 <fieldset class="form">
-                    <legend>Update Email Subscription</legend>
+                    <legend>Update Email Notifications</legend>
                     @Html.AntiForgeryToken()
-                    <input type="submit" style="width: auto;"
-                           value="@(CurrentUser.EmailAllowed ? "Unsubscribe" : "Subscribe")"
-                           title="@(CurrentUser.EmailAllowed ? "Unsubscribe" : "Subscribe")" />
+                    <div class="form-field">
+                        <input name="emailAllowed" id="CurrentUser_EmailAllowed" type="checkbox" checked="@(CurrentUser.EmailAllowed)" value="true">
+                        <label class="checkbox" for="CurrentUser_EmailAllowed">Users can contact me through the <em>Contact Owners form</em></label>
+                        <p class="smaller">
+                            This subscription allows other registered users of the site to contact you
+                            about packages that you own using the <em>Contact Owners</em> form, or to request
+                            that you become an owner of their package. Unsubscribing means users cannot contact you for these reasons.
+                        </p>
+                    </div>
+                    <div class="form-field">
+                        <input name="notifyPackagePushed" id="CurrentUser_NotifyPackagePushed" type="checkbox" checked="@(CurrentUser.NotifyPackagePushed)" value="true">
+                        <label class="checkbox" for="CurrentUser_NotifyPackagePushed">Notify me when a package is pushed to @Config.Current.Brand using my account</label>
+                        <p class="smaller">
+                            This subscription will send a notification whenever a package is pushed using your account.
+                            We recommend to enable this subscription so that you can inspect whether a package was pushed intentional or not.
+                            Unsubscribing means no notification will be sent on push.
+                        </p>
+                    </div>
+                    <input type="submit" value="Save" title="Save changes to email notifications" />
                 </fieldset>
             }
-        </text>,
-        content: @<text>
-            <p>
-                You are <em>@(CurrentUser.EmailAllowed ? "" : "not")</em> subscribed to email notifications.
-                @if (!CurrentUser.EmailAllowed)
-                {
-                    @: However, we will still send important account management and security notices.
-                }
-            </p>
+
             <p>
-                <strong>Important:</strong> Subscribing allows other registered users of the site to contact you
-                about packages that you own using the <em>Contact Owners</em> form, or to request that you become an owner
-                of their package. Unsubscribing means users cannot contact you for these reasons.
+                <br /> <strong>Note:</strong> We will <em>always</em> send important account management and security notices.
             </p>
-        </text>)
+            </text>)
 
     @**************************
         Profile Picture

tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs

@@ -123,13 +123,13 @@ public async Task UpdatesEmailAllowedSetting()
                 var controller = GetController<UsersController>();
                 controller.SetCurrentUser(user);
                 GetMock<IUserService>()
-                    .Setup(u => u.ChangeEmailSubscriptionAsync(user, false))
+                    .Setup(u => u.ChangeEmailSubscriptionAsync(user, false, true))
                     .Returns(Task.CompletedTask);
 
-                var result = await controller.ChangeEmailSubscription(false);
+                var result = await controller.ChangeEmailSubscription(false, true);
 
                 ResultAssert.IsRedirectToRoute(result, new { action = "Account" });
-                GetMock<IUserService>().Verify(u => u.ChangeEmailSubscriptionAsync(user, false));
+                GetMock<IUserService>().Verify(u => u.ChangeEmailSubscriptionAsync(user, false, true));
             }
         }
 

tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs

@@ -512,8 +512,8 @@ public void WillSendEmailToAllOwners()
                     Id = "smangit",
                     Owners = new[]
                     {
-                        new User { EmailAddress = "yung@example.com", EmailAllowed = true },
-                        new User { EmailAddress = "flynt@example.com", EmailAllowed = true }
+                        new User { EmailAddress = "yung@example.com", NotifyPackagePushed = true },
+                        new User { EmailAddress = "flynt@example.com", NotifyPackagePushed = true }
                     }
                 };
                 var package = new Package
@@ -547,8 +547,8 @@ public void WillNotSendEmailToOwnerThatOptsOut()
                     Id = "smangit",
                     Owners = new[]
                     {
-                        new User { EmailAddress = "yung@example.com", EmailAllowed = true },
-                        new User { EmailAddress = "flynt@example.com", EmailAllowed = false }
+                        new User { EmailAddress = "yung@example.com", NotifyPackagePushed = true },
+                        new User { EmailAddress = "flynt@example.com", NotifyPackagePushed = false }
                     }
                 };
                 var package = new Package

tests/NuGetGallery.Facts/Services/UserServiceFacts.cs

@@ -363,17 +363,44 @@ public async Task WritesAuditRecord()
         public class TheUpdateProfileMethod
         {
             [Fact]
-            public async Task SavesEmailAllowedSetting()
+            public async Task SavesEmailSettings()
             {
-                var user = new User { EmailAddress = "old@example.org", EmailAllowed = true };
+                var user = new User { EmailAddress = "old@example.org", EmailAllowed = true, NotifyPackagePushed = true};
                 var service = new TestableUserService();
                 service.MockUserRepository
                        .Setup(r => r.GetAll())
                        .Returns(new[] { user }.AsQueryable());
+
+                // Disable notifications
+                await service.ChangeEmailSubscriptionAsync(user, false, false);
+                Assert.Equal(false, user.EmailAllowed);
+                Assert.Equal(false, user.NotifyPackagePushed);
+
+                // Enable contact notifications
+                await service.ChangeEmailSubscriptionAsync(user, true, false);
+                Assert.Equal(true, user.EmailAllowed);
+                Assert.Equal(false, user.NotifyPackagePushed);
+
+                // Disable notifications
+                await service.ChangeEmailSubscriptionAsync(user, false, false);
+                Assert.Equal(false, user.EmailAllowed);
+                Assert.Equal(false, user.NotifyPackagePushed);
 
-                await service.ChangeEmailSubscriptionAsync(user, false);
+                // Enable package pushed notifications
+                await service.ChangeEmailSubscriptionAsync(user, false, true);
+                Assert.Equal(false, user.EmailAllowed);
+                Assert.Equal(true, user.NotifyPackagePushed);
 
+                // Disable notifications
+                await service.ChangeEmailSubscriptionAsync(user, false, false);
                 Assert.Equal(false, user.EmailAllowed);
+                Assert.Equal(false, user.NotifyPackagePushed);
+
+                // Enable all notifications
+                await service.ChangeEmailSubscriptionAsync(user, true, true);
+                Assert.Equal(true, user.EmailAllowed);
+                Assert.Equal(true, user.NotifyPackagePushed);
+
                 service.MockUserRepository
                        .Verify(r => r.CommitChangesAsync());
             }
@@ -383,7 +410,7 @@ public async Task ThrowsArgumentExceptionForNullUser()
             {
                 var service = new TestableUserService();
 
-                await ContractAssert.ThrowsArgNullAsync(async () => await service.ChangeEmailSubscriptionAsync(null, emailAllowed: true), "user");
+                await ContractAssert.ThrowsArgNullAsync(async () => await service.ChangeEmailSubscriptionAsync(null, emailAllowed: true, notifyPackagePushed: true), "user");
             }
         }