Allow categories with null position, which means sort them based on activity. Mix absolutely positioned (position is not null) categories with null position categories.

This commit is contained in:
Neil Lalonde 2013-12-16 15:13:43 -05:00
parent f1a7b63afc
commit 341adc93a4
11 changed files with 154 additions and 43 deletions

View File

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

View File

@ -100,7 +100,12 @@
<section class='field'>
<label>{{i18n category.position}}</label>
{{textField value=position}}
<span {{action disableDefaultPosition}}>{{textField value=position disabled=defaultPosition class="position-input"}}</span>
&nbsp;{{i18n or}}&nbsp;
<button {{bindAttr class=":btn defaultPosition:btn-primary"}} {{action toggleDefaultPosition}}>
<span {{bindAttr class="defaultPosition::hidden"}}><i class="fa fa-check"></i></span>
{{i18n category.default_position}}
</button>
</section>
<section class='field'>

View File

@ -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)
}

View File

@ -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

View File

@ -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

View File

@ -1007,6 +1007,7 @@ en:
add_permission: "Add Permission"
this_year: "this year"
position: "position"
default_position: "Default Position"
parent: "Parent Category"
flagging:

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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