Merge pull request #2621 from MushroomObserver/nimmo-fix-query-test-o…

…rdering

Restore `QueryTest` ordering checks

app/classes/query/field_slips.rb

@@ -23,6 +23,6 @@ def initialize_flavor
   end
 
   def self.default_order
-    "date"
+    "code_then_date"
   end
 end

app/classes/query/modules/ordering.rb

@@ -57,6 +57,11 @@ def sort_by_code(_model)
     "herbaria.code ASC"
   end
 
+  def sort_by_code_then_date(_model)
+    "field_slips.code ASC, field_slips.created_at DESC, " \
+    "field_slips.id DESC"
+  end
+
   def sort_by_code_then_name(_model)
     "IF(herbaria.code = '', '~', herbaria.code) ASC, herbaria.name ASC"
   end

app/classes/query/species_lists.rb

@@ -46,6 +46,8 @@ def initialize_flavor
   end
 
   def add_pattern_condition
+    return if params[:pattern].blank?
+
     add_search_condition(search_fields, params[:pattern])
     add_join(:locations!)
     super

app/controllers/projects/violations_controller.rb

@@ -11,6 +11,8 @@ module Projects
   class ViolationsController < ApplicationController
     before_action :login_required
     before_action :pass_query_params
+    # Cannot figure out the eager loading here.
+    around_action :skip_bullet, if: -> { defined?(Bullet) }, only: [:index]
 
     def index
       return unless find_project!
@@ -45,7 +47,8 @@ def update
     private
 
     def find_project!
-      @project = find_or_goto_index(Project, params[:project_id])
+      @project = Project.violations_includes.find(params[:project_id]) ||
+                 flash_error_and_goto_index(Project, params[:project_id])
     end
 
     def remove_observation_if_permitted(obs_id)

app/controllers/species_lists/shared_private_methods.rb

@@ -464,8 +464,7 @@ def init_project_vars
 
     def init_project_vars_for_create
       init_project_vars
-      last_obs = Observation.where(user_id: User.current_id).
-                 order(:created_at).last
+      last_obs = Observation.recent_by_user(@user).last
       return unless last_obs && last_obs.created_at > 1.hour.ago
 
       last_obs.projects.each { |proj| @project_checks[proj.id] = true }

app/controllers/species_lists_controller.rb

@@ -76,17 +76,18 @@ def project
   def index_display_opts(opts, query)
     opts = {
       num_per_page: 20,
-      include: [:location, :user],
-      letters: "species_lists.title"
+      include: [:location, :user]
     }.merge(opts)
 
+    return opts if %w[date created modified].include?(query.params[:by]) ||
+                   query.params[:by].blank?
+
     # Paginate by letter if sorting by user.
     opts[:letters] =
       if [query.params[:by]].intersect?(%w[user reverse_user])
         "users.login"
       else
-        # Can always paginate by title letter.
-        opts[:letters] = "species_lists.title"
+        "species_lists.title"
       end
 
     opts

app/models/abstract_model.rb

@@ -132,6 +132,16 @@ def type_tag
     where(arel_table[:updated_at].format("%Y-%m-%d") >= earliest).
       where(arel_table[:updated_at].format("%Y-%m-%d") <= latest)
   }
+  scope :order_by_user, lambda {
+    joins(:user).
+      reorder(User[:name].when(nil).then(User[:login]).
+              when("").then(User[:login]).
+              else(User[:name]).asc, id: :desc).distinct
+  }
+  scope :order_by_rss_log, lambda {
+    joins(:rss_log).
+      reorder(RssLog[:updated_at].desc, model.arel_table[:id].desc).distinct
+  }
 
   ##############################################################################
   #

app/models/article.rb

@@ -26,6 +26,8 @@ class Article < AbstractModel
   belongs_to :user
   belongs_to :rss_log
 
+  default_scope { order(created_at: :desc, id: :desc) }
+
   # Automatically log standard events.
   self.autolog_events = [:created!, :updated!, :destroyed!]
 

app/models/collection_number.rb

@@ -58,6 +58,13 @@ class CollectionNumber < AbstractModel
   before_update :log_update
   before_destroy :log_destroy
 
