load article's author only when necessary

(ActionItem3097)

load article's author only when necessary
(ActionItem3097)
Leandro Santos · Daniela Feitosa
1 parent aa47fe88
Showing 5 changed files with 69 additions and 28 deletions Show diff stats
app/controllers/my_profile/cms_controller.rb
app/models/article.rb
test/functional/cms_controller_test.rb
test/functional/content_viewer_controller_test.rb
test/unit/article_test.rb
@@ -144,6 +144,7 @@ class CmsController &lt; MyProfileController
     @article.profile = profile
     @article.last_changed_by = user
+    @article.created_by = user
     translations if @article.translatable?
@@ -40,12 +40,6 @@ class Article &lt; ActiveRecord::Base
   # xss_terminate plugin can't sanitize array fields
   before_save :sanitize_tag_list
-  before_create do |article|
-    if article.last_changed_by_id
-      article.author_name = Person.find(article.last_changed_by_id).name
-    end
-  end
-
   belongs_to :profile
   validates_presence_of :profile_id, :name
   validates_presence_of :slug, :path, :if => lambda { |article| !article.name.blank? }
@@ -55,6 +49,7 @@ class Article &lt; ActiveRecord::Base
   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? }
   belongs_to :last_changed_by, :class_name => 'Person', :foreign_key => 'last_changed_by_id'
+  belongs_to :created_by, :class_name => 'Person', :foreign_key => 'created_by_id'
   has_many :comments, :class_name => 'Comment', :foreign_key => 'source_id', :dependent => :destroy, :order => 'created_at asc'
@@ -89,6 +84,11 @@ class Article &lt; ActiveRecord::Base
         article.parent = article.profile.blog
       end
     end
+
+    if article.created_by
+      article.author_name = article.created_by.name
+    end
+
   end
   after_destroy :destroy_activity
@@ -630,18 +630,20 @@ class Article &lt; ActiveRecord::Base
   def author(version_number = nil)
     if version_number
-      version = versions.find_by_version(version_number)
+      version = self.versions.find_by_version(version_number)
       author_id = version.last_changed_by_id if version
-      Person.exists?(author_id) ? Person.find(author_id) : nil
     else
-      if versions.empty?
-        last_changed_by
-      else
-        author_id = versions.first.last_changed_by_id
-        Person.exists?(author_id) ? Person.find(author_id) : nil
-      end
+      author_id = self.created_by_id
     end
-  end
+
+    begin
+      @author = Person.find(author_id) if !@author || @author.id != author_id
+    rescue
+      @author = nil
+    end
+
+    @author
+   end
   def author_name(version_number = nil)
     person = author(version_number)
@@ -1228,7 +1228,7 @@ class CmsControllerTest &lt; ActionController::TestCase
   should 'allow user edit article if he is owner and has publish permission' do
     c = Community.create!(:name => 'test_comm', :identifier => 'test_comm')
     u = create_user_with_permission('test_user', 'publish_content', c)
-    a = create(Article, :profile => c, :name => 'test_article', :last_changed_by => u)
+    a = create(Article, :profile => c, :name => 'test_article', :created_by => u)
     login_as :test_user
     @controller.stubs(:user).returns(u)
@@ -1774,6 +1774,31 @@ class CmsControllerTest &lt; ActionController::TestCase
     assert_equal 'first version', Article.find(article.id).name
   end
+  should 'set created_by when creating article' do
+    login_as(profile.identifier)
+
+    post :new, :type => 'TinyMceArticle', :profile => profile.identifier, :article => { :name => 'changed by me', :body => 'content ...' }
+
+    a = profile.articles.find_by_path('changed-by-me')
+    assert_not_nil a
+    assert_equal profile, a.created_by
+  end
+
+  should 'not change created_by when updating article' do
+    other_person = create_user('otherperson').person
+
+    a = profile.articles.build(:name => 'my article')
+    a.created_by = other_person
+    a.save!
+
+    login_as(profile.identifier)
+    post :edit, :profile => profile.identifier, :id => a.id, :article => { :body => 'new content for this article' }
+
+    a.reload
+
+    assert_equal other_person, a.created_by
+  end
+
   protected
   # FIXME this is to avoid adding an extra dependency for a proper JSON parser.
@@ -743,7 +743,7 @@ class ContentViewerControllerTest &lt; ActionController::TestCase
     c = Community.create!(:name => 'test_com')
     u = create_user_with_permission('test_user', 'publish_content', c)
     login_as u.identifier
