diff --git a/app/assets/javascripts/discourse/models/category.js b/app/assets/javascripts/discourse/models/category.js index 4c63b01ce54..029fca27e66 100644 --- a/app/assets/javascripts/discourse/models/category.js +++ b/app/assets/javascripts/discourse/models/category.js @@ -49,24 +49,27 @@ Discourse.Category = Discourse.Model.extend({ } return Discourse.ajax(url, { - contentType: 'application/json', - dataType: 'json', - data: JSON.stringify({ + data: { name: this.get('name'), color: this.get('color'), text_color: this.get('text_color'), hotness: this.get('hotness'), secure: this.get('secure'), - permissions: _.map( - this.get('permissions'), function(p){ - return { group_name: p.group_name, permission_type: p.permission.id}; - }), + permissions: this.get('permissionsForUpdate'), auto_close_days: this.get('auto_close_days') - }), + }, type: this.get('id') ? 'PUT' : 'POST' }); }, + permissionsForUpdate: function(){ + var rval = {}; + _.each(this.get("permissions"),function(p){ + rval[p.group_name] = p.permission.id; + }); + return rval; + }.property("permissions"), + destroy: function(callback) { return Discourse.ajax("/categories/" + (this.get('slug') || this.get('id')), { type: 'DELETE' }); }, diff --git a/app/assets/javascripts/discourse/routes/list_categories_route.js b/app/assets/javascripts/discourse/routes/list_categories_route.js index 6ccd4b0cec7..15bb2ee7e01 100644 --- a/app/assets/javascripts/discourse/routes/list_categories_route.js +++ b/app/assets/javascripts/discourse/routes/list_categories_route.js @@ -12,7 +12,10 @@ Discourse.ListCategoriesRoute = Discourse.Route.extend(Discourse.ModelReady, { events: { createCategory: function() { - Discourse.Route.showModal(this, 'editCategory', Discourse.Category.create({ color: 'AB9364', text_color: 'FFFFFF', hotness: 5 })); + Discourse.Route.showModal(this, 'editCategory', Discourse.Category.create({ + color: 'AB9364', text_color: 'FFFFFF', hotness: 5, group_permissions: [{group_name: "everyone", permission_type: 1}], + available_groups: Discourse.Site.instance().group_names + })); this.controllerFor('editCategory').set('selectedTab', 'general'); } }, 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 a15f34b4353..6a73794be57 100644 --- a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars +++ b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars @@ -60,17 +60,17 @@ {{#unless isUncategorized}}
-
diff --git a/app/assets/javascripts/discourse/views/category_chooser_view.js b/app/assets/javascripts/discourse/views/category_chooser_view.js index 0808fb89726..c37aacd95b0 100644 --- a/app/assets/javascripts/discourse/views/category_chooser_view.js +++ b/app/assets/javascripts/discourse/views/category_chooser_view.js @@ -14,7 +14,10 @@ Discourse.CategoryChooserView = Discourse.ComboboxView.extend({ init: function() { this._super(); - this.set('content', Discourse.Category.list()); + // TODO perhaps allow passing a param in to select if we need full or not + this.set('content', _.filter(Discourse.Category.list(), function(c){ + return c.permission == Discourse.PermissionType.FULL; + })); }, none: function() { diff --git a/app/assets/stylesheets/application/modal.css.scss b/app/assets/stylesheets/application/modal.css.scss index 9306705d03a..df574efb6a6 100644 --- a/app/assets/stylesheets/application/modal.css.scss +++ b/app/assets/stylesheets/application/modal.css.scss @@ -262,3 +262,22 @@ } } } + +.permission-selector{ + width: 300px; +} +.permission-list{ + list-style:none; + margin: 0 0 15px; + padding: 0; + .name { + display: inline-block; + min-width: 80px; + } + .icon-remove-sign { + margin-left: 5px; + } + li { + margin-bottom: 10px; + } +} diff --git a/app/assets/stylesheets/components/badges.css.scss b/app/assets/stylesheets/components/badges.css.scss index 829b306a774..2b8d0eae8b0 100644 --- a/app/assets/stylesheets/components/badges.css.scss +++ b/app/assets/stylesheets/components/badges.css.scss @@ -82,7 +82,7 @@ .badge-group { @extend %badge; - padding: 3px 2px 3px 8px; + padding: 3px 5px; color: $black; text-shadow: 0 1px 0 rgba($white, 0.2); background-color: #ddd; diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index de5c1e83389..4684c845e2e 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -52,16 +52,18 @@ class CategoriesController < ApplicationController [:name, :color, :text_color] end - def category_param_keys - [required_param_keys, :hotness, :read_restricted, :permissions, :auto_close_days].flatten! - end - def category_params required_param_keys.each do |key| params.require(key) end - params.permit(*category_param_keys) + if p = params[:permissions] + p.each do |k,v| + p[k] = v.to_i + end + end + + params.permit(*required_param_keys, :hotness, :auto_close_days, :permissions => [*p.try(:keys)]) end def fetch_category diff --git a/app/models/category.rb b/app/models/category.rb index d4a7594ef30..20f971f8df4 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -50,7 +50,9 @@ class Category < ActiveRecord::Base } delegate :post_template, to: 'self.class' - attr_accessor :displayable_topics + # permission is just used by serialization + # we may consider wrapping this in another spot + attr_accessor :displayable_topics, :permission def self.scoped_to_permissions(guardian, permission_types) @@ -64,17 +66,19 @@ class Category < ActiveRecord::Base SELECT c.id FROM categories c WHERE ( NOT c.read_restricted AND - NOT EXISTS( - SELECT 1 FROM category_groups cg WHERE cg.category_id = categories.id ) - ) OR EXISTS( - SELECT 1 FROM category_groups cg - WHERE permission_type in (?) AND - cg.category_id = categories.id AND - group_id IN ( - SELECT g.group_id FROM group_users g where g.user_id = ? - ) + ( + NOT EXISTS( + SELECT 1 FROM category_groups cg WHERE cg.category_id = categories.id ) + ) OR EXISTS( + SELECT 1 FROM category_groups cg + WHERE permission_type in (?) AND + cg.category_id = categories.id AND + group_id IN ( + SELECT g.group_id FROM group_users g where g.user_id = ? UNION SELECT ? + ) + ) ) - )", permission_types,(!guardian || guardian.user.blank?) ? -1 : guardian.user.id) + )", permission_types,(!guardian || guardian.user.blank?) ? -1 : guardian.user.id, Group[:everyone].id) end end @@ -169,6 +173,10 @@ class Category < ActiveRecord::Base # on save. end + def permissions=(permissions) + set_permissions(permissions) + end + def apply_permissions if @permissions category_groups.destroy_all @@ -196,7 +204,7 @@ class Category < ActiveRecord::Base group = group.id if Group === group # subtle, using Group[] ensures the group exists in the DB - group = Group[group].id unless Fixnum === group + group = Group[group.to_sym].id unless Fixnum === group permission = CategoryGroup.permission_types[permission] unless Fixnum === permission [group, permission] diff --git a/app/models/site.rb b/app/models/site.rb index 60acda463d3..d4aa6d65f2c 100644 --- a/app/models/site.rb +++ b/app/models/site.rb @@ -25,11 +25,24 @@ class Site TrustLevel.all end + def group_names + @group_name ||= Group.pluck(:name) + end + def categories - Category - .topic_create_allowed(@guardian) - .latest - .includes(:topic_only_relative_url) + @categories ||= begin + categories = Category + .secured(@guardian) + .latest + .includes(:topic_only_relative_url).to_a + + allowed_topic_create = Set.new(Category.topic_create_allowed(@guardian).pluck(:id)) + + categories.each do |category| + category.permission = CategoryGroup.permission_types[:full] if allowed_topic_create.include?(category.id) + end + categories + end end def archetypes diff --git a/app/serializers/basic_category_serializer.rb b/app/serializers/basic_category_serializer.rb index 18e166c6899..29b45de3940 100644 --- a/app/serializers/basic_category_serializer.rb +++ b/app/serializers/basic_category_serializer.rb @@ -9,6 +9,7 @@ class BasicCategorySerializer < ApplicationSerializer :description, :topic_url, :hotness, - :read_restricted + :read_restricted, + :permission end diff --git a/app/serializers/category_serializer.rb b/app/serializers/category_serializer.rb index 03a484e1278..b565f20269f 100644 --- a/app/serializers/category_serializer.rb +++ b/app/serializers/category_serializer.rb @@ -1,10 +1,10 @@ class CategorySerializer < BasicCategorySerializer - attributes :read_restricted, :groups, :available_groups, :auto_close_days, :group_permissions + attributes :read_restricted, :available_groups, :auto_close_days, :group_permissions def group_permissions @group_permissions ||= begin - perms = object.category_groups.joins(:group).order("groups.name").map do |cg| + perms = object.category_groups.joins(:group).includes(:group).order("groups.name").map do |cg| { permission_type: cg.permission_type, group_name: cg.group.name @@ -18,7 +18,7 @@ class CategorySerializer < BasicCategorySerializer end def available_groups - Group.order("name").pluck(:name) - group_permissions.map{|g| g[:group_name]} + Group.order(:name).pluck(:name) - group_permissions.map{|g| g[:group_name]} end end diff --git a/app/serializers/site_serializer.rb b/app/serializers/site_serializer.rb index 0caf55df44b..d67f65c4e5c 100644 --- a/app/serializers/site_serializer.rb +++ b/app/serializers/site_serializer.rb @@ -3,7 +3,8 @@ class SiteSerializer < ApplicationSerializer attributes :default_archetype, :notification_types, :post_types, - :uncategorized_slug + :uncategorized_slug, + :group_names has_many :categories, serializer: BasicCategorySerializer, embed: :objects diff --git a/lib/guardian.rb b/lib/guardian.rb index a58e791b8b0..d2c9b093bbb 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -238,8 +238,19 @@ class Guardian can_create_post?(parent) end + def can_create_topic_on_category?(category) + can_create_post?(nil) && ( + !category || + Category.topic_create_allowed(self).where(:id => category.id).count == 1 + ) + end + def can_create_post?(parent) - !SpamRulesEnforcer.block?(@user) + !SpamRulesEnforcer.block?(@user) && ( + !parent || + !parent.category || + Category.post_create_allowed(self).where(:id => parent.category.id).count == 1 + ) end def can_create_post_on_topic?(topic) diff --git a/lib/topic_creator.rb b/lib/topic_creator.rb index 107952e1399..828db0d4351 100644 --- a/lib/topic_creator.rb +++ b/lib/topic_creator.rb @@ -27,9 +27,9 @@ class TopicCreator topic_params[:archetype] = @opts[:archetype] if @opts[:archetype].present? topic_params[:subtype] = @opts[:subtype] if @opts[:subtype].present? - @guardian.ensure_can_create!(Topic) - category = Category.where(name: @opts[:category]).first + + @guardian.ensure_can_create!(Topic,category) topic_params[:category_id] = category.id if category.present? topic_params[:meta_data] = @opts[:meta_data] if @opts[:meta_data].present? topic_params[:created_at] = Time.zone.parse(@opts[:created_at].to_s) if @opts[:created_at].present? diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 63f707fb23d..ca52bcf451a 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -276,8 +276,27 @@ describe Guardian do end end + describe 'a Topic' do + it 'should check for full permissions' do + category = Fabricate(:category) + category.set_permissions(:everyone => :create_post) + category.save + Guardian.new(user).can_create?(Topic,category).should be_false + end + end + describe 'a Post' do + it "is false on readonly categories" do + category = Fabricate(:category) + topic.category = category + category.set_permissions(:everyone => :readonly) + category.save + + Guardian.new(topic.user).can_create?(Post, topic).should be_false + + end + it "is false when not logged in" do Guardian.new.can_create?(Post, topic).should be_false end diff --git a/spec/controllers/categories_controller_spec.rb b/spec/controllers/categories_controller_spec.rb index b71fecdd2ab..7ca122bef18 100644 --- a/spec/controllers/categories_controller_spec.rb +++ b/spec/controllers/categories_controller_spec.rb @@ -1,13 +1,13 @@ -require 'spec_helper' +require "spec_helper" describe CategoriesController do - describe 'create' do + describe "create" do - it 'requires the user to be logged in' do + it "requires the user to be logged in" do lambda { xhr :post, :create }.should raise_error(Discourse::NotLoggedIn) end - describe 'logged in' do + describe "logged in" do before do @user = log_in(:moderator) end @@ -18,55 +18,66 @@ describe CategoriesController do response.should be_forbidden end - it 'raises an exception when the name is missing' do - lambda { xhr :post, :create, color: 'ff0', text_color: 'fff' }.should raise_error(ActionController::ParameterMissing) + it "raises an exception when the name is missing" do + lambda { xhr :post, :create, color: "ff0", text_color: "fff" }.should raise_error(ActionController::ParameterMissing) end - it 'raises an exception when the color is missing' do - lambda { xhr :post, :create, name: 'hello', text_color: 'fff' }.should raise_error(ActionController::ParameterMissing) + it "raises an exception when the color is missing" do + lambda { xhr :post, :create, name: "hello", text_color: "fff" }.should raise_error(ActionController::ParameterMissing) end - it 'raises an exception when the text color is missing' do - lambda { xhr :post, :create, name: 'hello', color: 'ff0' }.should raise_error(ActionController::ParameterMissing) + it "raises an exception when the text color is missing" do + lambda { xhr :post, :create, name: "hello", color: "ff0" }.should raise_error(ActionController::ParameterMissing) end - describe 'failure' do + describe "failure" do before do @category = Fabricate(:category, user: @user) - xhr :post, :create, name: @category.name, color: 'ff0', text_color: 'fff' + xhr :post, :create, name: @category.name, color: "ff0", text_color: "fff" end it { should_not respond_with(:success) } - it 'returns errors on a duplicate category name' do - response.code.to_i.should == 422 + it "returns errors on a duplicate category name" do + response.status.should == 422 end end - describe 'success' do - before do - xhr :post, :create, name: 'hello', color: 'ff0', text_color: 'fff' + describe "success" do + it "works" do + readonly = CategoryGroup.permission_types[:readonly] + create_post = CategoryGroup.permission_types[:create_post] + + xhr :post, :create, name: "hello", color: "ff0", text_color: "fff", + hotness: 2, + auto_close_days: 3, + permissions: { + "everyone" => readonly, + "staff" => create_post + } + + response.status.should == 200 + category = Category.first + category.category_groups.map{|g| [g.group_id, g.permission_type]}.sort.should == [ + [Group[:everyone].id, readonly],[Group[:staff].id,create_post] + ] + category.name.should == "hello" + category.color.should == "ff0" + category.hotness.should == 2 + category.auto_close_days.should == 3 end - - it 'creates a category' do - Category.count.should == 1 - end - - it { should respond_with(:success) } - end - end end - describe 'destroy' do + describe "destroy" do - it 'requires the user to be logged in' do - lambda { xhr :delete, :destroy, id: 'category'}.should raise_error(Discourse::NotLoggedIn) + it "requires the user to be logged in" do + lambda { xhr :delete, :destroy, id: "category"}.should raise_error(Discourse::NotLoggedIn) end - describe 'logged in' do + describe "logged in" do before do @user = log_in @category = Fabricate(:category, user: @user) @@ -86,14 +97,14 @@ describe CategoriesController do end - describe 'update' do + describe "update" do - it 'requires the user to be logged in' do + it "requires the user to be logged in" do lambda { xhr :put, :update, id: 'category'}.should raise_error(Discourse::NotLoggedIn) end - describe 'logged in' do + describe "logged in" do before do @user = log_in(:moderator) @category = Fabricate(:category, user: @user) @@ -117,37 +128,46 @@ describe CategoriesController do lambda { xhr :put, :update, id: @category.slug, name: 'asdf', color: 'fff' }.should raise_error(ActionController::ParameterMissing) end - describe 'failure' do + describe "failure" do before do - @other_category = Fabricate(:category, name: 'Other', user: @user ) - xhr :put, :update, id: @category.id, name: @other_category.name, color: 'ff0', text_color: 'fff' + @other_category = Fabricate(:category, name: "Other", user: @user ) + xhr :put, :update, id: @category.id, name: @other_category.name, color: "ff0", text_color: "fff" end - it 'returns errors on a duplicate category name' do + it "returns errors on a duplicate category name" do response.should_not be_success end - it 'returns errors on a duplicate category name' do + it "returns errors on a duplicate category name" do response.code.to_i.should == 422 end end - describe 'success' do - before do - # might as well test this while at it - @category.set_permissions(:admins => :full) - @category.save + describe "success" do - xhr :put, :update, id: @category.id, name: 'science', color: '000', text_color: '0ff', group_names: Group[:staff].name, read_restricted: 'true' + it "updates the group correctly" do + + readonly = CategoryGroup.permission_types[:readonly] + create_post = CategoryGroup.permission_types[:create_post] + + xhr :put, :update, id: @category.id, name: "hello", color: "ff0", text_color: "fff", + hotness: 2, + auto_close_days: 3, + permissions: { + "everyone" => readonly, + "staff" => create_post + } + + response.status.should == 200 @category.reload - end + @category.category_groups.map{|g| [g.group_id, g.permission_type]}.sort.should == [ + [Group[:everyone].id, readonly],[Group[:staff].id,create_post] + ] + @category.name.should == "hello" + @category.color.should == "ff0" + @category.hotness.should == 2 + @category.auto_close_days.should == 3 - it 'updates the group correctly' do - @category.name.should == 'science' - @category.color.should == '000' - @category.text_color.should == '0ff' - @category.read_restricted?.should be_true - @category.groups.count.should == 1 end end end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index f2d69c37536..afb16f5f6e7 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -61,6 +61,12 @@ describe Category do Category.secured(guardian).count.should == 4 Category.post_create_allowed(guardian).count.should == 3 Category.topic_create_allowed(guardian).count.should == 2 # explicitly allowed once, default allowed once + + # everyone has special semantics, test it as well + can_post_category.set_permissions(:everyone => :create_post) + can_post_category.save + + Category.post_create_allowed(guardian).count.should == 3 end end diff --git a/spec/models/site_spec.rb b/spec/models/site_spec.rb index a4ffd55a3fb..771400e7469 100644 --- a/spec/models/site_spec.rb +++ b/spec/models/site_spec.rb @@ -11,6 +11,7 @@ describe Site do category.set_permissions(:everyone => :create_post) category.save - Site.new(Guardian.new(user)).categories.count.should == 0 + # TODO clean up querying so we can make sure we have the correct permission set + Site.new(Guardian.new(user)).categories[0].permission.should_not == CategoryGroup.permission_types[:full] end end