comments: fewer db round trips on creation

Writes a vote directly to avoid vote_thusly doing round trips to check if one
exists, etc.

Removes redundant transactions from controllers from lobsters#899. Rails already creates
a transaction for the .save.

Unifies Story cache updating. Previously recalculate_hotness! was called twice
on comment creation. Moves comment counting into the db.

Shorter transaction should reduce the frequence of
lobsters/lobsters-ansible/issues/39 but seems unlikely to eliminate it as the
create + upvote transactions for stories + comments still read/write from
stories, comments, and votes.

app/controllers/comments_controller.rb

@@ -51,7 +51,7 @@ def create
         content_type: "text/html", locals: {comment: comment}
     end
 
-    if comment.valid? && params[:preview].blank? && ActiveRecord::Base.transaction { comment.save }
+    if comment.valid? && params[:preview].blank? && comment.save
       comment.current_vote = {vote: 1}
       render_created_comment(comment)
     else

app/controllers/stories_controller.rb

@@ -20,7 +20,7 @@ def create
     @story.attributes = story_params
 
     if @story.valid? && !(@story.already_posted_recently? && !@story.seen_previous)
-      if ActiveRecord::Base.transaction { @story.save }
+      if @story.save
         ReadRibbon.where(user: @user, story: @story).first_or_create
         return redirect_to @story.comments_path
       end

app/models/comment.rb

@@ -29,10 +29,6 @@ class Comment < ApplicationRecord
   end
   after_create :record_initial_upvote, :mark_submitter, :deliver_reply_notifications,
     :deliver_mention_notifications, :log_hat_use
-  after_create do
-    # fire this once after record_initial_upvote
-    update_score_and_recalculate! 0, 0
-  end
   after_destroy :unassign_votes
 
   scope :deleted, -> { where(is_deleted: true) }
@@ -259,7 +255,7 @@ def delete_for_user(user, reason = nil)
     save(validate: false)
     Comment.record_timestamps = true
 
-    story.update_comments_count!
+    story.update_cached_columns
     self.user.refresh_counts!
   end
 
@@ -371,7 +367,7 @@ def update_score_and_recalculate!(score_delta, flag_delta)
         confidence_order = concat(lpad(char(65536 - floor(((confidence - -0.2) * 65535) / 1.2) using binary), 2, '0'), char(id & 0xff using binary))
       WHERE id = #{id.to_i}
     SQL
-    story.recalculate_hotness!
+    story.update_cached_columns
   end
 
   def gone_text
@@ -504,11 +500,11 @@ def plaintext_comment
   end
 
   def record_initial_upvote
-    Vote.vote_thusly_on_story_or_comment_for_user_because(
-      1, story_id, id, user_id, nil, false
-    )
+    # not calling vote_thusly to save round-trips of validations
+    Vote.create! story: story, comment: self, user: user, vote: 1
+    update_score_and_recalculate! 0, 0 # trigger db calculation
 
-    story.update_comments_count!
+    story.update_cached_columns
   end
 
   def score_for_user(u)
@@ -541,7 +537,7 @@ def to_param
   end
 
   def unassign_votes
-    story.update_comments_count!
+    story.update_cached_columns
   end
 
   def url
@@ -592,7 +588,7 @@ def undelete_for_user(user)
     save(validate: false)
     Comment.record_timestamps = true
 
-    story.update_comments_count!
+    story.update_cached_columns
     self.user.refresh_counts!
   end
 

app/models/story.rb

@@ -157,7 +157,7 @@ class Story < ApplicationRecord
   before_save :log_moderation
   before_save :fix_bogus_chars
   after_create :mark_submitter, :record_initial_upvote
-  after_save :update_merged_into_story_comments, :recalculate_hotness!, :update_story_text
+  after_save :update_cached_columns, :update_story_text
 
   validate do
     if url.present?
@@ -270,8 +270,8 @@ def most_recent_similar
 
   def self.recalculate_all_hotnesses!
     # do the front page first, since find_each can't take an order
-    Story.order("id DESC").limit(100).each(&:recalculate_hotness!)
-    Story.find_each(&:recalculate_hotness!)
+    Story.order("id DESC").limit(100).each(&:update_cached_columns)
+    Story.find_each(&:update_cached_columns)
     true
   end
 
@@ -620,10 +620,6 @@ def merge_story_short_id
     merged_story_id ? merged_into_story.try(:short_id) : nil
   end
 
-  def recalculate_hotness!
-    update_column :hotness, calculated_hotness
-  end
-
   def record_initial_upvote
     Vote.vote_thusly_on_story_or_comment_for_user_because(1, id, nil, user_id, nil, false)
   end
@@ -812,17 +808,11 @@ def update_availability
     end
   end
 
-  def update_comments_count!
-    comments = merged_comments
+  def update_cached_columns
+    update_column :comments_count, merged_comments.active.count
+    merged_into_story&.update_cached_columns
 
-    # calculate count after removing deleted comments and threads
-    update_column :comments_count, (comments.count { |c| !c.is_gone? })
-    update_merged_into_story_comments
-    recalculate_hotness!
-  end
-
-  def update_merged_into_story_comments
-    merged_into_story&.update_comments_count!
+    update_column :hotness, calculated_hotness
   end
 
   def update_story_text

spec/models/story_spec.rb

@@ -405,7 +405,7 @@ def fake_response(content, type, code = "200")
     end
   end
 
-  describe "#update_comments_count!" do
+  describe "#update_cached_columns" do
     context "with a merged_into_story" do
       let(:merged_into_story) { create(:story) }
       let(:story) { create(:story, merged_into_story: merged_into_story) }
@@ -414,7 +414,7 @@ def fake_response(content, type, code = "200")
         expect(story.comments_count).to eq 0
         expect(merged_into_story.comments_count).to eq 0
         create(:comment, story: story)
-        story.update_comments_count!
+        story.update_cached_columns
         expect(story.comments_count).to eq 1
         expect(merged_into_story.comments_count).to eq 1
       end