Merge pull request #5126 from dodona-edu/fix/no-reply

Do not send emails to github noreply

app/models/repository.rb

@@ -117,8 +117,13 @@ def process_activities_email_errors_delayed(kwargs = {})
   end
 
   def process_activities_email_errors(kwargs = {})
-    kwargs[:user] = admins.first if kwargs.empty? && admins.any?
-    kwargs[:email] = Rails.application.config.dodona_email if kwargs.empty?
+    recipient_is_invalid = kwargs.empty? || kwargs[:email]&.end_with?('@users.noreply.github.com')
+
+    if recipient_is_invalid && admins.any?
+      kwargs[:user] = admins.first
+    elsif recipient_is_invalid
+      kwargs[:email] = Rails.application.config.dodona_email
+    end
 
     process_activities
   rescue AggregatedConfigErrors => e

test/models/repository_test.rb

@@ -374,10 +374,47 @@ def teardown
     @remote.copy_dir(@echo.path, new_dir)
     @remote.commit('copy exercise')
 
+    @repository.reset
+    assert_difference 'ActionMailer::Base.deliveries.size', +1 do
+      @repository.process_activities_email_errors(email: '[email protected]')
+    end
+
+    assert_equal '[email protected]', ActionMailer::Base.deliveries.last.to[0]
+  end
+
+  test 'should not send a mail to github noreply' do
+    # make sure commit fails
+    @repository.stubs(:commit).returns([false, ['commit fail']])
+
+    # add an activity to make sure that commit will be executed inside @repository.process_activities
+    new_dir = 'echo2'
+    @remote.copy_dir(@echo.path, new_dir)
+    @remote.commit('copy exercise')
+
+    @repository.reset
+    assert_difference 'ActionMailer::Base.deliveries.size', +1 do
+      @repository.process_activities_email_errors(email: '[email protected]')
+    end
+
+    assert_equal '[email protected]', ActionMailer::Base.deliveries.last.to[0]
+  end
+
+  test 'should email first admin if present' do
+    # make sure commit fails
+    @repository.stubs(:commit).returns([false, ['commit fail']])
+    @repository.admins << create(:staff, email: '[email protected]')
+
+    # add an activity to make sure that commit will be executed inside @repository.process_activities
+    new_dir = 'echo2'
+    @remote.copy_dir(@echo.path, new_dir)
+    @remote.commit('copy exercise')
+
     @repository.reset
     assert_difference 'ActionMailer::Base.deliveries.size', +1 do
       @repository.process_activities_email_errors
     end
+
+    assert_equal '[email protected]', ActionMailer::Base.deliveries.last.to[0]
   end
 
   test 'has allowed course should filter correctly' do