Commit 99d66a607b3f6b72beeb8e2d9fb0a724c2452044
1 parent
3d858878
Exists in
master
and in
29 other branches
Adding (real) authors to articles
Showing
9 changed files
with
79 additions
and
26 deletions
Show diff stats
app/controllers/my_profile/cms_controller.rb
app/models/article.rb
| ... | ... | @@ -13,6 +13,12 @@ class Article < ActiveRecord::Base |
| 13 | 13 | # xss_terminate plugin can't sanitize array fields |
| 14 | 14 | before_save :sanitize_tag_list |
| 15 | 15 | |
| 16 | + before_save do |article| | |
| 17 | + if article.author_id | |
| 18 | + article.author_name = Person.find(article.author_id).name | |
| 19 | + end | |
| 20 | + end | |
| 21 | + | |
| 16 | 22 | belongs_to :profile |
| 17 | 23 | validates_presence_of :profile_id, :name |
| 18 | 24 | validates_presence_of :slug, :path, :if => lambda { |article| !article.name.blank? } |
| ... | ... | @@ -20,6 +26,7 @@ class Article < ActiveRecord::Base |
| 20 | 26 | validates_uniqueness_of :slug, :scope => ['profile_id', 'parent_id'], :message => N_('The title (article name) is already being used by another article, please use another title.'), :if => lambda { |article| !article.slug.blank? } |
| 21 | 27 | |
| 22 | 28 | belongs_to :last_changed_by, :class_name => 'Person', :foreign_key => 'last_changed_by_id' |
| 29 | + belongs_to :author, :class_name => 'Person', :foreign_key => 'author_id' | |
| 23 | 30 | |
| 24 | 31 | has_many :comments, :class_name => 'Comment', :foreign_key => 'source_id', :dependent => :destroy, :order => 'created_at asc' |
| 25 | 32 | |
| ... | ... | @@ -278,7 +285,7 @@ class Article < ActiveRecord::Base |
| 278 | 285 | if last_comment |
| 279 | 286 | {:date => last_comment.created_at, :author_name => last_comment.author_name, :author_url => last_comment.author_url} |
| 280 | 287 | else |
| 281 | - {:date => updated_at, :author_name => author.name, :author_url => author.url} | |
| 288 | + {:date => updated_at, :author_name => author_name, :author_url => author ? author.url : nil} | |
| 282 | 289 | end |
| 283 | 290 | end |
| 284 | 291 | |
| ... | ... | @@ -430,7 +437,7 @@ class Article < ActiveRecord::Base |
| 430 | 437 | end |
| 431 | 438 | |
| 432 | 439 | def allow_post_content?(user = nil) |
| 433 | - user && (user.has_permission?('post_content', profile) || allow_publish_content?(user) && (user == self.creator)) | |
| 440 | + user && (user.has_permission?('post_content', profile) || allow_publish_content?(user) && (user == author)) | |
| 434 | 441 | end |
| 435 | 442 | |
| 436 | 443 | def allow_publish_content?(user = nil) |
| ... | ... | @@ -527,16 +534,8 @@ class Article < ActiveRecord::Base |
| 527 | 534 | false |
| 528 | 535 | end |
| 529 | 536 | |
| 530 | - def author | |
| 531 | - if reference_article | |
| 532 | - reference_article.author | |
| 533 | - else | |
| 534 | - last_changed_by || profile | |
| 535 | - end | |
| 536 | - end | |
| 537 | - | |
| 538 | 537 | def author_name |
| 539 | - setting[:author_name].blank? ? author.name : setting[:author_name] | |
| 538 | + author ? author.name : setting[:author_name] | |
| 540 | 539 | end |
| 541 | 540 | |
| 542 | 541 | alias :active_record_cache_key :cache_key |
| ... | ... | @@ -561,11 +560,6 @@ class Article < ActiveRecord::Base |
| 561 | 560 | truncate sanitize_html(self.lead), :length => 170, :omission => '...' |
| 562 | 561 | end |
| 563 | 562 | |
| 564 | - def creator | |
| 565 | - creator_id = versions[0][:last_changed_by_id] | |
| 566 | - creator_id && Profile.find(creator_id) | |
| 567 | - end | |
| 568 | - | |
| 569 | 563 | def notifiable? |
| 570 | 564 | false |
| 571 | 565 | end | ... | ... |
app/models/person.rb
| ... | ... | @@ -54,6 +54,8 @@ class Person < Profile |
| 54 | 54 | |
| 55 | 55 | has_many :scraps_sent, :class_name => 'Scrap', :foreign_key => :sender_id, :dependent => :destroy |
| 56 | 56 | |
| 57 | + has_many :creations, :class_name => 'Article', :foreign_key => :author_id | |
| 58 | + | |
| 57 | 59 | named_scope :more_popular, |
| 58 | 60 | :select => "#{Profile.qualified_column_names}, count(friend_id) as total", |
| 59 | 61 | :group => Profile.qualified_column_names, | ... | ... |
| ... | ... | @@ -0,0 +1,11 @@ |
| 1 | +class AddAuthorIdToArticle < ActiveRecord::Migration | |
| 2 | + def self.up | |
| 3 | + add_column :articles, :author_id, :integer | |
| 4 | + add_column :article_versions, :author_id, :integer | |
| 5 | + end | |
| 6 | + | |
| 7 | + def self.down | |
| 8 | + remove_column :articles, :author_id | |
| 9 | + remove_column :article_versions, :author_id, :integer | |
| 10 | + end | |
| 11 | +end | ... | ... |
db/migrate/20121024222938_set_authors_based_on_article_versions.rb
0 → 100644
| ... | ... | @@ -0,0 +1,9 @@ |
| 1 | +class SetAuthorsBasedOnArticleVersions < ActiveRecord::Migration | |
| 2 | + def self.up | |
| 3 | + update('UPDATE articles SET author_id = (SELECT profiles.id FROM articles as a INNER JOIN article_versions ON a.id = article_versions.article_id INNER JOIN profiles ON profiles.id = article_versions.last_changed_by_id WHERE article_versions.version = 1 AND articles.id = a.id)') | |
| 4 | + end | |
| 5 | + | |
| 6 | + def self.down | |
| 7 | + say "Can not be revesed!" | |
| 8 | + end | |
| 9 | +end | ... | ... |
db/schema.rb
| ... | ... | @@ -9,7 +9,7 @@ |
| 9 | 9 | # |
| 10 | 10 | # It's strongly recommended to check this file into your version control system. |
| 11 | 11 | |
| 12 | -ActiveRecord::Schema.define(:version => 20120825185219) do | |
| 12 | +ActiveRecord::Schema.define(:version => 20121024222938) do | |
| 13 | 13 | |
| 14 | 14 | create_table "abuse_reports", :force => true do |t| |
| 15 | 15 | t.integer "reporter_id" |
| ... | ... | @@ -86,6 +86,7 @@ ActiveRecord::Schema.define(:version => 20120825185219) do |
| 86 | 86 | t.string "language" |
| 87 | 87 | t.string "source_name" |
| 88 | 88 | t.integer "license_id" |
| 89 | + t.integer "author_id" | |
| 89 | 90 | end |
| 90 | 91 | |
| 91 | 92 | create_table "articles", :force => true do |t| |
| ... | ... | @@ -127,6 +128,7 @@ ActiveRecord::Schema.define(:version => 20120825185219) do |
| 127 | 128 | t.string "language" |
| 128 | 129 | t.string "source_name" |
| 129 | 130 | t.integer "license_id" |
| 131 | + t.integer "author_id" | |
| 130 | 132 | end |
| 131 | 133 | |
| 132 | 134 | add_index "articles", ["translation_of_id"], :name => "index_articles_on_translation_of_id" | ... | ... |
test/functional/cms_controller_test.rb
| ... | ... | @@ -1556,6 +1556,16 @@ class CmsControllerTest < ActionController::TestCase |
| 1556 | 1556 | assert_equal license, article.license |
| 1557 | 1557 | end |
| 1558 | 1558 | |
| 1559 | + should 'set author when creating article' do | |
| 1560 | + login_as(profile.identifier) | |
| 1561 | + | |
| 1562 | + post :new, :type => 'TinyMceArticle', :profile => profile.identifier, :article => { :name => 'Sample Article', :body => 'content ...' } | |
| 1563 | + | |
| 1564 | + a = profile.articles.find_by_path('sample-article') | |
| 1565 | + assert_not_nil a | |
| 1566 | + assert_equal profile, a.author | |
| 1567 | + end | |
| 1568 | + | |
| 1559 | 1569 | protected |
| 1560 | 1570 | |
| 1561 | 1571 | # FIXME this is to avoid adding an extra dependency for a proper JSON parser. | ... | ... |
test/unit/article_test.rb
| ... | ... | @@ -708,14 +708,6 @@ class ArticleTest < ActiveSupport::TestCase |
| 708 | 708 | assert_equal a.url, a.view_url |
| 709 | 709 | end |
| 710 | 710 | |
| 711 | - should 'know its author' do | |
| 712 | - assert_equal profile, Article.new(:last_changed_by => profile).author | |
| 713 | - end | |
| 714 | - | |
| 715 | - should 'use owning profile as author when we dont know who did the last change' do | |
| 716 | - assert_equal profile, Article.new(:last_changed_by => nil, :profile => profile).author | |
| 717 | - end | |
| 718 | - | |
| 719 | 711 | should 'have published_at' do |
| 720 | 712 | assert_respond_to Article.new, :published_at |
| 721 | 713 | end |
| ... | ... | @@ -1372,7 +1364,7 @@ class ArticleTest < ActiveSupport::TestCase |
| 1372 | 1364 | assert Article.method_defined?('author_name') |
| 1373 | 1365 | end |
| 1374 | 1366 | |
| 1375 | - should "the author_name returns the name od the article's author" do | |
| 1367 | + should "the author_name returns the name of the article's author" do | |
| 1376 | 1368 | author = mock() |
| 1377 | 1369 | author.expects(:name).returns('author name') |
| 1378 | 1370 | a = Article.new |
| ... | ... | @@ -1782,4 +1774,25 @@ class ArticleTest < ActiveSupport::TestCase |
| 1782 | 1774 | assert_equal license, article.license |
| 1783 | 1775 | end |
| 1784 | 1776 | |
| 1777 | + should 'be able to have an author' do | |
| 1778 | + author = fast_create(Person) | |
| 1779 | + article = Article.new | |
| 1780 | + assert_nothing_raised do | |
| 1781 | + article.author = author | |
| 1782 | + end | |
| 1783 | + end | |
| 1784 | + | |
| 1785 | + should 'set author_name before saving article if there is an author' do | |
| 1786 | + author = fast_create(Person) | |
| 1787 | + article = fast_create(Article, :profile_id => fast_create(Profile).id) | |
| 1788 | + article.author = author | |
| 1789 | + article.save! | |
| 1790 | + assert_equal author.name, article.author_name | |
| 1791 | + | |
| 1792 | + author_name = author.name | |
| 1793 | + author.destroy | |
| 1794 | + article.reload | |
| 1795 | + assert_equal author_name, article.author_name | |
| 1796 | + end | |
| 1797 | + | |
| 1785 | 1798 | end | ... | ... |
test/unit/person_test.rb
| ... | ... | @@ -1262,4 +1262,15 @@ class PersonTest < ActiveSupport::TestCase |
| 1262 | 1262 | |
| 1263 | 1263 | assert person.has_permission?('bli', Profile.new) |
| 1264 | 1264 | end |
| 1265 | + | |
| 1266 | + should 'have creations' do | |
| 1267 | + person = create_user('some-user').person | |
| 1268 | + a1 = fast_create(Article, :author_id => person.id) | |
| 1269 | + a2 = fast_create(Article, :author_id => person.id) | |
| 1270 | + a3 = fast_create(Article) | |
| 1271 | + | |
| 1272 | + assert_includes person.creations, a1 | |
| 1273 | + assert_includes person.creations, a2 | |
| 1274 | + assert_not_includes person.creations, a3 | |
| 1275 | + end | |
| 1265 | 1276 | end | ... | ... |