cap users in groups json

closes FOO-1436 flag=none TEST PLAN: 1) make a giant group with more memberships than your setting value 2) ask for the groups index with "include[]=users" 3) you only get the first N users in each group 4) you can still paginate through group memberships from the memberships endpoint successfully Change-Id: I211c48d7adcc444b9d9e05d22b38131add66be41 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/256219 Tested-by: Service Cloud Jenkins <[email protected]> Reviewed-by: Rob Orton <[email protected]> QA-Review: Ethan Vizitei <[email protected]> Product-Review: Ethan Vizitei <[email protected]>
ralacas · Jan 7, 2021 · 524148d · 524148d
1 parent e70901c
commit 524148d
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 2 deletions.
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
@@ -129,6 +129,11 @@
 #           "type": "object",
 #           "key": { "type": "string" },
 #           "value": { "type": "boolean" }
+#         },
+#         "users": {
+#           "description": "optional: A list of users that are members in the group. Returned only if include[]=users. WARNING: this collection's size is capped (if there are an extremely large number of users in the group (thousands) not all of them will be returned).  If you need to capture all the users in a group with certainty consider using the paginated /api/v1/groups/<group_id>/memberships endpoint.",
+#           "type": "array",
+#           "items": { "$ref": "User" }
 #         }
 #       }
 #     }

diff --git a/lib/api/v1/group.rb b/lib/api/v1/group.rb
@@ -49,9 +49,10 @@ def group_json(group, user, session, options = {})
     hash['leader'] = group.leader ? user_display_json(group.leader, group) : nil
 
     if includes.include?('users')
+      group_member_json_limit = Setting.get("group_json_user_cap", "1000").to_i
       users = group.grants_right?(@current_user, :read_as_admin) ?
-        group.users.order_by_sortable_name.distinct :
-        group.participating_users_in_context(sort: true, include_inactive_users: options[:include_inactive_users]).distinct
+        group.users.order_by_sortable_name.limit(group_member_json_limit).distinct :
+        group.participating_users_in_context(sort: true, include_inactive_users: options[:include_inactive_users]).limit(group_member_json_limit).distinct
       active_user_ids = nil
       if options[:include_inactive_users]
         active_user_ids = group.participating_users_in_context.pluck('id').to_set

diff --git a/spec/lib/api/v1/group_spec.rb b/spec/lib/api/v1/group_spec.rb
@@ -43,6 +43,16 @@
       expect(user_json["name"]).to eq(@user.name)
     end
 
+    it "caps the numer of users that will be returned" do
+      other_user = user_model
+      @group.add_user(other_user)
+      json = group_json(@group, @user, nil, :include_inactive_users => true, :include => ['users'])
+      expect(json["users"].length).to eq 2
+      Setting.set("group_json_user_cap", "1")
+      json = group_json(@group, @user, nil, :include_inactive_users => true, :include => ['users'])
+      expect(json["users"].length).to eq 1
+    end
+
     it "filter inactive users but do include users" do
       json = group_json(@group, @user, nil, :include => ['users'])
       expect(json["id"]).to eq @group.id