Merge branch 'issue-893' of https://github.com/McFoggy/gitbucket into McFoggy-issue-893

src/main/scala/gitbucket/core/controller/SystemSettingsController.scala

@@ -24,7 +24,8 @@ trait SystemSettingsControllerBase extends ControllerBase {
     "activityLogLimit"         -> trim(label("Limit of activity logs", optional(number()))),
     "ssh"                      -> trim(label("SSH access", boolean())),
     "sshPort"                  -> trim(label("SSH port", optional(number()))),
-    "smtp"                     -> optionalIfNotChecked("notification", mapping(
+    "useSMTP"                 -> trim(label("SMTP", boolean())),
+    "smtp"                     -> optionalIfNotChecked("useSMTP", mapping(
         "host"                     -> trim(label("SMTP Host", text(required))),
         "port"                     -> trim(label("SMTP Port", optional(number()))),
         "user"                     -> trim(label("SMTP User", optional(text()))),

src/main/scala/gitbucket/core/service/SystemSettingsService.scala

@@ -22,7 +22,8 @@ trait SystemSettingsService {
       settings.activityLogLimit.foreach(x => props.setProperty(ActivityLogLimit, x.toString))
       props.setProperty(Ssh, settings.ssh.toString)
       settings.sshPort.foreach(x => props.setProperty(SshPort, x.toString))
-      if(settings.notification) {
+      props.setProperty(UseSMTP, settings.useSMTP.toString)
+      if(settings.useSMTP) {
         settings.smtp.foreach { smtp =>
           props.setProperty(SmtpHost, smtp.host)
           smtp.port.foreach(x => props.setProperty(SmtpPort, x.toString))
@@ -75,7 +76,8 @@ trait SystemSettingsService {
         getOptionValue[Int](props, ActivityLogLimit, None),
         getValue(props, Ssh, false),
         getOptionValue(props, SshPort, Some(DefaultSshPort)),
-        if(getValue(props, Notification, false)){
+        getValue(props, UseSMTP, getValue(props, Notification, false)),   // handle migration scenario from only notification to useSMTP
+        if(getValue(props, UseSMTP, getValue(props, Notification, false))){
           Some(Smtp(
             getValue(props, SmtpHost, ""),
             getOptionValue(props, SmtpPort, Some(DefaultSmtpPort)),
@@ -125,6 +127,7 @@ object SystemSettingsService {
     activityLogLimit: Option[Int],
     ssh: Boolean,
     sshPort: Option[Int],
+    useSMTP: Boolean,
     smtp: Option[Smtp],
     ldapAuthentication: Boolean,
     ldap: Option[Ldap]){
@@ -172,6 +175,7 @@ object SystemSettingsService {
   private val ActivityLogLimit = "activity_log_limit"
   private val Ssh = "ssh"
   private val SshPort = "ssh.port"
+  private val UseSMTP = "useSMTP"
   private val SmtpHost = "smtp.host"
   private val SmtpPort = "smtp.port"
   private val SmtpUser = "smtp.user"

src/main/scala/gitbucket/core/util/Notifier.scala

@@ -37,7 +37,7 @@ trait Notifier extends RepositoryService with AccountService with IssuesService
 object Notifier {
   // TODO We want to be able to switch to mock.
   def apply(): Notifier = new SystemSettingsService {}.loadSystemSettings match {
-    case settings if settings.notification => new Mailer(settings.smtp.get)
+    case settings if (settings.notification && settings.useSMTP) => new Mailer(settings.smtp.get)
     case _ => new MockMailer
   }
 

src/main/twirl/gitbucket/core/admin/system.scala.html

@@ -224,14 +224,25 @@
           <!-- Notification email -->
           <!--====================================================================-->
           <hr>
-          <label class="strong">Notification email</label>
+          <label class="strong">Notifications</label>
           <fieldset>
             <label class="checkbox">
               <input type="checkbox" id="notification" name="notification"@if(settings.notification){ checked}/>
               Send notifications
             </label>
           </fieldset>
-          <div class="form-horizontal notification">
+          <!--====================================================================-->
+          <!-- Communication email -->
+          <!--====================================================================-->
+          <hr>
+          <label class="strong">Communication</label>
+          <fieldset>
+            <label class="checkbox">
+              <input type="checkbox" id="useSMTP" name="useSMTP" @if(settings.useSMTP){ checked}/>
+              use SMTP
+            </label>
+          </fieldset>
+          <div class="form-horizontal useSMTP">
             <div class="control-group">
               <label class="control-label" for="smtpHost">SMTP Host</label>
               <div class="controls">
@@ -292,8 +303,16 @@ $(function(){
     $('.ssh input').prop('disabled', !$(this).prop('checked'));
   }).change();
 
-  $('#notification').change(function(){
+  $('#useSMTP').change(function(){
-    $('.notification input').prop('disabled', !$(this).prop('checked'));
+    $('.useSMTP input').prop('disabled', !$(this).prop('checked'));
+
+    // With only SMTP in current version, notification cannot be enabled if no communication channel exists
+    $('#notification').prop('disabled', !$(this).prop('checked'));
+
+    if (!$(this).prop('checked')) {
+      // With only SMTP in current version, if SMTP is unchecked then we disable notification
+      $('#notification').prop('checked', false);
+    }
   }).change();
 
   $('#ldapAuthentication').change(function(){

src/test/scala/gitbucket/core/view/AvatarImageProviderSpec.scala

@@ -103,6 +103,7 @@ class AvatarImageProviderSpec extends Specification with Mockito {
       activityLogLimit         = None,
       ssh                      = false,
       sshPort                  = None,
+      useSMTP                  = false,
       smtp                     = None,
       ldapAuthentication       = false,
       ldap                     = None)