Merge branch 'main' into nimmo-name-resolver

app/controllers/observations/namings/votes_controller.rb

@@ -7,9 +7,9 @@ class VotesController < ApplicationController
     # Index breakdown of votes for a given naming.
     # Linked from: observations/show
     # Displayed on show obs via popup for JS users.
-    # Has its own route for non-js.
-    # Inputs: params[:naming_id] (naming)
-    # Outputs: @naming
+    # Has its own route for non-js access and testing.
+    # Inputs: params[:naming_id], [:observation_id]
+    # Outputs: @naming, @consensus
     def index
       pass_query_params
       @naming = find_or_goto_index(Naming, params[:naming_id].to_s)
@@ -72,8 +72,6 @@ def create_or_update_vote
       value = Vote.validate_value(value_str)
       raise("Bad value.") unless value
 
-      # N+1: Take the whole vote object and send it to change vote?
-      # Or how about returning obs.reload from observation.change_vote
       @consensus = ::Observation::NamingConsensus.new(observation)
       @consensus.change_vote(@naming, value, @user) # 2nd load (namings.reload)
       @observation = load_observation_naming_includes # 3rd load

app/controllers/observations/namings_controller.rb

@@ -216,7 +216,7 @@ def can_save?
     end
 
     def unproposed_name(warning)
-      if name_been_proposed?
+      if @consensus.name_been_proposed?(@name)
         flash_warning(warning.t)
       else
         true
@@ -324,10 +324,6 @@ def add_reasons(reasons)
       @reasons = @naming.init_reasons(reasons)
     end
 
-    def name_been_proposed?
-      @consensus.name_been_proposed?(@name)
-    end
-
     def respond_to_successful_update
       respond_to do |format|
         format.turbo_stream do

app/controllers/species_lists/shared_private_methods.rb

@@ -11,7 +11,6 @@ module SharedPrivateMethods
     ############################################################################
 
     def find_species_list!
-      # find_or_goto_index(SpeciesList, params[:id].to_s)
       SpeciesList.show_includes.safe_find(params[:id].to_s) ||
         flash_error_and_goto_index(SpeciesList, params[:id].to_s)
     end

app/helpers/namings_helper.rb

@@ -86,7 +86,6 @@ def observation_namings_table_rows(consensus)
   end
 
   # NEW - needs a current consensus object
-  # N+1: obs.consensus_naming and observation.owners_favorite?
   def naming_row_content(consensus, naming)
     vote = consensus.users_vote(naming, User.current) || Vote.new(value: 0)
     consensus_favorite = consensus.consensus_naming
@@ -389,15 +388,15 @@ def propose_naming_link(obs_id, text: :create_naming.t,
   end
 
   # N+1: can't move the calculation of observation.image_ids
-  # must query obs includes images, don't update table-footer
+  # must query obs includes images, so don't update table-footer
   def suggest_namings_link(obs)
     localizations = {
       processing_images: :suggestions_processing_images.t,
       processing_image: :suggestions_processing_image.t,
       processing_results: :suggestions_processing_results.t,
       error: :suggestions_error.t
     }.to_json
-    # NOTE: it does not actually commit to this path.
+    # NOTE: suggestions does not actually commit to this path, it's a js request
     results_url = add_query_param(
       naming_suggestions_for_observation_path(id: obs.id, names: :xxx)
     )

app/models/observation/naming_consensus.rb

@@ -1,10 +1,61 @@
 # frozen_string_literal: true
 
+# Observation::NamingConsensus
+#
+# This PORO isolates the code that determines the current state of naming/
+# voting on an obs, and handles naming and voting changes on observations.
+#
+# Instantiate a NamingConsensus to get a snapshot of the current state, or to
+# perform changes to that state via proposing or voting on namings.
+#
+# The goal is to safeguard against any unintentional db loads whenever the
+# observation's current "consensus" naming is calculated, which needs to
+# happen whenever an observation is shown, or whenever namings or votes change,
+# and the show_obs "namings_table" is redrawn.
+#
+# Instantiate this object with an observation that you've eager-loaded the
+# namings and votes on. The PORO will use the eager-loaded associations, and
+# only saves to the db when a naming or vote is changed/created. At this point,
+# you need to call @consensus.reload_namings_and_votes! to update the object.
+#
+# The object itself contains everything needed to draw the Namings "table" -
+# the views and controllers now should only access attributes and methods of a
+# NamingConsensus object, because all the Naming and Vote methods that caused
+# db lookups have been moved here.
+#
+#  name_been_proposed?::    Has someone proposed this Name already?
+#  owners_votes::           Get all of the onwer's Vote's for this Observation.
+#  owner_preference::       owners's unique prefered Name (if any) for this Obs
+#  consensus_naming::       Guess which Naming is responsible for consensus.
+#  calc_consensus::         Calculate and cache the consensus naming/name.
+#  dump_votes::             Dump all the Naming and Vote info as known by this
+#                           Observation and its associations.
+#
+# Methods that require passing a naming, called in views or controllers:
+#  user_voted?::            Has a given User voted on this Naming?
+#  owner_voted?::           Has the owner voted on a given Naming?
+#  users_vote::             Get a given User's Vote on this Naming.
+#  owners_vote::            Owner's Vote on a given Naming.
+#  users_favorite?::        Is this Naming the given User's favorite?
+#  owners_favorite?::       Is a given Naming one of the owner's favorite(s)?
+#  change_vote::            Change a given User's Vote for a given Naming.
+#  change_vote_with_log::   Also log the change (on the Obs)
+#  editable?::              Can owner change this Naming's Name?
+#  deletable?::             Can owner delete this Naming?
+#  calc_vote_table::        Who voted on this naming (via VotesController#index)
+#                           Note that many votes are anonymous, so...
+#  clean_votes::            Delete unused votes (via NamingsController#update)
+#  editable?::              Naming is editable?
+#  deletable?::             Naming is deletable?
+#
+# At the time this was written, vote form updates performed something like
+# 3N+2 db loads of naming votes per vote update (N in this case being the
+# number of proposed namings per obs, so 3 namings meant 11 vote lookups).
+# The vote table is the slowest table in the db, so it was extremely slow.
+#
 class Observation
   class NamingConsensus
-    attr_accessor :observation
-    attr_accessor :namings
-    attr_accessor :votes
+    attr_accessor :observation, :namings, :votes
 
     def initialize(observation)
       @observation = observation

app/models/vote.rb

@@ -9,21 +9,6 @@
 #  contained within a single Naming or not.
 #  Vote is responsible for very little except holding the value.
 #
-#  NC#change_vote::         Change a User's Vote for a given Naming.
-#  NC#calc_consensus::      Decide which Name is winner for an Observation.
-#  NC#owners_favorite?::    Is a given Naming the Observation owner's
-#                                    favorite?
-#  NC#users_favorite?::     Is a given Naming the given User's
-#                                    favorite?
-#  NC#refresh_vote_cache::  Refresh vote cache for all Observation's.
-#
-#  Naming#vote_sum::        Straight sum of Vote's for this Naming (tests)
-#  NC#user_voted?::         Has a given User voted for this Naming?
-#  NC#users_vote::          Get a given User's Vote for this Naming.
-#  NC#vote_percent::        Convert score for this Naming into a percentage.
-#  NC#users_favorite?::     Is this Naming the given User's favorite?
-#  NC#calc_vote_table::     Gather Vote info for this Naming.
-#
 #  == Attributes
 #
 #  id::                 Locally unique numerical id, starting at 1.