PERF: Avoid looking up the same group multiple times during bulk invite.

PERF: Avoid looking up the same group multiple times during bulk invite.

Also cache the guardian check because it does a query to check if the user is an owner of the group.

diff --git a/app/jobs/regular/bulk_invite.rb b/app/jobs/regular/bulk_invite.rb
index 61d2875..cd76fed 100644
--- a/app/jobs/regular/bulk_invite.rb
+++ b/app/jobs/regular/bulk_invite.rb
@@ -11,6 +11,8 @@ module Jobs
       @logs    = []
       @sent    = 0
       @failed  = 0
+      @groups = {}
+      @valid_groups = {}
     end
 
     def execute(args)
@@ -19,6 +21,7 @@ module Jobs
 
       @current_user = User.find_by(id: args[:current_user_id])
       raise Discourse::InvalidParameters.new(:current_user_id) unless @current_user
+      @guardian = Guardian.new(@current_user)
 
       csv_path = "#{Invite.base_directory}/#{filename}"
 
@@ -60,14 +63,13 @@ module Jobs
 
       if group_names
         group_names = group_names.split(';')
-        guardian = Guardian.new(@current_user)
 
         group_names.each { |group_name|
-          group_detail = Group.find_by_name(group_name)
+          group = fetch_group(group_name)
 
-          if group_detail && guardian.can_edit_group?(group_detail)
+          if group && can_edit_group?(group)
             # valid group
-            groups.push(group_detail)
+            groups.push(group)
           else
             # invalid group
             save_log "Invalid Group '#{group_name}' at line number '#{csv_line_number}'"
@@ -145,6 +147,30 @@ module Jobs
       end
     end
 
+    def fetch_group(group_name)
+      group_name = group_name.downcase
+      group = @groups[group_name]
+
+      unless group
+        group = Group.find_by("lower(name) = ?", group_name)
+        @groups[group_name] = group
+      end
+
+      group
+    end
+
+    def can_edit_group?(group)
+      group_name = group.name.downcase
+      result = @valid_groups[group_name]
+
+      unless result
+        result = @guardian.can_edit_group?(group)
+        @valid_groups[group_name] = result
+      end
+
+      result
+    end
+
   end
 
 end
diff --git a/lib/guardian/group_guardian.rb b/lib/guardian/group_guardian.rb
index 492924c..a9ad974 100644
--- a/lib/guardian/group_guardian.rb
+++ b/lib/guardian/group_guardian.rb
@@ -5,7 +5,7 @@ module GroupGuardian
   # Automatic groups are not represented in the GROUP_USERS
   # table and thus do not allow membership changes.
   def can_edit_group?(group)
-    can_log_group_changes?(group) && !group.automatic
+    !group.automatic && can_log_group_changes?(group)
   end
 
   def can_log_group_changes?(group)
diff --git a/spec/fixtures/csv/bulk_invite.csv b/spec/fixtures/csv/bulk_invite.csv
index ea2d897..9d0d3b0 100644
--- a/spec/fixtures/csv/bulk_invite.csv
+++ b/spec/fixtures/csv/bulk_invite.csv
@@ -1,2 +1,2 @@
 test2@discourse.org
-test@discourse.org,group1;group2,999
+test@discourse.org,GROUP1;group2,999

GitHub sha: ce4c8e95

1 Like