From 816f3c0e97c160636ae924f6ac78fc0bb02ed655 Mon Sep 17 00:00:00 2001 From: Ahmed Gagan Date: Tue, 21 Jul 2020 17:25:34 +0530 Subject: [PATCH] FEATURE: Per-member counts in group assignment summary (#80) --- .../discourse_assign/assign_controller.rb | 24 +++++++++ .../group_user_assigned_serializer.rb | 12 +++++ .../controllers/group-assignments.js.es6 | 30 ++++++----- .../routes/group-assignments.js.es6 | 6 ++- .../group-assignments-filter.js.es6 | 8 +-- .../components/group-assignments-filter.hbs | 2 +- .../discourse/templates/group/assignments.hbs | 2 +- config/routes.rb | 1 + plugin.rb | 2 + spec/requests/assign_controller_spec.rb | 54 +++++++++++++++++++ .../group-assignments-filter-test.js.es6 | 2 - 11 files changed, 119 insertions(+), 24 deletions(-) create mode 100644 app/serializers/group_user_assigned_serializer.rb diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb index 41177cc..b9775d9 100644 --- a/app/controllers/discourse_assign/assign_controller.rb +++ b/app/controllers/discourse_assign/assign_controller.rb @@ -124,6 +124,30 @@ module DiscourseAssign render json: { topics: serialize_data(topics, AssignedTopicSerializer) } end + def group_members + limit = (params[:limit] || 50).to_i + offset = params[:offset].to_i + + raise Discourse::InvalidParameters.new(:limit) if limit < 0 || limit > 1000 + raise Discourse::InvalidParameters.new(:offset) if offset < 0 + raise Discourse::NotFound.new if !params[:group_name].present? + + group = Group.find_by(name: params[:group_name]) + + guardian.ensure_can_see_group_members!(group) + + members = User + .joins("LEFT OUTER JOIN group_users g on users.id=g.user_id LEFT OUTER JOIN user_options uo on uo.user_id=users.id LEFT OUTER JOIN topic_custom_fields tcf ON tcf.value::int = users.id") + .where("tcf.name = 'assigned_to_id' AND g.group_id=? AND (users.id > 0)", group.id) + .order('COUNT(users.id) DESC') + .group('users.id') + .select('users.*, COUNT(users.id) as "assignments_count"') + .limit(limit) + .offset(offset) + + render json: { members: serialize_data(members, GroupUserAssignedSerializer) } + end + private def translate_failure(reason, user) diff --git a/app/serializers/group_user_assigned_serializer.rb b/app/serializers/group_user_assigned_serializer.rb new file mode 100644 index 0000000..b670e5c --- /dev/null +++ b/app/serializers/group_user_assigned_serializer.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class GroupUserAssignedSerializer < BasicUserSerializer + + attributes :assignments_count, + :username_lower + + def include_assignments_count + object.can_assign? + end + +end diff --git a/assets/javascripts/discourse-assign/controllers/group-assignments.js.es6 b/assets/javascripts/discourse-assign/controllers/group-assignments.js.es6 index f77a18a..1ecbd4a 100644 --- a/assets/javascripts/discourse-assign/controllers/group-assignments.js.es6 +++ b/assets/javascripts/discourse-assign/controllers/group-assignments.js.es6 @@ -1,31 +1,33 @@ import { inject as service } from "@ember/service"; import Controller, { inject as controller } from "@ember/controller"; +import { ajax } from "discourse/lib/ajax"; export default Controller.extend({ router: service(), application: controller(), loading: false, + offset: 0, findMembers(refresh) { + if (refresh) { + this.set("members", this.model.members); + return; + } + if (this.loading || !this.model) { return; } - if (!refresh && this.model.members.length >= this.model.user_count) { - this.set("application.showFooter", true); - return; + if (this.model.members.length >= this.offset + 50) { + this.set("loading", true); + this.set("offset", this.offset + 50); + ajax(`/assign/members/${this.groupName}?offset=${this.offset}`).then( + result => { + this.members.pushObjects(result.members); + this.set("loading", false); + } + ); } - - this.set("loading", true); - this.model - .findMembers({ order: "", asc: true, filter: null }, refresh) - .finally(() => { - this.setProperties({ - "application.showFooter": - this.model.members.length >= this.model.user_count, - loading: false - }); - }); }, actions: { diff --git a/assets/javascripts/discourse-assign/routes/group-assignments.js.es6 b/assets/javascripts/discourse-assign/routes/group-assignments.js.es6 index c70a634..fd90774 100644 --- a/assets/javascripts/discourse-assign/routes/group-assignments.js.es6 +++ b/assets/javascripts/discourse-assign/routes/group-assignments.js.es6 @@ -1,14 +1,16 @@ import Route from "@ember/routing/route"; +import { ajax } from "discourse/lib/ajax"; export default Route.extend({ model() { - return this.modelFor("group"); + return ajax(`/assign/members/${this.modelFor("group").get("name")}`); }, setupController(controller, model) { controller.setProperties({ model, - showing: "members" + members: [], + groupName: this.modelFor("group").get("name") }); controller.findMembers(true); diff --git a/assets/javascripts/discourse/components/group-assignments-filter.js.es6 b/assets/javascripts/discourse/components/group-assignments-filter.js.es6 index 0ce9cfe..873ac3d 100644 --- a/assets/javascripts/discourse/components/group-assignments-filter.js.es6 +++ b/assets/javascripts/discourse/components/group-assignments-filter.js.es6 @@ -7,12 +7,12 @@ export default Component.extend({ @discourseComputed( "siteSettings.prioritize_username_in_ux", "filter.username", - "filter.displayName" + "filter.name" ) - displayName(prioritize_username_in_ux, username, displayName) { + displayName(prioritize_username_in_ux, username, name) { if (prioritize_username_in_ux) { - return username; + return username.trim(); } - return displayName; + return (name || username).trim(); } }); diff --git a/assets/javascripts/discourse/templates/components/group-assignments-filter.hbs b/assets/javascripts/discourse/templates/components/group-assignments-filter.hbs index e052f57..be45773 100644 --- a/assets/javascripts/discourse/templates/components/group-assignments-filter.hbs +++ b/assets/javascripts/discourse/templates/components/group-assignments-filter.hbs @@ -1,6 +1,6 @@ {{#if show-avatar}} {{#link-to "group.assignments.show" filter.username_lower}} - {{avatar filter avatarTemplatePath="avatar_template" usernamePath="username" imageSize="small"}} {{displayName}} + {{avatar filter avatarTemplatePath="avatar_template" usernamePath="username" imageSize="small"}} {{displayName}} ({{filter.assignments_count}}) {{/link-to}} {{else}} {{#link-to "group.assignments.show" filter}} diff --git a/assets/javascripts/discourse/templates/group/assignments.hbs b/assets/javascripts/discourse/templates/group/assignments.hbs index 683ecda..ff29d89 100644 --- a/assets/javascripts/discourse/templates/group/assignments.hbs +++ b/assets/javascripts/discourse/templates/group/assignments.hbs @@ -2,7 +2,7 @@ {{#mobile-nav class="activity-nav" desktopClass="action-list activity-list nav-stacked" currentPath=router._router.currentPath}} {{#load-more selector=".activity-nav li" action=(action "loadMore")}} {{group-assignments-filter show-avatar=false filter="everyone" routeType=route_type}} - {{#each model.members as |member|}} + {{#each members as |member|}} {{group-assignments-filter show-avatar=true filter=member routeType=route_type}} {{/each}} {{conditional-loading-spinner condition=loading}} diff --git a/config/routes.rb b/config/routes.rb index 575166e..02ddb36 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -6,4 +6,5 @@ DiscourseAssign::Engine.routes.draw do put "/unassign" => "assign#unassign" get "/suggestions" => "assign#suggestions" get "/assigned" => "assign#assigned" + get "/members/:group_name" => "assign#group_members" end diff --git a/plugin.rb b/plugin.rb index e95850a..f7d8c7d 100644 --- a/plugin.rb +++ b/plugin.rb @@ -253,6 +253,8 @@ after_initialize do page = (params[:page].to_i || 0).to_i group = Group.find_by("name = ?", params[:groupname]) + guardian.ensure_can_see_group_members!(group) + raise Discourse::NotFound unless group raise Discourse::InvalidAccess unless current_user.can_assign? diff --git a/spec/requests/assign_controller_spec.rb b/spec/requests/assign_controller_spec.rb index 5737f64..f68437c 100644 --- a/spec/requests/assign_controller_spec.rb +++ b/spec/requests/assign_controller_spec.rb @@ -12,6 +12,8 @@ RSpec.describe DiscourseAssign::AssignController do let(:post) { Fabricate(:post) } let(:user2) { Fabricate(:active_user) } let(:nonadmin) { Fabricate(:user, groups: [default_allowed_group]) } + let(:normal_user) { Fabricate(:user) } + let(:normal_admin) { Fabricate(:admin) } describe 'only allow users from allowed groups' do before { sign_in(user2) } @@ -196,4 +198,56 @@ RSpec.describe DiscourseAssign::AssignController do end end + context '#group_members' do + include_context 'A 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) + + freeze_time 1.hour.from_now + TopicAssigner.new(post1.topic, user).assign(user) + + freeze_time 1.hour.from_now + TopicAssigner.new(post2.topic, user).assign(user2) + + freeze_time 1.hour.from_now + TopicAssigner.new(post3.topic, user).assign(user) + end + + it 'list members order by assignments_count' 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]) + end + + it "doesn't include members with no assignments" do + sign_in(user) + add_to_assign_allowed_group(nonadmin) + + 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]) + end + + it "404 error to non-group-members" do + sign_in(normal_user) + + get "/assign/members/#{get_assigned_allowed_group_name}.json" + expect(response.status).to eq(403) + end + + it "allows non-member-admin" do + sign_in(normal_admin) + + get "/assign/members/#{get_assigned_allowed_group_name}.json" + expect(response.status).to eq(200) + end + end end diff --git a/test/javascripts/components/group-assignments-filter-test.js.es6 b/test/javascripts/components/group-assignments-filter-test.js.es6 index 426bbbb..d049fa0 100644 --- a/test/javascripts/components/group-assignments-filter-test.js.es6 +++ b/test/javascripts/components/group-assignments-filter-test.js.es6 @@ -10,7 +10,6 @@ componentTest("display username", { this.set("filter", { id: 2, username: "Ahmed", - displayName: "Ahmed Gagan", name: "Ahmed Gagan", avatar_template: "/letter_avatar_proxy/v4/letter/a/8c91f0/{size}.png", title: "trust_level_0", @@ -32,7 +31,6 @@ componentTest("display name", { this.set("filter", { id: 2, username: "Ahmed", - displayName: "Ahmed Gagan", name: "Ahmed Gagan", avatar_template: "/letter_avatar_proxy/v4/letter/a/8c91f0/{size}.png", title: "trust_level_0",