From a9613163b54ae5729765a7e3d4ccc23563202cb7 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Tue, 8 Aug 2017 18:53:02 +0900 Subject: [PATCH] FEATURE: Force user to enter reason when requesting for group membership. --- .../javascripts/admin/templates/group.hbs | 11 ++++++ .../components/expanding-text-area.js.es6 | 0 .../components/group-membership-button.js.es6 | 14 ++------ .../request-group-membership-form.js.es6 | 34 +++++++++++++++++++ .../javascripts/discourse/models/group.js.es6 | 8 +++-- .../components/group-membership-button.hbs | 6 +--- .../discourse/templates/group-edit.hbs | 12 +++++++ .../modal/request-group-membership-form.hbs | 21 ++++++++++++ .../base/request-group-membership-form.scss | 5 +++ app/controllers/admin/groups_controller.rb | 4 ++- app/controllers/groups_controller.rb | 4 ++- app/serializers/basic_group_serializer.rb | 3 +- config/locales/client.en.yml | 6 ++++ config/locales/server.en.yml | 1 - ...d_membership_request_template_to_groups.rb | 5 +++ spec/integration/admin/groups_spec.rb | 4 ++- spec/integration/groups_spec.rb | 16 ++++++--- .../acceptance/group-edit-test.js.es6 | 7 ++-- .../javascripts/acceptance/groups-test.js.es6 | 28 +++++++++++++++ .../fixtures/groups-fixtures.js.es6 | 2 +- 20 files changed, 159 insertions(+), 32 deletions(-) rename app/assets/javascripts/{admin => discourse}/components/expanding-text-area.js.es6 (100%) create mode 100644 app/assets/javascripts/discourse/controllers/request-group-membership-form.js.es6 create mode 100644 app/assets/javascripts/discourse/templates/modal/request-group-membership-form.hbs create mode 100644 app/assets/stylesheets/common/base/request-group-membership-form.scss create mode 100644 db/migrate/20170731075604_add_membership_request_template_to_groups.rb diff --git a/app/assets/javascripts/admin/templates/group.hbs b/app/assets/javascripts/admin/templates/group.hbs index 4861454272d..736fab05de6 100644 --- a/app/assets/javascripts/admin/templates/group.hbs +++ b/app/assets/javascripts/admin/templates/group.hbs @@ -91,6 +91,17 @@ + {{#if model.allow_membership_requests}} +
+ + + {{expanding-text-area name="membership-request-template" + value=model.membership_request_template}} +
+ {{/if}} +
+ {{#if model.allow_membership_requests}} +
+ + + {{expanding-text-area name="membership-request-template" + value=model.membership_request_template + class="group-edit-membership-request-template"}} +
+ {{/if}} + {{plugin-outlet name="group-edit" args=(hash group=model)}} {{d-button action="save" class="btn-primary" disabled=saving label="save"}} diff --git a/app/assets/javascripts/discourse/templates/modal/request-group-membership-form.hbs b/app/assets/javascripts/discourse/templates/modal/request-group-membership-form.hbs new file mode 100644 index 00000000000..53bf6f98b8b --- /dev/null +++ b/app/assets/javascripts/discourse/templates/modal/request-group-membership-form.hbs @@ -0,0 +1,21 @@ +
+ {{#d-modal-body rawTitle=title}} +
+ + + {{expanding-text-area value=reason}} +
+ {{/d-modal-body}} + +
+ {{d-button class="btn-primary" + disabled=disableSubmit + label="groups.membership_request.submit" + action="requestMember"}} + + {{i18n 'cancel'}} + {{conditional-loading-spinner size="small" condition=loading}} +
+
diff --git a/app/assets/stylesheets/common/base/request-group-membership-form.scss b/app/assets/stylesheets/common/base/request-group-membership-form.scss new file mode 100644 index 00000000000..569ebfeb696 --- /dev/null +++ b/app/assets/stylesheets/common/base/request-group-membership-form.scss @@ -0,0 +1,5 @@ +.request-group-membership-form { + label { + font-weight: bold; + } +} diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index dd99b2fda03..e7e169d9462 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -97,6 +97,7 @@ class Admin::GroupsController < Admin::AdminController if group_params[:allow_membership_requests] group.allow_membership_requests = group_params[:allow_membership_requests] + group.membership_request_template = group_params[:membership_request_template] end if group_params[:owner_usernames].present? @@ -208,7 +209,8 @@ class Admin::GroupsController < Admin::AdminController :full_name, :default_notification_level, :usernames, - :owner_usernames + :owner_usernames, + :membership_request_template ) end end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index d73b87e4d6c..e8e2aed2c6c 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -239,6 +239,8 @@ class GroupsController < ApplicationController end def request_membership + params.require(:reason) + unless current_user.staff? RateLimiter.new(current_user, "request_group_membership", 1, 1.day).performed! end @@ -255,7 +257,7 @@ class GroupsController < ApplicationController 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), + raw: params[:reason], archetype: Archetype.private_message, target_usernames: usernames.join(','), skip_validations: true diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb index bb4fd9ff86d..ca74ab7c596 100644 --- a/app/serializers/basic_group_serializer.rb +++ b/app/serializers/basic_group_serializer.rb @@ -22,7 +22,8 @@ class BasicGroupSerializer < ApplicationSerializer :public_exit, :allow_membership_requests, :full_name, - :default_notification_level + :default_notification_level, + :membership_request_template def include_display_name? object.automatic diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 13f4101c1e7..d009742429d 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -427,6 +427,12 @@ en: closed_group: Closed Group is_group_user: "You are a member of this group" allow_membership_requests: "Allow users to send membership requests to group owners" + membership_request_template: "Custom template to display to users when sending a membership request" + membership_request: + submit: "Submit Request" + title: "Request to join @%{group_name}" + reason: "Let the group owners know why you belong in this group" + membership: "Membership" name: "Name" user_count: "Number of Members" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3e5da27ad55..b3559db9816 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -280,7 +280,6 @@ en: 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/db/migrate/20170731075604_add_membership_request_template_to_groups.rb b/db/migrate/20170731075604_add_membership_request_template_to_groups.rb new file mode 100644 index 00000000000..1e5fc4a9661 --- /dev/null +++ b/db/migrate/20170731075604_add_membership_request_template_to_groups.rb @@ -0,0 +1,5 @@ +class AddMembershipRequestTemplateToGroups < ActiveRecord::Migration + def change + add_column :groups, :membership_request_template, :text + end +end diff --git a/spec/integration/admin/groups_spec.rb b/spec/integration/admin/groups_spec.rb index 0781ff61c23..c553ba561a6 100644 --- a/spec/integration/admin/groups_spec.rb +++ b/spec/integration/admin/groups_spec.rb @@ -15,7 +15,8 @@ RSpec.describe "Managing groups as an admin" do name: 'testing', usernames: [admin.username, user.username].join(","), owner_usernames: [user.username].join(","), - allow_membership_requests: true + allow_membership_requests: true, + membership_request_template: 'Testing', } expect(response).to be_success @@ -25,6 +26,7 @@ RSpec.describe "Managing groups as an admin" do expect(group.name).to eq('testing') expect(group.users).to contain_exactly(admin, user) expect(group.allow_membership_requests).to eq(true) + expect(group.membership_request_template).to eq('Testing') end end diff --git a/spec/integration/groups_spec.rb b/spec/integration/groups_spec.rb index 410b2f77cb6..90d1c61b13a 100644 --- a/spec/integration/groups_spec.rb +++ b/spec/integration/groups_spec.rb @@ -564,6 +564,14 @@ describe "Groups" do end.to raise_error(Discourse::NotLoggedIn) end + it 'requires a reason' do + sign_in(user) + + expect do + xhr :post, "/groups/#{group.name}/request_membership" + end.to raise_error(ActionController::ParameterMissing) + end + it 'should create the right PM' do owner1 = Fabricate(:user, last_seen_at: Time.zone.now) owner2 = Fabricate(:user, last_seen_at: Time.zone.now - 1 .day) @@ -571,7 +579,8 @@ describe "Groups" do sign_in(user) - xhr :post, "/groups/#{group.name}/request_membership" + xhr :post, "/groups/#{group.name}/request_membership", + reason: 'Please add me in' expect(response).to be_success @@ -586,10 +595,7 @@ describe "Groups" do group_name: group.name )) - expect(post.raw).to eq(I18n.t( - 'groups.request_membership_pm.body', group_name: group.name - )) - + expect(post.raw).to eq('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-edit-test.js.es6 b/test/javascripts/acceptance/group-edit-test.js.es6 index 0c28f8af7f4..47fe847cf52 100644 --- a/test/javascripts/acceptance/group-edit-test.js.es6 +++ b/test/javascripts/acceptance/group-edit-test.js.es6 @@ -27,9 +27,7 @@ QUnit.test("Editing group", assert => { assert.ok(find('.group-members-input .item').length === 7, 'it should display group members'); assert.ok(find('.group-members-input-selector').length === 1, 'it should display input to add group members'); assert.ok(find('.group-members-input-selector .add[disabled]').length === 1, 'add members button should be disabled'); - }); - andThen(() => { assert.ok( find('.group-edit-allow-membership-requests[disabled]').length === 1, 'it should disable group allow_membership_request input' @@ -44,6 +42,11 @@ QUnit.test("Editing group", assert => { find('.group-edit-public-admission[disabled]').length === 1, 'it should disable group public admission input' ); + + assert.equal( + find('.group-edit-membership-request-template').length, 1, + 'it should display the membership request template field' + ); }); }); diff --git a/test/javascripts/acceptance/groups-test.js.es6 b/test/javascripts/acceptance/groups-test.js.es6 index 017f672a73e..0038ff093a2 100644 --- a/test/javascripts/acceptance/groups-test.js.es6 +++ b/test/javascripts/acceptance/groups-test.js.es6 @@ -85,6 +85,34 @@ QUnit.test("User Viewing Group", assert => { logIn(); Discourse.reset(); + visit("/groups"); + click('.group-index-request'); + + server.post('/groups/Macdonald/request_membership', () => { // eslint-disable-line no-undef + return [ + 200, + { "Content-Type": "application/json" }, + { relative_url: '/t/internationalization-localization/280' } + ]; + }); + + andThen(() => { + assert.equal(find('.modal-header').text().trim(), I18n.t( + 'groups.membership_request.title', { group_name: 'Macdonald' } + )); + + assert.equal(find('.request-group-membership-form textarea').val(), 'Please add me'); + }); + + click('.modal-footer .btn-primary'); + + andThen(() => { + assert.equal( + find('.fancy-title').text().trim(), + "Internationalization / localization" + ); + }); + visit("/groups/discourse"); click('.group-message-button'); diff --git a/test/javascripts/fixtures/groups-fixtures.js.es6 b/test/javascripts/fixtures/groups-fixtures.js.es6 index bd819b47d59..79f47e64cb1 100644 --- a/test/javascripts/fixtures/groups-fixtures.js.es6 +++ b/test/javascripts/fixtures/groups-fixtures.js.es6 @@ -1,3 +1,3 @@ export default { - "/groups.json": {"groups":[{"id":41,"automatic":false,"name":"discourse","user_count":0,"alias_level":0,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"has_messages":false,"flair_url":null,"flair_bg_color":null,"flair_color":null,"bio_raw":"","bio_cooked":null,"public_admission":true,"allow_membership_requests":false,"full_name":"Awesome Team"},{"id":42,"automatic":false,"name":"Macdonald","user_count":0,"alias_level":99,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"has_messages":false,"flair_url":null,"flair_bg_color":null,"flair_color":null,"bio_raw":null,"bio_cooked":null,"public_admission":false,"allow_membership_requests":true,"full_name":null}],"extras":{"group_user_ids":[]},"total_rows_groups":2,"load_more_groups":"/groups?page=1"} + "/groups.json": {"groups":[{"id":41,"automatic":false,"name":"discourse","user_count":0,"alias_level":0,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"has_messages":false,"flair_url":null,"flair_bg_color":null,"flair_color":null,"bio_raw":"","bio_cooked":null,"public_admission":true,"allow_membership_requests":false,"full_name":"Awesome Team"},{"id":42,"automatic":false,"name":"Macdonald","user_count":0,"alias_level":99,"visible":true,"automatic_membership_email_domains":"","automatic_membership_retroactive":false,"primary_group":false,"title":null,"grant_trust_level":null,"has_messages":false,"flair_url":null,"flair_bg_color":null,"flair_color":null,"bio_raw":null,"bio_cooked":null,"public_admission":false,"allow_membership_requests":true,"membership_request_template":"Please add me","full_name":null}],"extras":{"group_user_ids":[]},"total_rows_groups":2,"load_more_groups":"/groups?page=1"} }