From 4eb54f08b29a13b2a5653a4df5050c1ca07d2190 Mon Sep 17 00:00:00 2001 From: Mark VanLandingham Date: Mon, 28 Oct 2019 12:46:27 -0500 Subject: [PATCH] FEATURE: Site setting/UI to allow users to set their primary group (#8244) * FEATURE: Site setting/ui to allow users to set their primary group * prettier and remove logic from account template * added 1 to 43 to make web_hook_user_serializer_spec pass --- .../controllers/preferences/account.js.es6 | 14 ++++++-- .../javascripts/discourse/models/user.js.es6 | 3 +- .../routes/preferences-account.js.es6 | 3 +- .../templates/preferences/account.hbs | 13 ++++++++ app/controllers/users_controller.rb | 3 +- app/serializers/user_serializer.rb | 1 + app/services/user_updater.rb | 8 +++++ config/locales/client.en.yml | 3 ++ config/locales/server.en.yml | 2 ++ config/site_settings.yml | 3 ++ lib/guardian.rb | 8 +++++ spec/components/guardian_spec.rb | 33 +++++++++++++++++++ .../web_hook_user_serializer_spec.rb | 2 +- spec/services/user_updater_spec.rb | 25 ++++++++++++++ 14 files changed, 115 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 index a5b9e098a59..732c51ea736 100644 --- a/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences/account.js.es6 @@ -17,7 +17,7 @@ export default Controller.extend(CanCheckEmails, PreferencesTabController, { init() { this._super(...arguments); - this.saveAttrNames = ["name", "title"]; + this.saveAttrNames = ["name", "title", "primary_group_id"]; this.set("revoking", {}); }, @@ -26,6 +26,7 @@ export default Controller.extend(CanCheckEmails, PreferencesTabController, { newNameInput: null, newTitleInput: null, + newPrimaryGroupInput: null, passwordProgress: null, @@ -55,6 +56,14 @@ export default Controller.extend(CanCheckEmails, PreferencesTabController, { canSelectTitle: Ember.computed.gt("model.availableTitles.length", 0), + @computed("model.filteredGroups") + canSelectPrimaryGroup(primaryGroupOptions) { + return ( + primaryGroupOptions.length > 0 && + this.siteSettings.user_selected_primary_groups + ); + }, + @computed("model.is_anonymous") canChangePassword(isAnonymous) { if (isAnonymous) { @@ -131,7 +140,8 @@ export default Controller.extend(CanCheckEmails, PreferencesTabController, { this.model.setProperties({ name: this.newNameInput, - title: this.newTitleInput + title: this.newTitleInput, + primary_group_id: this.newPrimaryGroupInput }); return this.model diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index a16b24f80ea..1665a825ff3 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -266,7 +266,8 @@ const User = RestModel.extend({ "tracked_tags", "watched_tags", "watching_first_post_tags", - "date_of_birth" + "date_of_birth", + "primary_group_id" ]; const data = this.getProperties( diff --git a/app/assets/javascripts/discourse/routes/preferences-account.js.es6 b/app/assets/javascripts/discourse/routes/preferences-account.js.es6 index e654ff06225..6ea1e95b95b 100644 --- a/app/assets/javascripts/discourse/routes/preferences-account.js.es6 +++ b/app/assets/javascripts/discourse/routes/preferences-account.js.es6 @@ -21,7 +21,8 @@ export default RestrictedUserRoute.extend({ controller.setProperties({ model: user, newNameInput: user.get("name"), - newTitleInput: user.get("title") + newTitleInput: user.get("title"), + newPrimaryGroupInput: user.get("primary_group_id") }); }, diff --git a/app/assets/javascripts/discourse/templates/preferences/account.hbs b/app/assets/javascripts/discourse/templates/preferences/account.hbs index 9f71dccd95d..292af19e4d4 100644 --- a/app/assets/javascripts/discourse/templates/preferences/account.hbs +++ b/app/assets/javascripts/discourse/templates/preferences/account.hbs @@ -149,6 +149,19 @@ {{/if}} +{{#if canSelectPrimaryGroup}} +
+ +
+ {{combo-box + value=newPrimaryGroupInput + content=model.filteredGroups + none="user.primary_group.none"}} +
+
+{{/if}} + + {{#if canCheckEmails}}
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 8d8386fa1f4..5498a9933ba 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1429,7 +1429,8 @@ class UsersController < ApplicationController :website, :dismissed_banner_key, :profile_background_upload_url, - :card_background_upload_url + :card_background_upload_url, + :primary_group_id ] editable_custom_fields = User.editable_user_custom_fields(by_staff: current_user.try(:staff?)) diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 43c038ba046..52d1214cfdd 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -72,6 +72,7 @@ class UserSerializer < BasicUserSerializer :profile_view_count, :time_read, :recent_time_read, + :primary_group_id, :primary_group_name, :primary_group_flair_url, :primary_group_flair_bg_color, diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index ac1e489cf85..c5d539e1db4 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -82,6 +82,14 @@ class UserUpdater user.title = attributes[:title] end + if SiteSetting.user_selected_primary_groups && + attributes[:primary_group_id] && + attributes[:primary_group_id] != user.primary_group_id && + guardian.can_use_primary_group?(user, attributes[:primary_group_id]) + + user.primary_group_id = attributes[:primary_group_id] + end + CATEGORY_IDS.each do |attribute, level| if ids = attributes[attribute] CategoryUser.batch_set(user, level, ids) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 471a7f78deb..cad4dfdf51e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1300,6 +1300,9 @@ en: title: title: "Title" none: "(none)" + primary_group: + title: "Primary Group" + none: "(none)" filters: all: "All" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 2209536a1ba..119b727913c 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1928,6 +1928,8 @@ en: clean_up_inactive_users_after_days: "Number of days before an inactive user (trust level 0 without any posts) is removed. To disable clean up set to 0." + user_selected_primary_groups: "Allow users to set their own primary group" + user_website_domains_whitelist: "User website will be verified against these domains. Pipe-delimited list." allow_profile_backgrounds: "Allow users to upload profile backgrounds." diff --git a/config/site_settings.yml b/config/site_settings.yml index ce1fe165750..61ba59496c8 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -575,6 +575,9 @@ users: default: 730 min: 0 max: 3650 + user_selected_primary_groups: + default: false + client: true groups: enable_group_directory: diff --git a/lib/guardian.rb b/lib/guardian.rb index b89f2c6a825..d6c996937d0 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -316,6 +316,14 @@ class Guardian user.groups.where(title: title).exists? end + def can_use_primary_group?(user, group_id = nil) + return false if !user || !group_id + group = Group.find_by(id: group_id.to_i) + + user.group_ids.include?(group_id.to_i) && + (group ? !group.automatic : false) + end + def can_change_primary_group?(user) user && is_staff? end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 34a66c9b8c8..34a2eae70eb 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -2378,6 +2378,39 @@ describe Guardian do end end + describe 'can_use_primary_group?' do + fab!(:group) { Fabricate(:group, title: 'Groupie') } + + it 'is false without a logged in user' do + expect(Guardian.new(nil).can_use_primary_group?(user)).to be_falsey + end + + it 'is false with no group_id' do + user.update(groups: [group]) + expect(Guardian.new(user).can_use_primary_group?(user, nil)).to be_falsey + end + + it 'is false if the group does not exist' do + user.update(groups: [group]) + expect(Guardian.new(user).can_use_primary_group?(user, Group.last.id + 1)).to be_falsey + end + + it 'is false if the user is not a part of the group' do + user.update(groups: []) + expect(Guardian.new(user).can_use_primary_group?(user, group.id)).to be_falsey + end + + it 'is false if the group is automatic' do + user.update(groups: [Group.new(name: 'autooo', automatic: true)]) + expect(Guardian.new(user).can_use_primary_group?(user, group.id)).to be_falsey + end + + it 'is true if the user is a part of the group, and the group is custom' do + user.update(groups: [group]) + expect(Guardian.new(user).can_use_primary_group?(user, group.id)).to be_truthy + end + end + describe 'can_change_trust_level?' do it 'is false without a logged in user' do diff --git a/spec/serializers/web_hook_user_serializer_spec.rb b/spec/serializers/web_hook_user_serializer_spec.rb index 1c7f2ed8a2f..bdf0f593d65 100644 --- a/spec/serializers/web_hook_user_serializer_spec.rb +++ b/spec/serializers/web_hook_user_serializer_spec.rb @@ -23,7 +23,7 @@ RSpec.describe WebHookUserSerializer do it 'should only include the required keys' do count = serializer.as_json.keys.count - difference = count - 43 + difference = count - 44 expect(difference).to eq(0), lambda { message = "" diff --git a/spec/services/user_updater_spec.rb b/spec/services/user_updater_spec.rb index 8e2515498ff..52dc56b5ef0 100644 --- a/spec/services/user_updater_spec.rb +++ b/spec/services/user_updater_spec.rb @@ -249,6 +249,31 @@ describe UserUpdater do end end + context 'when updating primary group' do + let(:new_group) { Group.create(name: 'new_group') } + let(:user) { Fabricate(:user) } + + it 'updates when setting is enabled' do + SiteSetting.user_selected_primary_groups = true + user.groups << new_group + user.update(primary_group_id: nil) + UserUpdater.new(acting_user, user).update(primary_group_id: new_group.id) + + user.reload + expect(user.primary_group_id).to eq new_group.id + end + + it 'does not update when setting is disabled' do + SiteSetting.user_selected_primary_groups = false + user.groups << new_group + user.update(primary_group_id: nil) + UserUpdater.new(acting_user, user).update(primary_group_id: new_group.id) + + user.reload + expect(user.primary_group_id).to eq nil + end + end + context 'when update fails' do it 'returns false' do user = Fabricate(:user)