UX: Allow users to filter members on group page.

* Only admins are allowed to filter users by email.
This commit is contained in:
Guo Xiang Tan 2018-03-22 13:42:46 +08:00
parent 1cc0961566
commit f3b402ffd5
11 changed files with 171 additions and 25 deletions

View File

@ -1,9 +1,10 @@
import { popupAjaxError } from 'discourse/lib/ajax-error'; import { popupAjaxError } from 'discourse/lib/ajax-error';
import Group from 'discourse/models/group'; import Group from 'discourse/models/group';
import { default as computed, observes } from 'ember-addons/ember-computed-decorators'; import { default as computed, observes } from 'ember-addons/ember-computed-decorators';
import debounce from 'discourse/lib/debounce';
export default Ember.Controller.extend({ export default Ember.Controller.extend({
queryParams: ['order', 'desc'], queryParams: ['order', 'desc', 'filter'],
order: '', order: '',
desc: null, desc: null,
loading: false, loading: false,
@ -11,15 +12,26 @@ export default Ember.Controller.extend({
offset: null, offset: null,
isOwner: Ember.computed.alias('model.is_group_owner'), isOwner: Ember.computed.alias('model.is_group_owner'),
showActions: false, showActions: false,
filter: null,
filterInput: null,
@observes('order', 'desc') @observes("filterInput")
_setFilter: debounce(function() {
this.set("filter", this.get("filterInput"));
}, 500),
@observes('order', 'desc', 'filter')
refreshMembers() { refreshMembers() {
this.set('loading', true); this.set('loading', true);
const model = this.get('model');
this.get('model') && if (model) {
this.get('model') model.findMembers({
.findMembers({ order: this.get('order'), desc: this.get('desc') }) order: this.get('order'),
.finally(() => this.set('loading', false)); desc: this.get('desc'),
filter: this.get('filter'),
}).finally(() => this.set('loading', false));
}
}, },
@computed('model.members') @computed('model.members')
@ -32,6 +44,15 @@ export default Ember.Controller.extend({
return this.currentUser && this.currentUser.canManageGroup(model); return this.currentUser && this.currentUser.canManageGroup(model);
}, },
@computed
filterPlaceholder() {
if (this.currentUser && this.currentUser.admin) {
return "groups.members.filter_placeholder_admin";
} else {
return "groups.members.filter_placeholder";
}
},
actions: { actions: {
toggleActions() { toggleActions() {
this.toggleProperty("showActions"); this.toggleProperty("showActions");

View File

@ -33,13 +33,13 @@ const Group = RestModel.extend({
findMembers(params) { findMembers(params) {
if (Em.isEmpty(this.get('name'))) { return ; } if (Em.isEmpty(this.get('name'))) { return ; }
const self = this, offset = Math.min(this.get("user_count"), Math.max(this.get("offset"), 0)); const offset = Math.min(this.get("user_count"), Math.max(this.get("offset"), 0));
return Group.loadMembers(this.get("name"), offset, this.get("limit"), params).then(function (result) { return Group.loadMembers(this.get("name"), offset, this.get("limit"), params).then(result => {
var ownerIds = {}; var ownerIds = {};
result.owners.forEach(owner => ownerIds[owner.id] = true); result.owners.forEach(owner => ownerIds[owner.id] = true);
self.setProperties({ this.setProperties({
user_count: result.meta.total, user_count: result.meta.total,
limit: result.meta.limit, limit: result.meta.limit,
offset: result.meta.offset, offset: result.meta.offset,

View File

@ -3,13 +3,19 @@ export default Discourse.Route.extend({
return I18n.t('groups.members.title'); return I18n.t('groups.members.title');
}, },
model() { model(params) {
this._params = params;
return this.modelFor("group"); return this.modelFor("group");
}, },
setupController(controller, model) { setupController(controller, model) {
this.controllerFor("group").set("showing", "members"); this.controllerFor("group").set("showing", "members");
controller.set("model", model);
controller.setProperties({
model,
filterInput: this._params.filter
});
controller.refreshMembers(); controller.refreshMembers();
} }
}); });

View File

@ -1,3 +1,7 @@
{{text-field value=filterInput
placeholderKey=filterPlaceholder
class="group-username-filter no-blur"}}
{{#if hasMembers}} {{#if hasMembers}}
{{#load-more selector=".group-members tr" action="loadMore"}} {{#load-more selector=".group-members tr" action="loadMore"}}
<table class='group-members'> <table class='group-members'>

View File

@ -186,8 +186,8 @@ class GroupsController < ApplicationController
end end
users = group.users.human_users users = group.users.human_users
total = users.count total = users.count
members = users members = users
.order('NOT group_users.owner') .order('NOT group_users.owner')
.order(order) .order(order)
@ -200,6 +200,16 @@ class GroupsController < ApplicationController
.order(username_lower: dir) .order(username_lower: dir)
.where('group_users.owner') .where('group_users.owner')
if (filter = params[:filter]).present?
if current_user&.admin
owners = owners.filter_by_username_or_email(filter)
members = members.filter_by_username_or_email(filter)
else
owners = owners.filter_by_username(filter)
members = members.filter_by_username(filter)
end
end
render json: { render json: {
members: serialize_data(members, GroupUserSerializer), members: serialize_data(members, GroupUserSerializer),
owners: serialize_data(owners, GroupUserSerializer), owners: serialize_data(owners, GroupUserSerializer),

View File

@ -159,6 +159,25 @@ class User < ActiveRecord::Base
scope :not_suspended, -> { where('suspended_till IS NULL OR suspended_till <= ?', Time.zone.now) } scope :not_suspended, -> { where('suspended_till IS NULL OR suspended_till <= ?', Time.zone.now) }
scope :activated, -> { where(active: true) } scope :activated, -> { where(active: true) }
scope :filter_by_username, ->(filter) do
where('username_lower ILIKE ?', filter)
end
scope :filter_by_username_or_email, ->(filter) do
if filter =~ /.+@.+/
# probably an email so try the bypass
if user_id = UserEmail.where("lower(email) = ?", filter.downcase).pluck(:user_id).first
return where('users.id = ?', user_id)
end
end
joins(:primary_email)
.where(
'username_lower ILIKE :filter OR lower(user_emails.email) ILIKE :filter',
filter: "%#{filter}%"
)
end
module NewTopicDuration module NewTopicDuration
ALWAYS = -1 ALWAYS = -1
LAST_VISIT = -2 LAST_VISIT = -2

View File

@ -466,6 +466,8 @@ en:
activity: "Activity" activity: "Activity"
members: members:
title: "Members" title: "Members"
filter_placeholder_admin: "username or email"
filter_placeholder: "username"
remove_member: "Remove Member" remove_member: "Remove Member"
remove_member_description: "Remove <b>%{username}</b> from this group" remove_member_description: "Remove <b>%{username}</b> from this group"
make_owner: "Make Owner" make_owner: "Make Owner"

View File

@ -101,18 +101,6 @@ class AdminUserIndexQuery
end end
end end
def filter_by_user_with_bypass(filter)
if filter =~ /.+@.+/
# probably an email so try the bypass
user_id = UserEmail.where("lower(email) = ?", filter.downcase).pluck(:user_id).first
if user_id
return @query.where('users.id = ?', user_id)
end
end
@query.where('username_lower ILIKE :filter OR user_emails.email ILIKE :filter', filter: "%#{params[:filter]}%")
end
def filter_by_search def filter_by_search
if params[:email].present? if params[:email].present?
return @query.where('user_emails.email = ?', params[:email].downcase) return @query.where('user_emails.email = ?', params[:email].downcase)
@ -124,7 +112,7 @@ class AdminUserIndexQuery
if ip = IPAddr.new(filter) rescue nil if ip = IPAddr.new(filter) rescue nil
@query.where('ip_address <<= :ip OR registration_ip_address <<= :ip', ip: ip.to_cidr_s) @query.where('ip_address <<= :ip OR registration_ip_address <<= :ip', ip: ip.to_cidr_s)
else else
filter_by_user_with_bypass(filter) @query.filter_by_username_or_email(filter)
end end
end end
end end

View File

@ -1650,4 +1650,41 @@ describe User do
expect(inactive.active).to eq(true) expect(inactive.active).to eq(true)
end end
end end
describe '#filter_by_username' do
it 'should be able to filter by username' do
username = 'someuniqueusername'
user.update!(username: username)
expect(User.filter_by_username(username))
.to eq([user])
expect(User.filter_by_username('UNiQuE'))
.to eq([user])
end
end
describe '#filter_by_username_or_email' do
it 'should be able to filter by email' do
email = 'veryspecialtest@discourse.org'
user.update!(email: email)
expect(User.filter_by_username_or_email(email))
.to eq([user])
expect(User.filter_by_username_or_email('veryspeCiaLtest'))
.to eq([user])
end
it 'should be able to filter by username' do
username = 'someuniqueusername'
user.update!(username: username)
expect(User.filter_by_username_or_email(username))
.to eq([user])
expect(User.filter_by_username_or_email('UNiQuE'))
.to eq([user])
end
end
end end

View File

@ -403,6 +403,53 @@ describe GroupsController do
expect(members.map { |m| m["id"] }) expect(members.map { |m| m["id"] })
.to contain_exactly(user1.id, user2.id, user3.id) .to contain_exactly(user1.id, user2.id, user3.id)
end end
describe 'filterable' do
describe 'as a normal user' do
it "should not allow members to be filterable by email" do
email = 'uniquetest@discourse.org'
user1.update!(email: email)
get "/groups/#{group.name}/members.json", params: { filter: email }
expect(response.status).to eq(200)
members = JSON.parse(response.body)["members"]
expect(members).to eq([])
end
end
describe 'as an admin' do
before do
sign_in(Fabricate(:admin))
end
it "should allow members to be filterable by username" do
email = 'uniquetest@discourse.org'
user1.update!(email: email)
[email.upcase, 'QUEtes'].each do |filter|
get "/groups/#{group.name}/members.json", params: { filter: filter }
expect(response.status).to eq(200)
members = JSON.parse(response.body)["members"]
expect(members.map { |m| m["id"] }).to contain_exactly(user1.id)
end
end
it "should allow members to be filterable by email" do
username = 'uniquetest'
user1.update!(username: username)
[username.upcase, 'QUEtes'].each do |filter|
get "/groups/#{group.name}/members.json", params: { filter: filter }
expect(response.status).to eq(200)
members = JSON.parse(response.body)["members"]
expect(members.map { |m| m["id"] }).to contain_exactly(user1.id)
end
end
end
end
end end
describe "#edit" do describe "#edit" do

View File

@ -13,6 +13,12 @@ QUnit.test("Viewing Members as anon user", assert => {
count('.group-member-dropdown') === 0, count('.group-member-dropdown') === 0,
'it does not allow anon user to manage group members' 'it does not allow anon user to manage group members'
); );
assert.equal(
find('.group-username-filter').attr('placeholder'),
I18n.t('groups.members.filter_placeholder'),
'it should display the right filter placehodler'
);
}); });
}); });
@ -27,5 +33,11 @@ QUnit.test("Viewing Members as an admin user", assert => {
count('.group-member-dropdown') > 0, count('.group-member-dropdown') > 0,
'it allows admin user to manage group members' 'it allows admin user to manage group members'
); );
assert.equal(
find('.group-username-filter').attr('placeholder'),
I18n.t('groups.members.filter_placeholder_admin'),
'it should display the right filter placehodler'
);
}); });
}); });