Backout counter_cache (#1165)

It caused other issues like in tests, and I'm not sure it was a slam dunk for performance.   Instead we need to be smarter about how we decide to load this data.

.env.example

@@ -5,6 +5,9 @@ PORT=3000
 RACK_ENV=development
 RAILS_ENV=development
 
+# This makes Quepid display detailed error messages.
+QUEPID_CONSIDER_ALL_REQUESTS_LOCAL=true
+
 MAX_THREADS=2
 WEB_CONCURRENCY=2
 

app/controllers/api/v1/cases/dropdown_controller.rb

@@ -5,7 +5,7 @@ module V1
     module Cases
       class DropdownController < Api::ApiController
         def index
-          @cases = recent_cases 3
+          @cases = recent_cases 4
 
           respond_with @cases
         end

app/controllers/concerns/authentication/current_book_manager.rb

@@ -20,45 +20,23 @@ def check_book
     end
 
     def set_recent_books
-      @recent_books = recent_books(3)
+      @recent_books = recent_books(4)
     end
 
-    # rubocop:disable Metrics/MethodLength
     def recent_books count
       if current_user
-        # Using joins/includes will not return the proper list in the
-        # correct order because rails refuses to include the
-        # `book_metadata`.`last_viewed_at` column in the SELECT statement
-        # which will then cause the ordering not to work properly.
-        # So instead, we have this beauty!
-        sql = "
-          SELECT  DISTINCT `books`.`id`, `book_metadata`.`last_viewed_at`
-          FROM `books`
-          LEFT OUTER JOIN `book_metadata` ON `book_metadata`.`book_id` = `books`.`id`
-          LEFT OUTER JOIN `teams_books` ON `teams_books`.`book_id` = `books`.`id`
-          LEFT OUTER JOIN `teams` ON `teams`.`id` = `teams_books`.`team_id`
-          LEFT OUTER JOIN `teams_members` ON `teams_members`.`team_id` = `teams`.`id`
-          LEFT OUTER JOIN `users` ON `users`.`id` = `teams_members`.`member_id`
-          WHERE (`teams_members`.`member_id` = #{current_user.id} OR `books`.`owner_id` = #{current_user.id})
-          ORDER BY `book_metadata`.`last_viewed_at` DESC, `books`.`id` DESC
-          LIMIT #{count}
-        "
-
-        results = ActiveRecord::Base.connection.execute(sql)
-
-        book_ids = results.map do |row|
-          row.first.to_i
-        end
+        book_ids = current_user.book_metadata.order(last_viewed_at: :desc).limit(count).pluck(:book_id)
 
         # map to objects
-        # cases = Case.includes(:tries).where(id: [ case_ids ])
-        books = Book.where(id: [ book_ids ])
+        books = current_user.books_involved_with.where(id: book_ids)
+
+        # the WHERE IN clause doesn't guarantee returning in order, so this sorts the cases in order of last viewing.
         books = books.sort_by { |x| book_ids.index x.id }
+
       else
         books = []
       end
       books
     end
-    # rubocop:enable Metrics/MethodLength
   end
 end

app/controllers/concerns/authentication/current_case_manager.rb

@@ -35,47 +35,23 @@ def set_case
     end
 
     def set_recent_cases
-      @recent_cases = recent_cases(3)
+      @recent_cases = recent_cases(4)
     end
 
-    # rubocop:disable Metrics/MethodLength
     def recent_cases count
       if current_user
-        # Using joins/includes will not return the proper list in the
-        # correct order because rails refuses to include the
-        # `case_metadata`.`last_viewed_at` column in the SELECT statement
-        # which will then cause the ordering not to work properly.
-        # So instead, we have this beauty!
-        sql = "
-          SELECT  DISTINCT `cases`.`id`, `case_metadata`.`last_viewed_at`
-          FROM `cases`
-          LEFT OUTER JOIN `case_metadata` ON `case_metadata`.`case_id` = `cases`.`id`
-          LEFT OUTER JOIN `teams_cases` ON `teams_cases`.`case_id` = `cases`.`id`
-          LEFT OUTER JOIN `teams` ON `teams`.`id` = `teams_cases`.`team_id`
-          LEFT OUTER JOIN `teams_members` ON `teams_members`.`team_id` = `teams`.`id`
-          LEFT OUTER JOIN `users` ON `users`.`id` = `teams_members`.`member_id`
-          WHERE (`teams_members`.`member_id` = #{current_user.id} OR `cases`.`owner_id` = #{current_user.id})
-          AND (`cases`.`archived` = false OR `cases`.`archived` IS NULL)
-          ORDER BY `case_metadata`.`last_viewed_at` DESC, `cases`.`id` DESC
-          LIMIT #{count}
-        "
-
-        results = ActiveRecord::Base.connection.execute(sql)
-
-        case_ids = results.map do |row|
-          row.first.to_i
-        end
+        case_ids = current_user.case_metadata.order(last_viewed_at: :desc).limit(count).pluck(:case_id)
 
         # map to objects
-        # cases = Case.includes(:tries).where(id: [ case_ids ])
-        cases = Case.where(id: [ case_ids ])
+        cases = current_user.cases_involved_with.where(id: case_ids).not_archived
+
+        # the WHERE IN clause doesn't guarantee returning in order, so this sorts the cases in order of last viewing.
         cases = cases.sort_by { |x| case_ids.index x.id }
       else
         cases = []
       end
       cases
     end
-    # rubocop:enable Metrics/MethodLength
 
     def check_case
       render json: { message: 'Case not found!' }, status: :not_found unless @case

app/controllers/home_controller.rb

@@ -7,6 +7,7 @@ class HomeController < ApplicationController
   def show
     # with_counts adds a `case.queries_count` field, which avoids loading
     # all queries and makes bullet happy.
+    # @cases = recent_cases(30)
     @cases = @current_user.cases_involved_with.not_archived.with_counts
       .includes([ :metadata ])
       .order('`case_metadata`.`last_viewed_at` DESC, `cases`.`id` DESC')
@@ -15,15 +16,16 @@ def show
     @most_recent_cases = @cases[0...4].sort_by { |c| c.case_name.downcase }
 
     @most_recent_books = recent_books(4)
-    @lookup_for_books = {}
-    @most_recent_books.each do |book|
-      judged_by_current_user = book.judgements.where(user: @current_user).count
-      if judged_by_current_user.positive? && judged_by_current_user < book.query_doc_pairs.size
-        @lookup_for_books[book] = book.query_doc_pairs.size - judged_by_current_user
-      end
-    end
 
-    @most_recent_books.sort_by!(&:name)
+    # @lookup_for_books = {}
+    # @most_recent_books.each do |book|
+    #  judged_by_current_user = book.judgements.where(user: @current_user).count
+    #  if judged_by_current_user.positive? && judged_by_current_user < book.query_doc_pairs.size
+    #    @lookup_for_books[book] = book.query_doc_pairs.size - judged_by_current_user
+    #  end
+    # end
+
+    @most_recent_books = @most_recent_books.sort_by { |b| b.name.downcase }
 
     # Homepage is too slow so we have to cut some stuff out ;-(
     # candidate_cases = @cases.select { |kase| kase.scores.scored.count.positive? }

app/javascript/application2.js

@@ -12,6 +12,7 @@ import Cookies from 'js-cookie'
 LocalTime.start()
 
 Turbo.config.drive.progressBarDelay = 1
+Turbo.session.drive = false
 
 import "vega"
 import "vega-lite"

app/models/book.rb

@@ -9,7 +9,6 @@
 #  import_job                  :string(255)
 #  name                        :string(255)
 #  populate_job                :string(255)
-#  query_doc_pairs_count       :integer          default(0), not null
 #  show_rank                   :boolean          default(FALSE)
 #  support_implicit_judgements :boolean
 #  created_at                  :datetime         not null
@@ -66,6 +65,16 @@ class Book < ApplicationRecord
   # Scopes
   include ForUserScope
 
+  scope :with_counts, -> {
+                        select <<~SQL.squish
+                          books.*,
+                          (
+                            SELECT COUNT(query_doc_pairs.id) FROM query_doc_pairs
+                            WHERE book_id = books.id
+                          ) AS query_doc_pairs_count
+                        SQL
+                      }
+
   private
 
   def delete_attachments

app/models/query_doc_pair.rb

@@ -25,7 +25,7 @@
 #  fk_rails_...  (book_id => books.id)
 #
 class QueryDocPair < ApplicationRecord
-  belongs_to :book, counter_cache: true
+  belongs_to :book
   has_many :judgements, dependent: :destroy, autosave: true
 
   validates :query_text, presence: true, length: { maximum: 2048 }

app/views/home/_book_summary.html.erb

@@ -12,10 +12,7 @@
       </p>
       <span id="notification-book-<%= book.id %>">
       </span>
-      <!-- Look at https://github.com/ankane/prophet-ruby -->
-      <% if @lookup_for_books[book] && @lookup_for_books[book] > 0 %>
-        <p class="card-text text-danger"><i class="frog-icon" aria-hidden="true">🐸</i> You need to judge <%= @lookup_for_books[book] %> out <%= book.query_doc_pairs.size %> query/doc pairs.</p>
-      <% end %>
+
       <%= link_to 'Judge', book_judge_path(book), data: { turbo_prefetch: false }, class: 'btn btn-sm btn-primary', role: 'button' %>
     </div> 
   </div>

app/views/home/show.html.erb

@@ -54,7 +54,7 @@ Inspired by https://dev.to/themesberg/tutorial-how-to-build-a-simple-admin-dashb
                           </tr>
                         </thead>
                         <tbody>
-                          <%= render partial: "case", collection: @cases, as: :kase %>
+                          <%= render partial: "case", collection: @cases, as: :kase, cached: true %>
                         </tbody>
                       </table>
                 </div>

app/views/layouts/_dropdown_case.html.erb

@@ -0,0 +1,2 @@
+<li><%= link_to_core_case  kase.case_name, kase, kase.last_try_number %></li>
+<li><hr class="dropdown-divider"></li>

app/views/layouts/_header.html.erb

@@ -18,10 +18,7 @@
               RECENT CASES
             </li>
             <li><hr class="dropdown-divider"></li>
-            <% set_recent_cases.each do |kase| %>
-              <li><%= link_to_core_case  kase.case_name, kase, kase.last_try_number %></li>
-              <li><hr class="dropdown-divider"></li>
-            <% end %>
+            <%= render partial: 'layouts/dropdown_case', collection: set_recent_cases, as: :kase, cached: true %>
 
             <li class="actions">
               <div class="d-grid gap-2">

test/controllers/api/v1/cases/dropdown_controller_test.rb

@@ -87,15 +87,15 @@ class DropdownControllerTest < ActionController::TestCase
             assert_not_includes ids, archived.id
           end
 
-          test 'limits list to 3 cases' do
+          test 'limits list to 4 cases' do
             get :index
 
             assert_response :ok
 
             body  = response.parsed_body
             cases = body['all_cases']
 
-            assert cases.length <= 3
+            assert cases.length <= 4
           end
 
           test 'returns list of cases ordered by last viewed date' do

test/fixtures/books.yml

@@ -7,7 +7,6 @@
 #  import_job                  :string(255)
 #  name                        :string(255)
 #  populate_job                :string(255)
-#  query_doc_pairs_count       :integer          default(0), not null
 #  show_rank                   :boolean          default(FALSE)
 #  support_implicit_judgements :boolean
 #  created_at                  :datetime         not null

test/models/book_test.rb

@@ -9,7 +9,6 @@
 #  import_job                  :string(255)
 #  name                        :string(255)
 #  populate_job                :string(255)
-#  query_doc_pairs_count       :integer          default(0), not null
 #  show_rank                   :boolean          default(FALSE)
 #  support_implicit_judgements :boolean
 #  created_at                  :datetime         not null