FEATURE: Add limit and group exclusion to the directory items endpoint (#22667)

* FEATURE: Add limit and group exclusion to the directory items endpoint
This commit is contained in:
Jean 2023-07-18 15:09:32 -04:00 committed by GitHub
parent 2a96064e6b
commit defa8904b9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 93 additions and 3 deletions

View File

@ -2,6 +2,7 @@
class DirectoryItemsController < ApplicationController
PAGE_SIZE = 50
before_action :set_groups_exclusion, if: -> { params[:exclude_groups].present? }
def index
unless SiteSetting.enable_user_directory?
@ -24,6 +25,8 @@ class DirectoryItemsController < ApplicationController
result = result.includes(user: :primary_group)
end
result = apply_exclude_groups_filter(result)
if params[:exclude_usernames]
result =
result
@ -78,8 +81,11 @@ class DirectoryItemsController < ApplicationController
end
end
limit = [params[:limit].to_i, PAGE_SIZE].min if params[:limit].to_i > 0
limit ||= PAGE_SIZE
result_count = result.count
result = result.limit(PAGE_SIZE).offset(PAGE_SIZE * page).to_a
result = result.limit(limit).offset(limit * page).to_a
more_params = params.slice(:period, :order, :asc, :group, :user_field_ids).permit!
more_params[:page] = page + 1
@ -92,8 +98,10 @@ class DirectoryItemsController < ApplicationController
# Don't show the record unless you're not in the top positions already
if (position || 10) >= 10
your_item = DirectoryItem.where(period_type: period_type, user_id: current_user.id).first
result.insert(0, your_item) if your_item
unless @users_in_exclude_groups&.include?(current_user.id)
your_item = DirectoryItem.where(period_type: period_type, user_id: current_user.id).first
result.insert(0, your_item) if your_item
end
end
end
@ -127,4 +135,17 @@ class DirectoryItemsController < ApplicationController
},
)
end
private
def set_groups_exclusion
@exclude_group_names = params[:exclude_groups].split("|")
@exclude_group_ids = Group.where(name: @exclude_group_names).pluck(:id)
@users_in_exclude_groups = GroupUser.where(group_id: @exclude_group_ids).pluck(:user_id)
end
def apply_exclude_groups_filter(result)
result = result.where.not(user_id: @users_in_exclude_groups) if params[:exclude_groups]
result
end
end

View File

@ -17,6 +17,75 @@ RSpec.describe DirectoryItemsController do
expect(response).not_to be_successful
end
context "with limit parameter" do
let!(:users) { Array.new(DirectoryItemsController::PAGE_SIZE + 10) { Fabricate(:user) } }
before { DirectoryItem.refresh! }
it "limits the number of returned items" do
get "/directory_items.json", params: { period: "all", limit: 2 }
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["directory_items"].length).to eq(2)
end
it "does not exceed PAGE_SIZE if limit parameter is more than PAGE_SIZE" do
large_limit = DirectoryItemsController::PAGE_SIZE + 10
get "/directory_items.json", params: { period: "all", limit: large_limit }
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["directory_items"].length).to eq(DirectoryItemsController::PAGE_SIZE)
end
it "handles invalid limit parameters gracefully" do
get "/directory_items.json", params: { period: "all", limit: "invalid_limit" }
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["directory_items"]).not_to be_empty
end
end
context "with exclude_groups parameter" do
before { DirectoryItem.refresh! }
it "excludes users from specified groups" do
get "/directory_items.json", params: { period: "all", exclude_groups: group.name }
expect(response.status).to eq(200)
json = response.parsed_body
usernames = json["directory_items"].map { |item| item["user"]["username"] }
expect(usernames).not_to include("eviltrout", "stage_user")
end
it "handles non-existent group names gracefully" do
get "/directory_items.json", params: { period: "all", exclude_groups: "non_existent_group" }
expect(response.status).to eq(200)
json = response.parsed_body
user_names = json["directory_items"].map { |item| item["user"]["username"] }
expect(user_names).to include("eviltrout")
end
end
context "with exclude_groups parameter and current user in the top positions" do
before do
sign_in(evil_trout)
DirectoryItem.refresh!
end
it "doesn't include current user if they are already in the top positions" do
get "/directory_items.json", params: { period: "all", exclude_groups: group.name }
expect(response.status).to eq(200)
json = response.parsed_body
usernames = json["directory_items"].map { |item| item["user"]["username"] }
expect(usernames).not_to include("eviltrout")
end
end
context "without data" do
context "with a logged in user" do
before { sign_in(user) }