Commit 556c40eb38f359091349ac57ec7c461836e7e550
Exists in
staging
and in
42 other branches
Merge remote-tracking branch 'origin/master'
Showing
9 changed files
with
84 additions
and
30 deletions
Show diff stats
app/controllers/my_profile/cms_controller.rb
| @@ -144,6 +144,7 @@ class CmsController < MyProfileController | @@ -144,6 +144,7 @@ class CmsController < MyProfileController | ||
| 144 | 144 | ||
| 145 | @article.profile = profile | 145 | @article.profile = profile |
| 146 | @article.last_changed_by = user | 146 | @article.last_changed_by = user |
| 147 | + @article.created_by = user | ||
| 147 | 148 | ||
| 148 | translations if @article.translatable? | 149 | translations if @article.translatable? |
| 149 | 150 |
app/models/approve_article.rb
| @@ -48,7 +48,7 @@ class ApproveArticle < Task | @@ -48,7 +48,7 @@ class ApproveArticle < Task | ||
| 48 | end | 48 | end |
| 49 | 49 | ||
| 50 | def perform | 50 | def perform |
| 51 | - article.copy!(:name => name, :abstract => abstract, :body => body, :profile => target, :reference_article => article, :parent => article_parent, :highlighted => highlighted, :source => article.source, :last_changed_by_id => article.author_id) | 51 | + article.copy!(:name => name, :abstract => abstract, :body => body, :profile => target, :reference_article => article, :parent => article_parent, :highlighted => highlighted, :source => article.source, :last_changed_by_id => article.last_changed_by_id, :created_by_id => article.created_by_id) |
| 52 | end | 52 | end |
| 53 | 53 | ||
| 54 | def title | 54 | def title |
app/models/article.rb
| @@ -40,12 +40,6 @@ class Article < ActiveRecord::Base | @@ -40,12 +40,6 @@ class Article < ActiveRecord::Base | ||
| 40 | # xss_terminate plugin can't sanitize array fields | 40 | # xss_terminate plugin can't sanitize array fields |
| 41 | before_save :sanitize_tag_list | 41 | before_save :sanitize_tag_list |
| 42 | 42 | ||
| 43 | - before_create do |article| | ||
| 44 | - if article.last_changed_by_id | ||
| 45 | - article.author_name = Person.find(article.last_changed_by_id).name | ||
| 46 | - end | ||
| 47 | - end | ||
| 48 | - | ||
| 49 | belongs_to :profile | 43 | belongs_to :profile |
| 50 | validates_presence_of :profile_id, :name | 44 | validates_presence_of :profile_id, :name |
| 51 | validates_presence_of :slug, :path, :if => lambda { |article| !article.name.blank? } | 45 | validates_presence_of :slug, :path, :if => lambda { |article| !article.name.blank? } |
| @@ -55,6 +49,7 @@ class Article < ActiveRecord::Base | @@ -55,6 +49,7 @@ class Article < ActiveRecord::Base | ||
| 55 | 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? } | 49 | 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? } |
| 56 | 50 | ||
| 57 | belongs_to :last_changed_by, :class_name => 'Person', :foreign_key => 'last_changed_by_id' | 51 | belongs_to :last_changed_by, :class_name => 'Person', :foreign_key => 'last_changed_by_id' |
| 52 | + belongs_to :created_by, :class_name => 'Person', :foreign_key => 'created_by_id' | ||
| 58 | 53 | ||
| 59 | has_many :comments, :class_name => 'Comment', :foreign_key => 'source_id', :dependent => :destroy, :order => 'created_at asc' | 54 | has_many :comments, :class_name => 'Comment', :foreign_key => 'source_id', :dependent => :destroy, :order => 'created_at asc' |
| 60 | 55 | ||
| @@ -89,6 +84,11 @@ class Article < ActiveRecord::Base | @@ -89,6 +84,11 @@ class Article < ActiveRecord::Base | ||
| 89 | article.parent = article.profile.blog | 84 | article.parent = article.profile.blog |
| 90 | end | 85 | end |
| 91 | end | 86 | end |
| 87 | + | ||
| 88 | + if article.created_by | ||
| 89 | + article.author_name = article.created_by.name | ||
| 90 | + end | ||
| 91 | + | ||
| 92 | end | 92 | end |
| 93 | 93 | ||
| 94 | after_destroy :destroy_activity | 94 | after_destroy :destroy_activity |
| @@ -630,17 +630,13 @@ class Article < ActiveRecord::Base | @@ -630,17 +630,13 @@ class Article < ActiveRecord::Base | ||
| 630 | 630 | ||
| 631 | def author(version_number = nil) | 631 | def author(version_number = nil) |
| 632 | if version_number | 632 | if version_number |
| 633 | - version = versions.find_by_version(version_number) | 633 | + version = self.versions.find_by_version(version_number) |
| 634 | author_id = version.last_changed_by_id if version | 634 | author_id = version.last_changed_by_id if version |
| 635 | - Person.exists?(author_id) ? Person.find(author_id) : nil | ||
| 636 | else | 635 | else |
| 637 | - if versions.empty? | ||
| 638 | - last_changed_by | ||
| 639 | - else | ||
| 640 | - author_id = versions.first.last_changed_by_id | ||
| 641 | - Person.exists?(author_id) ? Person.find(author_id) : nil | ||
| 642 | - end | 636 | + author_id = self.created_by_id |
| 643 | end | 637 | end |
| 638 | + | ||
| 639 | + environment.people.find_by_id(author_id) | ||
| 644 | end | 640 | end |
| 645 | 641 | ||
| 646 | def author_name(version_number = nil) | 642 | def author_name(version_number = nil) |
| @@ -0,0 +1,17 @@ | @@ -0,0 +1,17 @@ | ||
| 1 | +class AddCreatedByToArticle < ActiveRecord::Migration | ||
| 2 | + def self.up | ||
| 3 | + add_column :articles, :created_by_id, :integer | ||
| 4 | + add_column :article_versions, :created_by_id, :integer | ||
| 5 | + | ||
| 6 | + execute("UPDATE article_versions SET created_by_id = last_changed_by_id") | ||
| 7 | + | ||
| 8 | + execute("UPDATE articles SET created_by_id = article_versions.created_by_id | ||
| 9 | +FROM article_versions WHERE article_versions.article_id = articles.id AND | ||
| 10 | +article_versions.version = 1") | ||
| 11 | + end | ||
| 12 | + | ||
| 13 | + def self.down | ||
| 14 | + remove_column :articles, :created_by_id | ||
| 15 | + remove_column :article_versions, :created_by_id | ||
| 16 | + end | ||
| 17 | +end |
db/schema.rb
| @@ -11,7 +11,7 @@ | @@ -11,7 +11,7 @@ | ||
| 11 | # | 11 | # |
| 12 | # It's strongly recommended to check this file into your version control system. | 12 | # It's strongly recommended to check this file into your version control system. |
| 13 | 13 | ||
| 14 | -ActiveRecord::Schema.define(:version => 20140408172149) do | 14 | +ActiveRecord::Schema.define(:version => 20140415125414) do |
| 15 | 15 | ||
| 16 | create_table "abuse_reports", :force => true do |t| | 16 | create_table "abuse_reports", :force => true do |t| |
| 17 | t.integer "reporter_id" | 17 | t.integer "reporter_id" |
| @@ -95,6 +95,7 @@ ActiveRecord::Schema.define(:version => 20140408172149) do | @@ -95,6 +95,7 @@ ActiveRecord::Schema.define(:version => 20140408172149) do | ||
| 95 | t.integer "license_id" | 95 | t.integer "license_id" |
| 96 | t.integer "image_id" | 96 | t.integer "image_id" |
| 97 | t.integer "position" | 97 | t.integer "position" |
| 98 | + t.integer "created_by_id" | ||
| 98 | end | 99 | end |
| 99 | 100 | ||
| 100 | add_index "article_versions", ["article_id"], :name => "index_article_versions_on_article_id" | 101 | add_index "article_versions", ["article_id"], :name => "index_article_versions_on_article_id" |
| @@ -140,6 +141,7 @@ ActiveRecord::Schema.define(:version => 20140408172149) do | @@ -140,6 +141,7 @@ ActiveRecord::Schema.define(:version => 20140408172149) do | ||
| 140 | t.integer "license_id" | 141 | t.integer "license_id" |
| 141 | t.integer "image_id" | 142 | t.integer "image_id" |
| 142 | t.integer "position" | 143 | t.integer "position" |
| 144 | + t.integer "created_by_id" | ||
| 143 | end | 145 | end |
| 144 | 146 | ||
| 145 | add_index "articles", ["comments_count"], :name => "index_articles_on_comments_count" | 147 | add_index "articles", ["comments_count"], :name => "index_articles_on_comments_count" |
test/functional/cms_controller_test.rb
| @@ -1228,7 +1228,7 @@ class CmsControllerTest < ActionController::TestCase | @@ -1228,7 +1228,7 @@ class CmsControllerTest < ActionController::TestCase | ||
| 1228 | should 'allow user edit article if he is owner and has publish permission' do | 1228 | should 'allow user edit article if he is owner and has publish permission' do |
| 1229 | c = Community.create!(:name => 'test_comm', :identifier => 'test_comm') | 1229 | c = Community.create!(:name => 'test_comm', :identifier => 'test_comm') |
| 1230 | u = create_user_with_permission('test_user', 'publish_content', c) | 1230 | u = create_user_with_permission('test_user', 'publish_content', c) |
| 1231 | - a = create(Article, :profile => c, :name => 'test_article', :last_changed_by => u) | 1231 | + a = create(Article, :profile => c, :name => 'test_article', :created_by => u) |
| 1232 | login_as :test_user | 1232 | login_as :test_user |
| 1233 | @controller.stubs(:user).returns(u) | 1233 | @controller.stubs(:user).returns(u) |
| 1234 | 1234 | ||
| @@ -1774,6 +1774,31 @@ class CmsControllerTest < ActionController::TestCase | @@ -1774,6 +1774,31 @@ class CmsControllerTest < ActionController::TestCase | ||
| 1774 | assert_equal 'first version', Article.find(article.id).name | 1774 | assert_equal 'first version', Article.find(article.id).name |
| 1775 | end | 1775 | end |
| 1776 | 1776 | ||
| 1777 | + should 'set created_by when creating article' do | ||
| 1778 | + login_as(profile.identifier) | ||
| 1779 | + | ||
| 1780 | + post :new, :type => 'TinyMceArticle', :profile => profile.identifier, :article => { :name => 'changed by me', :body => 'content ...' } | ||
| 1781 | + | ||
| 1782 | + a = profile.articles.find_by_path('changed-by-me') | ||
| 1783 | + assert_not_nil a | ||
| 1784 | + assert_equal profile, a.created_by | ||
| 1785 | + end | ||
| 1786 | + | ||
| 1787 | + should 'not change created_by when updating article' do | ||
| 1788 | + other_person = create_user('otherperson').person | ||
| 1789 | + | ||
| 1790 | + a = profile.articles.build(:name => 'my article') | ||
| 1791 | + a.created_by = other_person | ||
| 1792 | + a.save! | ||
| 1793 | + | ||
| 1794 | + login_as(profile.identifier) | ||
| 1795 | + post :edit, :profile => profile.identifier, :id => a.id, :article => { :body => 'new content for this article' } | ||
| 1796 | + | ||
| 1797 | + a.reload | ||
| 1798 | + | ||
| 1799 | + assert_equal other_person, a.created_by | ||
| 1800 | + end | ||
| 1801 | + | ||
| 1777 | protected | 1802 | protected |
| 1778 | 1803 | ||
| 1779 | # FIXME this is to avoid adding an extra dependency for a proper JSON parser. | 1804 | # FIXME this is to avoid adding an extra dependency for a proper JSON parser. |
test/functional/content_viewer_controller_test.rb
| @@ -743,7 +743,7 @@ class ContentViewerControllerTest < ActionController::TestCase | @@ -743,7 +743,7 @@ class ContentViewerControllerTest < ActionController::TestCase | ||
| 743 | c = Community.create!(:name => 'test_com') | 743 | c = Community.create!(:name => 'test_com') |
| 744 | u = create_user_with_permission('test_user', 'publish_content', c) | 744 | u = create_user_with_permission('test_user', 'publish_content', c) |
| 745 | login_as u.identifier | 745 | login_as u.identifier |
| 746 | - a = create(Article, :profile => c, :name => 'test-article', :last_changed_by => u, :published => false) | 746 | + a = create(Article, :profile => c, :name => 'test-article', :created_by => u, :published => false) |
| 747 | 747 | ||
| 748 | get :view_page, :profile => c.identifier, :page => a.path | 748 | get :view_page, :profile => c.identifier, :page => a.path |
| 749 | 749 |
test/unit/article_test.rb
| @@ -154,7 +154,7 @@ class ArticleTest < ActiveSupport::TestCase | @@ -154,7 +154,7 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 154 | assert a4.errors[:slug.to_s].present? | 154 | assert a4.errors[:slug.to_s].present? |
| 155 | end | 155 | end |
| 156 | 156 | ||
| 157 | - should 'record who did the last change' do | 157 | + should 'last_changed_by be a person' do |
| 158 | a = profile.articles.build(:name => 'test') | 158 | a = profile.articles.build(:name => 'test') |
| 159 | 159 | ||
| 160 | # must be a person | 160 | # must be a person |
| @@ -167,6 +167,19 @@ class ArticleTest < ActiveSupport::TestCase | @@ -167,6 +167,19 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 167 | end | 167 | end |
| 168 | end | 168 | end |
| 169 | 169 | ||
| 170 | + should 'created_by be a person' do | ||
| 171 | + a = profile.articles.build(:name => 'test') | ||
| 172 | + | ||
| 173 | + # must be a person | ||
| 174 | + assert_raise ActiveRecord::AssociationTypeMismatch do | ||
| 175 | + a.created_by = Profile.new | ||
| 176 | + end | ||
| 177 | + assert_nothing_raised do | ||
| 178 | + a.created_by = Person.new | ||
| 179 | + a.save! | ||
| 180 | + end | ||
| 181 | + end | ||
| 182 | + | ||
| 170 | should 'not show private documents as recent' do | 183 | should 'not show private documents as recent' do |
| 171 | p = create_user('usr1').person | 184 | p = create_user('usr1').person |
| 172 | Article.destroy_all | 185 | Article.destroy_all |
| @@ -802,7 +815,7 @@ class ArticleTest < ActiveSupport::TestCase | @@ -802,7 +815,7 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 802 | should 'allow author to edit if is publisher' do | 815 | should 'allow author to edit if is publisher' do |
| 803 | c = fast_create(Community) | 816 | c = fast_create(Community) |
| 804 | p = create_user_with_permission('test_user', 'publish_content', c) | 817 | p = create_user_with_permission('test_user', 'publish_content', c) |
| 805 | - a = create(Article, :name => 'a test article', :last_changed_by => p, :profile_id => c.id) | 818 | + a = create(Article, :name => 'a test article', :created_by => p, :profile_id => c.id) |
| 806 | 819 | ||
| 807 | assert a.allow_post_content?(p) | 820 | assert a.allow_post_content?(p) |
| 808 | end | 821 | end |
| @@ -1380,17 +1393,17 @@ class ArticleTest < ActiveSupport::TestCase | @@ -1380,17 +1393,17 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 1380 | 1393 | ||
| 1381 | should "the author_name returns the name of the article's author" do | 1394 | should "the author_name returns the name of the article's author" do |
| 1382 | author = fast_create(Person) | 1395 | author = fast_create(Person) |
| 1383 | - a = create(Article, :name => 'a test article', :last_changed_by => author, :profile_id => profile.id) | 1396 | + a = create(Article, :name => 'a test article', :created_by => author, :profile_id => profile.id) |
| 1384 | assert_equal author.name, a.author_name | 1397 | assert_equal author.name, a.author_name |
| 1385 | author.destroy | 1398 | author.destroy |
| 1386 | - a.reload | 1399 | + a = Article.find(a.id) |
| 1387 | a.author_name = 'some name' | 1400 | a.author_name = 'some name' |
| 1388 | assert_equal 'some name', a.author_name | 1401 | assert_equal 'some name', a.author_name |
| 1389 | end | 1402 | end |
| 1390 | 1403 | ||
| 1391 | should 'retrieve latest info from topic when has no comments' do | 1404 | should 'retrieve latest info from topic when has no comments' do |
| 1392 | forum = fast_create(Forum, :name => 'Forum test', :profile_id => profile.id) | 1405 | forum = fast_create(Forum, :name => 'Forum test', :profile_id => profile.id) |
| 1393 | - post = fast_create(TextileArticle, :name => 'First post', :profile_id => profile.id, :parent_id => forum.id, :updated_at => Time.now, :last_changed_by_id => profile.id) | 1406 | + post = fast_create(TextileArticle, :name => 'First post', :profile_id => profile.id, :parent_id => forum.id, :updated_at => Time.now, :last_changed_by_id => profile.id, :created_by_id => profile.id) |
| 1394 | assert_equal post.updated_at, post.info_from_last_update[:date] | 1407 | assert_equal post.updated_at, post.info_from_last_update[:date] |
| 1395 | assert_equal profile.name, post.info_from_last_update[:author_name] | 1408 | assert_equal profile.name, post.info_from_last_update[:author_name] |
| 1396 | assert_equal profile.url, post.info_from_last_update[:author_url] | 1409 | assert_equal profile.url, post.info_from_last_update[:author_url] |
| @@ -1744,30 +1757,30 @@ class ArticleTest < ActiveSupport::TestCase | @@ -1744,30 +1757,30 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 1744 | 1757 | ||
| 1745 | should 'set author_name before creating article if there is an author' do | 1758 | should 'set author_name before creating article if there is an author' do |
| 1746 | author = fast_create(Person) | 1759 | author = fast_create(Person) |
| 1747 | - article = create(Article, :name => 'Test', :profile => profile, :last_changed_by => author) | 1760 | + article = create(Article, :name => 'Test', :profile => profile, :created_by => author) |
| 1748 | assert_equal author.name, article.author_name | 1761 | assert_equal author.name, article.author_name |
| 1749 | 1762 | ||
| 1750 | author_name = author.name | 1763 | author_name = author.name |
| 1751 | author.destroy | 1764 | author.destroy |
| 1752 | - article.reload | 1765 | + article = Article.find(article.id) |
| 1753 | assert_equal author_name, article.author_name | 1766 | assert_equal author_name, article.author_name |
| 1754 | end | 1767 | end |
| 1755 | 1768 | ||
| 1756 | should "author_id return the author id of the article's author" do | 1769 | should "author_id return the author id of the article's author" do |
| 1757 | author = fast_create(Person) | 1770 | author = fast_create(Person) |
| 1758 | - article = create(Article, :name => 'Test', :profile => profile, :last_changed_by => author) | 1771 | + article = create(Article, :name => 'Test', :profile => profile, :created_by => author) |
| 1759 | assert_equal author.id, article.author_id | 1772 | assert_equal author.id, article.author_id |
| 1760 | end | 1773 | end |
| 1761 | 1774 | ||
| 1762 | should "author_id return nil if there is no article's author" do | 1775 | should "author_id return nil if there is no article's author" do |
| 1763 | - article = create(Article, :name => 'Test', :profile => profile, :last_changed_by => nil) | 1776 | + article = create(Article, :name => 'Test', :profile => profile, :created_by => nil) |
| 1764 | assert_nil article.author_id | 1777 | assert_nil article.author_id |
| 1765 | end | 1778 | end |
| 1766 | 1779 | ||
| 1767 | should "return the author of a specific version" do | 1780 | should "return the author of a specific version" do |
| 1768 | author1 = fast_create(Person) | 1781 | author1 = fast_create(Person) |
| 1769 | author2 = fast_create(Person) | 1782 | author2 = fast_create(Person) |
| 1770 | - article = create(Article, :name => 'first version', :profile => profile, :last_changed_by => author1) | 1783 | + article = create(Article, :name => 'first version', :profile => profile, :created_by => author1, :last_changed_by => author1) |
| 1771 | article.name = 'second version' | 1784 | article.name = 'second version' |
| 1772 | article.last_changed_by = author2 | 1785 | article.last_changed_by = author2 |
| 1773 | article.save | 1786 | article.save |
| @@ -1778,7 +1791,7 @@ class ArticleTest < ActiveSupport::TestCase | @@ -1778,7 +1791,7 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 1778 | should "return the author_name of a specific version" do | 1791 | should "return the author_name of a specific version" do |
| 1779 | author1 = fast_create(Person) | 1792 | author1 = fast_create(Person) |
| 1780 | author2 = fast_create(Person) | 1793 | author2 = fast_create(Person) |
| 1781 | - article = create(Article, :name => 'first version', :profile => profile, :last_changed_by => author1) | 1794 | + article = create(Article, :name => 'first version', :profile => profile, :created_by => author1) |
| 1782 | article.name = 'second version' | 1795 | article.name = 'second version' |
| 1783 | article.last_changed_by = author2 | 1796 | article.last_changed_by = author2 |
| 1784 | article.save | 1797 | article.save |
test/unit/forum_helper_test.rb
| @@ -38,7 +38,7 @@ class ForumHelperTest < ActiveSupport::TestCase | @@ -38,7 +38,7 @@ class ForumHelperTest < ActiveSupport::TestCase | ||
| 38 | 38 | ||
| 39 | should 'return post update if it has no comments' do | 39 | should 'return post update if it has no comments' do |
| 40 | author = create_user('forum test author').person | 40 | author = create_user('forum test author').person |
| 41 | - some_post = create(TextileArticle, :name => 'First post', :profile => profile, :parent => forum, :published => true, :last_changed_by => author) | 41 | + some_post = create(TextileArticle, :name => 'First post', :profile => profile, :parent => forum, :published => true, :created_by => author) |
| 42 | assert some_post.comments.empty? | 42 | assert some_post.comments.empty? |
| 43 | out = last_topic_update(some_post) | 43 | out = last_topic_update(some_post) |
| 44 | assert_match some_post.updated_at.to_s, out | 44 | assert_match some_post.updated_at.to_s, out |