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.

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

Add link to package in Contact Owners email #5202

Merged
merged 2 commits into from
Dec 19, 2017
Merged
Changes from 1 commit
Commits
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
Next Next commit
add url to package in contact owners email
Scott Bommarito committed Dec 18, 2017
commit 5f5ef20e25bd0d94b9213eb6319f2db4e1fe4149
1 change: 1 addition & 0 deletions src/NuGetGallery/Controllers/PackagesController.cs
Original file line number Diff line number Diff line change
@@ -948,6 +948,7 @@ public virtual ActionResult ContactOwners(string id, ContactOwnersViewModel cont
_messageService.SendContactOwnersMessage(
fromAddress,
package,
Url.Package(package, false),
contactForm.Message,
Url.AccountSettings(relativeUrl: false),
contactForm.CopySender);
2 changes: 1 addition & 1 deletion src/NuGetGallery/Services/IMessageService.cs
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ namespace NuGetGallery
{
public interface IMessageService
{
void SendContactOwnersMessage(MailAddress fromAddress, PackageRegistration packageRegistration, string message, string emailSettingsUrl, bool copyFromAddress);
void SendContactOwnersMessage(MailAddress fromAddress, PackageRegistration packageRegistration, string packageUrl, string message, string emailSettingsUrl, bool copyFromAddress);
void ReportAbuse(ReportPackageRequest report);
void ReportMyPackage(ReportPackageRequest report);
void SendNewAccountEmail(MailAddress toAddress, string confirmationUrl);
11 changes: 6 additions & 5 deletions src/NuGetGallery/Services/MessageService.cs
Original file line number Diff line number Diff line change
@@ -129,17 +129,17 @@ public void ReportMyPackage(ReportPackageRequest request)
}
}

public void SendContactOwnersMessage(MailAddress fromAddress, PackageRegistration packageRegistration, string message, string emailSettingsUrl, bool copySender)
public void SendContactOwnersMessage(MailAddress fromAddress, PackageRegistration packageRegistration, string packageUrl, string message, string emailSettingsUrl, bool copySender)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: may be format the parameters on new line?

{
string subject = "[{0}] Message for owners of the package '{1}'";
string body = @"_User {0} <{1}> sends the following message to the owners of Package '{2}'._
string body = @"_User {0} <{1}> sends the following message to the owners of Package '[{2}]({3})'._

{3}
{4}

-----------------------------------------------
<em style=""font-size: 0.8em;"">
To stop receiving contact emails as an owner of this package, sign in to the {4} and
[change your email notification settings]({5}).
To stop receiving contact emails as an owner of this package, sign in to the {5} and
[change your email notification settings]({6}).
</em>";

body = String.Format(
@@ -148,6 +148,7 @@ [change your email notification settings]({5}).
fromAddress.DisplayName,
fromAddress.Address,
packageRegistration.Id,
packageUrl,
message,
Config.GalleryOwner.DisplayName,
emailSettingsUrl);
Original file line number Diff line number Diff line change
@@ -1235,8 +1235,9 @@ public void HtmlEncodesMessageContent()
It.IsAny<PackageRegistration>(),
It.IsAny<string>(),
It.IsAny<string>(),
It.IsAny<string>(),
false))
.Callback<MailAddress, PackageRegistration, string, string, bool>((_, __, msg, ___, ____) => sentMessage = msg);
.Callback<MailAddress, PackageRegistration, string, string, string, bool>((_, __, ___, msg, ____, _____) => sentMessage = msg);
var package = new PackageRegistration { Id = "factory" };

var packageService = new Mock<IPackageService>();
@@ -1265,6 +1266,7 @@ public void CallsSendContactOwnersMessageWithUserInfo()
s => s.SendContactOwnersMessage(
It.IsAny<MailAddress>(),
It.IsAny<PackageRegistration>(),
It.IsAny<string>(),
"I like the cut of your jib",
It.IsAny<string>(), false));
var package = new PackageRegistration { Id = "factory" };
34 changes: 20 additions & 14 deletions tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs
Original file line number Diff line number Diff line change
@@ -208,7 +208,7 @@ public void WillCopySenderIfAsked()
};
var messageService = TestableMessageService.Create(GetConfigurationService());

