diff --git a/app/assets/javascripts/admin/templates/admin.js.handlebars b/app/assets/javascripts/admin/templates/admin.js.handlebars index 240909f3c..98c1b46dd 100644 --- a/app/assets/javascripts/admin/templates/admin.js.handlebars +++ b/app/assets/javascripts/admin/templates/admin.js.handlebars @@ -9,7 +9,7 @@
  • {{#linkTo 'adminSiteContents'}}{{i18n admin.site_content.title}}{{/linkTo}}
  • {{/if}}
  • {{#linkTo 'adminUsersList.active'}}{{i18n admin.users.title}}{{/linkTo}}
  • - +
  • {{#linkTo 'admin.groups'}}{{i18n admin.groups.title}}{{/linkTo}}
  • {{#linkTo 'admin.email_logs'}}{{i18n admin.email_logs.title}}{{/linkTo}}
  • {{#linkTo 'adminFlags.active'}}{{i18n admin.flags.title}}{{/linkTo}}
  • {{#if Discourse.currentUser.admin}} diff --git a/app/assets/javascripts/discourse/models/category.js b/app/assets/javascripts/discourse/models/category.js index 7a8707391..4044a41e5 100644 --- a/app/assets/javascripts/discourse/models/category.js +++ b/app/assets/javascripts/discourse/models/category.js @@ -15,6 +15,9 @@ Discourse.Category = Discourse.Model.extend({ if (!this.get('color')) this.set('color', Discourse.SiteSettings.uncategorized_color); if (!this.get('text_color')) this.set('text_color', Discourse.SiteSettings.uncategorized_text_color); } + + this.set("availableGroups", Em.A(this.get("available_groups"))); + this.set("groups", Em.A(this.groups)); }, url: function() { @@ -40,7 +43,9 @@ Discourse.Category = Discourse.Model.extend({ name: this.get('name'), color: this.get('color'), text_color: this.get('text_color'), - hotness: this.get('hotness') + hotness: this.get('hotness'), + secure: this.get('secure'), + group_names: this.get('groups').join(",") }, type: this.get('id') ? 'PUT' : 'POST' }); @@ -48,6 +53,17 @@ Discourse.Category = Discourse.Model.extend({ destroy: function(callback) { return Discourse.ajax("/categories/" + (this.get('slug') || this.get('id')), { type: 'DELETE' }); + }, + + addGroup: function(group){ + this.get("groups").addObject(group); + this.get("availableGroups").removeObject(group); + }, + + + removeGroup: function(group){ + this.get("groups").removeObject(group); + this.get("availableGroups").addObject(group); } }); diff --git a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars index b927eca69..74fcc303c 100644 --- a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars @@ -23,10 +23,30 @@ {{/if}} +
    + + + {{#if secure}} +
    + {{i18n category.allowed_groups}} + {{#each groups}} + {{this}} x + {{/each}} +
    + {{view Ember.Select contentBinding="availableGroups" valueBinding="view.selectedGroup"}} + + {{/if}} +
    + + {{/unless}}
    @@ -58,4 +78,4 @@ -{{/with}} \ No newline at end of file +{{/with}} diff --git a/app/assets/javascripts/discourse/views/modal/edit_category_view.js b/app/assets/javascripts/discourse/views/modal/edit_category_view.js index 055296fd1..264c76efb 100644 --- a/app/assets/javascripts/discourse/views/modal/edit_category_view.js +++ b/app/assets/javascripts/discourse/views/modal/edit_category_view.js @@ -93,6 +93,16 @@ Discourse.EditCategoryView = Discourse.ModalBodyView.extend({ return false; }, + addGroup: function(){ + this.get("category").addGroup(this.get("selectedGroup")); + }, + + removeGroup: function(group){ + // OBVIOUS, Ember treats this as Ember.String, we need a real string here + group = group + ""; + this.get("category").removeGroup(group); + }, + saveCategory: function() { var categoryView = this; this.set('saving', true); diff --git a/app/assets/stylesheets/application/colorpicker.css.scss b/app/assets/stylesheets/application/colorpicker.css.scss index fd02861fa..ea43917b6 100644 --- a/app/assets/stylesheets/application/colorpicker.css.scss +++ b/app/assets/stylesheets/application/colorpicker.css.scss @@ -18,7 +18,7 @@ vertical-align: middle; padding-top: 4px; padding-left: 15px; - max-width: 320px; + max-width: 300px; .colorpicker { border: 1px solid $darkish_gray; diff --git a/app/assets/stylesheets/vendor/bootstrap.css.scss b/app/assets/stylesheets/vendor/bootstrap.css.scss index 734b3cd2f..2b3803dad 100644 --- a/app/assets/stylesheets/vendor/bootstrap.css.scss +++ b/app/assets/stylesheets/vendor/bootstrap.css.scss @@ -265,7 +265,6 @@ body { } select, textarea { display: inline-block; - height: 18px; padding: 4px; margin-bottom: 9px; font-size: 13px; diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index de9946abf..7cb0ab4fa 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -48,7 +48,7 @@ class CategoriesController < ApplicationController end def category_param_keys - [required_param_keys, :hotness].flatten! + [required_param_keys, :hotness, :secure, :group_names].flatten! end def category_params diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index cfc5971fd..c4fb6c7b8 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -92,7 +92,7 @@ class PostsController < ApplicationController result = {post: post_serializer.as_json} if revisor.category_changed.present? - result[:category] = CategorySerializer.new(revisor.category_changed, scope: guardian, root: false).as_json + result[:category] = BasicCategorySerializer.new(revisor.category_changed, scope: guardian, root: false).as_json end render_json_dump(result) diff --git a/app/models/category.rb b/app/models/category.rb index 92d5ff70d..d4d37930c 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -99,6 +99,15 @@ class Category < ActiveRecord::Base self.secure end + def group_names=(names) + # this line bothers me, destroying in AR can not seem to be queued, thinking of extending it + category_groups.destroy_all unless new_record? + ids = Group.where(name: names.split(",")).select(:id).map(&:id) + ids.each do |id| + category_groups.build(group_id: id) + end + end + def deny(group) if group == :all self.secure = true @@ -108,7 +117,8 @@ class Category < ActiveRecord::Base def allow(group) if group == :all self.secure = false - category_groups.clear + # this is kind of annoying, there should be a clean way of queuing this stuff + category_groups.destroy_all unless new_record? else groups.push(group) end diff --git a/app/models/category_list.rb b/app/models/category_list.rb index 9afb9e330..cca2b731e 100644 --- a/app/models/category_list.rb +++ b/app/models/category_list.rb @@ -9,7 +9,15 @@ class CategoryList .includes(:featured_users) .where('topics.visible' => true) .order('categories.topics_week desc, categories.topics_month desc, categories.topics_year desc') - .to_a + + allowed_ids = current_user ? current_user.secure_category_ids : nil + if allowed_ids.present? + @categories = @categories.where("categories.secure = 'f' OR categories.id in (?)", allowed_ids) + else + @categories = @categories.where("categories.secure = 'f'") + end + + @categories = @categories.to_a # Support for uncategorized topics uncategorized_topics = Topic diff --git a/app/models/user.rb b/app/models/user.rb index 93b7d21a8..13abe4fbc 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -564,7 +564,7 @@ class User < ActiveRecord::Base end def secure_category_ids - cats = self.moderator? ? Category.select(:id).where(:secure, true) : secure_categories.select('categories.id') + cats = self.staff? ? Category.select(:id).where(secure: true) : secure_categories.select('categories.id') cats.map{|c| c.id} end diff --git a/app/serializers/basic_category_serializer.rb b/app/serializers/basic_category_serializer.rb new file mode 100644 index 000000000..ace9b75d8 --- /dev/null +++ b/app/serializers/basic_category_serializer.rb @@ -0,0 +1,13 @@ +class BasicCategorySerializer < ApplicationSerializer + + attributes :id, + :name, + :color, + :text_color, + :slug, + :topic_count, + :description, + :topic_url, + :hotness + +end diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index 9e640780c..2e8037422 100644 --- a/app/serializers/category_serializer.rb +++ b/app/serializers/category_serializer.rb @@ -1,13 +1,13 @@ -class CategorySerializer < ApplicationSerializer +class CategorySerializer < BasicCategorySerializer - attributes :id, - :name, - :color, - :text_color, - :slug, - :topic_count, - :description, - :topic_url, - :hotness + attributes :secure, :groups, :available_groups + + def groups + @groups ||= object.groups.order("name").all.map(&:name) + end + + def available_groups + Group.order("name").map(&:name) - groups + end end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 31fbcb2c0..d0af195b2 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -758,6 +758,11 @@ en: change_in_category_topic: "Edit Description" hotness: "Hotness" already_used: 'This color has been used by another category' + is_secure: "Secure category?" + add_group: "Add Group" + security: "Security" + allowed_groups: "Allowed Groups:" + flagging: title: 'Why are you flagging this post?' diff --git a/spec/components/category_list_spec.rb b/spec/components/category_list_spec.rb index 470033def..dbea089c5 100644 --- a/spec/components/category_list_spec.rb +++ b/spec/components/category_list_spec.rb @@ -16,23 +16,11 @@ describe CategoryList do let!(:topic) { Fabricate(:topic)} let(:category) { category_list.categories.first } - it "has a category" do + it "has the right category" do category.should be_present - end - - it "has the uncategorized label" do category.name.should == SiteSetting.uncategorized_name - end - - it "has the uncategorized slug" do category.slug.should == SiteSetting.uncategorized_name - end - - it "has one topic this week" do category.topics_week.should == 1 - end - - it "contains the topic in featured_topics" do category.featured_topics.should == [topic] end @@ -40,6 +28,23 @@ describe CategoryList do end + context "security" do + it "properly hide secure categories" do + admin = Fabricate(:admin) + user = Fabricate(:user) + + cat = Fabricate(:category) + topic = Fabricate(:topic, category: cat) + cat.deny(:all) + cat.allow(Group[:admins]) + cat.save + + CategoryList.new(admin).categories.count.should == 1 + CategoryList.new(user).categories.count.should == 0 + CategoryList.new(nil).categories.count.should == 0 + end + end + context "with a category" do let!(:topic_category) { Fabricate(:category) } diff --git a/spec/controllers/categories_controller_spec.rb b/spec/controllers/categories_controller_spec.rb index f00169982..7e05df58d 100644 --- a/spec/controllers/categories_controller_spec.rb +++ b/spec/controllers/categories_controller_spec.rb @@ -134,22 +134,21 @@ describe CategoriesController do describe 'success' do before do - xhr :put, :update, id: @category.id, name: 'science', color: '000', text_color: '0ff' + # might as well test this as well + @category.allow(Group[:admins]) + @category.save + + xhr :put, :update, id: @category.id, name: 'science', color: '000', text_color: '0ff', group_names: Group[:staff].name, secure: 'true' @category.reload end - it 'updates the name' do + it 'updates the group correctly' do @category.name.should == 'science' - end - - it 'updates the color' do @category.color.should == '000' - end - - it 'updates the text color' do @category.text_color.should == '0ff' + @category.secure?.should be_true + @category.groups.count.should == 1 end - end end