From 193f8301a48b3ef57ce4b7d94484008ea92f6840 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 20 Dec 2016 14:26:15 +0800 Subject: [PATCH] FIX: Do not show automatic groups to normal users. --- app/controllers/groups_controller.rb | 11 ++++++---- spec/integration/groups_spec.rb | 32 ++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index dd0265ad5ff..67b3de9938b 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -14,10 +14,13 @@ class GroupsController < ApplicationController page_size = 30 page = params[:page]&.to_i || 0 - groups = Group.order(user_count: :desc, name: :asc) - .where(visible: true) - .offset(page * page_size) - .limit(page_size) + groups = Group.order(user_count: :desc, name: :asc).where(visible: true) + + if !guardian.is_admin? + groups = groups.where(automatic: false) + end + + groups = groups.offset(page * page_size).limit(page_size) render json: { groups: serialize_data(groups, BasicGroupSerializer), diff --git a/spec/integration/groups_spec.rb b/spec/integration/groups_spec.rb index ddd273d39c0..348f69f90e9 100644 --- a/spec/integration/groups_spec.rb +++ b/spec/integration/groups_spec.rb @@ -5,10 +5,16 @@ describe "Groups" do let(:group) { Fabricate(:group, users: [user]) } describe 'viewing groups' do - it 'should return the right response' do - group.update_attributes!(visible: true) - other_group = Fabricate(:group, name: '0000', visible: true) + let(:other_group) do + Fabricate(:group, name: '0000', visible: true, automatic: false) + end + before do + other_group + group.update_attributes!(automatic: true, visible: true) + end + + it 'should return the right response' do get "/groups.json" expect(response).to be_success @@ -17,9 +23,27 @@ describe "Groups" do group_ids = response_body["groups"].map { |g| g["id"] } - expect(group_ids).to include(group.id, other_group.id) + expect(group_ids).to include(other_group.id) + expect(group_ids).to_not include(group.id) expect(response_body["load_more_groups"]).to eq("/groups?page=1") end + + context 'viewing as an admin' do + it 'should display automatic groups' do + sign_in(Fabricate(:admin)) + + get "/groups.json" + + expect(response).to be_success + + response_body = JSON.parse(response.body) + + group_ids = response_body["groups"].map { |g| g["id"] } + + expect(group_ids).to include(group.id, other_group.id) + expect(response_body["load_more_groups"]).to eq("/groups?page=1") + end + end end describe "checking if a group can be mentioned" do