messageService.SendContactOwnersMessage(from, package, "Test message", "http://someurl/", true);
messageService.SendContactOwnersMessage(from, package, "http://someurl/", "Test message", "http://someotherurl/", true);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe distinguish the urls, like http://package-url/ and http://email-settings-url?

Should we have test to validate correct package url included?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


var messages = messageService.MockMailSender.Sent;
Assert.Equal(2, messages.Count);
@@ -226,30 +226,36 @@ public void WillCopySenderIfAsked()
[Fact]
public void WillSendEmailToAllOwners()
{
var from = new MailAddress("[email protected]", "flossy");
var id = "smangit";
var owner1Email = "[email protected]";
var owner2Email = "[email protected]";
var userEmail = "[email protected]";
var from = new MailAddress(userEmail, "flossy");
var package = new PackageRegistration
{
Id = "smangit",
Id = id,
Owners = new[]
{
new User { EmailAddress = "[email protected]", EmailAllowed = true },
new User { EmailAddress = "[email protected]", EmailAllowed = true }
new User { EmailAddress = owner1Email, EmailAllowed = true },
new User { EmailAddress = owner2Email, EmailAllowed = true }
}
};

var messageService = TestableMessageService.Create(GetConfigurationService());

messageService.SendContactOwnersMessage(from, package, "Test message", "http://someurl/", false);
var packageUrl = "http://someurl/";
messageService.SendContactOwnersMessage(from, package, packageUrl, "Test message", "http://someotherurl/", false);
var message = messageService.MockMailSender.Sent.Last();

Assert.Equal("[email protected]", message.To[0].Address);
Assert.Equal("[email protected]", message.To[1].Address);
Assert.Equal(owner1Email, message.To[0].Address);
Assert.Equal(owner2Email, message.To[1].Address);
Assert.Equal(TestGalleryOwner, message.From);
Assert.Equal("[email protected]", message.ReplyToList.Single().Address);
Assert.Contains("[Joe Shmoe] Message for owners of the package 'smangit'", message.Subject);
Assert.Equal(userEmail, message.ReplyToList.Single().Address);
Assert.Contains($"[Joe Shmoe] Message for owners of the package '{id}'", message.Subject);
Assert.Contains("Test message", message.Body);
Assert.Contains(
"User flossy &lt;[email protected]&gt; sends the following message to the owners of Package 'smangit'.", message.Body);
$"User flossy &lt;{userEmail}&gt; sends the following message to the owners of Package '[{id}]({packageUrl})'.",
message.Body);
}

[Fact]
@@ -268,7 +274,7 @@ public void WillNotSendEmailToOwnerThatOptsOut()

var messageService = TestableMessageService.Create(GetConfigurationService());

messageService.SendContactOwnersMessage(from, package, "Test message", "http://someurl/", false);
messageService.SendContactOwnersMessage(from, package, "http://someurl/", "Test message", "http://someotherurl/", false);
var message = messageService.MockMailSender.Sent.Last();

Assert.Equal("[email protected]", message.To[0].Address);
@@ -290,7 +296,7 @@ public void WillNotAttemptToSendIfNoOwnersAllow()
};

var messageService = TestableMessageService.Create(GetConfigurationService());
messageService.SendContactOwnersMessage(from, package, "Test message", "http://someurl/", false);
messageService.SendContactOwnersMessage(from, package, "http://someurl/", "Test message", "http://someotherurl/", false);

Assert.Empty(messageService.MockMailSender.Sent);
}
@@ -307,7 +313,7 @@ public void WillNotCopySenderIfNoOwnersAllow()
};

var messageService = TestableMessageService.Create(GetConfigurationService());
messageService.SendContactOwnersMessage(from, package, "Test message", "http://someurl/", false);
messageService.SendContactOwnersMessage(from, package, "http://someurl/", "Test message", "http://someotherurl/", false);

Assert.Empty(messageService.MockMailSender.Sent);
}