FEATURE: Don't create PM for successful automatic backups

FEATURE: Don't create PM for successful automatic backups
diff --git a/lib/backup_restore/backuper.rb b/lib/backup_restore/backuper.rb
index 4fdd194..49c4252 100644
--- a/lib/backup_restore/backuper.rb
+++ b/lib/backup_restore/backuper.rb
@@ -276,18 +276,18 @@ module BackupRestore
     end
 
     def notify_user
+      return if @success && @user.id == Discourse::SYSTEM_USER_ID
+
       log "Notifying '#{@user.username}' of the end of the backup..."
       status = @success ? :backup_succeeded : :backup_failed
 
-      post = SystemMessage.create_from_system_user(@user, status,
-        logs: Discourse::Utils.pretty_logs(@logs)
+      post = SystemMessage.create_from_system_user(
+        @user, status, logs: Discourse::Utils.pretty_logs(@logs)
       )
 
-      if !@success && @user.id == Discourse::SYSTEM_USER_ID
+      if @user.id == Discourse::SYSTEM_USER_ID
         post.topic.invite_group(@user, Group[:admins])
       end
-
-      post
     rescue => ex
       log "Something went wrong while notifying user.", ex
     end

GitHub
sha: 0bc1fa8a

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/backup-growth-rate/104756/4

1 Like

We might want a test about that :wink:

I agree. Having a test would be nice, but the Backuper in its current state is nearly untestable. That’s probably why no test exists yet. It would either require heavy refactoring or lots of stubbing and/or fiddling with instance variables. If you don’t mind, I’d like to shelf this for now.

1 Like

I agree, testing for testing’s sake isn’t desirable…