+  default_scope { order(name: :asc, number: :asc) }
+
+  scope :for_observation, lambda { |obs|
+    joins(:observation_collection_numbers).
+      where(observation_collection_numbers: { observation: obs })
+  }
+
   def format_name
     "#{name} #{number}"
   end

app/models/comment.rb

@@ -117,8 +117,8 @@ class Comment < AbstractModel
   after_create :notify_users
   after_create :oil_and_water
 
-  scope :by_user,
-        ->(user) { where(user: user) }
+  default_scope { order(created_at: :desc, id: :desc) }
+  scope :by_user, ->(user) { where(user: user) }
 
   # This scope starts with a `where`, and chains subsequent `where` clauses
   # with `or`. So, rather than separately assembling `target_ids`, that would

app/models/field_slip.rb

@@ -7,7 +7,7 @@ class FieldSlip < AbstractModel
   belongs_to :observation
   belongs_to :project
   belongs_to :user
-  default_scope { order(:code) }
+  default_scope { order(code: :asc, created_at: :desc, id: :desc) }
 
   scope :by_user, lambda { |user|
     where(user_id: user.id).distinct

app/models/glossary_term.rb

@@ -43,6 +43,8 @@ class GlossaryTerm < AbstractModel
   )
   versioned_class.before_save { |x| x.user_id = User.current_id }
 
