Finalize read only and post only categories, finished off UI work

app/assets/javascripts/discourse/models/category.js

@@ -49,24 +49,27 @@ Discourse.Category = Discourse.Model.extend({
     }
 
     return Discourse.ajax(url, {
-      contentType: 'application/json',
+      data: {
-      dataType: 'json',
-      data: JSON.stringify({
         name: this.get('name'),
         color: this.get('color'),
         text_color: this.get('text_color'),
         hotness: this.get('hotness'),
         secure: this.get('secure'),
-        permissions: _.map(
+        permissions: this.get('permissionsForUpdate'),
-                this.get('permissions'), function(p){
-                  return { group_name: p.group_name, permission_type: p.permission.id};
-                }),
         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' });
   },

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');
     }
   },

app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars

@@ -60,17 +60,17 @@
     {{#unless isUncategorized}}
       <div {{bindAttr class=":modal-tab :options-tab securitySelected::invisible"}}>
         <section class='field'>
-          <ul>
+          <ul class='permission-list'>
             {{#each permissions}}
               <li>
-                <span class="badge-group">{{group_name}}</span>
+                <span class="name"><span class="badge-group">{{group_name}}</span></span>
                 <span class="permission">{{permission.description}}</span>
                 <a {{action removePermission this}}><i class="icon icon-remove-sign"></i></a>
               </li>
             {{/each}}
           </ul>
           {{view Ember.Select contentBinding="availableGroups" valueBinding="selectedGroup"}}
-          {{view Ember.Select optionValuePath="content.id" optionLabelPath="content.description" contentBinding="availablePermissions" valueBinding="selectedPermission"}}
+          {{view Ember.Select class="permission-selector" optionValuePath="content.id" optionLabelPath="content.description" contentBinding="availablePermissions" valueBinding="selectedPermission"}}
           <button {{action addPermission selectedGroup selectedPermission}} class="btn btn-small">{{i18n category.add_group}}</button>
         </section>
       </div>

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() {

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;
+  }
+}

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;

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

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,6 +66,7 @@ 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(
@@ -71,10 +74,11 @@ class Category < ActiveRecord::Base
                         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 = ?
+                          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]

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
+    @categories ||= begin
-      .topic_create_allowed(@guardian)
+      categories = Category
+        .secured(@guardian)
         .latest
-      .includes(:topic_only_relative_url)
+        .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

app/serializers/basic_category_serializer.rb

@@ -9,6 +9,7 @@ class BasicCategorySerializer < ApplicationSerializer
              :description,
              :topic_url,
              :hotness,
-             :read_restricted
+             :read_restricted,
+             :permission
 
 end

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

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

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)

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?

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

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
+      it "raises an exception when the name is missing" do
-        lambda { xhr :post, :create, color: 'ff0', text_color: 'fff' }.should raise_error(ActionController::ParameterMissing)
+        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
+      it "raises an exception when the color is missing" do
-        lambda { xhr :post, :create, name: 'hello', text_color: 'fff' }.should raise_error(ActionController::ParameterMissing)
+        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
+      it "raises an exception when the text color is missing" do
-        lambda { xhr :post, :create, name: 'hello', color: 'ff0' }.should raise_error(ActionController::ParameterMissing)
+        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
+        it "returns errors on a duplicate category name" do
-          response.code.to_i.should == 422
+          response.status.should == 422
         end
       end
 
 
-      describe 'success' do
+      describe "success" do
-        before do
+        it "works" do
-          xhr :post, :create, name: 'hello', color: 'ff0', text_color: 'fff'
+          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
+    it "requires the user to be logged in" do
-      lambda { xhr :delete, :destroy, id: 'category'}.should raise_error(Discourse::NotLoggedIn)
+      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 )
+          @other_category = Fabricate(:category, name: "Other", user: @user )
-          xhr :put, :update, id: @category.id, name: @other_category.name, color: 'ff0', text_color: 'fff'
+          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
+      describe "success" do
-        before do
-          # might as well test this while at it
-          @category.set_permissions(:admins => :full)
-          @category.save
 
-          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

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

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