diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb index 031e231..842b318 100644 --- a/app/controllers/discourse_assign/assign_controller.rb +++ b/app/controllers/discourse_assign/assign_controller.rb @@ -125,7 +125,7 @@ module DiscourseAssign guardian.ensure_can_see_group_members!(group) - members = + users_with_assignments_count = User .joins("LEFT OUTER JOIN group_users g ON g.user_id = users.id") .joins( @@ -140,25 +140,22 @@ module DiscourseAssign .limit(limit) .offset(offset) - members = members.where(<<~SQL, pattern: "%#{params[:filter]}%") if params[:filter] + users_with_assignments_count = + users_with_assignments_count.where(<<~SQL, pattern: "%#{params[:filter]}%") if params[ users.name ILIKE :pattern OR users.username_lower ILIKE :pattern SQL - - group_assignments = - Topic - .joins("JOIN assignments a ON a.topic_id = topics.id") - .where(<<~SQL, group_id: group.id) - a.assigned_to_id = :group_id AND a.assigned_to_type = 'Group' AND a.active - SQL - .pluck(:topic_id) - - assignments = - TopicQuery.new(current_user).group_topics_assigned_results(group).pluck("topics.id") + :filter + ] + group_assignments_count = Assignment.active_for_group(group).count + users_assignments_count = + users_with_assignments_count.reduce(0) do |sum, assignment| + sum + assignment.assignments_count + end render json: { - members: serialize_data(members, GroupUserAssignedSerializer), - assignment_count: (assignments | group_assignments).count, - group_assignment_count: group_assignments.count, + members: serialize_data(users_with_assignments_count, GroupUserAssignedSerializer), + assignment_count: users_assignments_count + group_assignments_count, + group_assignment_count: group_assignments_count, } end diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 2a614db..8d0b897 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -15,6 +15,8 @@ class Assignment < ActiveRecord::Base ) } + scope :active_for_group, ->(group) { where(assigned_to: group, active: true) } + before_validation :default_status validate :validate_status, if: -> { SiteSetting.enable_assign_status } diff --git a/spec/fabricators/assignment_fabricator.rb b/spec/fabricators/assignment_fabricator.rb new file mode 100644 index 0000000..5a3e6a4 --- /dev/null +++ b/spec/fabricators/assignment_fabricator.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +Fabricator(:topic_assignment, class_name: :assignment) do + topic + target { |attrs| attrs[:topic] } + target_type "Topic" + assigned_by_user { Fabricate(:user) } + assigned_to { Fabricate(:user) } +end + +Fabricator(:post_assignment, class_name: :assignment) do + topic { |attrs| attrs[:post]&.topic || Fabricate(:topic) } + target { |attrs| attrs[:post] || Fabricate(:post, topic: attrs[:topic]) } + target_type "Post" + assigned_by_user { Fabricate(:user) } + assigned_to { Fabricate(:user) } +end diff --git a/spec/models/assignment_spec.rb b/spec/models/assignment_spec.rb new file mode 100644 index 0000000..a223aa6 --- /dev/null +++ b/spec/models/assignment_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe Assignment do + fab!(:group) { Fabricate(:group) } + fab!(:user1) { Fabricate(:user) } + fab!(:user2) { Fabricate(:user) } + fab!(:group_user1) { Fabricate(:group_user, user: user1, group: group) } + fab!(:group_user1) { Fabricate(:group_user, user: user2, group: group) } + + fab!(:wrong_group) { Fabricate(:group) } + + before { SiteSetting.assign_enabled = true } + + describe "#active_for_group" do + it "returns active assignments for the group" do + assignment1 = Fabricate(:topic_assignment, assigned_to: group) + assignment2 = Fabricate(:post_assignment, assigned_to: group) + Fabricate(:post_assignment, assigned_to: group, active: false) + Fabricate(:post_assignment, assigned_to: user1) + Fabricate(:topic_assignment, assigned_to: wrong_group) + + expect(Assignment.active_for_group(group)).to contain_exactly(assignment1, assignment2) + end + end +end diff --git a/spec/requests/assign_controller_spec.rb b/spec/requests/assign_controller_spec.rb index 7aeefc4..c93228b 100644 --- a/spec/requests/assign_controller_spec.rb +++ b/spec/requests/assign_controller_spec.rb @@ -364,56 +364,98 @@ RSpec.describe DiscourseAssign::AssignController do describe "#group_members" do include_context "with group that is allowed to assign" - fab!(:post1) { Fabricate(:post) } - fab!(:post2) { Fabricate(:post) } - fab!(:post3) { Fabricate(:post) } - before do add_to_assign_allowed_group(user2) add_to_assign_allowed_group(user) - Assigner.new(post1.topic, user).assign(user) - Assigner.new(post2.topic, user).assign(user2) - Assigner.new(post3.topic, user).assign(user) + topic = Fabricate(:topic) + post_in_same_topic = Fabricate(:post, topic: topic) + + Fabricate(:post_assignment, assigned_to: user, target: topic, assigned_by_user: user) + Fabricate( + :topic_assignment, + assigned_to: user, + target: post_in_same_topic, + assigned_by_user: user, + ) + Fabricate(:topic_assignment, assigned_to: user2, assigned_by_user: user) + Fabricate(:topic_assignment, assigned_to: user, assigned_by_user: user) + + Fabricate(:topic_assignment, assigned_to: assign_allowed_group, assigned_by_user: user) + Fabricate(:post_assignment, assigned_to: assign_allowed_group, assigned_by_user: user) end - it "list members order by assignments_count" do - sign_in(user) + describe "members" do + describe "without filter" do + it "list members ordered by the number of assignments" do + sign_in(user) - get "/assign/members/#{get_assigned_allowed_group_name}.json" - expect(response.status).to eq(200) - expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to match_array( - [user.id, user2.id], - ) + get "/assign/members/#{get_assigned_allowed_group_name}.json" + members = JSON.parse(response.body)["members"] + + expect(response.status).to eq(200) + expect(members[0]).to include({ "id" => user.id, "assignments_count" => 3 }) + expect(members[1]).to include({ "id" => user2.id, "assignments_count" => 1 }) + end + + it "doesn't include members with no assignments" do + sign_in(user) + add_to_assign_allowed_group(non_admin) + + get "/assign/members/#{get_assigned_allowed_group_name}.json" + expect(response.status).to eq(200) + expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to_not include( + non_admin.id, + ) + end + end + + describe "with filter" do + it "returns members as according to filter" do + sign_in(user) + + get "/assign/members/#{get_assigned_allowed_group_name}.json", params: { filter: "a" } + expect(response.status).to eq(200) + expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to match_array( + [user.id, user2.id], + ) + + get "/assign/members/#{get_assigned_allowed_group_name}.json", params: { filter: "david" } + expect(response.status).to eq(200) + expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to match_array( + [user2.id], + ) + + get "/assign/members/#{get_assigned_allowed_group_name}.json", + params: { + filter: "Taylor", + } + expect(response.status).to eq(200) + expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to match_array( + [user2.id], + ) + end + end end - it "doesn't include members with no assignments" do - sign_in(user) - add_to_assign_allowed_group(non_admin) + describe "assignment_count" do + it "returns the total number of assignments for group users and the group" do + sign_in(user) - get "/assign/members/#{get_assigned_allowed_group_name}.json" - expect(response.status).to eq(200) - expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to match_array( - [user.id, user2.id], - ) + get "/assign/members/#{get_assigned_allowed_group_name}.json" + + expect(JSON.parse(response.body)["assignment_count"]).to eq(6) + end end - it "returns members as according to filter" do - sign_in(user) + describe "group_assignment_count" do + it "returns the number of assignments assigned to the group" do + sign_in(user) - get "/assign/members/#{get_assigned_allowed_group_name}.json", params: { filter: "a" } - expect(response.status).to eq(200) - expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to match_array( - [user.id, user2.id], - ) + get "/assign/members/#{get_assigned_allowed_group_name}.json" - get "/assign/members/#{get_assigned_allowed_group_name}.json", params: { filter: "david" } - expect(response.status).to eq(200) - expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to match_array([user2.id]) - - get "/assign/members/#{get_assigned_allowed_group_name}.json", params: { filter: "Taylor" } - expect(response.status).to eq(200) - expect(JSON.parse(response.body)["members"].map { |m| m["id"] }).to match_array([user2.id]) + expect(JSON.parse(response.body)["group_assignment_count"]).to eq(2) + end end it "404 error to non-group-members" do