-
Notifications
You must be signed in to change notification settings - Fork 644
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
Conversation
@@ -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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -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) |
There was a problem hiding this comment.
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?
#5087