Merge branch 'master_register_performance' into 'master'

Improve register performance - Send activation code with delayed job - Avoid unecessary user save after update person Signed-off-by: Gustavo Jaruga <darksshades@gmail.com> Signed-off-by: Marcos Ronaldo <marcos.rpj2@gmail.com> See merge request !829

Merge branch 'master_register_performance' into 'master'
Improve register performance - Send activation code with delayed job - Avoid unecessary user save after update person Signed-off-by: Gustavo Jaruga <darksshades@gmail.com> Signed-off-by: Marcos Ronaldo <marcos.rpj2@gmail.com> See merge request !829
Rodrigo Souto
2 parents 5d4a0723 06162adb
Showing 7 changed files with 17 additions and 6 deletions Show diff stats
app/models/person.rb
app/models/profile.rb
app/models/user.rb
features/signup.feature
test/functional/search_controller_test.rb
test/integration/signup_test.rb
test/unit/person_test.rb
@@ -328,7 +328,7 @@ class Person &lt; Profile
   end
   after_update do |person|
-    person.user.save!
+    person.user.save! unless person.user.changes.blank?
   end
   def is_admin?(environment = nil)
@@ -692,15 +692,15 @@ private :generate_url, :url_options
   after_create :insert_default_article_set
   def insert_default_article_set
     if template
-      copy_articles_from template
+      self.save! if copy_articles_from template
     else
       default_set_of_articles.each do |article|
         article.profile = self
         article.advertise = false
         article.save!
       end
+      self.save!
     end
-    self.save!
   end
   # Override this method in subclasses of Profile to create a default article
@@ -721,10 +721,12 @@ private :generate_url, :url_options
   end
   def copy_articles_from other
+    return false if other.top_level_articles.empty?
     other.top_level_articles.each do |a|
       copy_article_tree a
     end
     self.articles.reload
+    true
   end
   def copy_article_tree(article, parent=nil)
@@ -445,7 +445,7 @@ class User &lt; ActiveRecord::Base
     def deliver_activation_code
       return if person.is_template?
-      UserMailer.activation_code(self).deliver unless self.activation_code.blank?
+      Delayed::Job.enqueue(UserMailer::Job.new(self, :activation_code)) unless self.activation_code.blank?
     end
     def delay_activation_check
@@ -16,6 +16,7 @@ Feature: signup
       | Full name             | José da Silva         |
     And wait for the captcha signup time
     And I press "Create my account"
+    And there are no pending jobs
     Then I should receive an e-mail on josesilva@example.com
     When I go to login page
     And I fill in "Username" with "josesilva"
@@ -27,6 +27,7 @@ class SearchControllerTest &lt; ActionController::TestCase
     # By pass user validation on person creation
     user = mock()
     user.stubs(:id).returns(1)
+    user.stubs(:changes).returns(nil)
     user.stubs(:valid?).returns(true)
     user.stubs(:email).returns('some@test.com')
     user.stubs(:save!).returns(true)
@@ -45,7 +45,7 @@ class SignupTest &lt; ActionDispatch::IntegrationTest
     assert_redirected_to controller: 'home', action: 'welcome'
     assert_equal count + 1, User.count
-    assert_equal mail_count + 1, ActionMailer::Base.deliveries.count
+    assert_includes Delayed::Job.all.map{|d|d.name}, 'UserMailer::Job'
   end
@@ -1937,11 +1937,18 @@ class PersonTest &lt; ActiveSupport::TestCase
   should 'a person follows many articles' do
     person = create_user('article_follower').person
-  
+
     1.upto(10).map do |n|
       person.following_articles <<  fast_create(Article, :profile_id => fast_create(Person))
     end
     assert_equal 10, person.following_articles.count
   end
+  should 'not save user after an update on person and user is not touched' do
+    user = create_user('testuser')
+    person = user.person
+    person.user.expects(:save!).never
+    person.save!
+  end
+
 end
	@@ -328,7 +328,7 @@ class Person < Profile		@@ -328,7 +328,7 @@ class Person < Profile
328	end	328	end
329		329
330	after_update do \|person\|	330	after_update do \|person\|
331	- person.user.save!	331	+ person.user.save! unless person.user.changes.blank?
332	end	332	end
333		333
334	def is_admin?(environment = nil)	334	def is_admin?(environment = nil)
	@@ -445,7 +445,7 @@ class User < ActiveRecord::Base		@@ -445,7 +445,7 @@ class User < ActiveRecord::Base
445		445
446	def deliver_activation_code	446	def deliver_activation_code
447	return if person.is_template?	447	return if person.is_template?
448	- UserMailer.activation_code(self).deliver unless self.activation_code.blank?	448	+ Delayed::Job.enqueue(UserMailer::Job.new(self, :activation_code)) unless self.activation_code.blank?
449	end	449	end
450		450
451	def delay_activation_check	451	def delay_activation_check
	@@ -45,7 +45,7 @@ class SignupTest < ActionDispatch::IntegrationTest		@@ -45,7 +45,7 @@ class SignupTest < ActionDispatch::IntegrationTest
45	assert_redirected_to controller: 'home', action: 'welcome'	45	assert_redirected_to controller: 'home', action: 'welcome'
46		46
47	assert_equal count + 1, User.count	47	assert_equal count + 1, User.count
48	- assert_equal mail_count + 1, ActionMailer::Base.deliveries.count	48	+ assert_includes Delayed::Job.all.map{\|d\|d.name}, 'UserMailer::Job'
49		49
50	end	50	end
51		51