diff --git a/app/assets/javascripts/discourse/controllers/edit_category_controller.js b/app/assets/javascripts/discourse/controllers/edit_category_controller.js
index 3e73bfdbb35..7dcf4d7f93b 100644
--- a/app/assets/javascripts/discourse/controllers/edit_category_controller.js
+++ b/app/assets/javascripts/discourse/controllers/edit_category_controller.js
@@ -12,6 +12,7 @@ Discourse.EditCategoryController = Discourse.ObjectController.extend(Discourse.M
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) {
@@ -22,6 +23,7 @@ Discourse.EditCategoryController = Discourse.ObjectController.extend(Discourse.M
onShow: function() {
this.changeSize();
this.titleChanged();
+ this.set('defaultPosition', this.get('position') === null);
},
changeSize: function() {
@@ -127,6 +129,17 @@ Discourse.EditCategoryController = Discourse.ObjectController.extend(Discourse.M
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'),
@@ -134,6 +147,7 @@ Discourse.EditCategoryController = Discourse.ObjectController.extend(Discourse.M
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) {
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 b8a4f76ef45..6057ca5700b 100644
--- a/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars
+++ b/app/assets/javascripts/discourse/templates/modal/edit_category.js.handlebars
@@ -100,7 +100,12 @@
- {{textField value=position}}
+ {{textField value=position disabled=defaultPosition class="position-input"}}
+ {{i18n or}}
+
diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb
index 4a8fc6539ca..8b9d0c1e421 100644
--- a/app/controllers/categories_controller.rb
+++ b/app/controllers/categories_controller.rb
@@ -60,7 +60,9 @@ class CategoriesController < ApplicationController
def update
guardian.ensure_can_edit!(@category)
json_result(@category, serializer: CategorySerializer) { |cat|
- cat.move_to(category_params[:position].to_i) if category_params[:position]
+ if category_params[:position]
+ category_params[:position] == 'default' ? cat.use_default_position : cat.move_to(category_params[:position].to_i)
+ end
category_params.delete(:position)
cat.update_attributes(category_params)
}
diff --git a/app/models/category.rb b/app/models/category.rb
index cf639d33eec..2aae060a439 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -364,7 +364,7 @@ end
# post_count :integer default(0), not null
# latest_post_id :integer
# latest_topic_id :integer
-# position :integer not null
+# position :integer
# parent_category_id :integer
#
# Indexes
diff --git a/app/models/category_list.rb b/app/models/category_list.rb
index 0c8a7dc3700..df862a4cef1 100644
--- a/app/models/category_list.rb
+++ b/app/models/category_list.rb
@@ -53,23 +53,36 @@ class CategoryList
# Find a list of all categories to associate the topics with
def find_categories
- @categories = Category
- .includes(:featured_users)
- .secured(@guardian)
- .order('position ASC')
+ @absolute_position_categories = Category
+ .includes(:featured_users)
+ .secured(@guardian)
+ .where('position IS NOT NULL')
+ .order('position ASC')
+ @default_position_categories = Category
+ .includes(:featured_users)
+ .secured(@guardian)
+ .where('position IS NULL')
+ .order('COALESCE(categories.posts_week, 0) DESC')
+ .order('COALESCE(categories.posts_month, 0) DESC')
+ .order('COALESCE(categories.posts_year, 0) DESC')
if latest_post_only?
- @categories = @categories.includes(:latest_post => {:topic => :last_poster} )
- # else
- # # This is only for the old 2-column layout.
- # # Use this when we support "organic" positioning of some/all categories.
- # @categories = @categories
- # .order('COALESCE(categories.topics_week, 0) DESC')
- # .order('COALESCE(categories.topics_month, 0) DESC')
- # .order('COALESCE(categories.topics_year, 0) DESC')
+ @absolute_position_categories = @absolute_position_categories.includes(:latest_post => {:topic => :last_poster} )
+ @default_position_categories = @default_position_categories.includes(:latest_post => {:topic => :last_poster} )
end
- @categories = @categories.to_a
+ @default_position_categories = @default_position_categories.to_a
+ @categories = []
+ index = 0
+ @absolute_position_categories.to_a.each do |c|
+ if c.position > index
+ @categories.push(*(@default_position_categories.shift(c.position - index)))
+ end
+ @categories << c
+ index = c.position + 1 if c.position >= index # handles duplicate position values
+ end
+ @categories.push *@default_position_categories # Whatever is left is put on the end
+
subcategories = {}
to_delete = Set.new
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index 2e564140bba..65745f6f12f 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -1007,6 +1007,7 @@ en:
add_permission: "Add Permission"
this_year: "this year"
position: "position"
+ default_position: "Default Position"
parent: "Parent Category"
flagging:
diff --git a/db/migrate/20131216164557_make_position_nullable_in_categories.rb b/db/migrate/20131216164557_make_position_nullable_in_categories.rb
new file mode 100644
index 00000000000..9b7dc619ac3
--- /dev/null
+++ b/db/migrate/20131216164557_make_position_nullable_in_categories.rb
@@ -0,0 +1,10 @@
+class MakePositionNullableInCategories < ActiveRecord::Migration
+ def up
+ change_column :categories, :position, :integer, null: true
+ end
+
+ def down
+ execute "update categories set position=0 where position is null"
+ change_column :categories, :position, :integer, null: false
+ end
+end
diff --git a/lib/concern/positionable.rb b/lib/concern/positionable.rb
index ec97a4442dc..fdeaf2f6fa9 100644
--- a/lib/concern/positionable.rb
+++ b/lib/concern/positionable.rb
@@ -2,36 +2,38 @@ module Concern
module Positionable
extend ActiveSupport::Concern
- included do
- before_save do
- self.position ||= self.class.count
+ def move_to(position_arg)
+
+ position = [[position_arg, 0].max, self.class.count - 1].min
+
+ if self.position.nil? or position > self.position
+ self.exec_sql "
+ UPDATE #{self.class.table_name}
+ SET position = position - 1
+ WHERE position > :current_position and position <= :new_position",
+ {current_position: self.position, new_position: position}
+ elsif position < self.position
+ self.exec_sql "
+ UPDATE #{self.class.table_name}
+ SET position = position + 1
+ WHERE position >= :new_position and position < :current_position",
+ {current_position: self.position, new_position: position}
+ else
+ # Not moving to a new position
+ return
end
- end
-
- def move_to(position)
-
- self.exec_sql "
- UPDATE #{self.class.table_name}
- SET position = position - 1
- WHERE position > :position AND position > 0", {position: self.position}
self.exec_sql "
UPDATE #{self.class.table_name}
SET position = :position
WHERE id = :id", {id: id, position: position}
+ end
+ def use_default_position
self.exec_sql "
- UPDATE #{self.class.table_name} t
- SET position = x.position - 1
- FROM (
- SELECT i.id, row_number()
- OVER(ORDER BY i.position asc,
- CASE WHEN i.id = :id THEN 0 ELSE 1 END ASC) AS position
- FROM #{self.class.table_name} i
- WHERE i.position IS NOT NULL
- ) x
- WHERE x.id = t.id AND t.position <> x.position - 1
- ", {id: id}
+ UPDATE #{self.class.table_name}
+ SET POSITION = null
+ WHERE id = :id", {id: id}
end
end
end
diff --git a/spec/components/category_list_spec.rb b/spec/components/category_list_spec.rb
index 97f8f8d177b..cc9e7163530 100644
--- a/spec/components/category_list_spec.rb
+++ b/spec/components/category_list_spec.rb
@@ -4,11 +4,11 @@ require 'category_list'
describe CategoryList do
let(:user) { Fabricate(:user) }
+ let(:admin) { Fabricate(:admin) }
let(:category_list) { CategoryList.new(Guardian.new user) }
context "security" do
it "properly hide secure categories" do
- admin = Fabricate(:admin)
user = Fabricate(:user)
cat = Fabricate(:category)
@@ -73,4 +73,35 @@ describe CategoryList do
end
+ describe 'category order' do
+ let(:category_ids) { CategoryList.new(Guardian.new(admin)).categories.map(&:id) - [SiteSetting.uncategorized_category_id] }
+
+ before do
+ uncategorized = Category.find(SiteSetting.uncategorized_category_id)
+ uncategorized.position = 100
+ uncategorized.save
+ end
+
+ it "returns topics in specified order" do
+ cat1, cat2 = Fabricate(:category, position: 1), Fabricate(:category, position: 0)
+ category_ids.should == [cat2.id, cat1.id]
+ end
+
+ it "returns default order categories" do
+ cat1, cat2 = Fabricate(:category, position: nil), Fabricate(:category, position: nil)
+ category_ids.should include(cat1.id)
+ category_ids.should include(cat2.id)
+ end
+
+ it "mixes default order categories with absolute position categories" do
+ cat1, cat2, cat3 = Fabricate(:category, position: 0), Fabricate(:category, position: 2), Fabricate(:category, position: nil)
+ category_ids.should == [cat1.id, cat3.id, cat2.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)
+ category_ids.should == [cat1.id, cat2.id, cat4.id, cat3.id]
+ end
+ end
+
end
diff --git a/spec/components/concern/positionable_spec.rb b/spec/components/concern/positionable_spec.rb
index fa8c3f8b151..82895ae75d5 100644
--- a/spec/components/concern/positionable_spec.rb
+++ b/spec/components/concern/positionable_spec.rb
@@ -37,8 +37,7 @@ describe Concern::Positionable do
TestItem.find(3).move_to(1)
positions.should == [0,3,1,2,4]
- # this is somewhat odd, but when there is not positioning
- # not much we can do
+ # this is somewhat odd, but when there is no such position, not much we can do
TestItem.find(1).move_to(5)
positions.should == [0,3,2,4,1]
@@ -47,7 +46,34 @@ describe Concern::Positionable do
item = TestItem.new
item.id = 7
item.save
- item.position.should == 5
+ item.position.should be_nil
+ 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
diff --git a/spec/controllers/categories_controller_spec.rb b/spec/controllers/categories_controller_spec.rb
index 8af52341c92..6c167b8745c 100644
--- a/spec/controllers/categories_controller_spec.rb
+++ b/spec/controllers/categories_controller_spec.rb
@@ -105,6 +105,8 @@ describe CategoriesController do
describe "logged in" do
+ let(:valid_attrs) { {id: @category.id, name: "hello", color: "ff0", text_color: "fff"} }
+
before do
@user = log_in(:moderator)
@category = Fabricate(:category, user: @user)
@@ -146,7 +148,6 @@ describe CategoriesController do
describe "success" do
it "updates the group correctly" do
-
readonly = CategoryGroup.permission_types[:readonly]
create_post = CategoryGroup.permission_types[:create_post]
@@ -167,7 +168,13 @@ describe CategoriesController do
@category.color.should == "ff0"
@category.hotness.should == 2
@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