From 84d46bceb96315fa0e290163add17a4dc966f997 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 13 Jun 2017 17:10:14 +0900 Subject: [PATCH] FIX: Create group membership request on behalf of user. --- .../components/group-membership-button.js.es6 | 15 ++--- .../javascripts/discourse/models/group.js.es6 | 13 +++-- .../components/group-membership-button.hbs | 5 ++ .../javascripts/discourse/templates/group.hbs | 4 +- .../discourse/templates/groups.hbs | 1 - app/controllers/groups_controller.rb | 33 +++++++---- config/locales/client.en.yml | 3 - config/locales/server.en.yml | 3 + config/routes.rb | 2 +- spec/integration/groups_spec.rb | 57 ++++++++++++------- 10 files changed, 84 insertions(+), 52 deletions(-) diff --git a/app/assets/javascripts/discourse/components/group-membership-button.js.es6 b/app/assets/javascripts/discourse/components/group-membership-button.js.es6 index b5911799093..b27ce66f79c 100644 --- a/app/assets/javascripts/discourse/components/group-membership-button.js.es6 +++ b/app/assets/javascripts/discourse/components/group-membership-button.js.es6 @@ -1,8 +1,10 @@ import { default as computed } from 'ember-addons/ember-computed-decorators'; import { popupAjaxError } from 'discourse/lib/ajax-error'; -import Group from 'discourse/models/group'; +import DiscourseURL from 'discourse/lib/url'; export default Ember.Component.extend({ + loading: false, + @computed("model.public") canJoinGroup(publicGroup) { return publicGroup; @@ -51,13 +53,12 @@ export default Ember.Component.extend({ requestMembership() { if (this.currentUser) { - const groupName = this.get('model.name'); + this.set('loading', true); - Group.loadOwners(groupName).then(result => { - const names = result.map(owner => owner.username).join(","); - const title = I18n.t('groups.request_membership_pm.title'); - const body = I18n.t('groups.request_membership_pm.body', { groupName }); - this.sendAction("createNewMessageViaParams", names, title, body); + this.get('model').requestMembership().then(result => { + DiscourseURL.routeTo(result.relative_url); + }).catch(popupAjaxError).finally(() => { + this.set('loading', false); }); } else { this._showLoginModal(); diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6 index a85025e410e..f14507c6abe 100644 --- a/app/assets/javascripts/discourse/models/group.js.es6 +++ b/app/assets/javascripts/discourse/models/group.js.es6 @@ -2,7 +2,6 @@ import { ajax } from 'discourse/lib/ajax'; import { default as computed, observes } from "ember-addons/ember-computed-decorators"; import GroupHistory from 'discourse/models/group-history'; import RestModel from 'discourse/models/rest'; -import { popupAjaxError } from 'discourse/lib/ajax-error'; const Group = RestModel.extend({ limit: 50, @@ -202,7 +201,13 @@ const Group = RestModel.extend({ data: { notification_level, user_id: userId }, type: "POST" }); - } + }, + + requestMembership() { + return ajax(`/groups/${this.get('name')}/request_membership`, { + type: "POST" + }); + }, }); Group.reopenClass({ @@ -216,10 +221,6 @@ Group.reopenClass({ return ajax("/groups/" + name + ".json").then(result => Group.create(result.basic_group)); }, - loadOwners(name) { - return ajax('/groups/' + name + '/owners.json').catch(popupAjaxError); - }, - loadMembers(name, offset, limit, params) { return ajax('/groups/' + name + '/members.json', { data: _.extend({ diff --git a/app/assets/javascripts/discourse/templates/components/group-membership-button.hbs b/app/assets/javascripts/discourse/templates/components/group-membership-button.hbs index fb1444bbf37..ed17b2a79bd 100644 --- a/app/assets/javascripts/discourse/templates/components/group-membership-button.hbs +++ b/app/assets/javascripts/discourse/templates/components/group-membership-button.hbs @@ -24,8 +24,13 @@ {{else}} {{d-button action="requestMembership" class="group-index-request" + disabled=loading icon="envelope" label="groups.request"}} + + {{#if loading}} + {{loading-spinner size="small"}} + {{/if}} {{/if}} {{else}} {{yield}} diff --git a/app/assets/javascripts/discourse/templates/group.hbs b/app/assets/javascripts/discourse/templates/group.hbs index 95fcabb36a7..24afa5a0c70 100644 --- a/app/assets/javascripts/discourse/templates/group.hbs +++ b/app/assets/javascripts/discourse/templates/group.hbs @@ -43,9 +43,7 @@ {{/each}} {{/mobile-nav}} - {{group-membership-button model=model - createNewMessageViaParams='createNewMessageViaParams' - showLogin='showLogin'}} + {{group-membership-button model=model showLogin='showLogin'}} diff --git a/app/assets/javascripts/discourse/templates/groups.hbs b/app/assets/javascripts/discourse/templates/groups.hbs index 98faa7e4174..296324e7737 100644 --- a/app/assets/javascripts/discourse/templates/groups.hbs +++ b/app/assets/javascripts/discourse/templates/groups.hbs @@ -45,7 +45,6 @@ {{#group-membership-button model=group - createNewMessageViaParams='createNewMessageViaParams' showMembershipStatus=true groupUserIds=groups.extras.group_user_ids showLogin='showLogin'}} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 1a027838d77..33b86d9e5b2 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -5,7 +5,8 @@ class GroupsController < ApplicationController :mentionable, :update, :messages, - :histories + :histories, + :request_membership ] skip_before_filter :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed] @@ -136,16 +137,6 @@ class GroupsController < ApplicationController } end - def owners - group = find_group(:group_id) - - owners = group.users.where('group_users.owner') - .order("users.last_seen_at DESC") - .limit(5) - - render_serialized(owners, GroupUserSerializer) - end - def add_members group = Group.find(params[:id]) group.public ? ensure_logged_in : guardian.ensure_can_edit!(group) @@ -238,7 +229,27 @@ class GroupsController < ApplicationController else render_json_error(group) end + end + def request_membership + unless current_user.staff? + RateLimiter.new(current_user, "request_group_membership", 1, 1.day).performed! + end + + group = find_group(:id) + group_name = group.name + username = current_user.username + + post = PostCreator.new(current_user, + title: I18n.t('groups.request_membership_pm.title', group_name: group_name), + raw: I18n.t('groups.request_membership_pm.body', group_name: group_name), + archetype: Archetype.private_message, + target_usernames: username, + target_group_names: group_name, + skip_validations: true + ).create! + + render json: success_json.merge(relative_url: post.topic.relative_url) end def set_notifications diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 673c0d08000..f4f41434969 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -407,9 +407,6 @@ en: full_name: 'Full Name' add_members: "Add Members" delete_member_confirm: "Remove '%{username}' from the '%{group}' group?" - request_membership_pm: - title: "Membership Request" - body: "I would like to apply for membership in @%{groupName}." name_placeholder: "Group name, no spaces, same as username rule" public: "Allow users to join/leave the group freely (Requires group to be visible)" empty: diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 6fa05371483..94070e7a367 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -274,6 +274,9 @@ en: trust_level_2: "trust_level_2" trust_level_3: "trust_level_3" trust_level_4: "trust_level_4" + request_membership_pm: + title: "Membership Request for @%{group_name}" + body: "I would like to apply for membership in @%{group_name}." education: until_posts: diff --git a/config/routes.rb b/config/routes.rb index 99f87cdf682..6b94848c0fe 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -430,7 +430,6 @@ Discourse::Application.routes.draw do get 'activity' => "groups#show" get 'activity/:filter' => "groups#show" get 'members' - get 'owners' get 'posts' get 'topics' get 'mentions' @@ -442,6 +441,7 @@ Discourse::Application.routes.draw do member do put "members" => "groups#add_members" delete "members" => "groups#remove_member" + post "request_membership" => "groups#request_membership" post "notifications" => "groups#set_notifications" end end diff --git a/spec/integration/groups_spec.rb b/spec/integration/groups_spec.rb index 788d72c1b1f..f5ab5fc75ba 100644 --- a/spec/integration/groups_spec.rb +++ b/spec/integration/groups_spec.rb @@ -151,26 +151,6 @@ describe "Groups" do end end - describe 'owners' do - let(:user1) { Fabricate(:user, last_seen_at: Time.zone.now) } - let(:user2) { Fabricate(:user, last_seen_at: Time.zone.now - 1 .day) } - let(:group) { Fabricate(:group, users: [user, user1, user2]) } - - it 'should return the right list of owners' do - group.add_owner(user1) - group.add_owner(user2) - - xhr :get, "/groups/#{group.name}/owners" - - expect(response).to be_success - - owners = JSON.parse(response.body) - - expect(owners.count).to eq(2) - expect(owners.map { |o| o["id"] }.sort).to eq([user1.id, user2.id]) - end - end - describe 'members' do let(:user1) do Fabricate(:user, @@ -555,4 +535,41 @@ describe "Groups" do end end end + + describe "requesting membership for a group" do + let(:new_user) { Fabricate(:user) } + + it 'requires the user to log in' do + expect do + xhr :post, "/groups/#{group.name}/request_membership" + end.to raise_error(Discourse::NotLoggedIn) + end + + it 'should create the right PM' do + sign_in(user) + + xhr :post, "/groups/#{group.name}/request_membership" + + expect(response).to be_success + + post = Post.last + topic = post.topic + body = JSON.parse(response.body) + + expect(body['relative_url']).to eq(topic.relative_url) + expect(post.user).to eq(user) + + expect(topic.title).to eq(I18n.t('groups.request_membership_pm.title', + group_name: group.name + )) + + expect(post.raw).to eq(I18n.t( + 'groups.request_membership_pm.body', group_name: group.name + )) + + expect(topic.archetype).to eq(Archetype.private_message) + expect(topic.allowed_users).to eq([user]) + expect(topic.allowed_groups).to eq([group]) + end + end end