From 1a620cb01fbebcc9273ad9025d0ce3cb1e78c6b3 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Thu, 20 May 2021 15:28:36 +1000
Subject: [PATCH] FEATURE: allow for notification of up to 20 group owners
 (#13081)

The 5 limit appears to be too low. Limiting to 20 group owners, though high
seems like a fairer limit.

Also... spec cleanup
---
 app/controllers/groups_controller.rb    | 6 ++++--
 spec/requests/groups_controller_spec.rb | 7 +------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 9dc1c43a401..576509eaaf9 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -480,6 +480,8 @@ class GroupsController < ApplicationController
     )
   end
 
+  MAX_NOTIFIED_OWNERS ||= 20
+
   def request_membership
     params.require(:reason)
 
@@ -487,14 +489,14 @@ class GroupsController < ApplicationController
 
     begin
       GroupRequest.create!(group: group, user: current_user, reason: params[:reason])
-    rescue ActiveRecord::RecordNotUnique => e
+    rescue ActiveRecord::RecordNotUnique
       return render json: failed_json.merge(error: I18n.t("groups.errors.already_requested_membership")), status: 409
     end
 
     usernames = [current_user.username].concat(
       group.users.where('group_users.owner')
         .order("users.last_seen_at DESC")
-        .limit(5)
+        .limit(MAX_NOTIFIED_OWNERS)
         .pluck("users.username")
     )
 
diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb
index 1f35f3dad2e..817ef5c63d0 100644
--- a/spec/requests/groups_controller_spec.rb
+++ b/spec/requests/groups_controller_spec.rb
@@ -1405,10 +1405,7 @@ describe GroupsController do
       it "will invite the user if their username and email are both invited" do
         new_user = Fabricate(:user)
         put "/groups/#{group.id}/members.json", params: { usernames: new_user.username, emails: new_user.email }
-
         expect(response.status).to eq(200)
-        body = response.parsed_body
-
         expect(new_user.reload.group_ids.include?(group.id)).to eq(true)
       end
 
@@ -1471,8 +1468,6 @@ describe GroupsController do
 
       it "raises an error if user to be removed is not found" do
         delete "/groups/#{group.id}/members.json", params: { user_id: -10 }
-
-        response_body = response.parsed_body
         expect(response.status).to eq(400)
       end
 
@@ -1627,7 +1622,7 @@ describe GroupsController do
     end
 
     it "sends a private message when accepted" do
-      group_request = GroupRequest.create!(group: group, user: other_user)
+      GroupRequest.create!(group: group, user: other_user)
       expect { put "/groups/#{group.id}/handle_membership_request.json", params: { user_id: other_user.id, accept: true } }
         .to change { Topic.count }.by(1)
         .and change { Post.count }.by(1)