From 4620dfe92d616bd801abf5fe1c890de30ff0a977 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Fri, 28 Jul 2017 11:37:10 +0900 Subject: [PATCH] FEATURE: Add group settngs to allow users to leave a group freely. https://meta.discourse.org/t/split-join-leave-freely-setting-on-groups/65565 --- .../admin/controllers/admin-group.js.es6 | 6 +- .../javascripts/admin/templates/group.hbs | 13 ++- .../components/group-membership-button.js.es6 | 11 ++- .../javascripts/discourse/models/group.js.es6 | 3 +- .../components/group-membership-button.hbs | 24 +++--- .../discourse/templates/group-edit.hbs | 19 +++- app/controllers/admin/groups_controller.rb | 33 +++++-- app/controllers/groups_controller.rb | 11 +-- app/models/group.rb | 3 +- app/models/watched_word.rb | 15 ++++ app/serializers/basic_group_serializer.rb | 3 +- app/services/group_action_logger.rb | 4 +- config/locales/client.en.yml | 3 +- db/fixtures/002_groups.rb | 4 +- .../20170728012754_split_public_in_groups.rb | 17 ++++ ...0170728054115_remove_public_from_groups.rb | 9 ++ spec/fabricators/group_fabricator.rb | 5 ++ spec/integration/groups_spec.rb | 42 ++++++--- spec/models/group_spec.rb | 2 +- spec/services/group_action_logger_spec.rb | 86 ++++++++++++++++--- .../acceptance/group-edit-test.js.es6 | 26 ++++-- .../group-membership-button-test.js.es6 | 46 ++++++++-- .../controllers/admin-group-test.js.es6 | 14 ++- .../fixtures/group-fixtures.js.es6 | 3 +- .../fixtures/groups-fixtures.js.es6 | 2 +- 25 files changed, 315 insertions(+), 89 deletions(-) create mode 100644 db/migrate/20170728012754_split_public_in_groups.rb create mode 100644 db/migrate/20170728054115_remove_public_from_groups.rb diff --git a/app/assets/javascripts/admin/controllers/admin-group.js.es6 b/app/assets/javascripts/admin/controllers/admin-group.js.es6 index 52dbc07791b..cab20f908f0 100644 --- a/app/assets/javascripts/admin/controllers/admin-group.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-group.js.es6 @@ -31,10 +31,10 @@ export default Ember.Controller.extend({ ]; }.property(), - @computed('model.visibility_level', 'model.public') - disableMembershipRequestSetting(visibility_level, publicGroup) { + @computed('model.visibility_level', 'model.public_admission') + disableMembershipRequestSetting(visibility_level, publicAdmission) { visibility_level = parseInt(visibility_level); - return (visibility_level !== 0) || publicGroup; + return (visibility_level !== 0) || publicAdmission; }, @computed('model.visibility_level', 'model.allow_membership_requests') diff --git a/app/assets/javascripts/admin/templates/group.hbs b/app/assets/javascripts/admin/templates/group.hbs index 7ff651e7c9d..caa25e5ab7b 100644 --- a/app/assets/javascripts/admin/templates/group.hbs +++ b/app/assets/javascripts/admin/templates/group.hbs @@ -61,10 +61,19 @@
+
+ +
+
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 b27ce66f79c..5661aec122a 100644 --- a/app/assets/javascripts/discourse/components/group-membership-button.js.es6 +++ b/app/assets/javascripts/discourse/components/group-membership-button.js.es6 @@ -5,9 +5,14 @@ import DiscourseURL from 'discourse/lib/url'; export default Ember.Component.extend({ loading: false, - @computed("model.public") - canJoinGroup(publicGroup) { - return publicGroup; + @computed("model.public_admission", "userIsGroupUser") + canJoinGroup(publicAdmission, userIsGroupUser) { + return publicAdmission && !userIsGroupUser; + }, + + @computed("model.public_exit", "userIsGroupUser") + canLeaveGroup(publicExit, userIsGroupUser) { + return publicExit && userIsGroupUser; }, @computed("model.is_group_user", "model.id", "groupUserIds") diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6 index 51ad388a1f3..995b536557a 100644 --- a/app/assets/javascripts/discourse/models/group.js.es6 +++ b/app/assets/javascripts/discourse/models/group.js.es6 @@ -144,7 +144,8 @@ const Group = RestModel.extend({ flair_bg_color: this.get('flairBackgroundHexColor'), flair_color: this.get('flairHexColor'), bio_raw: this.get('bio_raw'), - public: this.get('public'), + public_admission: this.get('public_admission'), + public_exit: this.get('public_exit'), allow_membership_requests: this.get('allow_membership_requests'), full_name: this.get('full_name'), default_notification_level: this.get('default_notification_level') 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 2d05e166889..a154287430a 100644 --- a/app/assets/javascripts/discourse/templates/components/group-membership-button.hbs +++ b/app/assets/javascripts/discourse/templates/components/group-membership-button.hbs @@ -1,17 +1,15 @@ {{#if canJoinGroup}} - {{#if userIsGroupUser}} - {{d-button action="leaveGroup" - class="btn-danger group-index-leave" - icon="user-times" - label="groups.leave" - disabled=updatingMembership}} - {{else}} - {{d-button action="joinGroup" - class="group-index-join" - icon="user-plus" - label="groups.join" - disabled=updatingMembership}} - {{/if}} + {{d-button action="joinGroup" + class="group-index-join" + icon="user-plus" + label="groups.join" + disabled=updatingMembership}} +{{else if canLeaveGroup}} + {{d-button action="leaveGroup" + class="btn-danger group-index-leave" + icon="user-times" + label="groups.leave" + disabled=updatingMembership}} {{else if model.allow_membership_requests}} {{#if userIsGroupUser}} {{#if showMembershipStatus}} diff --git a/app/assets/javascripts/discourse/templates/group-edit.hbs b/app/assets/javascripts/discourse/templates/group-edit.hbs index 03122d2abaa..d5552e5327d 100644 --- a/app/assets/javascripts/discourse/templates/group-edit.hbs +++ b/app/assets/javascripts/discourse/templates/group-edit.hbs @@ -21,11 +21,22 @@
+
+ +
+
@@ -34,7 +45,7 @@ {{input type='checkbox' checked=model.allow_membership_requests class="group-edit-allow-membership-requests" - disabled=model.public}} + disabled=model.public_admission}} {{i18n 'groups.allow_membership_requests'}} diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 7d54bb12634..1ad9367aa8e 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -67,7 +67,13 @@ class Admin::GroupsController < Admin::AdminController group.flair_url = group_params[:flair_url].presence group.flair_bg_color = group_params[:flair_bg_color].presence group.flair_color = group_params[:flair_color].presence - group.public = group_params[:public] if group_params[:public] + + %i{public_admission public_exit}.each do |key| + if group_params[key] + group.public_send("#{key}=", group_params[key]) + end + end + group.bio_raw = group_params[:bio_raw] if group_params[:bio_raw] group.full_name = group_params[:full_name] if group_params[:full_name] @@ -169,11 +175,26 @@ class Admin::GroupsController < Admin::AdminController def group_params params.require(:group).permit( - :name, :alias_level, :visibility_level, :automatic_membership_email_domains, - :automatic_membership_retroactive, :title, :primary_group, - :grant_trust_level, :incoming_email, :flair_url, :flair_bg_color, - :flair_color, :bio_raw, :public, :allow_membership_requests, :full_name, - :default_notification_level, :usernames, :owner_usernames + :name, + :alias_level, + :visibility_level, + :automatic_membership_email_domains, + :automatic_membership_retroactive, + :title, + :primary_group, + :grant_trust_level, + :incoming_email, + :flair_url, + :flair_bg_color, + :flair_color, + :bio_raw, + :public_admission, + :public_exit, + :allow_membership_requests, + :full_name, + :default_notification_level, + :usernames, + :owner_usernames ) end end diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 504dc251371..d73b87e4d6c 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -146,7 +146,7 @@ class GroupsController < ApplicationController def add_members group = Group.find(params[:id]) - group.public ? ensure_logged_in : guardian.ensure_can_edit!(group) + group.public_admission ? ensure_logged_in : guardian.ensure_can_edit!(group) users = if params[:usernames].present? @@ -163,7 +163,7 @@ class GroupsController < ApplicationController raise Discourse::NotFound if users.blank? - if group.public + if group.public_admission if !guardian.can_log_group_changes?(group) && current_user != users.first raise Discourse::InvalidAccess end @@ -201,7 +201,7 @@ class GroupsController < ApplicationController def remove_member group = Group.find(params[:id]) - group.public ? ensure_logged_in : guardian.ensure_can_edit!(group) + group.public_exit ? ensure_logged_in : guardian.ensure_can_edit!(group) user = if params[:user_id].present? @@ -216,7 +216,7 @@ class GroupsController < ApplicationController raise Discourse::NotFound unless user - if group.public + if group.public_exit if !guardian.can_log_group_changes?(group) && current_user != user raise Discourse::InvalidAccess end @@ -322,7 +322,8 @@ class GroupsController < ApplicationController :flair_color, :bio_raw, :full_name, - :public, + :public_admission, + :public_exit, :allow_membership_requests ) end diff --git a/app/models/group.rb b/app/models/group.rb index f7f37ab4105..f137ec8931c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -619,11 +619,12 @@ end # flair_color :string # bio_raw :text # bio_cooked :text -# public :boolean default(FALSE), not null +# public_admission :boolean default(FALSE), not null # allow_membership_requests :boolean default(FALSE), not null # full_name :string # default_notification_level :integer default(3), not null # visibility_level :integer default(0), not null +# public_exit :boolean default(FALSE), not null # # Indexes # diff --git a/app/models/watched_word.rb b/app/models/watched_word.rb index a0cc43e867a..d9af0071de1 100644 --- a/app/models/watched_word.rb +++ b/app/models/watched_word.rb @@ -51,3 +51,18 @@ class WatchedWord < ActiveRecord::Base end end + +# == Schema Information +# +# Table name: watched_words +# +# id :integer not null, primary key +# word :string not null +# action :integer not null +# created_at :datetime +# updated_at :datetime +# +# Indexes +# +# index_watched_words_on_action_and_word (action,word) UNIQUE +# diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb index e9cac82f26c..bb4fd9ff86d 100644 --- a/app/serializers/basic_group_serializer.rb +++ b/app/serializers/basic_group_serializer.rb @@ -18,7 +18,8 @@ class BasicGroupSerializer < ApplicationSerializer :flair_color, :bio_raw, :bio_cooked, - :public, + :public_admission, + :public_exit, :allow_membership_requests, :full_name, :default_notification_level diff --git a/app/services/group_action_logger.rb b/app/services/group_action_logger.rb index 9f1f429a8f2..58674016c37 100644 --- a/app/services/group_action_logger.rb +++ b/app/services/group_action_logger.rb @@ -24,7 +24,7 @@ class GroupActionLogger end def log_add_user_to_group(target_user) - @group.public || can_edit? + (target_user == @acting_user && @group.public_admission) || can_edit? GroupHistory.create!(default_params.merge( action: GroupHistory.actions[:add_user_to_group], @@ -33,7 +33,7 @@ class GroupActionLogger end def log_remove_user_from_group(target_user) - @group.public || can_edit? + (target_user == @acting_user && @group.public_exit) || can_edit? GroupHistory.create!(default_params.merge( action: GroupHistory.actions[:remove_user_from_group], diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 467e3a799b5..a8082a95de4 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -409,7 +409,8 @@ en: add_members: "Add Members" delete_member_confirm: "Remove '%{username}' from the '%{group}' group?" name_placeholder: "Group name, no spaces, same as username rule" - public: "Allow users to join/leave the group freely (Requires publicly visible group)" + public_admission: "Allow users to join the group freely (Requires publicly visible group)" + public_exit: "Allow users to leave the group freely" empty: posts: "There are no posts by members of this group." members: "There are no members in this group." diff --git a/db/fixtures/002_groups.rb b/db/fixtures/002_groups.rb index 487263f6abe..d7ae62c9445 100644 --- a/db/fixtures/002_groups.rb +++ b/db/fixtures/002_groups.rb @@ -7,8 +7,8 @@ Group.where(name: 'everyone').update_all(visibility_level: Group.visibility_leve ColumnDropper.drop( table: 'groups', - after_migration: 'AddVisibleBackToGroups', - columns: %w[visible], + after_migration: 'RemovePublicFromGroups', + columns: %w[visible public], on_drop: ->() { STDERR.puts 'Removing superflous visible group column!' } diff --git a/db/migrate/20170728012754_split_public_in_groups.rb b/db/migrate/20170728012754_split_public_in_groups.rb new file mode 100644 index 00000000000..31303986c53 --- /dev/null +++ b/db/migrate/20170728012754_split_public_in_groups.rb @@ -0,0 +1,17 @@ +class SplitPublicInGroups < ActiveRecord::Migration + def up + add_column :groups, :public_exit, :boolean, default: false, null: false + add_column :groups, :public_admission, :boolean, default: false, null: false + + ActiveRecord::Base.exec_sql <<~SQL + UPDATE groups + SET public_exit = true, public_admission = true + WHERE public = true + SQL + end + + def down + remove_column :groups, :public_exit + remove_column :groups, :public_admission + end +end diff --git a/db/migrate/20170728054115_remove_public_from_groups.rb b/db/migrate/20170728054115_remove_public_from_groups.rb new file mode 100644 index 00000000000..216bed0a6bb --- /dev/null +++ b/db/migrate/20170728054115_remove_public_from_groups.rb @@ -0,0 +1,9 @@ +class RemovePublicFromGroups < ActiveRecord::Migration + def up + # Defer dropping of the columns until the new application code has been deployed. + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/spec/fabricators/group_fabricator.rb b/spec/fabricators/group_fabricator.rb index d190d520e69..a4c80237c85 100644 --- a/spec/fabricators/group_fabricator.rb +++ b/spec/fabricators/group_fabricator.rb @@ -1,3 +1,8 @@ Fabricator(:group) do name { sequence(:name) { |n| "my_group_#{n}" } } end + +Fabricator(:public_group, from: :group) do + public_admission true + public_exit true +end diff --git a/spec/integration/groups_spec.rb b/spec/integration/groups_spec.rb index 5fbb9ffcaa6..410b2f77cb6 100644 --- a/spec/integration/groups_spec.rb +++ b/spec/integration/groups_spec.rb @@ -80,7 +80,14 @@ describe "Groups" do end describe "group can be updated" do - let(:group) { Fabricate(:group, name: 'test', users: [user], public: false) } + let(:group) do + Fabricate(:group, + name: 'test', + users: [user], + public_admission: false, + public_exit: false + ) + end before do sign_in(user) @@ -102,10 +109,11 @@ describe "Groups" do flair_url: 'fa-adjust', bio_raw: 'testing', full_name: 'awesome team', - public: true, + public_admission: true, + public_exit: true, allow_membership_requests: true } - end.to change { GroupHistory.count }.by(7) + end.to change { GroupHistory.count }.by(8) expect(response).to be_success @@ -116,7 +124,8 @@ describe "Groups" do expect(group.flair_url).to eq('fa-adjust') expect(group.bio_raw).to eq('testing') expect(group.full_name).to eq('awesome team') - expect(group.public).to eq(true) + expect(group.public_admission).to eq(true) + expect(group.public_exit).to eq(true) expect(group.allow_membership_requests).to eq(true) expect(GroupHistory.last.subject).to eq('allow_membership_requests') end @@ -225,7 +234,10 @@ describe "Groups" do context 'public group' do it 'should be fobidden' do - group.update_attributes!(public: true) + group.update_attributes!( + public_admission: true, + public_exit: true + ) expect { xhr :put, "/groups/#{group.id}/members", usernames: "bob" } .to raise_error(Discourse::NotLoggedIn) @@ -341,7 +353,10 @@ describe "Groups" do let(:other_user) { Fabricate(:user) } before do - group.update!(public: true) + group.update!( + public_admission: true, + public_exit: true + ) end context 'admin' do @@ -424,11 +439,7 @@ describe "Groups" do context 'public group' do let(:other_user) { Fabricate(:user) } - let(:group) { Fabricate(:group, users: [other_user]) } - - before do - group.update!(public: true) - end + let(:group) { Fabricate(:public_group, users: [other_user]) } context "admin" do it "removes by username" do @@ -484,7 +495,12 @@ describe "Groups" do context 'public group' do before do group.add_owner(user) - group.update_attributes!(public: true) + + group.update_attributes!( + public_admission: true, + public_exit: true + ) + GroupActionLogger.new(user, group).log_change_group_settings sign_in(user) end @@ -497,7 +513,7 @@ describe "Groups" do result = JSON.parse(response.body)["logs"].first expect(result["action"]).to eq(GroupHistory.actions[1].to_s) - expect(result["subject"]).to eq('public') + expect(result["subject"]).to eq('public_exit') expect(result["prev_value"]).to eq('f') expect(result["new_value"]).to eq('t') end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index ddf7fb47270..081577ca1c4 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -531,7 +531,7 @@ describe Group do let(:category) { Fabricate(:category) } it "should publish the group's categories to the client" do - group.update!(public: true, categories: [category]) + group.update!(public_admission: true, categories: [category]) message = MessageBus.track_publish { group.add(user) }.first diff --git a/spec/services/group_action_logger_spec.rb b/spec/services/group_action_logger_spec.rb index 0ec4efe2617..ec08846c770 100644 --- a/spec/services/group_action_logger_spec.rb +++ b/spec/services/group_action_logger_spec.rb @@ -36,32 +36,90 @@ RSpec.describe GroupActionLogger do end describe '#log_add_user_to_group' do - it 'should create the right record' do - subject.log_add_user_to_group(user) + describe 'as a group owner' do + it 'should create the right record' do + subject.log_add_user_to_group(user) - group_history = GroupHistory.last + group_history = GroupHistory.last - expect(group_history.action).to eq(GroupHistory.actions[:add_user_to_group]) - expect(group_history.acting_user).to eq(group_owner) - expect(group_history.target_user).to eq(user) + expect(group_history.action).to eq(GroupHistory.actions[:add_user_to_group]) + expect(group_history.acting_user).to eq(group_owner) + expect(group_history.target_user).to eq(user) + end + end + + context 'as a normal user' do + subject { described_class.new(user, group) } + + describe 'user cannot freely exit group' do + it 'should not be allowed to log the action' do + expect { subject.log_add_user_to_group(user) } + .to raise_error(Discourse::InvalidParameters) + end + end + + describe 'user can freely exit group' do + before do + group.update!(public_admission: true) + end + + it 'should create the right record' do + subject.log_add_user_to_group(user) + + group_history = GroupHistory.last + + expect(group_history.action).to eq(GroupHistory.actions[:add_user_to_group]) + expect(group_history.acting_user).to eq(user) + expect(group_history.target_user).to eq(user) + end + end end end describe '#log_remove_user_from_group' do - it 'should create the right record' do - subject.log_remove_user_from_group(user) + describe 'as group owner' do + it 'should create the right record' do + subject.log_remove_user_from_group(user) - group_history = GroupHistory.last + group_history = GroupHistory.last - expect(group_history.action).to eq(GroupHistory.actions[:remove_user_from_group]) - expect(group_history.acting_user).to eq(group_owner) - expect(group_history.target_user).to eq(user) + expect(group_history.action).to eq(GroupHistory.actions[:remove_user_from_group]) + expect(group_history.acting_user).to eq(group_owner) + expect(group_history.target_user).to eq(user) + end + end + + context 'as a normal user' do + subject { described_class.new(user, group) } + + describe 'user cannot freely exit group' do + it 'should not be allowed to log the action' do + expect { subject.log_remove_user_from_group(user) } + .to raise_error(Discourse::InvalidParameters) + end + end + + describe 'user can freely exit group' do + before do + group.update!(public_exit: true) + end + + it 'should create the right record' do + subject.log_remove_user_from_group(user) + + group_history = GroupHistory.last + + expect(group_history.action).to eq(GroupHistory.actions[:remove_user_from_group]) + expect(group_history.acting_user).to eq(user) + expect(group_history.target_user).to eq(user) + end + end end end describe '#log_change_group_settings' do it 'should create the right record' do - group.update_attributes!(public: true, created_at: Time.zone.now) + group.update_attributes!(public_admission: true, created_at: Time.zone.now) expect { subject.log_change_group_settings }.to change { GroupHistory.count }.by(1) @@ -69,7 +127,7 @@ RSpec.describe GroupActionLogger do expect(group_history.action).to eq(GroupHistory.actions[:change_group_setting]) expect(group_history.acting_user).to eq(group_owner) - expect(group_history.subject).to eq('public') + expect(group_history.subject).to eq('public_admission') expect(group_history.prev_value).to eq('f') expect(group_history.new_value).to eq('t') end diff --git a/test/javascripts/acceptance/group-edit-test.js.es6 b/test/javascripts/acceptance/group-edit-test.js.es6 index 676ee7a2fc1..0c28f8af7f4 100644 --- a/test/javascripts/acceptance/group-edit-test.js.es6 +++ b/test/javascripts/acceptance/group-edit-test.js.es6 @@ -12,7 +12,17 @@ QUnit.test("Editing group", assert => { assert.ok(find('.group-flair-inputs').length === 1, 'it should display avatar flair inputs'); assert.ok(find('.group-edit-bio').length === 1, 'it should display group bio input'); assert.ok(find('.group-edit-full-name').length === 1, 'it should display group full name input'); - assert.ok(find('.group-edit-public').length === 1, 'it should display group public input'); + + assert.ok( + find('.group-edit-public-admission').length === 1, + 'it should display group public admission input' + ); + + assert.ok( + find('.group-edit-public-exit').length === 1, + 'it should display group public exit input' + ); + assert.ok(find('.group-edit-allow-membership-requests').length === 1, 'it should display group allow_membership_requets input'); 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'); @@ -20,14 +30,20 @@ QUnit.test("Editing group", assert => { }); andThen(() => { - assert.ok(find('.group-edit-allow-membership-requests[disabled]').length === 1, 'it should disable group allow_membership_request input'); + assert.ok( + find('.group-edit-allow-membership-requests[disabled]').length === 1, + 'it should disable group allow_membership_request input' + ); }); - click('.group-edit-public'); + click('.group-edit-public-admission'); click('.group-edit-allow-membership-requests'); andThen(() => { - assert.ok(find('.group-edit-public[disabled]').length === 1, 'it should disable group public input'); + assert.ok( + find('.group-edit-public-admission[disabled]').length === 1, + 'it should disable group public admission input' + ); }); }); @@ -37,4 +53,4 @@ QUnit.test("Editing group as an anonymous user", assert => { andThen(() => { assert.ok(count('.group-members tr') > 0, "it should redirect to members page for an anonymous user"); }); -}); \ No newline at end of file +}); diff --git a/test/javascripts/components/group-membership-button-test.js.es6 b/test/javascripts/components/group-membership-button-test.js.es6 index 9f404c151b5..1937d380886 100644 --- a/test/javascripts/components/group-membership-button-test.js.es6 +++ b/test/javascripts/components/group-membership-button-test.js.es6 @@ -2,18 +2,52 @@ moduleFor('component:group-membership-button'); QUnit.test('canJoinGroup', function(assert) { this.subject().setProperties({ - model: { public: false } + model: { public_admission: false, is_group_user: true } }); - assert.equal(this.subject().get("canJoinGroup"), false, "non public group cannot be joined"); + assert.equal( + this.subject().get("canJoinGroup"), false, + "can't join group if public_admission is false" + ); - this.subject().set("model.public", true); + this.subject().set("model.public_admission", true); - assert.equal(this.subject().get("canJoinGroup"), true, "public group can be joined"); + assert.equal( + this.subject().get("canJoinGroup"), false, + "can't join group if user is already in the group" + ); - this.subject().setProperties({ currentUser: null, model: { public: true } }); + this.subject().set("model.is_group_user", false); - assert.equal(this.subject().get("canJoinGroup"), true, "can't join group when not logged in"); + assert.equal( + this.subject().get("canJoinGroup"), true, + "allowed to join group" + ); +}); + +QUnit.test('canLeaveGroup', function(assert) { + this.subject().setProperties({ + model: { public_exit: false, is_group_user: false } + }); + + assert.equal( + this.subject().get("canLeaveGroup"), false, + "can't leave group if public_exit is false" + ); + + this.subject().set("model.public_exit", true); + + assert.equal( + this.subject().get("canLeaveGroup"), false, + "can't leave group if user is not in the group" + ); + + this.subject().set("model.is_group_user", true); + + assert.equal( + this.subject().get("canLeaveGroup"), true, + "allowed to leave group" + ); }); QUnit.test('userIsGroupUser', function(assert) { diff --git a/test/javascripts/controllers/admin-group-test.js.es6 b/test/javascripts/controllers/admin-group-test.js.es6 index b03ddf7422b..01d77cb5ee3 100644 --- a/test/javascripts/controllers/admin-group-test.js.es6 +++ b/test/javascripts/controllers/admin-group-test.js.es6 @@ -20,16 +20,22 @@ QUnit.test("disablePublicSetting", function(assert) { QUnit.test("disableMembershipRequestSetting", function(assert) { this.subject().setProperties({ - model: { visibility_level: 1, public: false, canEveryoneMention: true } + model: { visibility_level: 1, public_admission: false, canEveryoneMention: true } }); assert.equal(this.subject().get("disableMembershipRequestSetting"), true, "it should disable setting"); this.subject().set("model.visibility_level", 0); - assert.equal(this.subject().get("disableMembershipRequestSetting"), false, "it should enable setting"); + assert.equal( + this.subject().get("disableMembershipRequestSetting"), false, + "it should enable setting" + ); - this.subject().set("model.public", true); + this.subject().set("model.public_admission", true); - assert.equal(this.subject().get("disableMembershipRequestSetting"), true, "it should disalbe setting"); + assert.equal( + this.subject().get("disableMembershipRequestSetting"), true, + "it should disable setting" + ); }); diff --git a/test/javascripts/fixtures/group-fixtures.js.es6 b/test/javascripts/fixtures/group-fixtures.js.es6 index dbc9d32d579..291db11d23d 100644 --- a/test/javascripts/fixtures/group-fixtures.js.es6 +++ b/test/javascripts/fixtures/group-fixtures.js.es6 @@ -8,7 +8,8 @@ export default { "user_count":8, "alias_level":99, "visible":true, - "public":true, + "public_admission":true, + "public_exit":true, "flair_url": 'fa-adjust', "is_group_owner":true, "mentionable":true diff --git a/test/javascripts/fixtures/groups-fixtures.js.es6 b/test/javascripts/fixtures/groups-fixtures.js.es6 index 1baa42b560e..bd819b47d59 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":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":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,"full_name":null}],"extras":{"group_user_ids":[]},"total_rows_groups":2,"load_more_groups":"/groups?page=1"} }