From a9798f0c4737788ae0a8af9c39ca5daf88d39e3a Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Wed, 27 Mar 2019 13:30:59 +0200 Subject: [PATCH] FEATURE: Add page for all group membership requests. (#6909) --- .../controllers/group-requests.js.es6 | 122 ++++++++++++++++ .../discourse/controllers/group.js.es6 | 19 ++- .../discourse/routes/app-route-map.js.es6 | 1 + .../discourse/routes/group-requests.js.es6 | 21 +++ .../discourse/templates/group-requests.hbs | 51 +++++++ app/controllers/groups_controller.rb | 66 ++++++++- app/models/group.rb | 2 + app/models/group_request.rb | 4 + app/models/user.rb | 1 + app/serializers/group_requester_serializer.rb | 3 + config/locales/client.en.yml | 10 ++ config/locales/server.en.yml | 1 + config/routes.rb | 2 + .../20190117191606_create_group_requests.rb | 14 ++ spec/fabricators/group_request_fabricator.rb | 5 + spec/requests/groups_controller_spec.rb | 16 ++- .../acceptance/group-requests-test.js.es6 | 130 ++++++++++++++++++ 17 files changed, 464 insertions(+), 4 deletions(-) create mode 100644 app/assets/javascripts/discourse/controllers/group-requests.js.es6 create mode 100644 app/assets/javascripts/discourse/routes/group-requests.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/group-requests.hbs create mode 100644 app/models/group_request.rb create mode 100644 app/serializers/group_requester_serializer.rb create mode 100644 db/migrate/20190117191606_create_group_requests.rb create mode 100644 spec/fabricators/group_request_fabricator.rb create mode 100644 test/javascripts/acceptance/group-requests-test.js.es6 diff --git a/app/assets/javascripts/discourse/controllers/group-requests.js.es6 b/app/assets/javascripts/discourse/controllers/group-requests.js.es6 new file mode 100644 index 00000000000..ed07261f384 --- /dev/null +++ b/app/assets/javascripts/discourse/controllers/group-requests.js.es6 @@ -0,0 +1,122 @@ +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import Group from "discourse/models/group"; +import { + default as computed, + observes +} from "ember-addons/ember-computed-decorators"; +import debounce from "discourse/lib/debounce"; + +export default Ember.Controller.extend({ + queryParams: ["order", "desc", "filter"], + order: "", + desc: null, + loading: false, + limit: null, + offset: null, + filter: null, + filterInput: null, + application: Ember.inject.controller(), + + @observes("filterInput") + _setFilter: debounce(function() { + this.set("filter", this.get("filterInput")); + }, 500), + + @observes("order", "desc", "filter") + refreshRequesters(force) { + if (this.get("loading") || !this.get("model")) { + return; + } + + if ( + !force && + this.get("count") && + this.get("model.requesters.length") >= this.get("count") + ) { + this.set("application.showFooter", true); + return; + } + + this.set("loading", true); + this.set("application.showFooter", false); + + Group.loadMembers( + this.get("model.name"), + force ? 0 : this.get("model.requesters.length"), + this.get("limit"), + { + order: this.get("order"), + desc: this.get("desc"), + filter: this.get("filter"), + requesters: true + } + ).then(result => { + const requesters = (!force && this.get("model.requesters")) || []; + requesters.addObjects(result.members.map(m => Discourse.User.create(m))); + this.set("model.requesters", requesters); + + this.setProperties({ + loading: false, + count: result.meta.total, + limit: result.meta.limit, + offset: Math.min( + result.meta.offset + result.meta.limit, + result.meta.total + ) + }); + }); + }, + + @computed("model.requesters") + hasRequesters(requesters) { + return requesters && requesters.length > 0; + }, + + @computed + filterPlaceholder() { + if (this.currentUser && this.currentUser.admin) { + return "groups.members.filter_placeholder_admin"; + } else { + return "groups.members.filter_placeholder"; + } + }, + + handleRequest(data) { + ajax(`/groups/${this.get("model.id")}/handle_membership_request.json`, { + data, + type: "PUT" + }).catch(popupAjaxError); + }, + + actions: { + loadMore() { + this.refreshRequesters(); + }, + + acceptRequest(user) { + this.handleRequest({ user_id: user.get("id"), accept: true }); + user.setProperties({ + request_accepted: true, + request_denied: false + }); + }, + + undoAcceptRequest(user) { + ajax("/groups/" + this.get("model.id") + "/members.json", { + type: "DELETE", + data: { user_id: user.get("id") } + }).then(() => { + user.set("request_undone", true); + }); + }, + + denyRequest(user) { + this.handleRequest({ user_id: user.get("id") }); + user.setProperties({ + request_accepted: false, + request_denied: true + }); + } + } +}); diff --git a/app/assets/javascripts/discourse/controllers/group.js.es6 b/app/assets/javascripts/discourse/controllers/group.js.es6 index dcb095282b6..81302d755c4 100644 --- a/app/assets/javascripts/discourse/controllers/group.js.es6 +++ b/app/assets/javascripts/discourse/controllers/group.js.es6 @@ -15,8 +15,13 @@ export default Ember.Controller.extend({ showing: "members", destroying: null, - @computed("showMessages", "model.user_count", "canManageGroup") - tabs(showMessages, userCount, canManageGroup) { + @computed( + "showMessages", + "model.user_count", + "canManageGroup", + "model.allow_membership_requests" + ) + tabs(showMessages, userCount, canManageGroup, allowMembershipRequests) { const membersTab = Tab.create({ name: "members", route: "group.index", @@ -28,6 +33,16 @@ export default Ember.Controller.extend({ const defaultTabs = [membersTab, Tab.create({ name: "activity" })]; + if (canManageGroup && allowMembershipRequests) { + defaultTabs.push( + Tab.create({ + name: "requests", + i18nKey: "requests.title", + icon: "user-plus" + }) + ); + } + if (showMessages) { defaultTabs.push( Tab.create({ diff --git a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 index b9fc992d74b..edf1ac66966 100644 --- a/app/assets/javascripts/discourse/routes/app-route-map.js.es6 +++ b/app/assets/javascripts/discourse/routes/app-route-map.js.es6 @@ -66,6 +66,7 @@ export default function() { this.route("group", { path: "/g/:name", resetNamespace: true }, function() { this.route("members"); + this.route("requests"); this.route("activity", function() { this.route("posts"); diff --git a/app/assets/javascripts/discourse/routes/group-requests.js.es6 b/app/assets/javascripts/discourse/routes/group-requests.js.es6 new file mode 100644 index 00000000000..c469c9895ab --- /dev/null +++ b/app/assets/javascripts/discourse/routes/group-requests.js.es6 @@ -0,0 +1,21 @@ +export default Discourse.Route.extend({ + titleToken() { + return I18n.t("groups.requests.title"); + }, + + model(params) { + this._params = params; + return this.modelFor("group"); + }, + + setupController(controller, model) { + this.controllerFor("group").set("showing", "requests"); + + controller.setProperties({ + model, + filterInput: this._params.filter + }); + + controller.refreshRequesters(true); + } +}); diff --git a/app/assets/javascripts/discourse/templates/group-requests.hbs b/app/assets/javascripts/discourse/templates/group-requests.hbs new file mode 100644 index 00000000000..d04bd11e44d --- /dev/null +++ b/app/assets/javascripts/discourse/templates/group-requests.hbs @@ -0,0 +1,51 @@ +
+ {{text-field value=filterInput + placeholderKey=filterPlaceholder + class="group-username-filter no-blur"}} +
+ +{{#if hasRequesters}} + {{#load-more selector=".group-members tr" action=(action "loadMore")}} + + + {{group-index-toggle order=order desc=desc field='username_lower' i18nKey='username'}} + {{group-index-toggle order=order desc=desc field='requested_at' i18nKey='groups.member_requested'}} + + + + + + + {{#each model.requesters as |m|}} + + + + + + + + {{/each}} + +
{{i18n "groups.requests.reason"}}
+ {{user-info user=m skipName=skipName}} + + {{bound-date m.requested_at}} + {{m.reason}} + {{#if m.request_undone}} + {{i18n "groups.requests.undone"}} + {{else if m.request_accepted}} + {{i18n "groups.requests.accepted"}} + {{d-button action=(action "undoAcceptRequest") actionParam=m label="undo"}} + {{else if m.request_denied}} + {{i18n "groups.requests.denied"}} + {{else}} + {{d-button action=(action "acceptRequest") actionParam=m label="groups.requests.accept" class="btn-primary"}} + {{d-button action=(action "denyRequest") actionParam=m label="groups.requests.deny" class="btn-danger"}} + {{/if}} +
+ {{/load-more}} + + {{conditional-loading-spinner condition=loading}} +{{else}} +
{{i18n "groups.empty.requests"}}
+{{/if}} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index ee322a708bc..95aceaa8729 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -214,6 +214,39 @@ class GroupsController < ApplicationController dir = (params[:desc] && !params[:desc].blank?) ? 'DESC' : 'ASC' order = "" + if params[:requesters] + guardian.ensure_can_edit!(group) + + users = group.requesters + total = users.count + + if (filter = params[:filter]).present? + filter = filter.split(',') if filter.include?(',') + + if current_user&.admin + users = users.filter_by_username_or_email(filter) + else + users = users.filter_by_username(filter) + end + end + + users = users + .select("users.*, group_requests.reason, group_requests.created_at requested_at") + .order(params[:order] == 'requested_at' ? "group_requests.created_at #{dir}" : "") + .order(username_lower: dir) + .limit(limit) + .offset(offset) + + return render json: { + members: serialize_data(users, GroupRequesterSerializer), + meta: { + total: total, + limit: limit, + offset: offset + } + } + end + if params[:order] && %w{last_posted_at last_seen_at}.include?(params[:order]) order = "#{params[:order]} #{dir} NULLS LAST" elsif params[:order] == 'added_at' @@ -294,6 +327,26 @@ class GroupsController < ApplicationController end end + def handle_membership_request + group = Group.find_by(id: params[:id]) + raise Discourse::InvalidParameters.new(:id) if group.blank? + guardian.ensure_can_edit!(group) + + ActiveRecord::Base.transaction do + user = User.find_by(id: params[:user_id]) + raise Discourse::InvalidParameters.new(:user_id) if user.blank? + + if params[:accept] + group.add(user) + GroupActionLogger.new(current_user, group).log_add_user_to_group(user) + end + + GroupRequest.where(group_id: group.id, user_id: user.id).delete_all + end + + render json: success_json + end + def mentionable group = find_group(:group_id, ensure_can_see: false) @@ -369,14 +422,25 @@ class GroupsController < ApplicationController .pluck("users.username") ) + raw = <<~EOF + #{params[:reason]} + + --- + + #{I18n.t('groups.request_membership_pm.handle')} + + EOF + post = PostCreator.new(current_user, title: I18n.t('groups.request_membership_pm.title', group_name: group_name), - raw: params[:reason], + raw: raw, archetype: Archetype.private_message, target_usernames: usernames.join(','), skip_validations: true ).create! + GroupRequest.create!(group: group, user: current_user, reason: params[:reason]) + render json: success_json.merge(relative_url: post.topic.relative_url) end diff --git a/app/models/group.rb b/app/models/group.rb index 65bc2c7fd49..91417f93aa3 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -12,12 +12,14 @@ class Group < ActiveRecord::Base has_many :category_groups, dependent: :destroy has_many :group_users, dependent: :destroy + has_many :group_requests, dependent: :destroy has_many :group_mentions, dependent: :destroy has_many :group_archived_messages, dependent: :destroy has_many :categories, through: :category_groups has_many :users, through: :group_users + has_many :requesters, through: :group_requests, source: :user has_many :group_histories, dependent: :destroy has_and_belongs_to_many :web_hooks diff --git a/app/models/group_request.rb b/app/models/group_request.rb new file mode 100644 index 00000000000..0977ada1575 --- /dev/null +++ b/app/models/group_request.rb @@ -0,0 +1,4 @@ +class GroupRequest < ActiveRecord::Base + belongs_to :group + belongs_to :user +end diff --git a/app/models/user.rb b/app/models/user.rb index a2383068fb4..2acfad883b7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -55,6 +55,7 @@ class User < ActiveRecord::Base has_many :group_users, dependent: :destroy has_many :groups, through: :group_users + has_many :group_requests, dependent: :destroy has_many :secure_categories, through: :groups, source: :categories has_many :user_uploads, dependent: :destroy diff --git a/app/serializers/group_requester_serializer.rb b/app/serializers/group_requester_serializer.rb new file mode 100644 index 00000000000..4fda1153e3a --- /dev/null +++ b/app/serializers/group_requester_serializer.rb @@ -0,0 +1,3 @@ +class GroupRequesterSerializer < BasicUserSerializer + attributes :reason, :requested_at +end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 0b6f2719dba..4397e0e38d0 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -430,10 +430,19 @@ en: groups: member_added: "Added" + member_requested: "Requested at" add_members: title: "Add Members" description: "Manage the membership of this group" usernames: "Usernames" + requests: + title: "Requests" + reason: "Reason" + accept: "Accept" + accepted: "accepted" + deny: "Deny" + denied: "denied" + undone: "request undone" manage: title: "Manage" name: "Name" @@ -464,6 +473,7 @@ en: empty: posts: "There are no posts by members of this group." members: "There are no members in this group." + requests: "There are no membership requests for this group." mentions: "There are no mentions of this group." messages: "There are no messages for this group." topics: "There are no topics by members of this group." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 37f28c1e945..b73d57f0077 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -379,6 +379,7 @@ en: trust_level_4: "trust_level_4" request_membership_pm: title: "Membership Request for @%{group_name}" + handle: "handle membership request" education: until_posts: diff --git a/config/routes.rb b/config/routes.rb index 64bb07a8c54..68620af6952 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -511,6 +511,7 @@ Discourse::Application.routes.draw do %w{ activity activity/:filter + requests messages messages/inbox messages/archive @@ -527,6 +528,7 @@ Discourse::Application.routes.draw do put "members" => "groups#add_members" delete "members" => "groups#remove_member" post "request_membership" => "groups#request_membership" + put "handle_membership_request" => "groups#handle_membership_request" post "notifications" => "groups#set_notifications" end end diff --git a/db/migrate/20190117191606_create_group_requests.rb b/db/migrate/20190117191606_create_group_requests.rb new file mode 100644 index 00000000000..0453b99a219 --- /dev/null +++ b/db/migrate/20190117191606_create_group_requests.rb @@ -0,0 +1,14 @@ +class CreateGroupRequests < ActiveRecord::Migration[5.2] + def change + create_table :group_requests do |t| + t.integer :group_id + t.integer :user_id + t.text :reason + + t.timestamps + end + + add_index :group_requests, :group_id + add_index :group_requests, :user_id + end +end diff --git a/spec/fabricators/group_request_fabricator.rb b/spec/fabricators/group_request_fabricator.rb new file mode 100644 index 00000000000..caa2fc70fbd --- /dev/null +++ b/spec/fabricators/group_request_fabricator.rb @@ -0,0 +1,5 @@ +Fabricator(:group_request) do + user + group + reason { sequence(:reason) { |n| "group request #{n}" } } +end diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 1e42565627d..51cf5865b49 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -724,6 +724,20 @@ describe GroupsController do .to contain_exactly(user1.id, user2.id, user3.id) end + it "can show group requests" do + sign_in(Fabricate(:admin)) + + user4 = Fabricate(:user) + request4 = Fabricate(:group_request, user: user4, group: group) + + get "/groups/#{group.name}/members.json", params: { requesters: true } + + members = JSON.parse(response.body)["members"] + expect(members.length).to eq(1) + expect(members.first["username"]).to eq(user4.username) + expect(members.first["reason"]).to eq(request4.reason) + end + describe 'filterable' do describe 'as a normal user' do it "should not allow members to be filterable by email" do @@ -1284,7 +1298,7 @@ describe GroupsController do group_name: group.name )) - expect(post.raw).to eq('Please add me in') + expect(post.raw).to start_with('Please add me in') expect(topic.archetype).to eq(Archetype.private_message) expect(topic.allowed_users).to contain_exactly(user, owner1, owner2) expect(topic.allowed_groups).to eq([]) diff --git a/test/javascripts/acceptance/group-requests-test.js.es6 b/test/javascripts/acceptance/group-requests-test.js.es6 new file mode 100644 index 00000000000..132ba3f0f2a --- /dev/null +++ b/test/javascripts/acceptance/group-requests-test.js.es6 @@ -0,0 +1,130 @@ +import { acceptance } from "helpers/qunit-helpers"; +import { parsePostData } from "helpers/create-pretender"; + +let requests = []; + +acceptance("Group Requests", { + loggedIn: true, + pretend(server, helper) { + server.get("/groups/Macdonald.json", () => { + return helper.response({ + group: { + id: 42, + automatic: false, + name: "Macdonald", + user_count: 1, + mentionable_level: 0, + messageable_level: 0, + visibility_level: 0, + automatic_membership_email_domains: "", + automatic_membership_retroactive: false, + primary_group: false, + title: "Macdonald", + grant_trust_level: null, + incoming_email: null, + has_messages: false, + flair_url: null, + flair_bg_color: "", + flair_color: "", + bio_raw: null, + bio_cooked: null, + public_admission: false, + public_exit: false, + allow_membership_requests: true, + full_name: "Macdonald", + default_notification_level: 3, + membership_request_template: "", + is_group_user: true, + is_group_owner: true, + is_group_owner_display: true, + mentionable: false, + messageable: false + }, + extras: { + visible_group_names: ["discourse", "Macdonald"] + } + }); + }); + + server.get("/groups/Macdonald/members.json", () => { + return helper.response({ + members: [ + { + id: 19, + username: "eviltrout", + name: "Robin Ward", + avatar_template: + "/user_avatar/meta.discourse.org/eviltrout/{size}/5275_2.png", + reason: "Please accept my membership request.", + requested_at: "2019-01-31T12:00:00.000Z" + }, + { + id: 20, + username: "eviltrout2", + name: "Robin Ward", + avatar_template: + "/user_avatar/meta.discourse.org/eviltrout/{size}/5275_2.png", + reason: "Please accept another membership request.", + requested_at: "2019-01-31T14:00:00.000Z" + } + ], + meta: { total: 2, limit: 50, offset: 0 } + }); + }); + + server.put("/groups/42/handle_membership_request.json", request => { + const body = parsePostData(request.requestBody); + requests.push([body["user_id"], body["accept"]]); + return helper.success(); + }); + } +}); + +QUnit.test("Group Requests", async assert => { + await visit("/groups/Macdonald/requests"); + + assert.equal(find(".group-members tr").length, 2); + assert.equal( + find(".group-members tr:first-child td:nth-child(1)") + .text() + .trim() + .replace(/\s+/g, " "), + "eviltrout Robin Ward" + ); + assert.equal( + find(".group-members tr:first-child td:nth-child(3)") + .text() + .trim(), + "Please accept my membership request." + ); + assert.equal( + find(".group-members tr:first-child .btn-primary") + .text() + .trim(), + "Accept" + ); + assert.equal( + find(".group-members tr:first-child .btn-danger") + .text() + .trim(), + "Deny" + ); + + await click(".group-members tr:first-child .btn-primary"); + assert.ok( + find(".group-members tr:first-child td:nth-child(4)") + .text() + .trim() + .indexOf("accepted") === 0 + ); + assert.deepEqual(requests, [["19", "true"]]); + + await click(".group-members tr:last-child .btn-danger"); + assert.equal( + find(".group-members tr:last-child td:nth-child(4)") + .text() + .trim(), + "denied" + ); + assert.deepEqual(requests, [["19", "true"], ["20", undefined]]); +});