Add fixed_category_positions site setting to handle whether categories are ordered by specified positions or by activity.

app/assets/javascripts/discourse/controllers/discovery/categories.js.es6

@@ -32,6 +32,10 @@ export default Discourse.DiscoveryController.extend({
     return Discourse.User.currentProp('staff');
   }.property(),
 
+  canOrder: function() {
+    return this.get('canEdit') && Discourse.SiteSettings.fixed_category_positions;
+  }.property('Discourse.SiteSettings.fixed_category_positions'),
+
   moveCategory: function(categoryId, position){
     this.get('model.categories').moveCategory(categoryId, position);
   },

app/assets/javascripts/discourse/controllers/edit-category.js.es6

@@ -12,7 +12,6 @@ export default Discourse.ObjectController.extend(Discourse.ModalFunctionality, {
   securitySelected: Ember.computed.equal('selectedTab', 'security'),
   settingsSelected: Ember.computed.equal('selectedTab', 'settings'),
   foregroundColors: ['FFFFFF', '000000'],
-  defaultPosition:  false,
 
   parentCategories: function() {
     return Discourse.Category.list().filter(function (c) {
@@ -29,7 +28,6 @@ export default Discourse.ObjectController.extend(Discourse.ModalFunctionality, {
   onShow: function() {
     this.changeSize();
     this.titleChanged();
-    this.set('defaultPosition', this.get('position') === null);
   },
 
   changeSize: function() {
@@ -116,6 +114,10 @@ export default Discourse.ObjectController.extend(Discourse.ModalFunctionality, {
     return !this.get('isUncategorized') && this.get('id');
   }.property('isUncategorized', 'id'),
 
+  showPositionInput: function() {
+    return Discourse.SiteSettings.fixed_category_positions;
+  }.property('Discourse.SiteSettings.fixed_category_positions'),
+
   actions: {
 
     selectGeneral: function() {
@@ -148,17 +150,6 @@ export default Discourse.ObjectController.extend(Discourse.ModalFunctionality, {
       this.get('model').removePermission(permission);
     },
 
-    toggleDefaultPosition: function() {
-      this.toggleProperty('defaultPosition');
-    },
-
-    disableDefaultPosition: function() {
-      this.set('defaultPosition', false);
-      Em.run.schedule('afterRender', function() {
-        this.$('.position-input').focus();
-      });
-    },
-
     saveCategory: function() {
       var self = this,
           model = this.get('model'),
@@ -166,7 +157,6 @@ export default Discourse.ObjectController.extend(Discourse.ModalFunctionality, {
 
       this.set('saving', true);
       model.set('parentCategory', parentCategory);
-      if (this.get('defaultPosition')) { model.set('position', 'default'); }
 
       self.set('saving', false);
       this.get('model').save().then(function(result) {

app/assets/javascripts/discourse/templates/discovery/categories.js.handlebars

@@ -7,7 +7,7 @@
           <th class='latest'>{{i18n categories.latest}}</th>
           <th class='stats topics'>{{i18n categories.topics}}</th>
           <th class='stats posts'>{{i18n categories.posts}}
-            {{#if canEdit}}<button title='{{i18n categories.toggle_ordering}}' class='btn toggle-admin no-text' {{action toggleOrdering}}><i class='fa fa-wrench'></i></button>{{/if}}
+            {{#if canOrder}}<button title='{{i18n categories.toggle_ordering}}' class='btn toggle-admin no-text' {{action toggleOrdering}}><i class='fa fa-wrench'></i></button>{{/if}}
           </th>
         </tr>
       </thead>

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

@@ -123,15 +123,12 @@
           </section>
         {{/if}}
 
-        <section class='field'>
+        {{#if showPositionInput}}
-          <label>{{i18n category.position}}</label>
+          <section class='field'>
-          <span {{action disableDefaultPosition}}>{{textField value=position disabled=defaultPosition class="position-input"}}</span>
+            <label>{{i18n category.position}}</label>
-          &nbsp;{{i18n or}}&nbsp;
+            {{textField value=position class="position-input"}}
-          <button {{bind-attr class=":btn defaultPosition:btn-primary"}} {{action toggleDefaultPosition}}>
+          </section>
-            <span {{bind-attr class="defaultPosition::hidden"}}><i class="fa fa-check"></i></span>
+        {{/if}}
-            {{i18n category.default_position}}
-          </button>
-        </section>
       </div>
     {{/unless}}
   </div>

app/controllers/categories_controller.rb

@@ -50,19 +50,19 @@ class CategoriesController < ApplicationController
   def create
     guardian.ensure_can_create!(Category)
 
+    position = category_params.delete(:position)
+
     @category = Category.create(category_params.merge(user: current_user))
     return render_json_error(@category) unless @category.save
 
-    @category.move_to(category_params[:position].to_i) if category_params[:position]
+    @category.move_to(position.to_i) if position
     render_serialized(@category, CategorySerializer)
   end
 
   def update
     guardian.ensure_can_edit!(@category)
     json_result(@category, serializer: CategorySerializer) { |cat|
-      if category_params[:position]
+      cat.move_to(category_params[:position].to_i) if category_params[:position]
-        category_params[:position] == 'default' ? cat.use_default_position : cat.move_to(category_params[:position].to_i)
-      end
       if category_params.key? :email_in and category_params[:email_in].length == 0
         # properly null the value so the database constrain doesn't catch us
         category_params[:email_in] = nil

app/models/category_list.rb

@@ -57,16 +57,19 @@ class CategoryList
       @categories = Category
                         .includes(:featured_users, subcategories: [:topic_only_relative_url])
                         .secured(@guardian)
-                        .order('position asc')
+      if SiteSetting.fixed_category_positions
-                        .order('COALESCE(categories.posts_week, 0) DESC')
+        @categories = @categories.order('position ASC').order('id ASC')
-                        .order('COALESCE(categories.posts_month, 0) DESC')
+      else
-                        .order('COALESCE(categories.posts_year, 0) DESC')
+        @categories = @categories.order('COALESCE(categories.posts_week, 0) DESC')
-                        .to_a
+                                 .order('COALESCE(categories.posts_month, 0) DESC')
+                                 .order('COALESCE(categories.posts_year, 0) DESC')
+      end
 
       if latest_post_only?
         @categories  = @categories.includes(:latest_post => {:topic => :last_poster} )
       end
 
+      @categories = @categories.to_a
       subcategories = {}
       to_delete = Set.new
       @categories.each do |c|

app/models/concerns/positionable.rb

@@ -1,6 +1,12 @@
 module Positionable
   extend ActiveSupport::Concern
 
+  included do
+    before_save do
+      self.position ||= self.class.count
+    end
+  end
+
   def move_to(position_arg)
 
     position = [[position_arg, 0].max, self.class.count - 1].min
@@ -27,11 +33,4 @@ module Positionable
     SET position = :position
     WHERE id = :id", {id: id, position: position}
   end
-
-  def use_default_position
-    self.exec_sql "
-    UPDATE #{self.class.table_name}
-    SET POSITION = null
-    WHERE id = :id", {id: id}
-  end
 end

config/locales/server.en.yml

@@ -620,6 +620,7 @@ en:
     max_image_width: "Maximum allowed width of images in a post"
     max_image_height: "Maximum allowed height of images in a post"
     category_featured_topics: "Number of topics displayed per category on the /categories page. After changing this value, it takes up to 15 minutes for the categories page to update."
+    fixed_category_positions: "If checked, you will be able to arrange categories into a fixed order. If unchecked, categories are listed in order of activity."
     add_rel_nofollow_to_user_content: "Add rel nofollow to all submitted user content, except for internal links (including parent domains) changing this requires you update all your baked markdown with: \"rake posts:rebake\""
     exclude_rel_nofollow_domains: "A pipe-delimited list of domains where nofollow is not added (tld.com will automatically allow sub.tld.com as well)"
 

config/site_settings.yml

@@ -78,6 +78,9 @@ basic:
   category_featured_topics:
     client: true
     default: 3
+  fixed_category_positions:
+    client: true
+    default: false
   topics_per_page: 30
   posts_per_page:
     client: true

db/migrate/20131022045114_add_uncategorized_category.rb

@@ -9,7 +9,7 @@ class AddUncategorizedCategory < ActiveRecord::Migration
 
     result = execute "INSERT INTO categories
             (name,color,slug,description,text_color, user_id, created_at, updated_at, position)
-     VALUES ('#{name}', 'AB9364', 'uncategorized', '', 'FFFFFF', -1, now(), now(), 1 )
+     VALUES ('#{name}', 'AB9364', 'uncategorized', '', 'FFFFFF', -1, now(), now(), 0 )
      RETURNING id
     "
     category_id = result[0]["id"].to_i

db/migrate/20140120155706_add_lounge_category.rb

@@ -13,8 +13,8 @@ class AddLoungeCategory < ActiveRecord::Migration
         end
 
         result = execute "INSERT INTO categories
-                        (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted)
+                        (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted, position)
-                 VALUES ('#{name}', 'EEEEEE', '652D90', now(), now(), -1, '#{Slug.for(name)}', '#{description}', true)
+                 VALUES ('#{name}', 'EEEEEE', '652D90', now(), now(), -1, '#{Slug.for(name)}', '#{description}', true, 3)
                  RETURNING id"
         category_id = result[0]["id"].to_i
 

db/migrate/20140122043508_add_meta_category.rb

@@ -8,8 +8,8 @@ class AddMetaCategory < ActiveRecord::Migration
         name = I18n.t('meta_category_name')
         if Category.exec_sql("SELECT 1 FROM categories where name ilike '#{name}'").count == 0
           result = execute "INSERT INTO categories
-                          (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted)
+                          (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted, position)
-                   VALUES ('#{name}', '808281', 'FFFFFF', now(), now(), -1, '#{Slug.for(name)}', '#{description}', true)
+                   VALUES ('#{name}', '808281', 'FFFFFF', now(), now(), -1, '#{Slug.for(name)}', '#{description}', true, 1)
                    RETURNING id"
           category_id = result[0]["id"].to_i
 

db/migrate/20140227201005_add_staff_category.rb

@@ -7,8 +7,8 @@ class AddStaffCategory < ActiveRecord::Migration
         name = I18n.t('staff_category_name')
         if Category.exec_sql("SELECT 1 FROM categories where name ilike '#{name}'").count == 0
           result = execute "INSERT INTO categories
-                          (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted)
+                          (name, color, text_color, created_at, updated_at, user_id, slug, description, read_restricted, position)
-                   VALUES ('#{name}', '283890', 'FFFFFF', now(), now(), -1, '#{Slug.for(name)}', '#{description}', true)
+                   VALUES ('#{name}', '283890', 'FFFFFF', now(), now(), -1, '#{Slug.for(name)}', '#{description}', true, 2)
                    RETURNING id"
           category_id = result[0]["id"].to_i
 

db/migrate/20140515220111_init_fixed_category_positions_value.rb

@@ -0,0 +1,15 @@
+class InitFixedCategoryPositionsValue < ActiveRecord::Migration
+  def up
+    # Look at existing categories to determine if positions have been specified
+    result = Category.exec_sql("SELECT count(*) FROM categories WHERE position IS NOT NULL")
+
+    # Greater than 4 because uncategorized, meta, staff, lounge all have positions by default
+    if result[0]['count'].to_i > 4
+      execute "INSERT INTO site_settings (name, data_type, value, created_at, updated_at) VALUES ('fixed_category_positions', 5, 't', now(), now())"
+    end
+  end
+
+  def down
+    execute "DELETE FROM site_settings WHERE name = 'fixed_category_positions'"
+  end
+end

spec/components/category_list_spec.rb

@@ -89,29 +89,41 @@ describe CategoryList do
       uncategorized.save
     end
 
-    it "returns topics in specified order" do
+    context 'fixed_category_positions is enabled' do
-      cat1, cat2 = Fabricate(:category, position: 1), Fabricate(:category, position: 0)
+      before do
-      category_ids.should == [cat2.id, cat1.id]
+        SiteSetting.stubs(:fixed_category_positions).returns(true)
+      end
+
+      it "returns categories in specified order" do
+        cat1, cat2 = Fabricate(:category, position: 1), Fabricate(:category, position: 0)
+        category_ids.should == [cat2.id, cat1.id]
+      end
+
+      it "handles duplicate position values" do
+        cat1, cat2, cat3, cat4 = Fabricate(:category, position: 0), Fabricate(:category, position: 0), Fabricate(:category, position: nil), Fabricate(:category, position: 0)
+        first_three = category_ids[0,3] # The order is not deterministic
+        first_three.should include(cat1.id)
+        first_three.should include(cat2.id)
+        first_three.should include(cat4.id)
+        category_ids[-1].should == cat3.id
+      end
     end
 
-    it "returns default order categories" do
+    context 'fixed_category_positions is disabled' do
-      cat1, cat2 = Fabricate(:category, position: nil), Fabricate(:category, position: nil)
+      before do
-      category_ids.should include(cat1.id)
+        SiteSetting.stubs(:fixed_category_positions).returns(false)
-      category_ids.should include(cat2.id)
+      end
-    end
 
-    it "default always at the end" do
+      it "returns categories in order of activity" do
-      cat1, cat2, cat3 = Fabricate(:category, position: 0), Fabricate(:category, position: 2), Fabricate(:category, position: nil)
+        cat1 = Fabricate(:category, position: 0, posts_week: 1, posts_month: 1, posts_year: 1)
-      category_ids.should == [cat1.id, cat2.id, cat3.id]
+        cat2 = Fabricate(:category, position: 1, posts_week: 2, posts_month: 1, posts_year: 1)
-    end
+        category_ids.should == [cat2.id, cat1.id]
+      end
 
-    it "handles duplicate position values" do
+      it "returns categories in order of id when there's no activity" do
-      cat1, cat2, cat3, cat4 = Fabricate(:category, position: 0), Fabricate(:category, position: 0), Fabricate(:category, position: nil), Fabricate(:category, position: 0)
+        cat1, cat2 = Fabricate(:category, position: 1), Fabricate(:category, position: 0)
-      first_three = category_ids[0,3] # The order is not deterministic
+        category_ids.should == [cat1.id, cat2.id]
-      first_three.should include(cat1.id)
+      end
-      first_three.should include(cat2.id)
-      first_three.should include(cat4.id)
-      category_ids[-1].should == cat3.id
     end
   end
 

spec/components/concern/positionable_spec.rb

@@ -45,34 +45,7 @@ describe Positionable do
       item = TestItem.new
       item.id = 7
       item.save
-      item.position.should be_nil
+      item.position.should == 5
-    end
-
-    it "can set records to have null position" do
-      5.times do |i|
-        Topic.exec_sql("insert into test_items(id,position) values(#{i}, #{i})")
-      end
-
-      TestItem.find(2).use_default_position
-      TestItem.find(2).position.should be_nil
-
-      TestItem.find(1).move_to(4)
-      TestItem.order('id ASC').pluck(:position).should == [0,4,nil,2,3]
-    end
-
-    it "can maintain null positions when moving things around" do
-      5.times do |i|
-        Topic.exec_sql("insert into test_items(id,position) values(#{i}, null)")
-      end
-
-      TestItem.find(2).move_to(0)
-      TestItem.order('id asc').pluck(:position).should == [nil,nil,0,nil,nil]
-      TestItem.find(0).move_to(4)
-      TestItem.order('id asc').pluck(:position).should == [4,nil,0,nil,nil]
-      TestItem.find(2).move_to(1)
-      TestItem.order('id asc').pluck(:position).should == [4,nil,1,nil,nil]
-      TestItem.find(0).move_to(1)
-      TestItem.order('id asc').pluck(:position).should == [1,nil,2,nil,nil]
     end
   end
 end

spec/controllers/categories_controller_spec.rb

@@ -165,13 +165,6 @@ describe CategoriesController do
           @category.color.should == "ff0"
           @category.auto_close_hours.should == 72
         end
-
-        it "can set category to use default position" do
-          xhr :put, :update, valid_attrs.merge(position: 'default')
-          response.should be_success
-          @category.reload
-          @category.position.should be_nil
-        end
       end
     end