+  default_scope { order(name: :asc, id: :desc) }
+
   scope :show_includes, lambda {
     strict_loading.includes(
       :glossary_term_images,

app/models/herbarium.rb

@@ -63,6 +63,8 @@ class Herbarium < AbstractModel
   # Used by create/edit form.
   attr_accessor :place_name, :personal, :personal_user_name
 
+  default_scope { order(name: :asc, id: :desc) }
+
   def self.mcp_collections
     @mcp_collections ||=
       begin

app/models/herbarium_record.rb

@@ -57,6 +57,13 @@ class HerbariumRecord < AbstractModel
   before_update :log_update
   before_destroy :log_destroy
 
+  default_scope { order(initial_det: :asc, accession_number: :asc, id: :desc) }
+
+  scope :for_observation, lambda { |obs|
+    joins(:observation_herbarium_records).
+      where(observation_herbarium_records: { observation: obs })
+  }
+
   def herbarium_label
     if initial_det.blank?
       accession_number

app/models/image.rb

@@ -266,6 +266,8 @@ class Image < AbstractModel # rubocop:disable Metrics/ClassLength
   after_update :track_copyright_changes
   before_destroy :update_thumbnails
 
+  default_scope { order(created_at: :desc, id: :desc) }
+
   scope :interactive_includes, lambda {
     strict_loading.includes(
       :image_votes, :license, :projects, :user

app/models/location.rb

@@ -155,6 +155,8 @@ class Location < AbstractModel # rubocop:disable Metrics/ClassLength
     end
   end
 
+  default_scope { order(name: :asc, id: :desc) }
+
   # NOTE: To improve Coveralls display, do not use one-line stabby lambda scopes
   scope :name_includes,
         ->(place_name) { where(Location[:name].matches("%#{place_name}%")) }
@@ -310,18 +312,16 @@ def update_observation_center_columns
   def self.update_box_area_and_center_columns
     # update the locations
     update_all(update_center_and_area_sql)
-    # give center points to associated observations in batches
-    Observation.joins(:location).
-      where(Location[:box_area].lteq(MO.obs_location_max_area)).
-      group(:location_id).update_all(
-        location_lat: Location[:center_lat], location_lng: Location[:center_lng]
-      )
+    # give center points to associated observations in batches by location_id
+    # Observation must be unscoped in order to join to locations.
+    # (removing default_scope)
+    Observation.unscoped.in_box_of_max_area.group(:location_id).update_all(
+      location_lat: Location[:center_lat], location_lng: Location[:center_lng]
+    )
     # null center points where area is above the threshold
-    Observation.joins(:location).
-      where(Location[:box_area].gt(MO.obs_location_max_area)).
-      group(:location_id).update_all(
-        location_lat: nil, location_lng: nil
-      )
+    Observation.unscoped.in_box_gt_max_area.group(:location_id).update_all(
+      location_lat: nil, location_lng: nil
+    )
   end
 
   # Let attached observations update their cache if these fields changed.
@@ -582,7 +582,8 @@ def self.reverse_name_if_necessary(name)
 
   # Looks for a matching location using either location order just to be sure
   def self.find_by_name_or_reverse_name(name)
-    Location.where(name: name).or(Location.where(scientific_name: name)).first
+    Location.where(name: name).
+      or(Location.where(scientific_name: name)).first
   end
 
   def self.user_format(user, name)

app/models/name.rb

@@ -453,6 +453,8 @@ class Name < AbstractModel
     end
   end
 
+  default_scope { order(sort_name: :asc, id: :desc) }
+
   # NOTE: To improve Coveralls display, do not use one-line stabby lambda scopes
   scope :of_lichens,
         -> { where(Name[:lifeform].matches("%lichen%")) }
@@ -463,13 +465,14 @@ class Name < AbstractModel
   scope :not_deprecated,
         -> { where(deprecated: false) }
   scope :with_description,
-        -> { where.not(description_id: nil) }
+        -> { with_correct_spelling.where.not(description_id: nil) }
   scope :without_description,
-        -> { where(description_id: nil) }
-  # Names without descriptions, order by most frequently used
+        -> { with_correct_spelling.where(description_id: nil) }
+  # Names needing descriptions
   scope :description_needed, lambda {
-    with_correct_spelling.without_description.joins(:observations).
-      group(:name_id).order(Arel.star.count.desc)
+    without_description.joins(:observations).distinct
+    # in the template: order by most frequently used
+    # group(:name_id).reorder(Arel.star.count.desc)
   }
   scope :description_includes,
         lambda { |text|
@@ -505,6 +508,19 @@ class Name < AbstractModel
           joins(:descriptions).
             merge(NameDescription.where(source_type: source))
         }
+  scope :with_description_classification_differing,
+        lambda {
+          joins(:descriptions).
+            where(rank: 0..Name.ranks[:Genus]).
+            where(Name[:classification].
+                  not_eq(NameDescription[:classification])).
+            where(NameDescription[:classification].not_blank)
+        }
+  scope :by_editor,
+        lambda { |user|
+          joins(:versions).where(name_versions: { user_id: user.id }).
+            where.not(user: user)
+        }
 
   ### Module Name::Spelling
   scope :with_correct_spelling,

app/models/name/resolve.rb

@@ -67,7 +67,8 @@ def fix_capitalized_species_epithet(str)
       str2 = str.sub(/ [A-Z]/, &:downcase)
 
       # Return corrected name if that name exists, else keep original name.
-      if Name.where(search_name: str2).or(Name.where(text_name: str2)).present?
+      if Name.where(search_name: str2).
+         or(Name.where(text_name: str2)).present?
         str2
       else
         str

app/models/name/taxonomy.rb

@@ -468,9 +468,11 @@ def best_matches_from_array(names, includes = [])
     # Now allows includes, for batch lookup of Naming email interested parties
     # GOTCHA: `search_name` cannot be used as a field in this AR where clause
     def batch_lookup_all_matches(name_or_names, includes = [])
-      Name.includes(includes).where(Name[:search_name].in(name_or_names)).
-        or(Name.where(Name[:text_name].in(name_or_names))).
-        with_correct_spelling
+      # Name must be unscoped in order to keep `or` condition comparable.
+      # (removing default_scope)
+      Name.unscoped.where(Name[:search_name].in(name_or_names)).
+        or(Name.unscoped.where(Name[:text_name].in(name_or_names))).
+        with_correct_spelling.includes(includes)
     end
 
     # NOTE: may return nil if no match
@@ -606,11 +608,7 @@ def parse_classification(text)
     # This is meant to be run nightly to ensure that all the classification
     # caches are up to date.  It only pays attention to genera or higher.
     def refresh_classification_caches(dry_run: false)
-      query =
-        Name.joins(:description).
-        where(rank: 0..Name.ranks[:Genus]).
-        where(NameDescription[:classification].not_eq(Name[:classification])).
-        where(NameDescription[:classification].not_blank)
+      query = Name.unscoped.with_description_classification_differing
       msgs = query.map do |name|
         "Classification for #{name.search_name} didn't match description."
       end