Commit 5451b3b986d042c303e35db730682cbfcf09ab18
1 parent
590bfeff
Exists in
master
and in
22 other branches
ActionItem466: changing article categorization
we'll categorize articles in every category from the actual category chosen by the user to the root of its hierarchy. git-svn-id: https://svn.colivre.coop.br/svn/noosfero/trunk@2068 3f533792-8f58-4932-b0fe-aaf55b0a4547
Showing
7 changed files
with
150 additions
and
3 deletions
Show diff stats
app/models/article.rb
@@ -12,7 +12,35 @@ class Article < ActiveRecord::Base | @@ -12,7 +12,35 @@ class Article < ActiveRecord::Base | ||
12 | 12 | ||
13 | has_many :comments, :dependent => :destroy | 13 | has_many :comments, :dependent => :destroy |
14 | 14 | ||
15 | - has_and_belongs_to_many :categories | 15 | + has_many :article_categorizations, :conditions => { :virtual => false } |
16 | + has_many :categories, :through => :article_categorizations | ||
17 | + | ||
18 | + def pending_categorizations | ||
19 | + @pending_categorizations ||= [] | ||
20 | + end | ||
21 | + | ||
22 | + def add_category(c) | ||
23 | + if self.id | ||
24 | + ArticleCategorization.create!(:category => c, :article => self) | ||
25 | + else | ||
26 | + pending_categorizations << c | ||
27 | + end | ||
28 | + end | ||
29 | + | ||
30 | + def category_ids=(ids) | ||
31 | + ArticleCategorization.remove_all_for(self) | ||
32 | + ids.each do |item| | ||
33 | + add_category(Category.find(item)) | ||
34 | + end | ||
35 | + end | ||
36 | + | ||
37 | + after_create :create_pending_categorizations | ||
38 | + def create_pending_categorizations | ||
39 | + pending_categorizations.each do |item| | ||
40 | + ArticleCategorization.create!(:category => item, :article => self) | ||
41 | + end | ||
42 | + pending_categorizations.clear | ||
43 | + end | ||
16 | 44 | ||
17 | acts_as_taggable | 45 | acts_as_taggable |
18 | N_('Tag list') | 46 | N_('Tag list') |
app/models/article_categorization.rb
@@ -2,4 +2,20 @@ class ArticleCategorization < ActiveRecord::Base | @@ -2,4 +2,20 @@ class ArticleCategorization < ActiveRecord::Base | ||
2 | set_table_name :articles_categories | 2 | set_table_name :articles_categories |
3 | belongs_to :article | 3 | belongs_to :article |
4 | belongs_to :category | 4 | belongs_to :category |
5 | + | ||
6 | + after_create :associate_with_entire_hierarchy | ||
7 | + def associate_with_entire_hierarchy | ||
8 | + return if virtual | ||
9 | + | ||
10 | + c = category.parent | ||
11 | + while !c.nil? && !self.class.find(:first, :conditions => {:article_id => article, :category_id => c}) | ||
12 | + self.class.create!(:article => article, :category => c, :virtual => true) | ||
13 | + c = c.parent | ||
14 | + end | ||
15 | + end | ||
16 | + | ||
17 | + def self.remove_all_for(article) | ||
18 | + self.delete_all(:article_id => article.id) | ||
19 | + end | ||
20 | + | ||
5 | end | 21 | end |
@@ -0,0 +1,11 @@ | @@ -0,0 +1,11 @@ | ||
1 | +class AddVirtualFlagToCategorizations < ActiveRecord::Migration | ||
2 | + def self.up | ||
3 | + add_column :articles_categories, :virtual, :boolean, :default => false | ||
4 | + add_column :categories_profiles, :virtual, :boolean, :default => false | ||
5 | + end | ||
6 | + | ||
7 | + def self.down | ||
8 | + remove_column :articles_categories, :virtual | ||
9 | + remove_column :categories_profiles, :virtual | ||
10 | + end | ||
11 | +end |
db/schema.rb
@@ -9,7 +9,7 @@ | @@ -9,7 +9,7 @@ | ||
9 | # | 9 | # |
10 | # It's strongly recommended to check this file into your version control system. | 10 | # It's strongly recommended to check this file into your version control system. |
11 | 11 | ||
12 | -ActiveRecord::Schema.define(:version => 42) do | 12 | +ActiveRecord::Schema.define(:version => 43) do |
13 | 13 | ||
14 | create_table "article_versions", :force => true do |t| | 14 | create_table "article_versions", :force => true do |t| |
15 | t.integer "article_id" | 15 | t.integer "article_id" |
@@ -68,6 +68,7 @@ ActiveRecord::Schema.define(:version => 42) do | @@ -68,6 +68,7 @@ ActiveRecord::Schema.define(:version => 42) do | ||
68 | create_table "articles_categories", :id => false, :force => true do |t| | 68 | create_table "articles_categories", :id => false, :force => true do |t| |
69 | t.integer "article_id" | 69 | t.integer "article_id" |
70 | t.integer "category_id" | 70 | t.integer "category_id" |
71 | + t.boolean "virtual", :default => false | ||
71 | end | 72 | end |
72 | 73 | ||
73 | add_index "articles_categories", ["category_id"], :name => "index_articles_categories_on_category_id" | 74 | add_index "articles_categories", ["category_id"], :name => "index_articles_categories_on_category_id" |
@@ -104,6 +105,7 @@ ActiveRecord::Schema.define(:version => 42) do | @@ -104,6 +105,7 @@ ActiveRecord::Schema.define(:version => 42) do | ||
104 | create_table "categories_profiles", :id => false, :force => true do |t| | 105 | create_table "categories_profiles", :id => false, :force => true do |t| |
105 | t.integer "profile_id" | 106 | t.integer "profile_id" |
106 | t.integer "category_id" | 107 | t.integer "category_id" |
108 | + t.boolean "virtual", :default => false | ||
107 | end | 109 | end |
108 | 110 | ||
109 | create_table "comments", :force => true do |t| | 111 | create_table "comments", :force => true do |t| |
test/functional/cms_controller_test.rb
@@ -281,7 +281,7 @@ class CmsControllerTest < Test::Unit::TestCase | @@ -281,7 +281,7 @@ class CmsControllerTest < Test::Unit::TestCase | ||
281 | # post is in c1 and c3 | 281 | # post is in c1 and c3 |
282 | post :new, :type => TextileArticle.name, :profile => profile.identifier, :article => { :name => 'adding-categories-test', :category_ids => [ c1.id, c3.id] } | 282 | post :new, :type => TextileArticle.name, :profile => profile.identifier, :article => { :name => 'adding-categories-test', :category_ids => [ c1.id, c3.id] } |
283 | 283 | ||
284 | - saved = TextileArticle.find_by_name('adding-categories-test') | 284 | + saved = profile.articles.find_by_name('adding-categories-test') |
285 | assert_includes saved.categories, c1 | 285 | assert_includes saved.categories, c1 |
286 | assert_not_includes saved.categories, c2 | 286 | assert_not_includes saved.categories, c2 |
287 | assert_includes saved.categories, c3 | 287 | assert_includes saved.categories, c3 |
test/unit/article_categorization_test.rb
@@ -17,4 +17,48 @@ class ArticleCategorizationTest < Test::Unit::TestCase | @@ -17,4 +17,48 @@ class ArticleCategorizationTest < Test::Unit::TestCase | ||
17 | assert_equal category, ArticleCategorization.new(:category => category).category | 17 | assert_equal category, ArticleCategorization.new(:category => category).category |
18 | end | 18 | end |
19 | 19 | ||
20 | + should 'create instances for the entire hierarchy' do | ||
21 | + c1 = Category.create!(:name => 'c1', :environment => Environment.default) | ||
22 | + c2 = c1.children.create!(:name => 'c2', :environment => Environment.default) | ||
23 | + | ||
24 | + p = create_user('testuser').person | ||
25 | + a = p.articles.create!(:name => 'test') | ||
26 | + | ||
27 | + assert_difference ArticleCategorization, :count, 2 do | ||
28 | + ArticleCategorization.create!(:category => c2, :article => a) | ||
29 | + end | ||
30 | + | ||
31 | + assert_equal 2, ArticleCategorization.find_all_by_article_id(a.id).size | ||
32 | + end | ||
33 | + | ||
34 | + should 'not duplicate entry for category that is parent of two others' do | ||
35 | + c1 = Category.create!(:name => 'c1', :environment => Environment.default) | ||
36 | + c2 = c1.children.create!(:name => 'c2', :environment => Environment.default) | ||
37 | + c3 = c1.children.create!(:name => 'c3', :environment => Environment.default) | ||
38 | + | ||
39 | + p = create_user('testuser').person | ||
40 | + a = p.articles.create!(:name => 'test') | ||
41 | + | ||
42 | + assert_difference ArticleCategorization, :count, 3 do | ||
43 | + ac = ArticleCategorization.create!(:category => c2, :article => a) | ||
44 | + ac = ArticleCategorization.create!(:category => c3, :article => a) | ||
45 | + end | ||
46 | + end | ||
47 | + | ||
48 | + should 'remove all instances for a given article' do | ||
49 | + c1 = Category.create!(:name => 'c1', :environment => Environment.default) | ||
50 | + c2 = c1.children.create!(:name => 'c2', :environment => Environment.default) | ||
51 | + c3 = c1.children.create!(:name => 'c3', :environment => Environment.default) | ||
52 | + | ||
53 | + p = create_user('testuser').person | ||
54 | + a = p.articles.create!(:name => 'test') | ||
55 | + | ||
56 | + ac = ArticleCategorization.create!(:category => c2, :article => a) | ||
57 | + ac = ArticleCategorization.create!(:category => c3, :article => a) | ||
58 | + | ||
59 | + assert_difference ArticleCategorization, :count, -3 do | ||
60 | + ArticleCategorization.remove_all_for(a) | ||
61 | + end | ||
62 | + end | ||
63 | + | ||
20 | end | 64 | end |
test/unit/article_test.rb
@@ -183,6 +183,8 @@ class ArticleTest < Test::Unit::TestCase | @@ -183,6 +183,8 @@ class ArticleTest < Test::Unit::TestCase | ||
183 | article.categories << c1 | 183 | article.categories << c1 |
184 | article.categories << c2 | 184 | article.categories << c2 |
185 | end | 185 | end |
186 | + | ||
187 | + assert_equivalent [c1,c2], article.categories(true) | ||
186 | end | 188 | end |
187 | 189 | ||
188 | should 'remove comments when removing article' do | 190 | should 'remove comments when removing article' do |
@@ -324,4 +326,48 @@ class ArticleTest < Test::Unit::TestCase | @@ -324,4 +326,48 @@ class ArticleTest < Test::Unit::TestCase | ||
324 | 326 | ||
325 | end | 327 | end |
326 | 328 | ||
329 | + should 'categorize in the entire category hierarchy' do | ||
330 | + c1 = Category.create!(:environment => Environment.default, :name => 'c1') | ||
331 | + c2 = c1.children.create!(:environment => Environment.default, :name => 'c2') | ||
332 | + c3 = c2.children.create!(:environment => Environment.default, :name => 'c3') | ||
333 | + | ||
334 | + owner = create_user('testuser').person | ||
335 | + art = owner.articles.create!(:name => 'ytest') | ||
336 | + | ||
337 | + art.add_category(c3) | ||
338 | + | ||
339 | + assert_equal [c3], art.categories(true) | ||
340 | + assert_equal [art], c2.articles(true) | ||
341 | + | ||
342 | + assert_includes c3.articles(true), art | ||
343 | + assert_includes c2.articles(true), art | ||
344 | + assert_includes c1.articles(true), art | ||
345 | + end | ||
346 | + | ||
347 | + should 'redefine the entire category set at once' do | ||
348 | + c1 = Category.create!(:environment => Environment.default, :name => 'c1') | ||
349 | + c2 = c1.children.create!(:environment => Environment.default, :name => 'c2') | ||
350 | + c3 = c2.children.create!(:environment => Environment.default, :name => 'c3') | ||
351 | + c4 = c1.children.create!(:environment => Environment.default, :name => 'c4') | ||
352 | + owner = create_user('testuser').person | ||
353 | + art = owner.articles.create!(:name => 'ytest') | ||
354 | + | ||
355 | + art.add_category(c4) | ||
356 | + | ||
357 | + art.category_ids = [c2,c3].map(&:id) | ||
358 | + | ||
359 | + assert_equivalent [c2, c3], art.categories(true) | ||
360 | + end | ||
361 | + | ||
362 | + should 'be able to create an article already with categories' do | ||
363 | + c1 = Category.create!(:environment => Environment.default, :name => 'c1') | ||
364 | + c2 = Category.create!(:environment => Environment.default, :name => 'c2') | ||
365 | + | ||
366 | + p = create_user('testinguser').person | ||
367 | + a = p.articles.create!(:name => 'test', :category_ids => [c1.id, c2.id]) | ||
368 | + | ||
369 | + assert_equivalent [c1, c2], a.categories(true) | ||
370 | + | ||
371 | + end | ||
372 | + | ||
327 | end | 373 | end |