Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: We should use `category_id` instead of `category_name` to perform 

operations, now that the subcategory names are not unique.
This commit is contained in:
Robin Ward 2014-07-16 15:39:39 -04:00
parent 612dcb5805
commit fb8dda7f42
14 changed files with 51 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
app/assets/javascripts/discourse/controllers/topic-bulk-actions.js.es6
View File

@ -67,10 +67,10 @@ export default Ember.ArrayController.extend(Discourse.ModalFunctionality, {
},
changeCategory: function() {
var category = Discourse.Category.findById(parseInt(this.get('newCategoryId'), 10)),
categoryName = (category ? category.get('name') : null),
var categoryId = parseInt(this.get('newCategoryId'), 10) || 0,
category = Discourse.Category.findById(categoryId),
self = this;
this.perform({type: 'change_category', category_name: categoryName}).then(function(topics) {
this.perform({type: 'change_category', category_id: categoryId}).then(function(topics) {
topics.forEach(function(t) {
t.set('category', category);
});

10
app/assets/javascripts/discourse/controllers/topic_controller.js
View File

@ -214,7 +214,6 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected
},
finishedEditingTopic: function() {
var topicController = this;
if (this.get('editingTopic')) {
var topic = this.get('model');
@ -230,6 +229,7 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected
});
// save the modifications
var self = this;
topic.save().then(function(result){
// update the title if it has been changed (cleaned up) server-side
var title = result.basic_topic.title;
@ -238,10 +238,10 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected
title: title,
fancy_title: fancy_title
});
topicController.set('topicSaving', false);
self.set('topicSaving', false);
}, function(error) {
topicController.set('editingTopic', true);
topicController.set('topicSaving', false);
self.set('editingTopic', true);
self.set('topicSaving', false);
if (error && error.responseText) {
bootbox.alert($.parseJSON(error.responseText).errors[0]);
} else {
@ -250,7 +250,7 @@ Discourse.TopicController = Discourse.ObjectController.extend(Discourse.Selected
});
// close editing mode
topicController.set('editingTopic', false);
self.set('editingTopic', false);
}
},

2
app/assets/javascripts/discourse/models/topic.js
View File

@ -193,7 +193,7 @@ Discourse.Topic = Discourse.Model.extend({
return Discourse.ajax(this.get('url'), {
type: 'PUT',
data: { title: this.get('title'), category: this.get('category.name') }
data: { title: this.get('title'), category_id: this.get('category.id') }
});
},

14
app/assets/javascripts/discourse/models/topic_tracking_state.js
View File

@ -124,7 +124,7 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
row.highest_post_number = topic.highest_post_number;
if (topic.category) {
row.category_name = topic.category.name;
row.category_id = topic.category.id;
}
tracker.states["t" + topic.id] = row;
@ -137,7 +137,7 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
this.set("messageCount", this.get("messageCount") + 1);
},
countNew: function(category_name){
countNew: function(category_id){
return _.chain(this.states)
.where({last_read_post_number: null})
.where(function(topic) {
@ -145,7 +145,7 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
return (topic.notification_level !== 0 && !topic.notification_level) ||
topic.notification_level >= Discourse.Topic.NotificationLevel.TRACKING;
})
.where(function(topic){ return topic.category_name === category_name || !category_name;})
.where(function(topic){ return topic.category_id === category_id || !category_id;})
.value()
.length;
},
@ -159,22 +159,22 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
});
},
countUnread: function(category_name){
countUnread: function(category_id){
return _.chain(this.states)
.where(function(topic){
return topic.last_read_post_number !== null &&
topic.last_read_post_number < topic.highest_post_number;
})
.where(function(topic) { return topic.notification_level >= Discourse.Topic.NotificationLevel.TRACKING})
.where(function(topic){ return topic.category_name === category_name || !category_name;})
.where(function(topic){ return topic.category_id === category_id || !category_id;})
.value()
.length;
},
countCategory: function(category) {
countCategory: function(category_id) {
var count = 0;
_.each(this.states, function(topic){
if (topic.category_name === category) {
if (topic.category_id === category_id) {
count += (topic.last_read_post_number === null ||
topic.last_read_post_number < topic.highest_post_number) ? 1 : 0;
}

4
app/controllers/posts_controller.rb
View File

@ -106,11 +106,11 @@ class PostsController < ApplicationController
# to stay consistent with the create api,
# we should allow for title changes and category changes here
# we should also move all of this to a post updater.
if post.post_number == 1 && (params[:title] || params[:post][:category])
if post.post_number == 1 && (params[:title] || params[:post][:category_id])
post.topic.acting_user = current_user
post.topic.title = params[:title] if params[:title]
Topic.transaction do
post.topic.change_category(params[:post][:category])
post.topic.change_category_to_id(params[:post][:category_id].to_i)
post.topic.save
end

2
app/controllers/topics_controller.rb
View File

@ -113,7 +113,7 @@ class TopicsController < ApplicationController
success = false
Topic.transaction do
success = topic.save && topic.change_category(params[:category])
success = topic.save && topic.change_category_to_id(params[:category_id].to_i)
end
# this is used to return the title to the client as it may have been changed by "TextCleaner"

8
app/models/topic.rb
View File

@ -493,13 +493,12 @@ class Topic < ActiveRecord::Base
new_post
end
# Changes the category to a new name
def change_category(name)
def change_category_to_id(category_id)
# If the category name is blank, reset the attribute
if name.blank?
if (category_id.nil? || category_id.to_i == 0)
cat = Category.find_by(id: SiteSetting.uncategorized_category_id)
else
cat = Category.find_by(name: name)
cat = Category.where(id: category_id).first
end
return true if cat == category
@ -507,7 +506,6 @@ class Topic < ActiveRecord::Base
changed_to_category(cat)
end
def remove_allowed_user(username)
user = User.find_by(username: username)
if user

4
app/models/topic_tracking_state.rb
View File

@ -14,7 +14,7 @@ class TopicTrackingState
:highest_post_number,
:last_read_post_number,
:created_at,
:category_name,
:category_id,
:notification_level
def self.publish_new(topic)
@ -118,7 +118,7 @@ class TopicTrackingState
topics.created_at,
highest_post_number,
last_read_post_number,
c.name AS category_name,
c.id AS category_id,
tu.notification_level
FROM users u
INNER JOIN user_stats AS us ON us.user_id = u.id

7
app/serializers/topic_tracking_state_serializer.rb
View File

@ -1,3 +1,8 @@
class TopicTrackingStateSerializer < ApplicationSerializer
attributes :topic_id, :highest_post_number, :last_read_post_number, :created_at, :category_name, :notification_level
attributes :topic_id,
:highest_post_number,
:last_read_post_number,
:created_at,
:category_id,
:notification_level
end

2
lib/topics_bulk_action.rb
View File

@ -26,7 +26,7 @@ class TopicsBulkAction
def change_category
topics.each do |t|
if guardian.can_edit?(t)
@changed_ids << t.id if t.change_category(@operation[:category_name])
@changed_ids << t.id if t.change_category_to_id(@operation[:category_id])
end
end
end

4
spec/components/topics_bulk_action_spec.rb
View File

@ -18,7 +18,7 @@ describe TopicsBulkAction do
context "when the user can edit the topic" do
it "changes the category and returns the topic_id" do
tba = TopicsBulkAction.new(topic.user, [topic.id], type: 'change_category', category_name: category.name)
tba = TopicsBulkAction.new(topic.user, [topic.id], type: 'change_category', category_id: category.id)
topic_ids = tba.perform!
topic_ids.should == [topic.id]
topic.reload
@ -29,7 +29,7 @@ describe TopicsBulkAction do
context "when the user can't edit the topic" do
it "doesn't change the category" do
Guardian.any_instance.expects(:can_edit?).returns(false)
tba = TopicsBulkAction.new(topic.user, [topic.id], type: 'change_category', category_name: category.name)
tba = TopicsBulkAction.new(topic.user, [topic.id], type: 'change_category', category_id: category.id)
topic_ids = tba.perform!
topic_ids.should == []
topic.reload

12
spec/controllers/topics_controller_spec.rb
View File

@ -719,8 +719,8 @@ describe TopicsController do
end
it 'triggers a change of category' do
Topic.any_instance.expects(:change_category).with('incredible').returns(true)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category: 'incredible'
Topic.any_instance.expects(:change_category_to_id).with(123).returns(true)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category_id: 123
end
it "returns errors with invalid titles" do
@ -729,8 +729,8 @@ describe TopicsController do
end
it "returns errors with invalid categories" do
Topic.any_instance.expects(:change_category).returns(false)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category: ''
Topic.any_instance.expects(:change_category_to_id).returns(false)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title
expect(response).not_to be_success
end
@ -740,8 +740,8 @@ describe TopicsController do
end
it "can add a category to an uncategorized topic" do
Topic.any_instance.expects(:change_category).with('incredible').returns(true)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category: 'incredible'
Topic.any_instance.expects(:change_category_to_id).with(456).returns(true)
xhr :put, :update, topic_id: @topic.id, slug: @topic.title, category_id: 456
response.should be_success
end
end

2
spec/models/category_spec.rb
View File

@ -236,7 +236,7 @@ describe Category do
describe "trying to change the category topic's category" do
before do
@new_cat = Fabricate(:category, name: '2nd Category', user: @category.user)
@topic.change_category(@new_cat.name)
@topic.change_category_to_id(@new_cat.id)
@topic.reload
@category.reload
end

22
spec/models/topic_spec.rb
View File

@ -802,7 +802,7 @@ describe Topic do
let(:category) { Fabricate(:category) }
before do
topic.change_category(category.name)
topic.change_category_to_id(category.id)
end
it "creates a new revision" do
@ -811,7 +811,7 @@ describe Topic do
context "removing a category" do
before do
topic.change_category(nil)
topic.change_category_to_id(nil)
end
it "creates a new revision" do
@ -850,12 +850,12 @@ describe Topic do
describe 'without a previous category' do
it 'should not change the topic_count when not changed' do
lambda { @topic.change_category(@topic.category.name); @category.reload }.should_not change(@category, :topic_count)
lambda { @topic.change_category_to_id(@topic.category.id); @category.reload }.should_not change(@category, :topic_count)
end
describe 'changed category' do
before do
@topic.change_category(@category.name)
@topic.change_category_to_id(@category.id)
@category.reload
end
@ -867,14 +867,14 @@ describe Topic do
end
it "doesn't change the category when it can't be found" do
@topic.change_category('made up')
@topic.change_category_to_id(12312312)
@topic.category_id.should == SiteSetting.uncategorized_category_id
end
end
describe 'with a previous category' do
before do
@topic.change_category(@category.name)
@topic.change_category_to_id(@category.id)
@topic.reload
@category.reload
end
@ -884,18 +884,18 @@ describe Topic do
end
it "doesn't change the topic_count when the value doesn't change" do
lambda { @topic.change_category(@category.name); @category.reload }.should_not change(@category, :topic_count)
lambda { @topic.change_category_to_id(@category.id); @category.reload }.should_not change(@category, :topic_count)
end
it "doesn't reset the category when given a name that doesn't exist" do
@topic.change_category('made up')
@topic.change_category_to_id(55556)
@topic.category_id.should be_present
end
describe 'to a different category' do
before do
@new_category = Fabricate(:category, user: @user, name: '2nd category')
@topic.change_category(@new_category.name)
@topic.change_category_to_id(@new_category.id)
@topic.reload
@new_category.reload
@category.reload
@ -918,13 +918,13 @@ describe Topic do
let!(:topic) { Fabricate(:topic, category: Fabricate(:category)) }
it 'returns false' do
topic.change_category('').should eq(false) # don't use "be_false" here because it would also match nil
topic.change_category_to_id(nil).should eq(false) # don't use "be_false" here because it would also match nil
end
end
describe 'when the category exists' do
before do
@topic.change_category(nil)
@topic.change_category_to_id(nil)
@category.reload
end