From a23509cbf39843a690a141f50d301f20acca5351 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 21 Mar 2018 16:32:08 +0800 Subject: [PATCH] UX: Limit the number of group names displayed on user page. --- .../discourse/controllers/groups.js.es6 | 2 +- .../javascripts/discourse/models/user.js.es6 | 19 ++++++++-- .../discourse/routes/groups.js.es6 | 1 + .../javascripts/discourse/templates/user.hbs | 6 ++++ .../stylesheets/common/base/groups.scss | 5 +++ app/controllers/groups_controller.rb | 23 +++++++----- spec/requests/groups_controller_spec.rb | 36 ++++++++++++++++--- 7 files changed, 76 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/groups.js.es6 b/app/assets/javascripts/discourse/controllers/groups.js.es6 index 35cfbf1dc38..006dd3baabc 100644 --- a/app/assets/javascripts/discourse/controllers/groups.js.es6 +++ b/app/assets/javascripts/discourse/controllers/groups.js.es6 @@ -7,7 +7,7 @@ export default Ember.Controller.extend({ order: null, asc: null, filter: "", - type: "", + type: null, @computed("model.extras.type_filters") types(typeFilters) { diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index e54caf27395..e6aeb9f676b 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -342,13 +342,26 @@ const User = RestModel.extend({ ua.action_type === UserAction.TYPES.topics; }, + numGroupsToDisplay: 2, + @computed("groups.[]") - displayGroups() { + filteredGroups() { const groups = this.get('groups') || []; - const filtered = groups.filter(group => { + + return groups.filter(group => { return !group.automatic || group.name === "moderators"; }); - return filtered.length === 0 ? null : filtered; + }, + + @computed("filteredGroups", "numGroupsToDisplay") + displayGroups(filteredGroups, numGroupsToDisplay) { + const groups = filteredGroups.slice(0, numGroupsToDisplay); + return groups.length === 0 ? null : groups; + }, + + @computed("filteredGroups", "numGroupsToDisplay") + showMoreGroupsLink(filteredGroups, numGroupsToDisplay) { + return filteredGroups.length > numGroupsToDisplay; }, // The user's stat count, excluding PMs. diff --git a/app/assets/javascripts/discourse/routes/groups.js.es6 b/app/assets/javascripts/discourse/routes/groups.js.es6 index 55681ff90bd..9b53f67906d 100644 --- a/app/assets/javascripts/discourse/routes/groups.js.es6 +++ b/app/assets/javascripts/discourse/routes/groups.js.es6 @@ -8,6 +8,7 @@ export default Discourse.Route.extend({ asc: { refreshModel: true, replace: true }, filter: { refreshModel: true }, type: { refreshModel: true, replace: true }, + username: { refreshModel: true }, }, refreshQueryWithoutTransition: true, diff --git a/app/assets/javascripts/discourse/templates/user.hbs b/app/assets/javascripts/discourse/templates/user.hbs index 7f5e25820c2..469dc539539 100644 --- a/app/assets/javascripts/discourse/templates/user.hbs +++ b/app/assets/javascripts/discourse/templates/user.hbs @@ -169,14 +169,20 @@ {{/if}} {{/if}} + {{#if model.displayGroups}}
{{i18n 'groups.title' count=model.displayGroups.length}}
{{#each model.displayGroups as |group|}} {{#link-to 'group' group.name class="group-link"}}{{group.name}}{{/link-to}} {{/each}} + + {{#link-to "groups" (query-params username=model.username)}} + ... + {{/link-to}}
{{/if}} + {{#if canDeleteUser}} {{d-button action="adminDelete" icon="exclamation-triangle" label="user.admin_delete" class="btn-danger"}} {{/if}} diff --git a/app/assets/stylesheets/common/base/groups.scss b/app/assets/stylesheets/common/base/groups.scss index 7740978859a..490a21e3577 100644 --- a/app/assets/stylesheets/common/base/groups.scss +++ b/app/assets/stylesheets/common/base/groups.scss @@ -8,6 +8,11 @@ .groups-filter { display: inline-block; float: right; + + .groups-name-filter { + position: relative; + top: 1px; + } } .groups-table { diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index bf1d0daf760..df2b3f82b1c 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -14,13 +14,13 @@ class GroupsController < ApplicationController skip_before_action :check_xhr, only: [:show] TYPE_FILTERS = { - my: Proc.new { |groups, current_user| - raise Discourse::NotFound unless current_user - Group.member_of(groups, current_user) + my: Proc.new { |groups, user| + raise Discourse::NotFound unless user + Group.member_of(groups, user) }, - owner: Proc.new { |groups, current_user| - raise Discourse::NotFound unless current_user - Group.owner_of(groups, current_user) + owner: Proc.new { |groups, user| + raise Discourse::NotFound unless user + Group.owner_of(groups, user) }, public: Proc.new { |groups| groups.where(public_admission: true, automatic: false) @@ -53,9 +53,14 @@ class GroupsController < ApplicationController type_filters = TYPE_FILTERS.keys + if username = params[:username] + groups = TYPE_FILTERS[:my].call(groups, User.find_by_username(username)) + type_filters = type_filters - [:my, :owner] + end + unless guardian.is_staff? # hide automatic groups from all non stuff to de-clutter page - groups = groups.where(automatic: false) + groups = groups.where("automatic IS FALSE OR groups.id = #{Group::AUTO_GROUPS[:moderators]}") type_filters.delete(:automatic) end @@ -71,6 +76,8 @@ class GroupsController < ApplicationController group_users = GroupUser.where(group: groups, user: current_user) user_group_ids = group_users.pluck(:group_id) owner_group_ids = group_users.where(owner: true).pluck(:group_id) + else + type_filters = type_filters - [:my, :owner] end count = groups.count @@ -83,7 +90,7 @@ class GroupsController < ApplicationController owner_group_ids: owner_group_ids || [] ), extras: { - type_filters: current_user ? type_filters : type_filters - [:my, :owner] + type_filters: type_filters }, total_rows_groups: count, load_more_groups: groups_path(page: page + 1, type: type), diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index aa88e21d737..b6bf99bd07a 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' describe GroupsController do let(:user) { Fabricate(:user) } let(:group) { Fabricate(:group, users: [user]) } + let(:moderator_group_id) { Group::AUTO_GROUPS[:moderators] } describe '#index' do let(:staff_group) do @@ -45,7 +46,7 @@ describe GroupsController do expect(response.status).to eq(200) - group_ids = [group.id, other_group.id] + group_ids = [moderator_group_id, group.id, other_group.id] group_ids.reverse! if !is_asc expect(JSON.parse(response.body)["groups"].map { |group| group["id"] }) @@ -62,7 +63,7 @@ describe GroupsController do expect(response.status).to eq(200) expect(JSON.parse(response.body)["groups"].map { |group| group["id"] }) - .to eq([other_group.id, group.id]) + .to eq([other_group.id, group.id, moderator_group_id]) end end end @@ -72,7 +73,7 @@ describe GroupsController do staff_group get "/groups.json" - expect(response).to be_success + expect(response.status).to eq(200) response_body = JSON.parse(response.body) @@ -81,13 +82,40 @@ describe GroupsController do expect(group_ids).to include(group.id) expect(group_ids).to_not include(staff_group.id) expect(response_body["load_more_groups"]).to eq("/groups?page=1") - expect(response_body["total_rows_groups"]).to eq(1) + expect(response_body["total_rows_groups"]).to eq(2) expect(response_body["extras"]["type_filters"].map(&:to_sym)).to eq( described_class::TYPE_FILTERS.keys - [:my, :owner, :automatic] ) end + context 'viewing groups of another user' do + describe 'when an invalid username is given' do + it 'should return the right response' do + group + get "/groups.json", params: { username: 'asdasd' } + + expect(response.status).to eq(404) + end + end + + it 'should return the right response' do + user2 = Fabricate(:user) + group + sign_in(user2) + + get "/groups.json", params: { username: user.username } + + expect(response.status).to eq(200) + + response_body = JSON.parse(response.body) + + group_ids = response_body["groups"].map { |g| g["id"] } + + expect(group_ids).to contain_exactly(group.id) + end + end + context 'viewing as an admin' do let(:admin) { Fabricate(:admin) }