-    a = create(Article, :profile => c, :name => 'test-article', :last_changed_by => u, :published => false)
+    a = create(Article, :profile => c, :name => 'test-article', :created_by => u, :published => false)
     get :view_page, :profile => c.identifier, :page => a.path
@@ -154,7 +154,7 @@ class ArticleTest &lt; ActiveSupport::TestCase
     assert a4.errors[:slug.to_s].present?
   end
-  should 'record who did the last change' do
+  should 'last_changed_by be a person' do
     a = profile.articles.build(:name => 'test')
     # must be a person
@@ -167,6 +167,19 @@ class ArticleTest &lt; ActiveSupport::TestCase
     end
   end
+  should 'created_by be a person' do
+    a = profile.articles.build(:name => 'test')
+
+    # must be a person
+    assert_raise ActiveRecord::AssociationTypeMismatch do
+      a.created_by = Profile.new
+    end
+    assert_nothing_raised do
+      a.created_by = Person.new
+      a.save!
+    end
+  end
+
   should 'not show private documents as recent' do
     p = create_user('usr1').person
     Article.destroy_all
@@ -802,7 +815,7 @@ class ArticleTest &lt; ActiveSupport::TestCase
   should 'allow author to edit if is publisher' do
     c = fast_create(Community)
     p = create_user_with_permission('test_user', 'publish_content', c)
-    a = create(Article, :name => 'a test article', :last_changed_by => p, :profile_id => c.id)
+    a = create(Article, :name => 'a test article', :created_by => p, :profile_id => c.id)
     assert a.allow_post_content?(p)
   end
@@ -1380,17 +1393,17 @@ class ArticleTest &lt; ActiveSupport::TestCase
   should "the author_name returns the name of the article's author" do
     author = fast_create(Person)
-    a = create(Article, :name => 'a test article', :last_changed_by => author, :profile_id => profile.id)
+    a = create(Article, :name => 'a test article', :created_by => author, :profile_id => profile.id)
     assert_equal author.name, a.author_name
     author.destroy
-    a.reload
+    a = Article.find(a.id)
     a.author_name = 'some name'
     assert_equal 'some name', a.author_name
   end
   should 'retrieve latest info from topic when has no comments' do
     forum = fast_create(Forum, :name => 'Forum test', :profile_id => profile.id)
-    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)
+    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)
     assert_equal post.updated_at, post.info_from_last_update[:date]
     assert_equal profile.name, post.info_from_last_update[:author_name]
     assert_equal profile.url, post.info_from_last_update[:author_url]
@@ -1744,30 +1757,30 @@ class ArticleTest &lt; ActiveSupport::TestCase
   should 'set author_name before creating article if there is an author' do
     author = fast_create(Person)
-    article = create(Article, :name => 'Test', :profile => profile, :last_changed_by => author)
+    article = create(Article, :name => 'Test', :profile => profile, :created_by => author)
     assert_equal author.name, article.author_name
     author_name = author.name
     author.destroy
-    article.reload
+    article = Article.find(article.id)
     assert_equal author_name, article.author_name
   end
   should "author_id return the author id of the article's author" do
     author = fast_create(Person)
-    article = create(Article, :name => 'Test', :profile => profile, :last_changed_by => author)
+    article = create(Article, :name => 'Test', :profile => profile, :created_by => author)
     assert_equal author.id, article.author_id
   end
   should "author_id return nil if there is no article's author" do
-    article = create(Article, :name => 'Test', :profile => profile, :last_changed_by => nil)
+    article = create(Article, :name => 'Test', :profile => profile, :created_by => nil)
     assert_nil article.author_id
   end
   should "return the author of a specific version" do
     author1 = fast_create(Person)
     author2 = fast_create(Person)
-    article = create(Article, :name => 'first version', :profile => profile, :last_changed_by => author1)
+    article = create(Article, :name => 'first version', :profile => profile, :created_by => author1, :last_changed_by => author1)
     article.name = 'second version'
     article.last_changed_by = author2
     article.save
@@ -1778,7 +1791,7 @@ class ArticleTest &lt; ActiveSupport::TestCase
   should "return the author_name of a specific version" do
     author1 = fast_create(Person)
     author2 = fast_create(Person)
-    article = create(Article, :name => 'first version', :profile => profile, :last_changed_by => author1)
+    article = create(Article, :name => 'first version', :profile => profile, :created_by => author1)
     article.name = 'second version'
     article.last_changed_by = author2
     article.save
	@@ -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
	@@ -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.
	@@ -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