From f50b757bb47223541dc5ab07d6e4ecf7ee1f55ad Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 10:20:34 -0800 Subject: [PATCH 01/39] Stash Jason's original SQL lookup_names --- .../query/modules/jasons_lookup_names.rb | 191 ++++++++++++++++++ app/classes/query/modules/lookup_names.rb | 6 +- 2 files changed, 193 insertions(+), 4 deletions(-) create mode 100644 app/classes/query/modules/jasons_lookup_names.rb diff --git a/app/classes/query/modules/jasons_lookup_names.rb b/app/classes/query/modules/jasons_lookup_names.rb new file mode 100644 index 0000000000..2f7abb9427 --- /dev/null +++ b/app/classes/query/modules/jasons_lookup_names.rb @@ -0,0 +1,191 @@ +# frozen_string_literal: true + +# Helper methods to help parsing name instances from parameter strings. +module Query::Modules::JasonsLookupNames + def lookup_names_by_name(args) + unless (vals = args[:names]) + complain_about_unused_flags!(args) + return + end + + orig_names = given_names(vals, args) + min_names = add_synonyms_if_necessary(orig_names, args) + min_names2 = add_subtaxa_if_necessary(min_names, args) + min_names = add_synonyms_again(min_names, min_names2, args) + min_names -= orig_names if args[:exclude_original_names] + min_names.map { |min_name| min_name[0] } + end + + # ---------------------------------------------------------------------------- + + private + + def given_names(vals, args) + min_names = find_exact_name_matches(vals) + if args[:exclude_original_names] + add_other_spellings(min_names) + else + min_names + end + end + + def add_synonyms_if_necessary(min_names, args) + if args[:include_synonyms] + add_synonyms(min_names) + elsif !args[:exclude_original_names] + add_other_spellings(min_names) + else + min_names + end + end + + def add_subtaxa_if_necessary(min_names, args) + if args[:include_subtaxa] + add_subtaxa(min_names) + elsif args[:include_immediate_subtaxa] + add_immediate_subtaxa(min_names) + else + min_names + end + end + + def add_synonyms_again(min_names, min_names2, args) + if min_names.length >= min_names2.length + min_names + elsif args[:include_synonyms] + add_synonyms(min_names2) + else + add_other_spellings(min_names2) + end + end + + def complain_about_unused_flags!(args) + complain_about_unused_flag!(args, :include_synonyms) + complain_about_unused_flag!(args, :include_subtaxa) + complain_about_unused_flag!(args, :include_nonconsensus) + complain_about_unused_flag!(args, :exclude_consensus) + complain_about_unused_flag!(args, :exclude_original_names) + end + + def complain_about_unused_flag!(args, arg) + return if args[arg].nil? + + raise("Flag \"#{arg}\" is invalid without \"names\" parameter.") + end + + def find_exact_name_matches(vals) + vals.inject([]) do |result, val| + if /^\d+$/.match?(val.to_s) + result << minimal_name_data(Name.safe_find(val)) + else + result += find_matching_names(val) + end + result + end.uniq.compact + end + + def find_matching_names(name) + parse = Name.parse_name(name) + name2 = parse ? parse.search_name : Name.clean_incoming_string(name) + matches = Name.where(search_name: name2) if parse.author.present? + matches = Name.where(text_name: name2) if matches.empty? + matches.map { |name3| minimal_name_data(name3) } + end + + def add_other_spellings(min_names) + ids = min_names.map { |min_name| min_name[1] || min_name[0] } + return [] if ids.empty? + + Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE COALESCE(correct_spelling_id, id) IN (#{clean_id_set(ids)}) + )) + end + + def add_synonyms(min_names) + ids = min_names.map { |min_name| min_name[2] }.reject(&:nil?) + return min_names if ids.empty? + + min_names.reject { |min_name| min_name[2] } + + Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE synonym_id IN (#{clean_id_set(ids)}) + )) + end + + def add_subtaxa(min_names) + higher_names = genera_and_up(min_names) + lower_names = genera_and_down(min_names) + unless higher_names.empty? + min_names += Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE classification REGEXP ": _(#{higher_names.join("|")})_" + )) + end + min_names += Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE text_name REGEXP "^(#{lower_names.join("|")}) " + )) + min_names.uniq + end + + def add_immediate_subtaxa(min_names) + higher_names = genera_and_up(min_names) + lower_names = genera_and_down(min_names) + unless higher_names.empty? + min_names += Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE classification REGEXP ": _(#{higher_names.join("|")})_$" + AND text_name NOT LIKE "% %" + )) + end + min_names += Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE text_name REGEXP + "^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$" + )) + min_names.uniq + end + + def genera_and_up(min_names) + min_names.map { |min_name| min_name[3] }. + reject { |min_name| min_name.include?(" ") } + end + + def genera_and_down(min_names) + genera = {} + text_names = min_names.map { |min_name| min_name[3] } + # Make hash of all genera present. + text_names.each do |text_name| + genera[text_name] = true unless text_name.include?(" ") + end + # Remove species if genus also present. + text_names.reject do |text_name| + text_name.include?(" ") && genera[text_name.split(" ").first] + end.uniq + end + + # This ugliness with "minimal name data" is a way to avoid having Rails + # instantiate all the names (which can get quite huge if you start talking + # about all the children of Kingdom Fungi!) It allows us to use low-level + # mysql queries, and restricts the dataflow back and forth to the database + # to just the few columns we actually need. Unfortunately it is ugly, + # it totally violates the autonomy of Name, and it is probably hard to + # understand. But hopefully once we get it working it will remain stable. + # Blame it on me... -Jason, July 2019 + + def minimal_name_data(name) + return nil unless name + + [ + name.id, # 0 + name.correct_spelling_id, # 1 + name.synonym_id, # 2 + name.text_name # 3 + ] + end + + def minimal_name_columns + "id, correct_spelling_id, synonym_id, text_name" + end +end diff --git a/app/classes/query/modules/lookup_names.rb b/app/classes/query/modules/lookup_names.rb index ff800b7cfe..a2b660fa50 100644 --- a/app/classes/query/modules/lookup_names.rb +++ b/app/classes/query/modules/lookup_names.rb @@ -95,10 +95,8 @@ def add_other_spellings(min_names) ids = min_names.map { |min_name| min_name[1] || min_name[0] } return [] if ids.empty? - Name. - where(Name[:correct_spelling_id].coalesce(Name[:id]). - in(limited_id_set(ids))). - pluck(*minimal_name_columns) + Name.where(Name[:correct_spelling_id].coalesce(Name[:id]). + in(limited_id_set(ids))).pluck(*minimal_name_columns) end def add_synonyms(min_names) From 1a54c925aba595944150246d00afe8c775c642b0 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 10:20:54 -0800 Subject: [PATCH 02/39] Update jasons_lookup_names.rb --- app/classes/query/modules/jasons_lookup_names.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/classes/query/modules/jasons_lookup_names.rb b/app/classes/query/modules/jasons_lookup_names.rb index 2f7abb9427..b7fdeacffc 100644 --- a/app/classes/query/modules/jasons_lookup_names.rb +++ b/app/classes/query/modules/jasons_lookup_names.rb @@ -75,9 +75,9 @@ def complain_about_unused_flag!(args, arg) def find_exact_name_matches(vals) vals.inject([]) do |result, val| - if /^\d+$/.match?(val.to_s) + if /^\d+$/.match?(val.to_s) # from an id result << minimal_name_data(Name.safe_find(val)) - else + else # from a string result += find_matching_names(val) end result From 2124cadc489bed02a1742300659de235e1e413af Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 13:32:07 -0800 Subject: [PATCH 03/39] Start a new PORO for Query and other lookups This is intended to be an "omnivorous" looker-upper, that can handle a single string, ID, or instance, or a mixed array of those. It has methods to return ids or instances, or in the case of names, subtaxa/synonymy subsets for testing. --- app/classes/lookup.rb | 74 ++++++++ app/classes/lookup/external_sites.rb | 12 ++ app/classes/lookup/herbaria.rb | 12 ++ app/classes/lookup/herbarium_records.rb | 12 ++ app/classes/lookup/locations.rb | 15 ++ app/classes/lookup/names.rb | 191 ++++++++++++++++++++ app/classes/lookup/project_species_lists.rb | 26 +++ app/classes/lookup/projects.rb | 12 ++ app/classes/lookup/regions.rb | 12 ++ app/classes/lookup/species_lists.rb | 12 ++ app/classes/lookup/users.rb | 12 ++ 11 files changed, 390 insertions(+) create mode 100644 app/classes/lookup.rb create mode 100644 app/classes/lookup/external_sites.rb create mode 100644 app/classes/lookup/herbaria.rb create mode 100644 app/classes/lookup/herbarium_records.rb create mode 100644 app/classes/lookup/locations.rb create mode 100644 app/classes/lookup/names.rb create mode 100644 app/classes/lookup/project_species_lists.rb create mode 100644 app/classes/lookup/projects.rb create mode 100644 app/classes/lookup/regions.rb create mode 100644 app/classes/lookup/species_lists.rb create mode 100644 app/classes/lookup/users.rb diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb new file mode 100644 index 0000000000..eda12388d9 --- /dev/null +++ b/app/classes/lookup.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +class Lookup + attr_reader :model, :vals, :params + + def initialize(vals, params = {}) + @vals = prepare_vals(vals) + @params = params + end + + def prepare_vals(vals) + return [] if vals.blank? + + if vals.is_a?(Array) + vals + else + [vals] + end + end + + def ids + @ids ||= lookup_ids + end + + def instances + @instances ||= lookup_instances + end + + def lookup_ids + return [] unless @vals + + @vals = [@vals] unless @vals.is_a?(Array) + evaluate_values_as_ids + end + + def lookup_instances + return [] unless @vals + + @vals = [@vals] unless @vals.is_a?(Array) + evaluate_values_as_instances + end + + # This is checking for an instance, then sanity-checking for an instance of + # the wrong model, then for an ID, then yielding to the lookup lambda. + # In the last condition, `yield` means run any lambda block provided to this + # method. (It only looks up the record in the case it can't find an ID.) + def evaluate_values_as_ids + @vals.map do |val| + if val.is_a?(@model) + val.id + elsif val.is_a?(AbstractModel) + raise("Passed a #{val.class} to LookupIDs for #{@model}.") + elsif /^\d+$/.match?(val.to_s) + val + else # each lookup returns an array + lookup_method(val).map(&:id) + end + end.flatten.uniq.compact + end + + def evaluate_values_as_instances(&) + @vals.map do |val| + if val.is_a?(@model) + val + elsif val.is_a?(AbstractModel) + raise("Passed a #{val.class} to LookupIDs for #{@model}.") + elsif /^\d+$/.match?(val.to_s) + @model.find(val.to_i) + else # each lookup returns an array + lookup_method(val) + end + end.flatten.uniq.compact + end +end diff --git a/app/classes/lookup/external_sites.rb b/app/classes/lookup/external_sites.rb new file mode 100644 index 0000000000..c9f8dc27a6 --- /dev/null +++ b/app/classes/lookup/external_sites.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Lookup::ExternalSites < Lookup + def initialize(vals, params = {}) + super + @model = ExternalSite + end + + def lookup_method(name) + ExternalSite.where(name: name) + end +end diff --git a/app/classes/lookup/herbaria.rb b/app/classes/lookup/herbaria.rb new file mode 100644 index 0000000000..0348a3a51a --- /dev/null +++ b/app/classes/lookup/herbaria.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Lookup::Herbaria < Lookup + def initialize(vals, params = {}) + super + @model = Herbarium + end + + def lookup_method(name) + Herbarium.where(name: name) + end +end diff --git a/app/classes/lookup/herbarium_records.rb b/app/classes/lookup/herbarium_records.rb new file mode 100644 index 0000000000..7c65ae19b4 --- /dev/null +++ b/app/classes/lookup/herbarium_records.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Lookup::HerbariumRecords < Lookup + def initialize(vals, params = {}) + super + @model = HerbariumRecord + end + + def lookup_method(name) + HerbariumRecord.where(id: name) + end +end diff --git a/app/classes/lookup/locations.rb b/app/classes/lookup/locations.rb new file mode 100644 index 0000000000..eafa48f0eb --- /dev/null +++ b/app/classes/lookup/locations.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class Lookup::Locations < Lookup + def initialize(vals, params = {}) + super + @model = Location + end + + def lookup_method(name) + # Downcases and removes all punctuation, so it's a multi-string search + # e.g. "sonoma co california usa" + pattern = Location.clean_name(name.to_s).clean_pattern + Location.name_contains(pattern) + end +end diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb new file mode 100644 index 0000000000..f85de5c5a2 --- /dev/null +++ b/app/classes/lookup/names.rb @@ -0,0 +1,191 @@ +# frozen_string_literal: true + +class Lookup::Names < Lookup + def initialize(vals, params = {}) + super + @model = Name + end + + def lookup_ids + unless (@vals = params[:names]) + complain_about_unused_flags!(args) + return + end + + orig_names = given_names(@vals, params) + min_names = add_synonyms_if_necessary(orig_names, params) + min_names2 = add_subtaxa_if_necessary(min_names, params) + min_names = add_synonyms_again(min_names, min_names2, params) + min_names -= orig_names if params[:exclude_original_names] + min_names.map { |min_name| min_name[0] } + end + + def given_names(vals, args) + min_names = find_exact_name_matches(vals) + if args[:exclude_original_names] + add_other_spellings(min_names) + else + min_names + end + end + + def add_synonyms_if_necessary(min_names, args) + if args[:include_synonyms] + add_synonyms(min_names) + elsif !args[:exclude_original_names] + add_other_spellings(min_names) + else + min_names + end + end + + def add_subtaxa_if_necessary(min_names, args) + if args[:include_subtaxa] + add_subtaxa(min_names) + elsif args[:include_immediate_subtaxa] + add_immediate_subtaxa(min_names) + else + min_names + end + end + + def add_synonyms_again(min_names, min_names2, args) + if min_names.length >= min_names2.length + min_names + elsif args[:include_synonyms] + add_synonyms(min_names2) + else + add_other_spellings(min_names2) + end + end + + def complain_about_unused_flags!(args) + complain_about_unused_flag!(args, :include_synonyms) + complain_about_unused_flag!(args, :include_subtaxa) + complain_about_unused_flag!(args, :include_nonconsensus) + complain_about_unused_flag!(args, :exclude_consensus) + complain_about_unused_flag!(args, :exclude_original_names) + end + + def complain_about_unused_flag!(args, arg) + return if args[arg].nil? + + raise("Flag \"#{arg}\" is invalid without \"names\" parameter.") + end + + def find_exact_name_matches(vals) + vals.inject([]) do |result, val| + if /^\d+$/.match?(val.to_s) # from an id + result << minimal_name_data(Name.safe_find(val)) + else # from a string + result += find_matching_names(val) + end + result + end.uniq.compact + end + + def find_matching_names(name) + parse = Name.parse_name(name) + name2 = parse ? parse.search_name : Name.clean_incoming_string(name) + matches = Name.where(search_name: name2) if parse.author.present? + matches = Name.where(text_name: name2) if matches.empty? + matches.map { |name3| minimal_name_data(name3) } + end + + def add_other_spellings(min_names) + ids = min_names.map { |min_name| min_name[1] || min_name[0] } + return [] if ids.empty? + + Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE COALESCE(correct_spelling_id, id) IN (#{clean_id_set(ids)}) + )) + end + + def add_synonyms(min_names) + ids = min_names.map { |min_name| min_name[2] }.reject(&:nil?) + return min_names if ids.empty? + + min_names.reject { |min_name| min_name[2] } + + Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE synonym_id IN (#{clean_id_set(ids)}) + )) + end + + def add_subtaxa(min_names) + higher_names = genera_and_up(min_names) + lower_names = genera_and_down(min_names) + unless higher_names.empty? + min_names += Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE classification REGEXP ": _(#{higher_names.join("|")})_" + )) + end + min_names += Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE text_name REGEXP "^(#{lower_names.join("|")}) " + )) + min_names.uniq + end + + def add_immediate_subtaxa(min_names) + higher_names = genera_and_up(min_names) + lower_names = genera_and_down(min_names) + unless higher_names.empty? + min_names += Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE classification REGEXP ": _(#{higher_names.join("|")})_$" + AND text_name NOT LIKE "% %" + )) + end + min_names += Name.connection.select_rows(%( + SELECT #{minimal_name_columns} FROM names + WHERE text_name REGEXP + "^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$" + )) + min_names.uniq + end + + def genera_and_up(min_names) + min_names.map { |min_name| min_name[3] }. + reject { |min_name| min_name.include?(" ") } + end + + def genera_and_down(min_names) + genera = {} + text_names = min_names.map { |min_name| min_name[3] } + # Make hash of all genera present. + text_names.each do |text_name| + genera[text_name] = true unless text_name.include?(" ") + end + # Remove species if genus also present. + text_names.reject do |text_name| + text_name.include?(" ") && genera[text_name.split(" ").first] + end.uniq + end + + # This ugliness with "minimal name data" is a way to avoid having Rails + # instantiate all the names (which can get quite huge if you start talking + # about all the children of Kingdom Fungi!) It allows us to use low-level + # mysql queries, and restricts the dataflow back and forth to the database + # to just the few columns we actually need. Unfortunately it is ugly, + # it totally violates the autonomy of Name, and it is probably hard to + # understand. But hopefully once we get it working it will remain stable. + # Blame it on me... -Jason, July 2019 + + def minimal_name_data(name) + return nil unless name + + [ + name.id, # 0 + name.correct_spelling_id, # 1 + name.synonym_id, # 2 + name.text_name # 3 + ] + end + + def minimal_name_columns + "id, correct_spelling_id, synonym_id, text_name" + end +end diff --git a/app/classes/lookup/project_species_lists.rb b/app/classes/lookup/project_species_lists.rb new file mode 100644 index 0000000000..609c19db44 --- /dev/null +++ b/app/classes/lookup/project_species_lists.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class Lookup::ProjectSpeciesLists < Lookup + def initialize(vals, params = {}) + super + @model = SpeciesList + end + + def ids + @ids ||= lookup_method.map(&:id) + end + + def instances + @instances ||= lookup_method + end + + def lookup_method + # We're checking species lists for each project. + project_ids = Lookup::Projects.new(@vals).ids + return [] if project_ids.empty? + + # Have to map(&:id) because it doesn't return lookup_object_ids_by_name + SpeciesList.joins(:project_species_lists). + where(project_species_lists: { project_id: project_ids }).distinct + end +end diff --git a/app/classes/lookup/projects.rb b/app/classes/lookup/projects.rb new file mode 100644 index 0000000000..4a592f083b --- /dev/null +++ b/app/classes/lookup/projects.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Lookup::Projects < Lookup + def initialize(vals, params = {}) + super + @model = Project + end + + def lookup_method(name) + Project.where(title: name) + end +end diff --git a/app/classes/lookup/regions.rb b/app/classes/lookup/regions.rb new file mode 100644 index 0000000000..556ed65f7e --- /dev/null +++ b/app/classes/lookup/regions.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Lookup::Regions < Lookup + def initialize(vals, params = {}) + super + @model = Location + end + + def lookup_method(name) + Location.in_region(name.to_s.clean_pattern) + end +end diff --git a/app/classes/lookup/species_lists.rb b/app/classes/lookup/species_lists.rb new file mode 100644 index 0000000000..6da944b4d5 --- /dev/null +++ b/app/classes/lookup/species_lists.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Lookup::SpeciesLists < Lookup + def initialize(vals, params = {}) + super + @model = SpeciesList + end + + def lookup_method(name) + SpeciesList.where(title: name) + end +end diff --git a/app/classes/lookup/users.rb b/app/classes/lookup/users.rb new file mode 100644 index 0000000000..8ab2e815b0 --- /dev/null +++ b/app/classes/lookup/users.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class Lookup::Users < Lookup + def initialize(vals, params = {}) + super + @model = User + end + + def lookup_method(name) + User.where(login: User.remove_bracketed_name(name)) + end +end From 7418da6960ad65fe8ce1f4bb5b84e308ac09ec63 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 14:03:35 -0800 Subject: [PATCH 04/39] Use more recent lookup_names --- app/classes/lookup/names.rb | 73 ++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index f85de5c5a2..1d654e25cd 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -96,72 +96,77 @@ def add_other_spellings(min_names) ids = min_names.map { |min_name| min_name[1] || min_name[0] } return [] if ids.empty? - Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE COALESCE(correct_spelling_id, id) IN (#{clean_id_set(ids)}) - )) + Name.where(Name[:correct_spelling_id].coalesce(Name[:id]). + in(limited_id_set(ids))).pluck(*minimal_name_columns) end def add_synonyms(min_names) - ids = min_names.map { |min_name| min_name[2] }.reject(&:nil?) + ids = min_names.filter_map { |min_name| min_name[2] } return min_names if ids.empty? min_names.reject { |min_name| min_name[2] } + - Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE synonym_id IN (#{clean_id_set(ids)}) - )) + Name.where(synonym_id: clean_id_set(ids).split(",")). + pluck(*minimal_name_columns) end def add_subtaxa(min_names) higher_names = genera_and_up(min_names) lower_names = genera_and_down(min_names) - unless higher_names.empty? - min_names += Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE classification REGEXP ": _(#{higher_names.join("|")})_" - )) - end - min_names += Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE text_name REGEXP "^(#{lower_names.join("|")}) " - )) - min_names.uniq + query = Name.where(id: min_names.map(&:first)) + query = add_lower_names(query, lower_names) + query = add_higher_names(query, higher_names) unless higher_names.empty? + query.distinct.pluck(*minimal_name_columns) + end + + def add_lower_names(query, names) + query.or(Name. + where(Name[:text_name] =~ /^(#{names.join("|")}) /)) + end + + def add_higher_names(query, names) + query.or(Name. + where(Name[:classification] =~ /: _(#{names.join("|")})_/)) end def add_immediate_subtaxa(min_names) higher_names = genera_and_up(min_names) lower_names = genera_and_down(min_names) + + query = Name.where(id: min_names.map(&:first)) + query = add_immediate_lower_names(query, lower_names) unless higher_names.empty? - min_names += Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE classification REGEXP ": _(#{higher_names.join("|")})_$" - AND text_name NOT LIKE "% %" - )) + query = add_immediate_higher_names(query, higher_names) end - min_names += Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE text_name REGEXP - "^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$" - )) - min_names.uniq + query.distinct.pluck(*minimal_name_columns) + end + + def add_immediate_lower_names(query, lower_names) + query.or(Name. + where(Name[:text_name] =~ + /^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$/)) + end + + def add_immediate_higher_names(query, higher_names) + query.or(Name. + where(Name[:classification] =~ /: _(#{higher_names.join("|")})_$/). + where.not(Name[:text_name].matches("% %"))) end def genera_and_up(min_names) - min_names.map { |min_name| min_name[3] }. + min_names.pluck(3). reject { |min_name| min_name.include?(" ") } end def genera_and_down(min_names) genera = {} - text_names = min_names.map { |min_name| min_name[3] } + text_names = min_names.pluck(3) # Make hash of all genera present. text_names.each do |text_name| genera[text_name] = true unless text_name.include?(" ") end # Remove species if genus also present. text_names.reject do |text_name| - text_name.include?(" ") && genera[text_name.split(" ").first] + text_name.include?(" ") && genera[text_name.split.first] end.uniq end From 051bfce2a668b0e4b5a85471aa5321d6b0b2d53f Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 14:10:43 -0800 Subject: [PATCH 05/39] Add test for the Lookup::Names PORO --- app/classes/lookup/names.rb | 4 ++ test/classes/lookup_test.rb | 131 ++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 test/classes/lookup_test.rb diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 1d654e25cd..685ed0a7b7 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -6,6 +6,10 @@ def initialize(vals, params = {}) @model = Name end + def lookup_instances + @ids.map { |id| Name.find(id) } + end + def lookup_ids unless (@vals = params[:names]) complain_about_unused_flags!(args) diff --git a/test/classes/lookup_test.rb b/test/classes/lookup_test.rb new file mode 100644 index 0000000000..a5df5eb5fb --- /dev/null +++ b/test/classes/lookup_test.rb @@ -0,0 +1,131 @@ +# frozen_string_literal: true + +require("test_helper") + +# tests of Lookup::Names +class Lookup::NamesTest + def create_test_name(name) + name = Name.new_name(Name.parse_name(name).params) + name.save + name + end + + def assert_lookup_names_by_name(expects, args) + actual = Lookup::Names.new(args).instances.sort_by(&:text_name) + expects = expects.sort_by(&:text_name) + # actual = actual.map { |id| Name.find(id) }.sort_by(&:text_name) + assert_name_arrays_equal(expects, actual) + end + + def test_lookup_names_by_name + User.current = rolf + + name1 = names(:macrolepiota) + name2 = names(:macrolepiota_rachodes) + name3 = names(:macrolepiota_rhacodes) + name4 = create_test_name("Pseudolepiota") + name5 = create_test_name("Pseudolepiota rachodes") + + name1.update(synonym_id: Synonym.create.id) + name4.update(synonym_id: name1.synonym_id) + name5.update(synonym_id: name2.synonym_id) + + assert_lookup_names_by_name([name1], names: ["Macrolepiota"]) + assert_lookup_names_by_name([name2], names: ["Macrolepiota rachodes"]) + assert_lookup_names_by_name([name1, name4], + names: ["Macrolepiota"], + include_synonyms: true) + assert_lookup_names_by_name([name2, name3, name5], + names: ["Macrolepiota rachodes"], + include_synonyms: true) + assert_lookup_names_by_name([name3, name5], + names: ["Macrolepiota rachodes"], + include_synonyms: true, + exclude_original_names: true) + assert_lookup_names_by_name([name1, name2, name3], + names: ["Macrolepiota"], + include_subtaxa: true) + assert_lookup_names_by_name([name1, name2, name3], + names: ["Macrolepiota"], + include_immediate_subtaxa: true) + assert_lookup_names_by_name([name1, name2, name3, name4, name5], + names: ["Macrolepiota"], + include_synonyms: true, + include_subtaxa: true) + assert_lookup_names_by_name([name2, name3, name4, name5], + names: ["Macrolepiota"], + include_synonyms: true, + include_subtaxa: true, + exclude_original_names: true) + + name5.update(synonym_id: nil) + name5 = Name.where(text_name: "Pseudolepiota rachodes").index_order.first + assert_lookup_names_by_name([name1, name2, name3, name4, name5], + names: ["Macrolepiota"], + include_synonyms: true, + include_subtaxa: true) + end + + def test_lookup_names_by_name2 + User.current = rolf + + name1 = names(:peltigeraceae) + name2 = names(:peltigera) + name3 = names(:petigera) + name4 = create_test_name("Peltigera canina") + name5 = create_test_name("Peltigera canina var. spuria") + name6 = create_test_name("Peltigera subg. Foo") + name7 = create_test_name("Peltigera subg. Foo sect. Bar") + + name4.update(classification: name2.classification) + name5.update(classification: name2.classification) + name6.update(classification: name2.classification) + name7.update(classification: name2.classification) + + assert_lookup_names_by_name([name2, name3], names: ["Peltigera"]) + assert_lookup_names_by_name([name2, name3], names: ["Petigera"]) + assert_lookup_names_by_name([name1, name2, name3, name4, name5, name6, + name7], + names: ["Peltigeraceae"], + include_subtaxa: true) + assert_lookup_names_by_name([name1, name2, name3], + names: ["Peltigeraceae"], + include_immediate_subtaxa: true) + assert_lookup_names_by_name([name2, name3, name4, name5, name6, name7], + names: ["Peltigera"], + include_subtaxa: true) + assert_lookup_names_by_name([name2, name3, name4, name6], + names: ["Peltigera"], + include_immediate_subtaxa: true) + assert_lookup_names_by_name([name6, name7], + names: ["Peltigera subg. Foo"], + include_immediate_subtaxa: true) + assert_lookup_names_by_name([name4, name5], + names: ["Peltigera canina"], + include_immediate_subtaxa: true) + end + + def test_lookup_names_by_name3 + User.current = rolf + + name1 = names(:lactarius) + name2 = create_test_name("Lactarius \"fakename\"") + name2.update(classification: name1.classification) + name2.save + + children = Name.index_order.where(Name[:text_name].matches("Lactarius %")) + + assert_lookup_names_by_name([name1] + children, + names: ["Lactarius"], + include_subtaxa: true) + + assert_lookup_names_by_name(children, + names: ["Lactarius"], + include_immediate_subtaxa: true, + exclude_original_names: true) + end + + def test_lookup_names_by_name4 + assert_lookup_names_by_name([], names: ["¡not a name!"]) + end +end From 748894764b2b8dbd6f014f441c735962acbb8bcf Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 14:23:25 -0800 Subject: [PATCH 06/39] Provide titles for testing and sanity checking --- app/classes/lookup.rb | 8 ++++++++ app/classes/lookup/external_sites.rb | 1 + app/classes/lookup/herbaria.rb | 1 + app/classes/lookup/herbarium_records.rb | 1 + app/classes/lookup/locations.rb | 1 + app/classes/lookup/names.rb | 1 + app/classes/lookup/project_species_lists.rb | 1 + app/classes/lookup/projects.rb | 1 + app/classes/lookup/regions.rb | 1 + app/classes/lookup/species_lists.rb | 1 + 10 files changed, 17 insertions(+) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index eda12388d9..424452b58b 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -26,6 +26,10 @@ def instances @instances ||= lookup_instances end + def titles + @titles ||= lookup_titles_from_instances + end + def lookup_ids return [] unless @vals @@ -40,6 +44,10 @@ def lookup_instances evaluate_values_as_instances end + def lookup_titles + @instances.map(&:"#{@name_column}") + end + # This is checking for an instance, then sanity-checking for an instance of # the wrong model, then for an ID, then yielding to the lookup lambda. # In the last condition, `yield` means run any lambda block provided to this diff --git a/app/classes/lookup/external_sites.rb b/app/classes/lookup/external_sites.rb index c9f8dc27a6..d9f6267bd8 100644 --- a/app/classes/lookup/external_sites.rb +++ b/app/classes/lookup/external_sites.rb @@ -4,6 +4,7 @@ class Lookup::ExternalSites < Lookup def initialize(vals, params = {}) super @model = ExternalSite + @name_column = :name end def lookup_method(name) diff --git a/app/classes/lookup/herbaria.rb b/app/classes/lookup/herbaria.rb index 0348a3a51a..3e7222625a 100644 --- a/app/classes/lookup/herbaria.rb +++ b/app/classes/lookup/herbaria.rb @@ -4,6 +4,7 @@ class Lookup::Herbaria < Lookup def initialize(vals, params = {}) super @model = Herbarium + @name_column = :name end def lookup_method(name) diff --git a/app/classes/lookup/herbarium_records.rb b/app/classes/lookup/herbarium_records.rb index 7c65ae19b4..eb1f039944 100644 --- a/app/classes/lookup/herbarium_records.rb +++ b/app/classes/lookup/herbarium_records.rb @@ -4,6 +4,7 @@ class Lookup::HerbariumRecords < Lookup def initialize(vals, params = {}) super @model = HerbariumRecord + @name_column = :id end def lookup_method(name) diff --git a/app/classes/lookup/locations.rb b/app/classes/lookup/locations.rb index eafa48f0eb..39937caf7a 100644 --- a/app/classes/lookup/locations.rb +++ b/app/classes/lookup/locations.rb @@ -4,6 +4,7 @@ class Lookup::Locations < Lookup def initialize(vals, params = {}) super @model = Location + @name_column = :name end def lookup_method(name) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 685ed0a7b7..6c8ae327b9 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -4,6 +4,7 @@ class Lookup::Names < Lookup def initialize(vals, params = {}) super @model = Name + @name_column = :search_name end def lookup_instances diff --git a/app/classes/lookup/project_species_lists.rb b/app/classes/lookup/project_species_lists.rb index 609c19db44..2880824396 100644 --- a/app/classes/lookup/project_species_lists.rb +++ b/app/classes/lookup/project_species_lists.rb @@ -4,6 +4,7 @@ class Lookup::ProjectSpeciesLists < Lookup def initialize(vals, params = {}) super @model = SpeciesList + @name_column = :title end def ids diff --git a/app/classes/lookup/projects.rb b/app/classes/lookup/projects.rb index 4a592f083b..d1a0b7e8fb 100644 --- a/app/classes/lookup/projects.rb +++ b/app/classes/lookup/projects.rb @@ -4,6 +4,7 @@ class Lookup::Projects < Lookup def initialize(vals, params = {}) super @model = Project + @name_column = :title end def lookup_method(name) diff --git a/app/classes/lookup/regions.rb b/app/classes/lookup/regions.rb index 556ed65f7e..bb554ad422 100644 --- a/app/classes/lookup/regions.rb +++ b/app/classes/lookup/regions.rb @@ -4,6 +4,7 @@ class Lookup::Regions < Lookup def initialize(vals, params = {}) super @model = Location + @name_column = :name end def lookup_method(name) diff --git a/app/classes/lookup/species_lists.rb b/app/classes/lookup/species_lists.rb index 6da944b4d5..f2c1aa1485 100644 --- a/app/classes/lookup/species_lists.rb +++ b/app/classes/lookup/species_lists.rb @@ -4,6 +4,7 @@ class Lookup::SpeciesLists < Lookup def initialize(vals, params = {}) super @model = SpeciesList + @name_column = :title end def lookup_method(name) From 80db727a1f82cf69a0339f3284effa39ad6642a7 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 16:19:20 -0800 Subject: [PATCH 07/39] Switch to use PORO --- app/classes/lookup/names.rb | 20 +++---- app/classes/query/modules/lookup_names.rb | 23 +++++--- app/classes/query/modules/lookup_objects.rb | 38 +++---------- app/models/abstract_model/scopes.rb | 63 +++------------------ 4 files changed, 43 insertions(+), 101 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 6c8ae327b9..ee2dfe8978 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -12,16 +12,16 @@ def lookup_instances end def lookup_ids - unless (@vals = params[:names]) - complain_about_unused_flags!(args) - return - end - - orig_names = given_names(@vals, params) - min_names = add_synonyms_if_necessary(orig_names, params) - min_names2 = add_subtaxa_if_necessary(min_names, params) - min_names = add_synonyms_again(min_names, min_names2, params) - min_names -= orig_names if params[:exclude_original_names] + # unless (@vals = params[:names]) + # complain_about_unused_flags!(args) + # return + # end + + orig_names = given_names(@vals, @params) + min_names = add_synonyms_if_necessary(orig_names, @params) + min_names2 = add_subtaxa_if_necessary(min_names, @params) + min_names = add_synonyms_again(min_names, min_names2, @params) + min_names -= orig_names if @params[:exclude_original_names] min_names.map { |min_name| min_name[0] } end diff --git a/app/classes/query/modules/lookup_names.rb b/app/classes/query/modules/lookup_names.rb index a2b660fa50..16fd17619b 100644 --- a/app/classes/query/modules/lookup_names.rb +++ b/app/classes/query/modules/lookup_names.rb @@ -8,13 +8,22 @@ def lookup_names_by_name(args) return end - orig_names = given_names(vals, args) - min_names = add_synonyms_if_necessary(orig_names, args) - min_names2 = add_subtaxa_if_necessary(min_names, args) - min_names = add_synonyms_again(min_names, min_names2, args) - min_names -= orig_names if args[:exclude_original_names] - min_names.pluck(0) - end + Lookup::Names.new(vals, **args).ids + end + + # def lookup_names_by_name(args) + # unless (vals = args[:names]) + # complain_about_unused_flags!(args) + # return + # end + + # orig_names = given_names(vals, args) + # min_names = add_synonyms_if_necessary(orig_names, args) + # min_names2 = add_subtaxa_if_necessary(min_names, args) + # min_names = add_synonyms_again(min_names, min_names2, args) + # min_names -= orig_names if args[:exclude_original_names] + # min_names.pluck(0) + # end # ------------------------------------------------------------------------ diff --git a/app/classes/query/modules/lookup_objects.rb b/app/classes/query/modules/lookup_objects.rb index 0ac50a924f..923cffafe3 100644 --- a/app/classes/query/modules/lookup_objects.rb +++ b/app/classes/query/modules/lookup_objects.rb @@ -3,57 +3,35 @@ # Helper methods to help parsing object instances from parameter strings. module Query::Modules::LookupObjects def lookup_external_sites_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - ExternalSite.where(name: name) - end + Lookup::ExternalSites.new(vals).ids end def lookup_herbaria_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - Herbarium.where(name: name) - end + Lookup::Herbaria.new(vals).ids end def lookup_herbarium_records_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - HerbariumRecord.where(id: name) - end + Lookup::HerbariumRecords.new(vals).ids end def lookup_locations_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - pattern = clean_pattern(Location.clean_name(name)) - Location.where("name LIKE ?", "%#{pattern}%") - end + Lookup::Locations.new(vals).ids end def lookup_projects_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - Project.where(title: name) - end + Lookup::Projects.new(vals).ids end def lookup_lists_for_projects_by_name(vals) - return unless vals - - project_ids = lookup_projects_by_name(vals) - return [] if project_ids.empty? - - SpeciesList.joins(:project_species_lists). - where(project_species_lists: { project_id: project_ids }).distinct. - map(&:id) + Lookup::ProjectSpeciesLists.new(vals).ids end def lookup_species_lists_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - SpeciesList.where(title: name) - end + Lookup::SpeciesLists.new(vals).ids end def lookup_users_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - User.where(login: User.remove_bracketed_name(name)) - end + Lookup::Users.new(vals).ids end # ------------------------------------------------------------------------ diff --git a/app/models/abstract_model/scopes.rb b/app/models/abstract_model/scopes.rb index a34acbb361..fa5b1eda85 100644 --- a/app/models/abstract_model/scopes.rb +++ b/app/models/abstract_model/scopes.rb @@ -274,84 +274,39 @@ def searchable_columns end def lookup_external_sites_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - ExternalSite.where(name: name) - end + Lookup::ExternalSites.new(vals).ids end def lookup_herbaria_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - Herbarium.where(name: name) - end + Lookup::Herbaria.new(vals).ids end def lookup_herbarium_records_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - HerbariumRecord.where(id: name) - end + Lookup::HerbariumRecords.new(vals).ids end def lookup_locations_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - pattern = Location.clean_name(name.to_s).clean_pattern - Location.name_contains(pattern) - end + Lookup::Locations.new(vals).ids end def lookup_regions_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - # does not lowercase it, because we want a match to the end of string - pattern = name.to_s.clean_pattern - Location.in_region(pattern) - end + Lookup::Regions.new(vals).ids end def lookup_projects_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - Project.where(title: name) - end + Lookup::Projects.new(vals).ids end def lookup_lists_for_projects_by_name(vals) - return unless vals - - project_ids = lookup_projects_by_name(vals) - return [] if project_ids.empty? - - # Have to map(&:id) because it doesn't return lookup_object_ids_by_name - SpeciesList.joins(:project_species_lists). - where(project_species_lists: { project_id: project_ids }). - distinct.map(&:id) + Lookup::ProjectSpeciesLists.new(vals).ids end def lookup_species_lists_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - SpeciesList.where(title: name) - end + Lookup::SpeciesLists.new(vals).ids end def lookup_users_by_name(vals) - lookup_object_ids_by_name(vals) do |name| - User.where(login: User.remove_bracketed_name(name)) - end - end - - # Used by scopes to get IDs when they're passed strings or instances. - # In the last condition, `yield` == run any block provided to this method. - # (Only in the case it doesn't have an ID does it look up the record.) - def lookup_object_ids_by_name(vals) - return unless vals - - vals = [vals] unless vals.is_a?(Array) - vals.map do |val| - if val.is_a?(AbstractModel) - val.id - elsif /^\d+$/.match?(val.to_s) - val - else - yield(val).map(&:id) - end - end.flatten.uniq.compact + Lookup::Users.new(vals).ids end end end From 9b097928d33ac77314933cb3c34aa42a5524c462 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 16:30:46 -0800 Subject: [PATCH 08/39] Update names.rb --- app/classes/lookup/names.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index ee2dfe8978..4f895fcd84 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -110,7 +110,7 @@ def add_synonyms(min_names) return min_names if ids.empty? min_names.reject { |min_name| min_name[2] } + - Name.where(synonym_id: clean_id_set(ids).split(",")). + Name.where(synonym_id: limited_id_set(ids)). pluck(*minimal_name_columns) end @@ -198,4 +198,10 @@ def minimal_name_data(name) def minimal_name_columns "id, correct_spelling_id, synonym_id, text_name" end + + # array of max of MO.query_max_array unique ids for use with Arel "in" + # where(.in(limited_id_set(ids))) + def limited_id_set(ids) + ids.map(&:to_i).uniq[0, MO.query_max_array] + end end From fb021cd0e20178eff7e81a88e974959393a2208c Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 22:24:00 -0800 Subject: [PATCH 09/39] Make lookup_names_by_name param signature same as others --- app/classes/lookup/names.rb | 90 ++--- app/classes/query/initializers/names.rb | 7 +- app/classes/query/modules/lookup_names.rb | 362 ++++++++++---------- app/classes/query/modules/lookup_objects.rb | 4 + app/classes/query/name_descriptions.rb | 2 +- test/classes/lookup_test.rb | 43 ++- test/classes/query/lookup_names_test.rb | 132 ------- 7 files changed, 256 insertions(+), 384 deletions(-) delete mode 100644 test/classes/query/lookup_names_test.rb diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 4f895fcd84..116d1fd239 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -12,30 +12,30 @@ def lookup_instances end def lookup_ids - # unless (@vals = params[:names]) - # complain_about_unused_flags!(args) - # return - # end - - orig_names = given_names(@vals, @params) - min_names = add_synonyms_if_necessary(orig_names, @params) - min_names2 = add_subtaxa_if_necessary(min_names, @params) - min_names = add_synonyms_again(min_names, min_names2, @params) + if @vals.blank? + complain_about_unused_flags! + return [] + end + + orig_names = given_names + min_names = add_synonyms_if_necessary(orig_names) + min_names2 = add_subtaxa_if_necessary(min_names) + min_names = add_synonyms_again(min_names, min_names2) min_names -= orig_names if @params[:exclude_original_names] min_names.map { |min_name| min_name[0] } end - def given_names(vals, args) - min_names = find_exact_name_matches(vals) - if args[:exclude_original_names] + def given_names + min_names = find_exact_name_matches(@vals) + if @params[:exclude_original_names] add_other_spellings(min_names) else min_names end end - def add_synonyms_if_necessary(min_names, args) - if args[:include_synonyms] + def add_synonyms_if_necessary(min_names) + if @params[:include_synonyms] add_synonyms(min_names) elsif !args[:exclude_original_names] add_other_spellings(min_names) @@ -44,8 +44,8 @@ def add_synonyms_if_necessary(min_names, args) end end - def add_subtaxa_if_necessary(min_names, args) - if args[:include_subtaxa] + def add_subtaxa_if_necessary(min_names) + if @params[:include_subtaxa] add_subtaxa(min_names) elsif args[:include_immediate_subtaxa] add_immediate_subtaxa(min_names) @@ -54,32 +54,32 @@ def add_subtaxa_if_necessary(min_names, args) end end - def add_synonyms_again(min_names, min_names2, args) + def add_synonyms_again(min_names, min_names2) if min_names.length >= min_names2.length min_names - elsif args[:include_synonyms] + elsif @params[:include_synonyms] add_synonyms(min_names2) else add_other_spellings(min_names2) end end - def complain_about_unused_flags!(args) - complain_about_unused_flag!(args, :include_synonyms) - complain_about_unused_flag!(args, :include_subtaxa) - complain_about_unused_flag!(args, :include_nonconsensus) - complain_about_unused_flag!(args, :exclude_consensus) - complain_about_unused_flag!(args, :exclude_original_names) + def complain_about_unused_flags! + complain_about_unused_flag!(@params, :include_synonyms) + complain_about_unused_flag!(@params, :include_subtaxa) + complain_about_unused_flag!(@params, :include_nonconsensus) + complain_about_unused_flag!(@params, :exclude_consensus) + complain_about_unused_flag!(@params, :exclude_original_names) end - def complain_about_unused_flag!(args, arg) - return if args[arg].nil? + def complain_about_unused_flag!(param) + return if @params[arg].nil? - raise("Flag \"#{arg}\" is invalid without \"names\" parameter.") + raise("Flag \"#{param}\" is invalid without \"names\" parameter.") end - def find_exact_name_matches(vals) - vals.inject([]) do |result, val| + def find_exact_name_matches + @vals.inject([]) do |result, val| if /^\d+$/.match?(val.to_s) # from an id result << minimal_name_data(Name.safe_find(val)) else # from a string @@ -117,19 +117,19 @@ def add_synonyms(min_names) def add_subtaxa(min_names) higher_names = genera_and_up(min_names) lower_names = genera_and_down(min_names) - query = Name.where(id: min_names.map(&:first)) - query = add_lower_names(query, lower_names) - query = add_higher_names(query, higher_names) unless higher_names.empty? - query.distinct.pluck(*minimal_name_columns) + @name_query = Name.where(id: min_names.map(&:first)) + @name_query = add_lower_names(lower_names) + @name_query = add_higher_names(higher_names) unless higher_names.empty? + @name_query.distinct.pluck(*minimal_name_columns) end - def add_lower_names(query, names) - query.or(Name. + def add_lower_names(names) + @name_query.or(Name. where(Name[:text_name] =~ /^(#{names.join("|")}) /)) end - def add_higher_names(query, names) - query.or(Name. + def add_higher_names(names) + @name_query.or(Name. where(Name[:classification] =~ /: _(#{names.join("|")})_/)) end @@ -137,22 +137,22 @@ def add_immediate_subtaxa(min_names) higher_names = genera_and_up(min_names) lower_names = genera_and_down(min_names) - query = Name.where(id: min_names.map(&:first)) - query = add_immediate_lower_names(query, lower_names) + @name_query = Name.where(id: min_names.map(&:first)) + @name_query = add_immediate_lower_names(lower_names) unless higher_names.empty? - query = add_immediate_higher_names(query, higher_names) + @name_query = add_immediate_higher_names(higher_names) end - query.distinct.pluck(*minimal_name_columns) + @name_query.distinct.pluck(*minimal_name_columns) end - def add_immediate_lower_names(query, lower_names) - query.or(Name. + def add_immediate_lower_names(lower_names) + @name_query.or(Name. where(Name[:text_name] =~ /^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$/)) end - def add_immediate_higher_names(query, higher_names) - query.or(Name. + def add_immediate_higher_names(higher_names) + @name_query.or(Name. where(Name[:classification] =~ /: _(#{higher_names.join("|")})_$/). where.not(Name[:text_name].matches("% %"))) end diff --git a/app/classes/query/initializers/names.rb b/app/classes/query/initializers/names.rb index 0f5f0cf3a0..4ab4d2a670 100644 --- a/app/classes/query/initializers/names.rb +++ b/app/classes/query/initializers/names.rb @@ -125,7 +125,7 @@ def initialize_name_parameters(*joins) table = params[:include_all_name_proposals] ? "namings" : "observations" column = "#{table}.name_id" - ids = lookup_names_by_name(names_parameters) + ids = lookup_names_by_name(params[:names], names_parameters) add_id_condition(column, ids, *joins) add_join(:observations, :namings) if params[:include_all_name_proposals] @@ -141,7 +141,8 @@ def force_empty_results def initialize_name_parameters_for_name_queries # Much simpler form for non-observation-based name queries. - add_id_condition("names.id", lookup_names_by_name(names_parameters)) + add_id_condition("names.id", + lookup_names_by_name(params[:names], names_parameters)) end # Copy only the names_parameters into a name_params hash we use here. @@ -149,7 +150,7 @@ def names_parameters name_params = names_parameter_declarations.dup name_params.transform_keys! { |k| k.to_s.chomp("?").to_sym } name_params.each_key { |k| name_params[k] = params[k] } - name_params + name_params.except(:names) end # ------------------------------------------------------------------------ diff --git a/app/classes/query/modules/lookup_names.rb b/app/classes/query/modules/lookup_names.rb index 16fd17619b..fa10e8c03e 100644 --- a/app/classes/query/modules/lookup_names.rb +++ b/app/classes/query/modules/lookup_names.rb @@ -2,14 +2,14 @@ # Helper methods to help parsing Name instances from parameter strings. module Query::Modules::LookupNames - def lookup_names_by_name(args) - unless (vals = args[:names]) - complain_about_unused_flags!(args) - return - end + # def lookup_names_by_name(args, params = {}) + # unless (vals = args[:names]) + # complain_about_unused_flags!(args) + # return + # end - Lookup::Names.new(vals, **args).ids - end + # Lookup::Names.new(vals, **args).ids + # end # def lookup_names_by_name(args) # unless (vals = args[:names]) @@ -27,178 +27,178 @@ def lookup_names_by_name(args) # ------------------------------------------------------------------------ - private - - def given_names(vals, args) - min_names = find_exact_name_matches(vals) - if args[:exclude_original_names] - add_other_spellings(min_names) - else - min_names - end - end - - def add_synonyms_if_necessary(min_names, args) - if args[:include_synonyms] - add_synonyms(min_names) - elsif !args[:exclude_original_names] - add_other_spellings(min_names) - else - min_names - end - end - - def add_subtaxa_if_necessary(min_names, args) - if args[:include_subtaxa] - add_subtaxa(min_names) - elsif args[:include_immediate_subtaxa] - add_immediate_subtaxa(min_names) - else - min_names - end - end - - def add_synonyms_again(min_names, min_names2, args) - if min_names.length >= min_names2.length - min_names - elsif args[:include_synonyms] - add_synonyms(min_names2) - else - add_other_spellings(min_names2) - end - end - - def complain_about_unused_flags!(args) - complain_about_unused_flag!(args, :include_synonyms) - complain_about_unused_flag!(args, :include_subtaxa) - complain_about_unused_flag!(args, :include_all_name_proposals) - complain_about_unused_flag!(args, :exclude_consensus) - complain_about_unused_flag!(args, :exclude_original_names) - end - - def complain_about_unused_flag!(args, arg) - return if args[arg].nil? - - raise("Flag \"#{arg}\" is invalid without \"names\" parameter.") - end - - def find_exact_name_matches(vals) - vals.inject([]) do |result, val| - if /^\d+$/.match?(val.to_s) - result << minimal_name_data(Name.safe_find(val)) - else - result + find_matching_names(val) - end - end.uniq.compact - end - - def find_matching_names(name) - parse = Name.parse_name(name) - name2 = parse ? parse.search_name : Name.clean_incoming_string(name) - matches = Name.where(search_name: name2) if parse&.author.present? - matches = Name.where(text_name: name2) if matches.empty? - matches.map { |name3| minimal_name_data(name3) } - end - - def add_other_spellings(min_names) - ids = min_names.map { |min_name| min_name[1] || min_name[0] } - return [] if ids.empty? - - Name.where(Name[:correct_spelling_id].coalesce(Name[:id]). - in(limited_id_set(ids))).pluck(*minimal_name_columns) - end - - def add_synonyms(min_names) - ids = min_names.filter_map { |min_name| min_name[2] } - return min_names if ids.empty? - - min_names.reject { |min_name| min_name[2] } + - Name.where(synonym_id: clean_id_set(ids).split(",")). - pluck(*minimal_name_columns) - end - - def add_subtaxa(min_names) - higher_names = genera_and_up(min_names) - lower_names = genera_and_down(min_names) - query = Name.where(id: min_names.map(&:first)) - query = add_lower_names(query, lower_names) - query = add_higher_names(query, higher_names) unless higher_names.empty? - query.distinct.pluck(*minimal_name_columns) - end - - def add_lower_names(query, names) - query.or(Name. - where(Name[:text_name] =~ /^(#{names.join("|")}) /)) - end - - def add_higher_names(query, names) - query.or(Name. - where(Name[:classification] =~ /: _(#{names.join("|")})_/)) - end - - def add_immediate_subtaxa(min_names) - higher_names = genera_and_up(min_names) - lower_names = genera_and_down(min_names) - - query = Name.where(id: min_names.map(&:first)) - query = add_immediate_lower_names(query, lower_names) - unless higher_names.empty? - query = add_immediate_higher_names(query, higher_names) - end - query.distinct.pluck(*minimal_name_columns) - end - - def add_immediate_lower_names(query, lower_names) - query.or(Name. - where(Name[:text_name] =~ - /^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$/)) - end - - def add_immediate_higher_names(query, higher_names) - query.or(Name. - where(Name[:classification] =~ /: _(#{higher_names.join("|")})_$/). - where.not(Name[:text_name].matches("% %"))) - end - - def genera_and_up(min_names) - min_names.pluck(3). - reject { |min_name| min_name.include?(" ") } - end - - def genera_and_down(min_names) - genera = {} - text_names = min_names.pluck(3) - # Make hash of all genera present. - text_names.each do |text_name| - genera[text_name] = true unless text_name.include?(" ") - end - # Remove species if genus also present. - text_names.reject do |text_name| - text_name.include?(" ") && genera[text_name.split.first] - end.uniq - end - - # This ugliness with "minimal name data" is a way to avoid having Rails - # instantiate all the names (which can get quite huge if you start talking - # about all the children of Kingdom Fungi!) It allows us to use low-level - # mysql queries, and restricts the dataflow back and forth to the database - # to just the few columns we actually need. Unfortunately it is ugly, - # it totally violates the autonomy of Name, and it is probably hard to - # understand. But hopefully once we get it working it will remain stable. - # Blame it on me... -Jason, July 2019 - - def minimal_name_data(name) - return nil unless name - - [ - name.id, # 0 - name.correct_spelling_id, # 1 - name.synonym_id, # 2 - name.text_name # 3 - ] - end - - def minimal_name_columns - [:id, :correct_spelling_id, :synonym_id, :text_name].freeze - end + # private + + # def given_names(vals, args) + # min_names = find_exact_name_matches(vals) + # if args[:exclude_original_names] + # add_other_spellings(min_names) + # else + # min_names + # end + # end + + # def add_synonyms_if_necessary(min_names, args) + # if args[:include_synonyms] + # add_synonyms(min_names) + # elsif !args[:exclude_original_names] + # add_other_spellings(min_names) + # else + # min_names + # end + # end + + # def add_subtaxa_if_necessary(min_names, args) + # if args[:include_subtaxa] + # add_subtaxa(min_names) + # elsif args[:include_immediate_subtaxa] + # add_immediate_subtaxa(min_names) + # else + # min_names + # end + # end + + # def add_synonyms_again(min_names, min_names2, args) + # if min_names.length >= min_names2.length + # min_names + # elsif args[:include_synonyms] + # add_synonyms(min_names2) + # else + # add_other_spellings(min_names2) + # end + # end + + # def complain_about_unused_flags!(args) + # complain_about_unused_flag!(args, :include_synonyms) + # complain_about_unused_flag!(args, :include_subtaxa) + # complain_about_unused_flag!(args, :include_all_name_proposals) + # complain_about_unused_flag!(args, :exclude_consensus) + # complain_about_unused_flag!(args, :exclude_original_names) + # end + + # def complain_about_unused_flag!(args, arg) + # return if args[arg].nil? + + # raise("Flag \"#{arg}\" is invalid without \"names\" parameter.") + # end + + # def find_exact_name_matches(vals) + # vals.inject([]) do |result, val| + # if /^\d+$/.match?(val.to_s) + # result << minimal_name_data(Name.safe_find(val)) + # else + # result + find_matching_names(val) + # end + # end.uniq.compact + # end + + # def find_matching_names(name) + # parse = Name.parse_name(name) + # name2 = parse ? parse.search_name : Name.clean_incoming_string(name) + # matches = Name.where(search_name: name2) if parse&.author.present? + # matches = Name.where(text_name: name2) if matches.empty? + # matches.map { |name3| minimal_name_data(name3) } + # end + + # def add_other_spellings(min_names) + # ids = min_names.map { |min_name| min_name[1] || min_name[0] } + # return [] if ids.empty? + + # Name.where(Name[:correct_spelling_id].coalesce(Name[:id]). + # in(limited_id_set(ids))).pluck(*minimal_name_columns) + # end + + # def add_synonyms(min_names) + # ids = min_names.filter_map { |min_name| min_name[2] } + # return min_names if ids.empty? + + # min_names.reject { |min_name| min_name[2] } + + # Name.where(synonym_id: clean_id_set(ids).split(",")). + # pluck(*minimal_name_columns) + # end + + # def add_subtaxa(min_names) + # higher_names = genera_and_up(min_names) + # lower_names = genera_and_down(min_names) + # query = Name.where(id: min_names.map(&:first)) + # query = add_lower_names(query, lower_names) + # query = add_higher_names(query, higher_names) unless higher_names.empty? + # query.distinct.pluck(*minimal_name_columns) + # end + + # def add_lower_names(query, names) + # query.or(Name. + # where(Name[:text_name] =~ /^(#{names.join("|")}) /)) + # end + + # def add_higher_names(query, names) + # query.or(Name. + # where(Name[:classification] =~ /: _(#{names.join("|")})_/)) + # end + + # def add_immediate_subtaxa(min_names) + # higher_names = genera_and_up(min_names) + # lower_names = genera_and_down(min_names) + + # query = Name.where(id: min_names.map(&:first)) + # query = add_immediate_lower_names(query, lower_names) + # unless higher_names.empty? + # query = add_immediate_higher_names(query, higher_names) + # end + # query.distinct.pluck(*minimal_name_columns) + # end + + # def add_immediate_lower_names(query, lower_names) + # query.or(Name. + # where(Name[:text_name] =~ + # /^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$/)) + # end + + # def add_immediate_higher_names(query, higher_names) + # query.or(Name. + # where(Name[:classification] =~ /: _(#{higher_names.join("|")})_$/). + # where.not(Name[:text_name].matches("% %"))) + # end + + # def genera_and_up(min_names) + # min_names.pluck(3). + # reject { |min_name| min_name.include?(" ") } + # end + + # def genera_and_down(min_names) + # genera = {} + # text_names = min_names.pluck(3) + # # Make hash of all genera present. + # text_names.each do |text_name| + # genera[text_name] = true unless text_name.include?(" ") + # end + # # Remove species if genus also present. + # text_names.reject do |text_name| + # text_name.include?(" ") && genera[text_name.split.first] + # end.uniq + # end + + # # This ugliness with "minimal name data" is a way to avoid having Rails + # # instantiate all the names (which can get quite huge if you start talking + # # about all the children of Kingdom Fungi!) It allows us to use low-level + # # mysql queries, and restricts the dataflow back and forth to the database + # # to just the few columns we actually need. Unfortunately it is ugly, + # # it totally violates the autonomy of Name, and it is probably hard to + # # understand. But hopefully once we get it working it will remain stable. + # # Blame it on me... -Jason, July 2019 + + # def minimal_name_data(name) + # return nil unless name + + # [ + # name.id, # 0 + # name.correct_spelling_id, # 1 + # name.synonym_id, # 2 + # name.text_name # 3 + # ] + # end + + # def minimal_name_columns + # [:id, :correct_spelling_id, :synonym_id, :text_name].freeze + # end end diff --git a/app/classes/query/modules/lookup_objects.rb b/app/classes/query/modules/lookup_objects.rb index 923cffafe3..c75a3c360a 100644 --- a/app/classes/query/modules/lookup_objects.rb +++ b/app/classes/query/modules/lookup_objects.rb @@ -18,6 +18,10 @@ def lookup_locations_by_name(vals) Lookup::Locations.new(vals).ids end + def lookup_names_by_name(vals, params = {}) + Lookup::Names.new(vals, **params).ids + end + def lookup_projects_by_name(vals) Lookup::Projects.new(vals).ids end diff --git a/app/classes/query/name_descriptions.rb b/app/classes/query/name_descriptions.rb index 80d53b5fd1..66cb2db2f9 100644 --- a/app/classes/query/name_descriptions.rb +++ b/app/classes/query/name_descriptions.rb @@ -31,7 +31,7 @@ def initialize_flavor add_by_user_condition add_desc_by_author_condition(:name) add_desc_by_editor_condition(:name) - names = lookup_names_by_name(names: params[:names]) + names = lookup_names_by_name(params[:names]) add_id_condition("name_descriptions.name_id", names) initialize_description_public_parameter(:name) initialize_name_descriptions_parameters diff --git a/test/classes/lookup_test.rb b/test/classes/lookup_test.rb index a5df5eb5fb..647a1de8e1 100644 --- a/test/classes/lookup_test.rb +++ b/test/classes/lookup_test.rb @@ -30,30 +30,29 @@ def test_lookup_names_by_name name4.update(synonym_id: name1.synonym_id) name5.update(synonym_id: name2.synonym_id) - assert_lookup_names_by_name([name1], names: ["Macrolepiota"]) - assert_lookup_names_by_name([name2], names: ["Macrolepiota rachodes"]) - assert_lookup_names_by_name([name1, name4], - names: ["Macrolepiota"], + assert_lookup_names_by_name([name1], ["Macrolepiota"]) + assert_lookup_names_by_name([name2], ["Macrolepiota rachodes"]) + assert_lookup_names_by_name([name1, name4], ["Macrolepiota"], include_synonyms: true) assert_lookup_names_by_name([name2, name3, name5], - names: ["Macrolepiota rachodes"], + ["Macrolepiota rachodes"], include_synonyms: true) assert_lookup_names_by_name([name3, name5], - names: ["Macrolepiota rachodes"], + ["Macrolepiota rachodes"], include_synonyms: true, exclude_original_names: true) assert_lookup_names_by_name([name1, name2, name3], - names: ["Macrolepiota"], + ["Macrolepiota"], include_subtaxa: true) assert_lookup_names_by_name([name1, name2, name3], - names: ["Macrolepiota"], + ["Macrolepiota"], include_immediate_subtaxa: true) assert_lookup_names_by_name([name1, name2, name3, name4, name5], - names: ["Macrolepiota"], + ["Macrolepiota"], include_synonyms: true, include_subtaxa: true) assert_lookup_names_by_name([name2, name3, name4, name5], - names: ["Macrolepiota"], + ["Macrolepiota"], include_synonyms: true, include_subtaxa: true, exclude_original_names: true) @@ -61,7 +60,7 @@ def test_lookup_names_by_name name5.update(synonym_id: nil) name5 = Name.where(text_name: "Pseudolepiota rachodes").index_order.first assert_lookup_names_by_name([name1, name2, name3, name4, name5], - names: ["Macrolepiota"], + ["Macrolepiota"], include_synonyms: true, include_subtaxa: true) end @@ -82,26 +81,26 @@ def test_lookup_names_by_name2 name6.update(classification: name2.classification) name7.update(classification: name2.classification) - assert_lookup_names_by_name([name2, name3], names: ["Peltigera"]) - assert_lookup_names_by_name([name2, name3], names: ["Petigera"]) + assert_lookup_names_by_name([name2, name3], ["Peltigera"]) + assert_lookup_names_by_name([name2, name3], ["Petigera"]) assert_lookup_names_by_name([name1, name2, name3, name4, name5, name6, name7], - names: ["Peltigeraceae"], + ["Peltigeraceae"], include_subtaxa: true) assert_lookup_names_by_name([name1, name2, name3], - names: ["Peltigeraceae"], + ["Peltigeraceae"], include_immediate_subtaxa: true) assert_lookup_names_by_name([name2, name3, name4, name5, name6, name7], - names: ["Peltigera"], + ["Peltigera"], include_subtaxa: true) assert_lookup_names_by_name([name2, name3, name4, name6], - names: ["Peltigera"], + ["Peltigera"], include_immediate_subtaxa: true) assert_lookup_names_by_name([name6, name7], - names: ["Peltigera subg. Foo"], + ["Peltigera subg. Foo"], include_immediate_subtaxa: true) assert_lookup_names_by_name([name4, name5], - names: ["Peltigera canina"], + ["Peltigera canina"], include_immediate_subtaxa: true) end @@ -116,16 +115,16 @@ def test_lookup_names_by_name3 children = Name.index_order.where(Name[:text_name].matches("Lactarius %")) assert_lookup_names_by_name([name1] + children, - names: ["Lactarius"], + ["Lactarius"], include_subtaxa: true) assert_lookup_names_by_name(children, - names: ["Lactarius"], + ["Lactarius"], include_immediate_subtaxa: true, exclude_original_names: true) end def test_lookup_names_by_name4 - assert_lookup_names_by_name([], names: ["¡not a name!"]) + assert_lookup_names_by_name([], ["¡not a name!"]) end end diff --git a/test/classes/query/lookup_names_test.rb b/test/classes/query/lookup_names_test.rb deleted file mode 100644 index ad75b8de2b..0000000000 --- a/test/classes/query/lookup_names_test.rb +++ /dev/null @@ -1,132 +0,0 @@ -# frozen_string_literal: true - -require("test_helper") - -# tests of Query::Modules::LookupNames to be included in QueryTest -module Query::LookupNamesTest - def create_test_name(name) - name = Name.new_name(Name.parse_name(name).params) - name.save - name - end - - def assert_lookup_names_by_name(expects, args) - query = Query.new(:Name) - actual = query.lookup_names_by_name(args) - expects = expects.sort_by(&:text_name) - actual = actual.map { |id| Name.find(id) }.sort_by(&:text_name) - assert_name_arrays_equal(expects, actual) - end - - def test_lookup_names_by_name - User.current = rolf - - name1 = names(:macrolepiota) - name2 = names(:macrolepiota_rachodes) - name3 = names(:macrolepiota_rhacodes) - name4 = create_test_name("Pseudolepiota") - name5 = create_test_name("Pseudolepiota rachodes") - - name1.update(synonym_id: Synonym.create.id) - name4.update(synonym_id: name1.synonym_id) - name5.update(synonym_id: name2.synonym_id) - - assert_lookup_names_by_name([name1], names: ["Macrolepiota"]) - assert_lookup_names_by_name([name2], names: ["Macrolepiota rachodes"]) - assert_lookup_names_by_name([name1, name4], - names: ["Macrolepiota"], - include_synonyms: true) - assert_lookup_names_by_name([name2, name3, name5], - names: ["Macrolepiota rachodes"], - include_synonyms: true) - assert_lookup_names_by_name([name3, name5], - names: ["Macrolepiota rachodes"], - include_synonyms: true, - exclude_original_names: true) - assert_lookup_names_by_name([name1, name2, name3], - names: ["Macrolepiota"], - include_subtaxa: true) - assert_lookup_names_by_name([name1, name2, name3], - names: ["Macrolepiota"], - include_immediate_subtaxa: true) - assert_lookup_names_by_name([name1, name2, name3, name4, name5], - names: ["Macrolepiota"], - include_synonyms: true, - include_subtaxa: true) - assert_lookup_names_by_name([name2, name3, name4, name5], - names: ["Macrolepiota"], - include_synonyms: true, - include_subtaxa: true, - exclude_original_names: true) - - name5.update(synonym_id: nil) - name5 = Name.where(text_name: "Pseudolepiota rachodes").index_order.first - assert_lookup_names_by_name([name1, name2, name3, name4, name5], - names: ["Macrolepiota"], - include_synonyms: true, - include_subtaxa: true) - end - - def test_lookup_names_by_name2 - User.current = rolf - - name1 = names(:peltigeraceae) - name2 = names(:peltigera) - name3 = names(:petigera) - name4 = create_test_name("Peltigera canina") - name5 = create_test_name("Peltigera canina var. spuria") - name6 = create_test_name("Peltigera subg. Foo") - name7 = create_test_name("Peltigera subg. Foo sect. Bar") - - name4.update(classification: name2.classification) - name5.update(classification: name2.classification) - name6.update(classification: name2.classification) - name7.update(classification: name2.classification) - - assert_lookup_names_by_name([name2, name3], names: ["Peltigera"]) - assert_lookup_names_by_name([name2, name3], names: ["Petigera"]) - assert_lookup_names_by_name([name1, name2, name3, name4, name5, name6, - name7], - names: ["Peltigeraceae"], - include_subtaxa: true) - assert_lookup_names_by_name([name1, name2, name3], - names: ["Peltigeraceae"], - include_immediate_subtaxa: true) - assert_lookup_names_by_name([name2, name3, name4, name5, name6, name7], - names: ["Peltigera"], - include_subtaxa: true) - assert_lookup_names_by_name([name2, name3, name4, name6], - names: ["Peltigera"], - include_immediate_subtaxa: true) - assert_lookup_names_by_name([name6, name7], - names: ["Peltigera subg. Foo"], - include_immediate_subtaxa: true) - assert_lookup_names_by_name([name4, name5], - names: ["Peltigera canina"], - include_immediate_subtaxa: true) - end - - def test_lookup_names_by_name3 - User.current = rolf - - name1 = names(:lactarius) - name2 = create_test_name("Lactarius \"fakename\"") - name2.update(classification: name1.classification) - name2.save - - children = Name.index_order.where(Name[:text_name].matches("Lactarius %")) - - assert_lookup_names_by_name([name1] + children, - names: ["Lactarius"], - include_subtaxa: true) - - assert_lookup_names_by_name(children, - names: ["Lactarius"], - include_immediate_subtaxa: true, - exclude_original_names: true) - end - - def test_lookup_names_by_name4 - assert_lookup_names_by_name([], names: ["¡not a name!"]) - end -end From 57be97e21701ab998a0fb82cdc8deb9401a9d1ee Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 23:36:03 -0800 Subject: [PATCH 10/39] Update lookup.rb Simplify prepare_vals --- app/classes/lookup.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index 424452b58b..89b39017f1 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -11,11 +11,7 @@ def initialize(vals, params = {}) def prepare_vals(vals) return [] if vals.blank? - if vals.is_a?(Array) - vals - else - [vals] - end + [vals].flatten end def ids @@ -31,20 +27,20 @@ def titles end def lookup_ids - return [] unless @vals + return [] if @vals.blank? - @vals = [@vals] unless @vals.is_a?(Array) evaluate_values_as_ids end def lookup_instances - return [] unless @vals + return [] if @vals.blank? - @vals = [@vals] unless @vals.is_a?(Array) evaluate_values_as_instances end def lookup_titles + return [] if @vals.blank? + @instances.map(&:"#{@name_column}") end From e54eadd2e5d5011028a30daa6741c25dc618f939 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 23:36:29 -0800 Subject: [PATCH 11/39] Lookup::Names Try to tame code --- app/classes/lookup/names.rb | 203 +++++++++++++++++++----------------- 1 file changed, 108 insertions(+), 95 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 116d1fd239..5586686fa1 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -7,142 +7,141 @@ def initialize(vals, params = {}) @name_column = :search_name end + def prepare_vals(vals) + if @vals.blank? + complain_about_unused_flags! + return [] + end + + [vals].flatten + end + def lookup_instances + return [] if @vals.blank? + @ids.map { |id| Name.find(id) } end def lookup_ids - if @vals.blank? - complain_about_unused_flags! - return [] - end + return [] if @vals.blank? orig_names = given_names - min_names = add_synonyms_if_necessary(orig_names) - min_names2 = add_subtaxa_if_necessary(min_names) - min_names = add_synonyms_again(min_names, min_names2) - min_names -= orig_names if @params[:exclude_original_names] - min_names.map { |min_name| min_name[0] } + names = add_synonyms_if_necessary(orig_names) + names_plus_subtaxa = add_subtaxa_if_necessary(names) + names = add_synonyms_again(names, names_plus_subtaxa) + names -= orig_names if @params[:exclude_original_names] + names.map(&:id) end def given_names - min_names = find_exact_name_matches(@vals) + given_names = find_exact_name_matches(@vals) if @params[:exclude_original_names] - add_other_spellings(min_names) + add_other_spellings(given_names) else - min_names + given_names end end - def add_synonyms_if_necessary(min_names) + def find_exact_name_matches + @vals.inject([]) do |result, val| + if /^\d+$/.match?(val.to_s) # from an id + result << Name.where(id: val).select(*minimal_name_columns) + else # from a string + result += find_matching_names(val) + end + result + end.uniq.compact + end + + # NOTE: Name.parse_name returns a ParsedName instance which is a hash of + # various parts/formats of the name, NOT an Name instance + def find_matching_names(name) + parse = Name.parse_name(name) + srch_str = parse ? parse.search_name : Name.clean_incoming_string(name) + if parse.author.present? + matches = Name.where(search_name: srch_str).select(*minimal_name_columns) + end + return matches unless matches.empty? + + Name.where(text_name: srch_str).select(*minimal_name_columns) + end + + def add_synonyms_if_necessary(names) if @params[:include_synonyms] - add_synonyms(min_names) + add_synonyms(names) elsif !args[:exclude_original_names] - add_other_spellings(min_names) + add_other_spellings(names) else - min_names + names end end - def add_subtaxa_if_necessary(min_names) + def add_subtaxa_if_necessary(names) if @params[:include_subtaxa] - add_subtaxa(min_names) + add_subtaxa(names) elsif args[:include_immediate_subtaxa] - add_immediate_subtaxa(min_names) + add_immediate_subtaxa(names) else - min_names + names end end - def add_synonyms_again(min_names, min_names2) - if min_names.length >= min_names2.length - min_names + def add_synonyms_again(names, names_plus_subtaxa) + if names.length >= names_plus_subtaxa.length + names elsif @params[:include_synonyms] - add_synonyms(min_names2) + add_synonyms(names_plus_subtaxa) else - add_other_spellings(min_names2) + add_other_spellings(names_plus_subtaxa) end end - def complain_about_unused_flags! - complain_about_unused_flag!(@params, :include_synonyms) - complain_about_unused_flag!(@params, :include_subtaxa) - complain_about_unused_flag!(@params, :include_nonconsensus) - complain_about_unused_flag!(@params, :exclude_consensus) - complain_about_unused_flag!(@params, :exclude_original_names) - end - - def complain_about_unused_flag!(param) - return if @params[arg].nil? - - raise("Flag \"#{param}\" is invalid without \"names\" parameter.") - end - - def find_exact_name_matches - @vals.inject([]) do |result, val| - if /^\d+$/.match?(val.to_s) # from an id - result << minimal_name_data(Name.safe_find(val)) - else # from a string - result += find_matching_names(val) - end - result - end.uniq.compact - end - - def find_matching_names(name) - parse = Name.parse_name(name) - name2 = parse ? parse.search_name : Name.clean_incoming_string(name) - matches = Name.where(search_name: name2) if parse.author.present? - matches = Name.where(text_name: name2) if matches.empty? - matches.map { |name3| minimal_name_data(name3) } - end - - def add_other_spellings(min_names) - ids = min_names.map { |min_name| min_name[1] || min_name[0] } + def add_other_spellings(names) + ids = names.map { |name| name[:correct_spelling_id] || name[:id] } return [] if ids.empty? Name.where(Name[:correct_spelling_id].coalesce(Name[:id]). - in(limited_id_set(ids))).pluck(*minimal_name_columns) + in(limited_id_set(ids))).select(*minimal_name_columns) end - def add_synonyms(min_names) - ids = min_names.filter_map { |min_name| min_name[2] } - return min_names if ids.empty? + def add_synonyms(names) + ids = names.pluck(:synonym_id).compact + return names if ids.empty? - min_names.reject { |min_name| min_name[2] } + + names.reject { |name| name[:synonym_id] } + Name.where(synonym_id: limited_id_set(ids)). - pluck(*minimal_name_columns) + select(*minimal_name_columns) end - def add_subtaxa(min_names) - higher_names = genera_and_up(min_names) - lower_names = genera_and_down(min_names) - @name_query = Name.where(id: min_names.map(&:first)) + def add_subtaxa(names) + higher_names = genera_and_up(names) + lower_names = genera_and_down(names) + @name_query = Name.where(id: names.map(&:id)) @name_query = add_lower_names(lower_names) @name_query = add_higher_names(higher_names) unless higher_names.empty? - @name_query.distinct.pluck(*minimal_name_columns) + @name_query.distinct.select(*minimal_name_columns) end def add_lower_names(names) - @name_query.or(Name. - where(Name[:text_name] =~ /^(#{names.join("|")}) /)) + @name_query.or(Name.where(Name[:text_name] =~ /^(#{names.join("|")}) /)) end def add_higher_names(names) - @name_query.or(Name. - where(Name[:classification] =~ /: _(#{names.join("|")})_/)) + @name_query.or( + Name.where(Name[:classification] =~ /: _(#{names.join("|")})_/) + ) end - def add_immediate_subtaxa(min_names) - higher_names = genera_and_up(min_names) - lower_names = genera_and_down(min_names) + def add_immediate_subtaxa(names) + higher_names = genera_and_up(names) + lower_names = genera_and_down(names) - @name_query = Name.where(id: min_names.map(&:first)) + @name_query = Name.where(id: names.map(&:id)) @name_query = add_immediate_lower_names(lower_names) unless higher_names.empty? @name_query = add_immediate_higher_names(higher_names) end - @name_query.distinct.pluck(*minimal_name_columns) + @name_query.distinct.select(*minimal_name_columns) end def add_immediate_lower_names(lower_names) @@ -157,14 +156,14 @@ def add_immediate_higher_names(higher_names) where.not(Name[:text_name].matches("% %"))) end - def genera_and_up(min_names) - min_names.pluck(3). - reject { |min_name| min_name.include?(" ") } + def genera_and_up(names) + names.pluck(:text_name). + reject { |name| name.include?(" ") } end - def genera_and_down(min_names) + def genera_and_down(names) genera = {} - text_names = min_names.pluck(3) + text_names = names.pluck(:text_name) # Make hash of all genera present. text_names.each do |text_name| genera[text_name] = true unless text_name.include?(" ") @@ -184,19 +183,19 @@ def genera_and_down(min_names) # understand. But hopefully once we get it working it will remain stable. # Blame it on me... -Jason, July 2019 - def minimal_name_data(name) - return nil unless name + # def minimal_name_data(name) + # return nil unless name - [ - name.id, # 0 - name.correct_spelling_id, # 1 - name.synonym_id, # 2 - name.text_name # 3 - ] - end + # [ + # name.id, # 0 + # name.correct_spelling_id, # 1 + # name.synonym_id, # 2 + # name.text_name # 3 + # ] + # end def minimal_name_columns - "id, correct_spelling_id, synonym_id, text_name" + [:id, :correct_spelling_id, :synonym_id, :text_name] end # array of max of MO.query_max_array unique ids for use with Arel "in" @@ -204,4 +203,18 @@ def minimal_name_columns def limited_id_set(ids) ids.map(&:to_i).uniq[0, MO.query_max_array] end + + def complain_about_unused_flags! + complain_about_unused_flag!(:include_synonyms) + complain_about_unused_flag!(:include_subtaxa) + complain_about_unused_flag!(:include_nonconsensus) + complain_about_unused_flag!(:exclude_consensus) + complain_about_unused_flag!(:exclude_original_names) + end + + def complain_about_unused_flag!(param) + return if @params.blank? || @params[param].nil? + + raise("Flag \"#{param}\" is invalid without \"names\" parameter.") + end end From 1adbf348f4a82d3d23116ccc7d3a649da98dbc63 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 23:36:57 -0800 Subject: [PATCH 12/39] Fix scope callbacks --- app/models/abstract_model/scopes.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/abstract_model/scopes.rb b/app/models/abstract_model/scopes.rb index fa5b1eda85..8387fb9275 100644 --- a/app/models/abstract_model/scopes.rb +++ b/app/models/abstract_model/scopes.rb @@ -289,8 +289,8 @@ def lookup_locations_by_name(vals) Lookup::Locations.new(vals).ids end - def lookup_regions_by_name(vals) - Lookup::Regions.new(vals).ids + def lookup_names_by_name(vals, params = {}) + Lookup::Names.new(vals, **params).ids end def lookup_projects_by_name(vals) @@ -305,6 +305,10 @@ def lookup_species_lists_by_name(vals) Lookup::SpeciesLists.new(vals).ids end + def lookup_regions_by_name(vals) + Lookup::Regions.new(vals).ids + end + def lookup_users_by_name(vals) Lookup::Users.new(vals).ids end From 6c7b4e458e584e282716f2a9efd8c0930ed7b442 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 23:37:28 -0800 Subject: [PATCH 13/39] Update lookup_test.rb Fix inheritance, assertions --- test/classes/lookup_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/classes/lookup_test.rb b/test/classes/lookup_test.rb index 647a1de8e1..7e266e0ff2 100644 --- a/test/classes/lookup_test.rb +++ b/test/classes/lookup_test.rb @@ -2,16 +2,16 @@ require("test_helper") -# tests of Lookup::Names -class Lookup::NamesTest +class LookupTest < UnitTestCase + # tests of Lookup::Names def create_test_name(name) name = Name.new_name(Name.parse_name(name).params) name.save name end - def assert_lookup_names_by_name(expects, args) - actual = Lookup::Names.new(args).instances.sort_by(&:text_name) + def assert_lookup_names_by_name(expects, vals, params = {}) + actual = Lookup::Names.new(vals, **params).instances.sort_by(&:text_name) expects = expects.sort_by(&:text_name) # actual = actual.map { |id| Name.find(id) }.sort_by(&:text_name) assert_name_arrays_equal(expects, actual) From 97e95cee459d9ee8fb9686532f62d92ea53fd746 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 23:37:58 -0800 Subject: [PATCH 14/39] Update query_test.rb just remove former Lookup Names test --- test/classes/query_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/classes/query_test.rb b/test/classes/query_test.rb index e81f9e205d..1ba575e5a1 100644 --- a/test/classes/query_test.rb +++ b/test/classes/query_test.rb @@ -1160,7 +1160,6 @@ def test_coercable include Query::SpeciesListsTest include Query::UsersTest include Query::FiltersTest - include Query::LookupNamesTest ############################################################################## # From 46b2774c7bc24fb55651ccc4496fde327e323869 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sun, 26 Jan 2025 23:40:02 -0800 Subject: [PATCH 15/39] Update lookup.rb --- app/classes/lookup.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index 89b39017f1..9d52395868 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -23,7 +23,7 @@ def instances end def titles - @titles ||= lookup_titles_from_instances + @titles ||= lookup_titles end def lookup_ids From cf53343b6741af4da5bbf20c55993992b9586dfd Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 00:27:39 -0800 Subject: [PATCH 16/39] Further simplify --- app/classes/lookup/names.rb | 49 ++++++++++++++++++++----------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 5586686fa1..4002cc9421 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -2,13 +2,13 @@ class Lookup::Names < Lookup def initialize(vals, params = {}) - super @model = Name @name_column = :search_name + super end def prepare_vals(vals) - if @vals.blank? + if vals.blank? complain_about_unused_flags! return [] end @@ -16,41 +16,44 @@ def prepare_vals(vals) [vals].flatten end - def lookup_instances - return [] if @vals.blank? - - @ids.map { |id| Name.find(id) } - end - def lookup_ids return [] if @vals.blank? orig_names = given_names - names = add_synonyms_if_necessary(orig_names) + names = add_synonyms_if_necessary(orig_names) names_plus_subtaxa = add_subtaxa_if_necessary(names) - names = add_synonyms_again(names, names_plus_subtaxa) + names = add_synonyms_again(names, names_plus_subtaxa) names -= orig_names if @params[:exclude_original_names] names.map(&:id) end + # If we got params, look up all instances from the ids. + def lookup_instances + return [] if @vals.blank? + + ids.map { |id| Name.find(id) } + end + def given_names - given_names = find_exact_name_matches(@vals) if @params[:exclude_original_names] - add_other_spellings(given_names) + add_other_spellings(matches) else - given_names + matches end end - def find_exact_name_matches - @vals.inject([]) do |result, val| - if /^\d+$/.match?(val.to_s) # from an id - result << Name.where(id: val).select(*minimal_name_columns) + def matches + @matches ||= @vals.map do |val| + if val.is_a?(@model) + val.id + elsif val.is_a?(AbstractModel) + raise("Passed a #{val.class} to LookupIDs for #{@model}.") + elsif /^\d+$/.match?(val.to_s) # from an id + Name.where(id: val).select(*minimal_name_columns) else # from a string - result += find_matching_names(val) + find_matching_names(val) end - result - end.uniq.compact + end.flatten.uniq.compact end # NOTE: Name.parse_name returns a ParsedName instance which is a hash of @@ -58,7 +61,7 @@ def find_exact_name_matches def find_matching_names(name) parse = Name.parse_name(name) srch_str = parse ? parse.search_name : Name.clean_incoming_string(name) - if parse.author.present? + if parse&.author.present? matches = Name.where(search_name: srch_str).select(*minimal_name_columns) end return matches unless matches.empty? @@ -69,7 +72,7 @@ def find_matching_names(name) def add_synonyms_if_necessary(names) if @params[:include_synonyms] add_synonyms(names) - elsif !args[:exclude_original_names] + elsif !@params[:exclude_original_names] add_other_spellings(names) else names @@ -79,7 +82,7 @@ def add_synonyms_if_necessary(names) def add_subtaxa_if_necessary(names) if @params[:include_subtaxa] add_subtaxa(names) - elsif args[:include_immediate_subtaxa] + elsif @params[:include_immediate_subtaxa] add_immediate_subtaxa(names) else names From 98d897f7dc060f8f66649da47c14ce5b48f89932 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 00:27:49 -0800 Subject: [PATCH 17/39] Fix lookup_titles --- app/classes/lookup.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index 9d52395868..318f0d489c 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -32,6 +32,7 @@ def lookup_ids evaluate_values_as_ids end + # Could just look them up from the ids, but vals may already have instances def lookup_instances return [] if @vals.blank? @@ -41,7 +42,7 @@ def lookup_instances def lookup_titles return [] if @vals.blank? - @instances.map(&:"#{@name_column}") + instances.map(&:"#{@name_column}") end # This is checking for an instance, then sanity-checking for an instance of From fb24b387aa158459f638ee9fc5e93fce47896ecf Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 00:37:10 -0800 Subject: [PATCH 18/39] Update names.rb --- app/classes/lookup/names.rb | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 4002cc9421..dac5e54ab2 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -19,11 +19,10 @@ def prepare_vals(vals) def lookup_ids return [] if @vals.blank? - orig_names = given_names - names = add_synonyms_if_necessary(orig_names) + names = add_synonyms_if_necessary(original_names) names_plus_subtaxa = add_subtaxa_if_necessary(names) names = add_synonyms_again(names, names_plus_subtaxa) - names -= orig_names if @params[:exclude_original_names] + names -= original_names if @params[:exclude_original_names] names.map(&:id) end @@ -34,16 +33,16 @@ def lookup_instances ids.map { |id| Name.find(id) } end - def given_names - if @params[:exclude_original_names] - add_other_spellings(matches) - else - matches - end + def original_names + @original_names ||= if @params[:exclude_original_names] + add_other_spellings(original_matches) + else + original_matches + end end - def matches - @matches ||= @vals.map do |val| + def original_matches + @original_matches ||= @vals.map do |val| if val.is_a?(@model) val.id elsif val.is_a?(AbstractModel) @@ -60,7 +59,11 @@ def matches # various parts/formats of the name, NOT an Name instance def find_matching_names(name) parse = Name.parse_name(name) - srch_str = parse ? parse.search_name : Name.clean_incoming_string(name) + srch_str = if parse + parse.search_name + else + Name.clean_incoming_string(name) + end if parse&.author.present? matches = Name.where(search_name: srch_str).select(*minimal_name_columns) end From 33090e9f0411cd622c43ac075c5f200df7a48314 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 00:41:44 -0800 Subject: [PATCH 19/39] Delete jasons_lookup_names.rb --- .../query/modules/jasons_lookup_names.rb | 191 ------------------ 1 file changed, 191 deletions(-) delete mode 100644 app/classes/query/modules/jasons_lookup_names.rb diff --git a/app/classes/query/modules/jasons_lookup_names.rb b/app/classes/query/modules/jasons_lookup_names.rb deleted file mode 100644 index b7fdeacffc..0000000000 --- a/app/classes/query/modules/jasons_lookup_names.rb +++ /dev/null @@ -1,191 +0,0 @@ -# frozen_string_literal: true - -# Helper methods to help parsing name instances from parameter strings. -module Query::Modules::JasonsLookupNames - def lookup_names_by_name(args) - unless (vals = args[:names]) - complain_about_unused_flags!(args) - return - end - - orig_names = given_names(vals, args) - min_names = add_synonyms_if_necessary(orig_names, args) - min_names2 = add_subtaxa_if_necessary(min_names, args) - min_names = add_synonyms_again(min_names, min_names2, args) - min_names -= orig_names if args[:exclude_original_names] - min_names.map { |min_name| min_name[0] } - end - - # ---------------------------------------------------------------------------- - - private - - def given_names(vals, args) - min_names = find_exact_name_matches(vals) - if args[:exclude_original_names] - add_other_spellings(min_names) - else - min_names - end - end - - def add_synonyms_if_necessary(min_names, args) - if args[:include_synonyms] - add_synonyms(min_names) - elsif !args[:exclude_original_names] - add_other_spellings(min_names) - else - min_names - end - end - - def add_subtaxa_if_necessary(min_names, args) - if args[:include_subtaxa] - add_subtaxa(min_names) - elsif args[:include_immediate_subtaxa] - add_immediate_subtaxa(min_names) - else - min_names - end - end - - def add_synonyms_again(min_names, min_names2, args) - if min_names.length >= min_names2.length - min_names - elsif args[:include_synonyms] - add_synonyms(min_names2) - else - add_other_spellings(min_names2) - end - end - - def complain_about_unused_flags!(args) - complain_about_unused_flag!(args, :include_synonyms) - complain_about_unused_flag!(args, :include_subtaxa) - complain_about_unused_flag!(args, :include_nonconsensus) - complain_about_unused_flag!(args, :exclude_consensus) - complain_about_unused_flag!(args, :exclude_original_names) - end - - def complain_about_unused_flag!(args, arg) - return if args[arg].nil? - - raise("Flag \"#{arg}\" is invalid without \"names\" parameter.") - end - - def find_exact_name_matches(vals) - vals.inject([]) do |result, val| - if /^\d+$/.match?(val.to_s) # from an id - result << minimal_name_data(Name.safe_find(val)) - else # from a string - result += find_matching_names(val) - end - result - end.uniq.compact - end - - def find_matching_names(name) - parse = Name.parse_name(name) - name2 = parse ? parse.search_name : Name.clean_incoming_string(name) - matches = Name.where(search_name: name2) if parse.author.present? - matches = Name.where(text_name: name2) if matches.empty? - matches.map { |name3| minimal_name_data(name3) } - end - - def add_other_spellings(min_names) - ids = min_names.map { |min_name| min_name[1] || min_name[0] } - return [] if ids.empty? - - Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE COALESCE(correct_spelling_id, id) IN (#{clean_id_set(ids)}) - )) - end - - def add_synonyms(min_names) - ids = min_names.map { |min_name| min_name[2] }.reject(&:nil?) - return min_names if ids.empty? - - min_names.reject { |min_name| min_name[2] } + - Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE synonym_id IN (#{clean_id_set(ids)}) - )) - end - - def add_subtaxa(min_names) - higher_names = genera_and_up(min_names) - lower_names = genera_and_down(min_names) - unless higher_names.empty? - min_names += Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE classification REGEXP ": _(#{higher_names.join("|")})_" - )) - end - min_names += Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE text_name REGEXP "^(#{lower_names.join("|")}) " - )) - min_names.uniq - end - - def add_immediate_subtaxa(min_names) - higher_names = genera_and_up(min_names) - lower_names = genera_and_down(min_names) - unless higher_names.empty? - min_names += Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE classification REGEXP ": _(#{higher_names.join("|")})_$" - AND text_name NOT LIKE "% %" - )) - end - min_names += Name.connection.select_rows(%( - SELECT #{minimal_name_columns} FROM names - WHERE text_name REGEXP - "^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$" - )) - min_names.uniq - end - - def genera_and_up(min_names) - min_names.map { |min_name| min_name[3] }. - reject { |min_name| min_name.include?(" ") } - end - - def genera_and_down(min_names) - genera = {} - text_names = min_names.map { |min_name| min_name[3] } - # Make hash of all genera present. - text_names.each do |text_name| - genera[text_name] = true unless text_name.include?(" ") - end - # Remove species if genus also present. - text_names.reject do |text_name| - text_name.include?(" ") && genera[text_name.split(" ").first] - end.uniq - end - - # This ugliness with "minimal name data" is a way to avoid having Rails - # instantiate all the names (which can get quite huge if you start talking - # about all the children of Kingdom Fungi!) It allows us to use low-level - # mysql queries, and restricts the dataflow back and forth to the database - # to just the few columns we actually need. Unfortunately it is ugly, - # it totally violates the autonomy of Name, and it is probably hard to - # understand. But hopefully once we get it working it will remain stable. - # Blame it on me... -Jason, July 2019 - - def minimal_name_data(name) - return nil unless name - - [ - name.id, # 0 - name.correct_spelling_id, # 1 - name.synonym_id, # 2 - name.text_name # 3 - ] - end - - def minimal_name_columns - "id, correct_spelling_id, synonym_id, text_name" - end -end From 251b3f9100cd62fda52f04bc87afc603d33e6511 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 00:44:00 -0800 Subject: [PATCH 20/39] Update names.rb --- app/classes/lookup/names.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index dac5e54ab2..3cacf0cdea 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -55,8 +55,8 @@ def original_matches end.flatten.uniq.compact end - # NOTE: Name.parse_name returns a ParsedName instance which is a hash of - # various parts/formats of the name, NOT an Name instance + # NOTE: Name.parse_name returns a ParsedName instance, not an Name instance. + # A ParsedName is a hash of segments and formatted strings of the name. def find_matching_names(name) parse = Name.parse_name(name) srch_str = if parse From 0103a86c9a3167fc9d48ffa58bc5be7bfd432cdc Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 00:47:25 -0800 Subject: [PATCH 21/39] Update lookup.rb --- app/classes/lookup.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index 318f0d489c..3361287d40 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -63,7 +63,7 @@ def evaluate_values_as_ids end.flatten.uniq.compact end - def evaluate_values_as_instances(&) + def evaluate_values_as_instances @vals.map do |val| if val.is_a?(@model) val From 8b7deaa6aa1ed554b653da31b5912bad75a7c1a9 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 01:07:17 -0800 Subject: [PATCH 22/39] Cleanup --- app/classes/lookup.rb | 10 +- app/classes/lookup/names.rb | 2 +- app/classes/query/base.rb | 1 - app/classes/query/modules/lookup_names.rb | 204 ---------------------- 4 files changed, 4 insertions(+), 213 deletions(-) delete mode 100644 app/classes/query/modules/lookup_names.rb diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index 3361287d40..295eb31f87 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -45,10 +45,6 @@ def lookup_titles instances.map(&:"#{@name_column}") end - # This is checking for an instance, then sanity-checking for an instance of - # the wrong model, then for an ID, then yielding to the lookup lambda. - # In the last condition, `yield` means run any lambda block provided to this - # method. (It only looks up the record in the case it can't find an ID.) def evaluate_values_as_ids @vals.map do |val| if val.is_a?(@model) @@ -57,8 +53,8 @@ def evaluate_values_as_ids raise("Passed a #{val.class} to LookupIDs for #{@model}.") elsif /^\d+$/.match?(val.to_s) val - else # each lookup returns an array - lookup_method(val).map(&:id) + else + lookup_method(val).map(&:id) # each lookup returns an array end end.flatten.uniq.compact end @@ -71,7 +67,7 @@ def evaluate_values_as_instances raise("Passed a #{val.class} to LookupIDs for #{@model}.") elsif /^\d+$/.match?(val.to_s) @model.find(val.to_i) - else # each lookup returns an array + else lookup_method(val) end end.flatten.uniq.compact diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 3cacf0cdea..59b4b53d7e 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -26,7 +26,7 @@ def lookup_ids names.map(&:id) end - # If we got params, look up all instances from the ids. + # Look up all instances from the ids. Too complicated otherwise. def lookup_instances return [] if @vals.blank? diff --git a/app/classes/query/base.rb b/app/classes/query/base.rb index 122a15e744..4aafb6cc1b 100644 --- a/app/classes/query/base.rb +++ b/app/classes/query/base.rb @@ -13,7 +13,6 @@ class Query::Base include Query::Modules::Initialization include Query::Modules::Joining include Query::Modules::LookupObjects - include Query::Modules::LookupNames include Query::Modules::LowLevelQueries include Query::Modules::NestedQueries include Query::Modules::Ordering diff --git a/app/classes/query/modules/lookup_names.rb b/app/classes/query/modules/lookup_names.rb deleted file mode 100644 index fa10e8c03e..0000000000 --- a/app/classes/query/modules/lookup_names.rb +++ /dev/null @@ -1,204 +0,0 @@ -# frozen_string_literal: true - -# Helper methods to help parsing Name instances from parameter strings. -module Query::Modules::LookupNames - # def lookup_names_by_name(args, params = {}) - # unless (vals = args[:names]) - # complain_about_unused_flags!(args) - # return - # end - - # Lookup::Names.new(vals, **args).ids - # end - - # def lookup_names_by_name(args) - # unless (vals = args[:names]) - # complain_about_unused_flags!(args) - # return - # end - - # orig_names = given_names(vals, args) - # min_names = add_synonyms_if_necessary(orig_names, args) - # min_names2 = add_subtaxa_if_necessary(min_names, args) - # min_names = add_synonyms_again(min_names, min_names2, args) - # min_names -= orig_names if args[:exclude_original_names] - # min_names.pluck(0) - # end - - # ------------------------------------------------------------------------ - - # private - - # def given_names(vals, args) - # min_names = find_exact_name_matches(vals) - # if args[:exclude_original_names] - # add_other_spellings(min_names) - # else - # min_names - # end - # end - - # def add_synonyms_if_necessary(min_names, args) - # if args[:include_synonyms] - # add_synonyms(min_names) - # elsif !args[:exclude_original_names] - # add_other_spellings(min_names) - # else - # min_names - # end - # end - - # def add_subtaxa_if_necessary(min_names, args) - # if args[:include_subtaxa] - # add_subtaxa(min_names) - # elsif args[:include_immediate_subtaxa] - # add_immediate_subtaxa(min_names) - # else - # min_names - # end - # end - - # def add_synonyms_again(min_names, min_names2, args) - # if min_names.length >= min_names2.length - # min_names - # elsif args[:include_synonyms] - # add_synonyms(min_names2) - # else - # add_other_spellings(min_names2) - # end - # end - - # def complain_about_unused_flags!(args) - # complain_about_unused_flag!(args, :include_synonyms) - # complain_about_unused_flag!(args, :include_subtaxa) - # complain_about_unused_flag!(args, :include_all_name_proposals) - # complain_about_unused_flag!(args, :exclude_consensus) - # complain_about_unused_flag!(args, :exclude_original_names) - # end - - # def complain_about_unused_flag!(args, arg) - # return if args[arg].nil? - - # raise("Flag \"#{arg}\" is invalid without \"names\" parameter.") - # end - - # def find_exact_name_matches(vals) - # vals.inject([]) do |result, val| - # if /^\d+$/.match?(val.to_s) - # result << minimal_name_data(Name.safe_find(val)) - # else - # result + find_matching_names(val) - # end - # end.uniq.compact - # end - - # def find_matching_names(name) - # parse = Name.parse_name(name) - # name2 = parse ? parse.search_name : Name.clean_incoming_string(name) - # matches = Name.where(search_name: name2) if parse&.author.present? - # matches = Name.where(text_name: name2) if matches.empty? - # matches.map { |name3| minimal_name_data(name3) } - # end - - # def add_other_spellings(min_names) - # ids = min_names.map { |min_name| min_name[1] || min_name[0] } - # return [] if ids.empty? - - # Name.where(Name[:correct_spelling_id].coalesce(Name[:id]). - # in(limited_id_set(ids))).pluck(*minimal_name_columns) - # end - - # def add_synonyms(min_names) - # ids = min_names.filter_map { |min_name| min_name[2] } - # return min_names if ids.empty? - - # min_names.reject { |min_name| min_name[2] } + - # Name.where(synonym_id: clean_id_set(ids).split(",")). - # pluck(*minimal_name_columns) - # end - - # def add_subtaxa(min_names) - # higher_names = genera_and_up(min_names) - # lower_names = genera_and_down(min_names) - # query = Name.where(id: min_names.map(&:first)) - # query = add_lower_names(query, lower_names) - # query = add_higher_names(query, higher_names) unless higher_names.empty? - # query.distinct.pluck(*minimal_name_columns) - # end - - # def add_lower_names(query, names) - # query.or(Name. - # where(Name[:text_name] =~ /^(#{names.join("|")}) /)) - # end - - # def add_higher_names(query, names) - # query.or(Name. - # where(Name[:classification] =~ /: _(#{names.join("|")})_/)) - # end - - # def add_immediate_subtaxa(min_names) - # higher_names = genera_and_up(min_names) - # lower_names = genera_and_down(min_names) - - # query = Name.where(id: min_names.map(&:first)) - # query = add_immediate_lower_names(query, lower_names) - # unless higher_names.empty? - # query = add_immediate_higher_names(query, higher_names) - # end - # query.distinct.pluck(*minimal_name_columns) - # end - - # def add_immediate_lower_names(query, lower_names) - # query.or(Name. - # where(Name[:text_name] =~ - # /^(#{lower_names.join("|")}) [^[:blank:]]+( [^[:blank:]]+)?$/)) - # end - - # def add_immediate_higher_names(query, higher_names) - # query.or(Name. - # where(Name[:classification] =~ /: _(#{higher_names.join("|")})_$/). - # where.not(Name[:text_name].matches("% %"))) - # end - - # def genera_and_up(min_names) - # min_names.pluck(3). - # reject { |min_name| min_name.include?(" ") } - # end - - # def genera_and_down(min_names) - # genera = {} - # text_names = min_names.pluck(3) - # # Make hash of all genera present. - # text_names.each do |text_name| - # genera[text_name] = true unless text_name.include?(" ") - # end - # # Remove species if genus also present. - # text_names.reject do |text_name| - # text_name.include?(" ") && genera[text_name.split.first] - # end.uniq - # end - - # # This ugliness with "minimal name data" is a way to avoid having Rails - # # instantiate all the names (which can get quite huge if you start talking - # # about all the children of Kingdom Fungi!) It allows us to use low-level - # # mysql queries, and restricts the dataflow back and forth to the database - # # to just the few columns we actually need. Unfortunately it is ugly, - # # it totally violates the autonomy of Name, and it is probably hard to - # # understand. But hopefully once we get it working it will remain stable. - # # Blame it on me... -Jason, July 2019 - - # def minimal_name_data(name) - # return nil unless name - - # [ - # name.id, # 0 - # name.correct_spelling_id, # 1 - # name.synonym_id, # 2 - # name.text_name # 3 - # ] - # end - - # def minimal_name_columns - # [:id, :correct_spelling_id, :synonym_id, :text_name].freeze - # end -end From ad13a58c4943beea3ae2b38a2f04ecd59cdb22cd Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 01:32:37 -0800 Subject: [PATCH 23/39] Update lookup.rb --- app/classes/lookup.rb | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index 295eb31f87..db96e9b525 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -1,5 +1,37 @@ # frozen_string_literal: true +# Lookup +# +# A flexible looker-upper of records. It can handle any identifiers we're likely +# to throw at it: a string, ID, instance, or a mixed array of any of those. The +# `lookup_method` has to be configured in the Lookup child class, because the +# lookup column names are different for each model. +# +# Primarily used to get a clean set of ids for ActiveRecord query params. +# For example, indexes like "Observations for (given) Projects" can be filtered +# for more than one project at a time: "NEMF 2023" and "NEMF 2024". +# The observation query needs the project IDs, and Lookup just allows callers +# to send whatever param type is available. This is handy in the API and +# in searches. +# +# Create an instance of a child class with a string, instance or id, or a mixed +# array of any of these. Returns an array of ids, instances or strings (names) +# via instance methods `ids`, `instances` and `titles`. +# +# Use: +# project_ids = Lookup::Projects.new(["NEMF 2023", "NEMF 2024"]).ids +# Observation.where(project: project_ids) +# +# fred_ids = Lookup::Users.new(["Fred", "Freddie", "Freda", "Anni Frid"]).ids +# Image.where(user: fred_ids) +# +# Instance methods: +# (all return arrays) +# +# ids: Array of ids of records matching the values sent to the instance +# instances: Array of instances of those records +# titles: Array of names of those records, from @name_column set in subclass +# class Lookup attr_reader :model, :vals, :params From ef04eb2dc95d1669bb7afcf36c087d5c697598b8 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 01:39:46 -0800 Subject: [PATCH 24/39] Be sure model is defined. --- app/classes/lookup.rb | 2 ++ app/classes/lookup/external_sites.rb | 2 +- app/classes/lookup/herbaria.rb | 2 +- app/classes/lookup/herbarium_records.rb | 2 +- app/classes/lookup/locations.rb | 2 +- app/classes/lookup/project_species_lists.rb | 2 +- app/classes/lookup/projects.rb | 2 +- app/classes/lookup/regions.rb | 2 +- app/classes/lookup/species_lists.rb | 2 +- app/classes/lookup/users.rb | 3 ++- 10 files changed, 12 insertions(+), 9 deletions(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index db96e9b525..b9035b21cb 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -36,6 +36,8 @@ class Lookup attr_reader :model, :vals, :params def initialize(vals, params = {}) + raise("Lookup is only usable via the subclasses.") if @model.blank? + @vals = prepare_vals(vals) @params = params end diff --git a/app/classes/lookup/external_sites.rb b/app/classes/lookup/external_sites.rb index d9f6267bd8..b0788a6bd9 100644 --- a/app/classes/lookup/external_sites.rb +++ b/app/classes/lookup/external_sites.rb @@ -2,9 +2,9 @@ class Lookup::ExternalSites < Lookup def initialize(vals, params = {}) - super @model = ExternalSite @name_column = :name + super end def lookup_method(name) diff --git a/app/classes/lookup/herbaria.rb b/app/classes/lookup/herbaria.rb index 3e7222625a..0f0ed17456 100644 --- a/app/classes/lookup/herbaria.rb +++ b/app/classes/lookup/herbaria.rb @@ -2,9 +2,9 @@ class Lookup::Herbaria < Lookup def initialize(vals, params = {}) - super @model = Herbarium @name_column = :name + super end def lookup_method(name) diff --git a/app/classes/lookup/herbarium_records.rb b/app/classes/lookup/herbarium_records.rb index eb1f039944..86c8e59c9c 100644 --- a/app/classes/lookup/herbarium_records.rb +++ b/app/classes/lookup/herbarium_records.rb @@ -2,9 +2,9 @@ class Lookup::HerbariumRecords < Lookup def initialize(vals, params = {}) - super @model = HerbariumRecord @name_column = :id + super end def lookup_method(name) diff --git a/app/classes/lookup/locations.rb b/app/classes/lookup/locations.rb index 39937caf7a..1335b34738 100644 --- a/app/classes/lookup/locations.rb +++ b/app/classes/lookup/locations.rb @@ -2,9 +2,9 @@ class Lookup::Locations < Lookup def initialize(vals, params = {}) - super @model = Location @name_column = :name + super end def lookup_method(name) diff --git a/app/classes/lookup/project_species_lists.rb b/app/classes/lookup/project_species_lists.rb index 2880824396..de87bd73e1 100644 --- a/app/classes/lookup/project_species_lists.rb +++ b/app/classes/lookup/project_species_lists.rb @@ -2,9 +2,9 @@ class Lookup::ProjectSpeciesLists < Lookup def initialize(vals, params = {}) - super @model = SpeciesList @name_column = :title + super end def ids diff --git a/app/classes/lookup/projects.rb b/app/classes/lookup/projects.rb index d1a0b7e8fb..03377b68ba 100644 --- a/app/classes/lookup/projects.rb +++ b/app/classes/lookup/projects.rb @@ -2,9 +2,9 @@ class Lookup::Projects < Lookup def initialize(vals, params = {}) - super @model = Project @name_column = :title + super end def lookup_method(name) diff --git a/app/classes/lookup/regions.rb b/app/classes/lookup/regions.rb index bb554ad422..b883194518 100644 --- a/app/classes/lookup/regions.rb +++ b/app/classes/lookup/regions.rb @@ -2,9 +2,9 @@ class Lookup::Regions < Lookup def initialize(vals, params = {}) - super @model = Location @name_column = :name + super end def lookup_method(name) diff --git a/app/classes/lookup/species_lists.rb b/app/classes/lookup/species_lists.rb index f2c1aa1485..4eca46db63 100644 --- a/app/classes/lookup/species_lists.rb +++ b/app/classes/lookup/species_lists.rb @@ -2,9 +2,9 @@ class Lookup::SpeciesLists < Lookup def initialize(vals, params = {}) - super @model = SpeciesList @name_column = :title + super end def lookup_method(name) diff --git a/app/classes/lookup/users.rb b/app/classes/lookup/users.rb index 8ab2e815b0..c77a4fdc29 100644 --- a/app/classes/lookup/users.rb +++ b/app/classes/lookup/users.rb @@ -2,8 +2,9 @@ class Lookup::Users < Lookup def initialize(vals, params = {}) - super @model = User + @name_column = :login + super end def lookup_method(name) From a79921fe42772b7f17ba069349926569dbc81cfe Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 01:41:55 -0800 Subject: [PATCH 25/39] Switch to @title_column --- app/classes/lookup.rb | 4 ++-- app/classes/lookup/external_sites.rb | 2 +- app/classes/lookup/herbaria.rb | 2 +- app/classes/lookup/herbarium_records.rb | 2 +- app/classes/lookup/locations.rb | 2 +- app/classes/lookup/names.rb | 2 +- app/classes/lookup/project_species_lists.rb | 2 +- app/classes/lookup/projects.rb | 2 +- app/classes/lookup/regions.rb | 2 +- app/classes/lookup/species_lists.rb | 2 +- app/classes/lookup/users.rb | 2 +- 11 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index b9035b21cb..9072aad475 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -30,7 +30,7 @@ # # ids: Array of ids of records matching the values sent to the instance # instances: Array of instances of those records -# titles: Array of names of those records, from @name_column set in subclass +# titles: Array of names of those records, from @title_column set in subclass # class Lookup attr_reader :model, :vals, :params @@ -76,7 +76,7 @@ def lookup_instances def lookup_titles return [] if @vals.blank? - instances.map(&:"#{@name_column}") + instances.map(&:"#{@title_column}") end def evaluate_values_as_ids diff --git a/app/classes/lookup/external_sites.rb b/app/classes/lookup/external_sites.rb index b0788a6bd9..ac9a558e77 100644 --- a/app/classes/lookup/external_sites.rb +++ b/app/classes/lookup/external_sites.rb @@ -3,7 +3,7 @@ class Lookup::ExternalSites < Lookup def initialize(vals, params = {}) @model = ExternalSite - @name_column = :name + @title_column = :name super end diff --git a/app/classes/lookup/herbaria.rb b/app/classes/lookup/herbaria.rb index 0f0ed17456..d88e204046 100644 --- a/app/classes/lookup/herbaria.rb +++ b/app/classes/lookup/herbaria.rb @@ -3,7 +3,7 @@ class Lookup::Herbaria < Lookup def initialize(vals, params = {}) @model = Herbarium - @name_column = :name + @title_column = :name super end diff --git a/app/classes/lookup/herbarium_records.rb b/app/classes/lookup/herbarium_records.rb index 86c8e59c9c..5062ec7cc7 100644 --- a/app/classes/lookup/herbarium_records.rb +++ b/app/classes/lookup/herbarium_records.rb @@ -3,7 +3,7 @@ class Lookup::HerbariumRecords < Lookup def initialize(vals, params = {}) @model = HerbariumRecord - @name_column = :id + @title_column = :id super end diff --git a/app/classes/lookup/locations.rb b/app/classes/lookup/locations.rb index 1335b34738..05121e57ad 100644 --- a/app/classes/lookup/locations.rb +++ b/app/classes/lookup/locations.rb @@ -3,7 +3,7 @@ class Lookup::Locations < Lookup def initialize(vals, params = {}) @model = Location - @name_column = :name + @title_column = :name super end diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 59b4b53d7e..83241761f2 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -3,7 +3,7 @@ class Lookup::Names < Lookup def initialize(vals, params = {}) @model = Name - @name_column = :search_name + @title_column = :search_name super end diff --git a/app/classes/lookup/project_species_lists.rb b/app/classes/lookup/project_species_lists.rb index de87bd73e1..3898f38dbc 100644 --- a/app/classes/lookup/project_species_lists.rb +++ b/app/classes/lookup/project_species_lists.rb @@ -3,7 +3,7 @@ class Lookup::ProjectSpeciesLists < Lookup def initialize(vals, params = {}) @model = SpeciesList - @name_column = :title + @title_column = :title super end diff --git a/app/classes/lookup/projects.rb b/app/classes/lookup/projects.rb index 03377b68ba..0e34486b92 100644 --- a/app/classes/lookup/projects.rb +++ b/app/classes/lookup/projects.rb @@ -3,7 +3,7 @@ class Lookup::Projects < Lookup def initialize(vals, params = {}) @model = Project - @name_column = :title + @title_column = :title super end diff --git a/app/classes/lookup/regions.rb b/app/classes/lookup/regions.rb index b883194518..0df5016ad2 100644 --- a/app/classes/lookup/regions.rb +++ b/app/classes/lookup/regions.rb @@ -3,7 +3,7 @@ class Lookup::Regions < Lookup def initialize(vals, params = {}) @model = Location - @name_column = :name + @title_column = :name super end diff --git a/app/classes/lookup/species_lists.rb b/app/classes/lookup/species_lists.rb index 4eca46db63..3b480f6893 100644 --- a/app/classes/lookup/species_lists.rb +++ b/app/classes/lookup/species_lists.rb @@ -3,7 +3,7 @@ class Lookup::SpeciesLists < Lookup def initialize(vals, params = {}) @model = SpeciesList - @name_column = :title + @title_column = :title super end diff --git a/app/classes/lookup/users.rb b/app/classes/lookup/users.rb index c77a4fdc29..aae32b62c2 100644 --- a/app/classes/lookup/users.rb +++ b/app/classes/lookup/users.rb @@ -3,7 +3,7 @@ class Lookup::Users < Lookup def initialize(vals, params = {}) @model = User - @name_column = :login + @title_column = :login super end From c03974a31cb1c604e00c6b1c03b64f032521f61e Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 01:44:40 -0800 Subject: [PATCH 26/39] Update lookup.rb --- app/classes/lookup.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index 9072aad475..c506e09aa8 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -30,7 +30,7 @@ # # ids: Array of ids of records matching the values sent to the instance # instances: Array of instances of those records -# titles: Array of names of those records, from @title_column set in subclass +# titles: Array of titles of those records, @title_column set in subclass # class Lookup attr_reader :model, :vals, :params From 08c8dbfe77ce9b156ebc2c7f33e3c5501e410a55 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 01:46:20 -0800 Subject: [PATCH 27/39] Update lookup.rb --- app/classes/lookup.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index c506e09aa8..f01325a834 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -30,7 +30,8 @@ # # ids: Array of ids of records matching the values sent to the instance # instances: Array of instances of those records -# titles: Array of titles of those records, @title_column set in subclass +# titles: Array of names of those records, via @title_column set in subclass +# (A `names` method seemed too confusing, because Lookup::Names...) # class Lookup attr_reader :model, :vals, :params From 09c210854be6ab071b85516a7031dd1a4cd8e675 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 02:04:57 -0800 Subject: [PATCH 28/39] Update names.rb --- app/classes/lookup/names.rb | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 83241761f2..9fbde3bc9a 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -180,26 +180,11 @@ def genera_and_down(names) end.uniq end - # This ugliness with "minimal name data" is a way to avoid having Rails - # instantiate all the names (which can get quite huge if you start talking - # about all the children of Kingdom Fungi!) It allows us to use low-level - # mysql queries, and restricts the dataflow back and forth to the database - # to just the few columns we actually need. Unfortunately it is ugly, - # it totally violates the autonomy of Name, and it is probably hard to - # understand. But hopefully once we get it working it will remain stable. - # Blame it on me... -Jason, July 2019 - - # def minimal_name_data(name) - # return nil unless name - - # [ - # name.id, # 0 - # name.correct_spelling_id, # 1 - # name.synonym_id, # 2 - # name.text_name # 3 - # ] - # end - + # Selecting "minimal_name_columns" is a way to avoid having Rails instantiate + # all the names getting passed around (which can get quite huge if we've got + # all the children of Kingdom Fungi!) It allows us to use quicker AR selects, + # optimized to restrict the dataflow back and forth to the database to just + # the few columns we actually need. def minimal_name_columns [:id, :correct_spelling_id, :synonym_id, :text_name] end From a378239694d8598bd1743b9aae93ebb736909c64 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 02:07:19 -0800 Subject: [PATCH 29/39] Update names.rb --- app/classes/lookup/names.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 9fbde3bc9a..2a9e7b4a93 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -196,15 +196,13 @@ def limited_id_set(ids) end def complain_about_unused_flags! - complain_about_unused_flag!(:include_synonyms) - complain_about_unused_flag!(:include_subtaxa) - complain_about_unused_flag!(:include_nonconsensus) - complain_about_unused_flag!(:exclude_consensus) - complain_about_unused_flag!(:exclude_original_names) + return if @params.blank? + + @params.each_key { |param| complain_about_unused_flag!(param) } end def complain_about_unused_flag!(param) - return if @params.blank? || @params[param].nil? + return if @params[param].nil? raise("Flag \"#{param}\" is invalid without \"names\" parameter.") end From c31f5400ec52cb453efb48dbf31092b0cbf3eae3 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 02:10:29 -0800 Subject: [PATCH 30/39] Update names.rb --- app/classes/lookup/names.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index 2a9e7b4a93..f0f501a094 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -33,6 +33,8 @@ def lookup_instances ids.map { |id| Name.find(id) } end + # Could be quite a few more than the given set. + # Memoized to avoid recalculating, or passing the value around. def original_names @original_names ||= if @params[:exclude_original_names] add_other_spellings(original_matches) @@ -41,6 +43,7 @@ def original_names end end + # Matches for the given set, from the db. def original_matches @original_matches ||= @vals.map do |val| if val.is_a?(@model) From 2c83bc7c456042f2d0cca6759f884dcfa6d4c7c2 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 02:14:17 -0800 Subject: [PATCH 31/39] Update names.rb --- app/classes/lookup/names.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index f0f501a094..e20e5ecd83 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -26,14 +26,15 @@ def lookup_ids names.map(&:id) end - # Look up all instances from the ids. Too complicated otherwise. + # Re-lookup all instances from the matched ids. Too complicated to try to grab + # instances if they were in the given vals, because of `add_other_spellings`. def lookup_instances return [] if @vals.blank? ids.map { |id| Name.find(id) } end - # Could be quite a few more than the given set. + # "Original" names could turn out to be quite a few more than the given vals. # Memoized to avoid recalculating, or passing the value around. def original_names @original_names ||= if @params[:exclude_original_names] @@ -43,7 +44,7 @@ def original_names end end - # Matches for the given set, from the db. + # Matches for the given vals, from the db. def original_matches @original_matches ||= @vals.map do |val| if val.is_a?(@model) From 7c31da13ec266f170bf2f2779ff23438f711dc8e Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 02:15:45 -0800 Subject: [PATCH 32/39] Update names.rb --- app/classes/lookup/names.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index e20e5ecd83..c5c36f1397 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -61,12 +61,12 @@ def original_matches # NOTE: Name.parse_name returns a ParsedName instance, not an Name instance. # A ParsedName is a hash of segments and formatted strings of the name. - def find_matching_names(name) - parse = Name.parse_name(name) + def find_matching_names(val) + parse = Name.parse_name(val) srch_str = if parse parse.search_name else - Name.clean_incoming_string(name) + Name.clean_incoming_string(val) end if parse&.author.present? matches = Name.where(search_name: srch_str).select(*minimal_name_columns) From afab2c403f8246cb1eeeb2464f62d9ce1a7eb389 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 14:19:54 -0800 Subject: [PATCH 33/39] refactor model, title_column as constants, tests --- app/classes/lookup.rb | 14 +++- app/classes/lookup/external_sites.rb | 5 +- app/classes/lookup/herbaria.rb | 5 +- app/classes/lookup/herbarium_records.rb | 5 +- app/classes/lookup/locations.rb | 5 +- app/classes/lookup/names.rb | 5 +- app/classes/lookup/project_species_lists.rb | 5 +- app/classes/lookup/projects.rb | 5 +- app/classes/lookup/regions.rb | 5 +- app/classes/lookup/species_lists.rb | 5 +- app/classes/lookup/users.rb | 5 +- test/classes/lookup_test.rb | 75 +++++++++++++++++---- 12 files changed, 104 insertions(+), 35 deletions(-) diff --git a/app/classes/lookup.rb b/app/classes/lookup.rb index f01325a834..60e2c9fbab 100644 --- a/app/classes/lookup.rb +++ b/app/classes/lookup.rb @@ -33,12 +33,22 @@ # titles: Array of names of those records, via @title_column set in subclass # (A `names` method seemed too confusing, because Lookup::Names...) # +# Class constants: +# (defined in subclass) +# +# MODEL: +# TITLE_COLUMN: +# class Lookup - attr_reader :model, :vals, :params + attr_reader :vals, :params def initialize(vals, params = {}) - raise("Lookup is only usable via the subclasses.") if @model.blank? + unless defined?(self.class::MODEL) + raise("Lookup is only usable via the subclasses, like Lookup::Names.") + end + @model = self.class::MODEL + @title_column = self.class::TITLE_COLUMN @vals = prepare_vals(vals) @params = params end diff --git a/app/classes/lookup/external_sites.rb b/app/classes/lookup/external_sites.rb index ac9a558e77..f8a4d8d9e7 100644 --- a/app/classes/lookup/external_sites.rb +++ b/app/classes/lookup/external_sites.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::ExternalSites < Lookup + MODEL = ExternalSite + TITLE_COLUMN = :name + def initialize(vals, params = {}) - @model = ExternalSite - @title_column = :name super end diff --git a/app/classes/lookup/herbaria.rb b/app/classes/lookup/herbaria.rb index d88e204046..7928a11969 100644 --- a/app/classes/lookup/herbaria.rb +++ b/app/classes/lookup/herbaria.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::Herbaria < Lookup + MODEL = Herbarium + TITLE_COLUMN = :name + def initialize(vals, params = {}) - @model = Herbarium - @title_column = :name super end diff --git a/app/classes/lookup/herbarium_records.rb b/app/classes/lookup/herbarium_records.rb index 5062ec7cc7..f462b3bd94 100644 --- a/app/classes/lookup/herbarium_records.rb +++ b/app/classes/lookup/herbarium_records.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::HerbariumRecords < Lookup + MODEL = HerbariumRecord + TITLE_COLUMN = :id + def initialize(vals, params = {}) - @model = HerbariumRecord - @title_column = :id super end diff --git a/app/classes/lookup/locations.rb b/app/classes/lookup/locations.rb index 05121e57ad..1c69ed1d6e 100644 --- a/app/classes/lookup/locations.rb +++ b/app/classes/lookup/locations.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::Locations < Lookup + MODEL = Location + TITLE_COLUMN = :name + def initialize(vals, params = {}) - @model = Location - @title_column = :name super end diff --git a/app/classes/lookup/names.rb b/app/classes/lookup/names.rb index c5c36f1397..2f3e4ab7cc 100644 --- a/app/classes/lookup/names.rb +++ b/app/classes/lookup/names.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::Names < Lookup + MODEL = Name + TITLE_COLUMN = :search_name + def initialize(vals, params = {}) - @model = Name - @title_column = :search_name super end diff --git a/app/classes/lookup/project_species_lists.rb b/app/classes/lookup/project_species_lists.rb index 3898f38dbc..445367da22 100644 --- a/app/classes/lookup/project_species_lists.rb +++ b/app/classes/lookup/project_species_lists.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::ProjectSpeciesLists < Lookup + MODEL = SpeciesList + TITLE_COLUMN = :title + def initialize(vals, params = {}) - @model = SpeciesList - @title_column = :title super end diff --git a/app/classes/lookup/projects.rb b/app/classes/lookup/projects.rb index 0e34486b92..50de3279a1 100644 --- a/app/classes/lookup/projects.rb +++ b/app/classes/lookup/projects.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::Projects < Lookup + MODEL = Project + TITLE_COLUMN = :title + def initialize(vals, params = {}) - @model = Project - @title_column = :title super end diff --git a/app/classes/lookup/regions.rb b/app/classes/lookup/regions.rb index 0df5016ad2..c1829fb8f2 100644 --- a/app/classes/lookup/regions.rb +++ b/app/classes/lookup/regions.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::Regions < Lookup + MODEL = Location + TITLE_COLUMN = :name + def initialize(vals, params = {}) - @model = Location - @title_column = :name super end diff --git a/app/classes/lookup/species_lists.rb b/app/classes/lookup/species_lists.rb index 3b480f6893..c4132d5c98 100644 --- a/app/classes/lookup/species_lists.rb +++ b/app/classes/lookup/species_lists.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::SpeciesLists < Lookup + MODEL = SpeciesList + TITLE_COLUMN = :title + def initialize(vals, params = {}) - @model = SpeciesList - @title_column = :title super end diff --git a/app/classes/lookup/users.rb b/app/classes/lookup/users.rb index aae32b62c2..84f5877358 100644 --- a/app/classes/lookup/users.rb +++ b/app/classes/lookup/users.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true class Lookup::Users < Lookup + MODEL = User + TITLE_COLUMN = :login + def initialize(vals, params = {}) - @model = User - @title_column = :login super end diff --git a/test/classes/lookup_test.rb b/test/classes/lookup_test.rb index 7e266e0ff2..a23937e50c 100644 --- a/test/classes/lookup_test.rb +++ b/test/classes/lookup_test.rb @@ -3,20 +3,63 @@ require("test_helper") class LookupTest < UnitTestCase - # tests of Lookup::Names - def create_test_name(name) - name = Name.new_name(Name.parse_name(name).params) - name.save - name + def assert_lookup_objects_by_name(type, expects, vals, **) + lookup = "Lookup::#{type}".constantize + actual = lookup.new(vals, **).titles.sort + expects = expects.map(&:"#{lookup::TITLE_COLUMN}").sort + assert_arrays_equal(expects, actual) + end + + def assert_lookup_names_by_name(expects, vals, **) + assert_lookup_objects_by_name(:Names, expects, vals, **) + end + + def test_lookup_external_sites_by_name + expects = [external_sites(:inaturalist)] + assert_lookup_objects_by_name(:ExternalSites, expects, "iNaturalist") + end + + def test_lookup_herbaria_by_name + expects = [herbaria(:rolf_herbarium), herbaria(:dick_herbarium)] + assert_lookup_objects_by_name(:Herbaria, expects, expects.map(&:name)) + end + + def test_lookup_herbarium_records_by_name + expects = [herbarium_records(:coprinus_comatus_nybg_spec), + herbarium_records(:coprinus_comatus_rolf_spec)] + assert_lookup_objects_by_name(:HerbariumRecords, expects, expects.map(&:id)) + end + + def test_lookup_locations_by_name + expects = [locations(:salt_point), locations(:burbank)] + assert_lookup_objects_by_name(:Locations, expects, expects.map(&:name)) + end + + def test_lookup_projects_by_name + expects = [projects(:bolete_project)] + assert_lookup_objects_by_name(:Projects, expects, expects.map(&:title)) + end + + def test_lookup_project_species_lists_by_name + expects = [species_lists(:unknown_species_list)] + assert_lookup_objects_by_name(:ProjectSpeciesLists, expects, + "Bolete Project") + end + + def test_lookup_regions_by_name + expects = [locations(:point_reyes)] + assert_lookup_objects_by_name(:Regions, expects, + "Marin Co., California, USA") end - def assert_lookup_names_by_name(expects, vals, params = {}) - actual = Lookup::Names.new(vals, **params).instances.sort_by(&:text_name) - expects = expects.sort_by(&:text_name) - # actual = actual.map { |id| Name.find(id) }.sort_by(&:text_name) - assert_name_arrays_equal(expects, actual) + def test_lookup_species_lists_by_name + expects = [species_lists(:unknown_species_list)] + assert_lookup_objects_by_name(:SpeciesLists, expects, "List of mysteries") end + ######################################################################## + # tests of Lookup::Names + # def test_lookup_names_by_name User.current = rolf @@ -65,7 +108,7 @@ def test_lookup_names_by_name include_subtaxa: true) end - def test_lookup_names_by_name2 + def test_lookup_names_by_name_classifications User.current = rolf name1 = names(:peltigeraceae) @@ -104,7 +147,7 @@ def test_lookup_names_by_name2 include_immediate_subtaxa: true) end - def test_lookup_names_by_name3 + def test_lookup_names_by_name_invalid_classification User.current = rolf name1 = names(:lactarius) @@ -124,7 +167,13 @@ def test_lookup_names_by_name3 exclude_original_names: true) end - def test_lookup_names_by_name4 + def test_lookup_names_by_name_invalid assert_lookup_names_by_name([], ["¡not a name!"]) end + + def create_test_name(name) + name = Name.new_name(Name.parse_name(name).params) + name.save + name + end end From e50ccf88ee33cc76426e4492927d4cf6c8a45719 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 16:18:54 -0800 Subject: [PATCH 34/39] Allow new lookup capabilities in User --- app/classes/query/modules/validation.rb | 33 +++++++++++++++- test/classes/query/names_test.rb | 32 ++++++++++++---- test/classes/query_test.rb | 51 ++++++++++++++++++++++--- 3 files changed, 101 insertions(+), 15 deletions(-) diff --git a/app/classes/query/modules/validation.rb b/app/classes/query/modules/validation.rb index ce66243442..89f0773cad 100644 --- a/app/classes/query/modules/validation.rb +++ b/app/classes/query/modules/validation.rb @@ -85,7 +85,7 @@ def scalar_validate(arg, val, arg_type) send(:"validate_#{arg_type}", arg, val) elsif arg_type.is_a?(Class) && arg_type.respond_to?(:descends_from_active_record?) - validate_id(arg, val, arg_type) + validate_record_or_id_or_string(arg, val, arg_type) elsif arg_type.is_a?(Hash) validate_enum(arg, val, arg_type) else @@ -154,6 +154,25 @@ def validate_float(arg, val) end end + def validate_record_or_id_or_string(arg, val, type = ActiveRecord::Base) + if val.is_a?(type) + raise("Value for :#{arg} is an unsaved #{type} instance.") unless val.id + + # Cache the instance for later use, in case we both instantiate and + # execute query in the same action. + @params_cache ||= {} + @params_cache[arg] = val + val.id + elsif could_be_record_id?(val) + val.to_i + elsif val.is_a?(String) + val # lookup happens later + else + raise("Value for :#{arg} should be id, string or an #{type} instance, " \ + "got: #{val.inspect}") + end + end + def validate_string(arg, val) if val.is_a?(Integer) || val.is_a?(Float) || val.is_a?(String) || val.is_a?(Symbol) @@ -243,7 +262,17 @@ def validate_query(arg, val) def find_cached_parameter_instance(model, arg) @params_cache ||= {} - @params_cache[arg] ||= model.find(params[arg]) + # unless could_be_record_id?(arg) + # raise("Value for :#{arg} is not an id, got #{val.inspect}") + # end + # @params_cache[arg] ||= model.find(params[arg]) + @params_cache[arg] ||= if could_be_record_id?(params[arg]) + model.find(params[arg]) + else + lookup = "Lookup::#{model.name.pluralize}" + lookup = lookup.constantize + lookup.new(params[arg]).instances.first + end end def get_cached_parameter_instance(arg) diff --git a/test/classes/query/names_test.rb b/test/classes/query/names_test.rb index e69c192dd3..bb432c5c75 100644 --- a/test/classes/query/names_test.rb +++ b/test/classes/query/names_test.rb @@ -65,17 +65,35 @@ def test_name_by_editor assert_query(expects, :Name, by_editor: dick, by: :id) end - # non-arrays and arrays of strings don't work - def test_name_users - # users = users(:rolf).login - # expects = Name.where(user: users).index_order - # assert_query(expects, :Name, users: users) + def test_name_users_login + # single + expects = Name.where(user: users(:rolf)).index_order + assert_query(expects, :Name, users: users(:rolf).login) + # array + users = [users(:rolf), users(:mary)] + expects = Name.where(user: users).index_order + assert_query(expects, :Name, users: users.map(&:login)) + end + + def test_name_users_id + # single + users = users(:rolf).id + expects = Name.where(user: users).index_order + assert_query(expects, :Name, users: users) + # array users = [users(:rolf), users(:mary)].map(&:id) expects = Name.where(user: users).index_order assert_query(expects, :Name, users: users) - # users = [users(:rolf), users(:mary)].map(&:login) - # assert_query(expects, :Name, users: users) + end + + def test_name_users_instance + # single + users = users(:rolf) + expects = Name.where(user: users).index_order + assert_query(expects, :Name, users: users) + # array users = [users(:rolf), users(:mary)] + expects = Name.where(user: users).index_order assert_query(expects, :Name, users: users) end diff --git a/test/classes/query_test.rb b/test/classes/query_test.rb index 1ba575e5a1..32cfa3c4e7 100644 --- a/test/classes/query_test.rb +++ b/test/classes/query_test.rb @@ -51,8 +51,6 @@ def test_basic end def test_validate_params - @fungi = names(:fungi) - assert_raises(RuntimeError) { Query.lookup(:Name, xxx: true) } assert_raises(RuntimeError) { Query.lookup(:Name, by: [1, 2, 3]) } assert_raises(RuntimeError) { Query.lookup(:Name, by: true) } @@ -73,10 +71,14 @@ def test_validate_params Query.lookup(:Name, misspellings: true) end assert_raises(RuntimeError) { Query.lookup(:Name, misspellings: 123) } + end + + def test_validate_params_instances_by_user + @fungi = names(:fungi) # assert_raises(RuntimeError) { Query.lookup(:Image) } assert_raises(RuntimeError) { Query.lookup(:Image, by_user: :bogus) } - assert_raises(RuntimeError) { Query.lookup(:Image, by_user: "rolf") } + # assert_raises(RuntimeError) { Query.lookup(:Image, by_user: "rolf") } assert_raises(RuntimeError) { Query.lookup(:Image, by_user: @fungi) } assert_equal(rolf.id, Query.lookup(:Image, by_user: rolf).params[:by_user]) @@ -85,12 +87,33 @@ def test_validate_params assert_equal(rolf.id, Query.lookup(:Image, by_user: rolf.id.to_s). params[:by_user]) + assert_equal(rolf.login, + Query.lookup(:Image, by_user: rolf.login).params[:by_user]) + end + def test_validate_params_instances_users + @fungi = names(:fungi) + + assert_raises(RuntimeError) { Query.lookup(:Image, users: :bogus) } + assert_raises(RuntimeError) { Query.lookup(:Image, users: @fungi) } + assert_equal([rolf.id], + Query.lookup(:Image, users: rolf).params[:users]) + assert_equal([rolf.id], + Query.lookup(:Image, users: rolf.id).params[:users]) + assert_equal([rolf.id], + Query.lookup(:Image, users: rolf.id.to_s).params[:users]) + assert_equal([rolf.login], + Query.lookup(:Image, users: rolf.login).params[:users]) + end + + def test_validate_params_ids # Oops, this query is generic, # doesn't know to require Name instances here. # assert_raises(RuntimeError) { Query.lookup(:Name, ids: rolf) } - assert_raises(RuntimeError) { Query.lookup(:Name, ids: "one") } - assert_raises(RuntimeError) { Query.lookup(:Name, ids: "1,2,3") } + # assert_raises(RuntimeError) { Query.lookup(:Name, ids: "one") } + # assert_raises(RuntimeError) { Query.lookup(:Name, ids: "1,2,3") } + assert_equal([], Query.lookup(:Name, ids: "one")) + assert_equal([], Query.lookup(:Name, ids: "1,2,3")) assert_equal([names(:fungi).id], Query.lookup(:Name, ids: names(:fungi).id.to_s).params[:ids]) @@ -113,7 +136,9 @@ def test_validate_params assert_equal([rolf.id, mary.id, junk.id], Query.lookup(:User, ids: [rolf, mary.id, junk.id.to_s]).params[:ids]) + end + def test_validate_params_pattern # assert_raises(RuntimeError) { Query.lookup(:Name) } assert_raises(RuntimeError) do Query.lookup(:Name, pattern: true) @@ -130,28 +155,42 @@ def test_validate_params Query.lookup(:Name, pattern: "rolf").params[:pattern]) assert_equal("rolf", Query.lookup(:Name, pattern: :rolf).params[:pattern]) + end + def test_validate_params_join assert_equal(["table"], Query.lookup(:Name, join: :table).params[:join]) assert_equal(%w[table1 table2], Query.lookup(:Name, join: [:table1, :table2]). params[:join]) + end + + def test_validate_params_tables assert_equal(["table"], Query.lookup(:Name, tables: :table).params[:tables]) assert_equal(%w[table1 table2], Query.lookup(:Name, tables: [:table1, :table2]). params[:tables]) + end + + def test_validate_params_where assert_equal(["foo = bar"], Query.lookup(:Name, where: "foo = bar").params[:where]) assert_equal(["foo = bar", "id in (1,2,3)"], Query.lookup(:Name, where: ["foo = bar", "id in (1,2,3)"]). params[:where]) + end + + def test_validate_params_group assert_equal("names.id", Query.lookup(:Name, group: "names.id").params[:group]) + assert_raises(RuntimeError) { Query.lookup(:Name, group: %w[1 2]) } + end + + def test_validate_params_order assert_equal("id DESC", Query.lookup(:Name, order: "id DESC").params[:order]) - assert_raises(RuntimeError) { Query.lookup(:Name, group: %w[1 2]) } assert_raises(RuntimeError) { Query.lookup(:Name, order: %w[1 2]) } end From 66dbd64938147384c222a5d6fe7eb3148d37457c Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 17:15:40 -0800 Subject: [PATCH 35/39] Adjust validation to actually do the lookup --- app/classes/query/modules/validation.rb | 7 ++++++- test/classes/query_test.rb | 12 ++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/classes/query/modules/validation.rb b/app/classes/query/modules/validation.rb index 89f0773cad..0c2a8cb8e9 100644 --- a/app/classes/query/modules/validation.rb +++ b/app/classes/query/modules/validation.rb @@ -166,7 +166,12 @@ def validate_record_or_id_or_string(arg, val, type = ActiveRecord::Base) elsif could_be_record_id?(val) val.to_i elsif val.is_a?(String) - val # lookup happens later + lookup = "Lookup::#{type.name.pluralize}" + lookup = lookup.constantize + result = lookup.new(val).ids&.first + raise("Couldn't find an id for : #{val.inspect}") unless result + + result else raise("Value for :#{arg} should be id, string or an #{type} instance, " \ "got: #{val.inspect}") diff --git a/test/classes/query_test.rb b/test/classes/query_test.rb index 32cfa3c4e7..c66bf1ae6f 100644 --- a/test/classes/query_test.rb +++ b/test/classes/query_test.rb @@ -87,7 +87,7 @@ def test_validate_params_instances_by_user assert_equal(rolf.id, Query.lookup(:Image, by_user: rolf.id.to_s). params[:by_user]) - assert_equal(rolf.login, + assert_equal(rolf.id, Query.lookup(:Image, by_user: rolf.login).params[:by_user]) end @@ -102,7 +102,7 @@ def test_validate_params_instances_users Query.lookup(:Image, users: rolf.id).params[:users]) assert_equal([rolf.id], Query.lookup(:Image, users: rolf.id.to_s).params[:users]) - assert_equal([rolf.login], + assert_equal([rolf.id], Query.lookup(:Image, users: rolf.login).params[:users]) end @@ -110,10 +110,10 @@ def test_validate_params_ids # Oops, this query is generic, # doesn't know to require Name instances here. # assert_raises(RuntimeError) { Query.lookup(:Name, ids: rolf) } - # assert_raises(RuntimeError) { Query.lookup(:Name, ids: "one") } - # assert_raises(RuntimeError) { Query.lookup(:Name, ids: "1,2,3") } - assert_equal([], Query.lookup(:Name, ids: "one")) - assert_equal([], Query.lookup(:Name, ids: "1,2,3")) + assert_raises(RuntimeError) { Query.lookup(:Name, ids: "one") } + assert_raises(RuntimeError) { Query.lookup(:Name, ids: "1,2,3") } + assert_equal([names(:fungi).id], + Query.lookup(:Name, ids: names(:fungi).text_name).params[:ids]) assert_equal([names(:fungi).id], Query.lookup(:Name, ids: names(:fungi).id.to_s).params[:ids]) From 8a4870b71b398173886f5e7d7fb103978fa7cbae Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 17:28:33 -0800 Subject: [PATCH 36/39] Adjust validation - lookup strings immediately, in case of instance param --- app/classes/query/modules/validation.rb | 23 ++++++++++------------- test/classes/query_test.rb | 7 +++---- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/app/classes/query/modules/validation.rb b/app/classes/query/modules/validation.rb index 0c2a8cb8e9..95b0c44e68 100644 --- a/app/classes/query/modules/validation.rb +++ b/app/classes/query/modules/validation.rb @@ -166,12 +166,7 @@ def validate_record_or_id_or_string(arg, val, type = ActiveRecord::Base) elsif could_be_record_id?(val) val.to_i elsif val.is_a?(String) - lookup = "Lookup::#{type.name.pluralize}" - lookup = lookup.constantize - result = lookup.new(val).ids&.first - raise("Couldn't find an id for : #{val.inspect}") unless result - - result + lookup_record_by_name(type, val).id else raise("Value for :#{arg} should be id, string or an #{type} instance, " \ "got: #{val.inspect}") @@ -267,16 +262,10 @@ def validate_query(arg, val) def find_cached_parameter_instance(model, arg) @params_cache ||= {} - # unless could_be_record_id?(arg) - # raise("Value for :#{arg} is not an id, got #{val.inspect}") - # end - # @params_cache[arg] ||= model.find(params[arg]) @params_cache[arg] ||= if could_be_record_id?(params[arg]) model.find(params[arg]) else - lookup = "Lookup::#{model.name.pluralize}" - lookup = lookup.constantize - lookup.new(params[arg]).instances.first + lookup_record_by_name(model, params[arg]) end end @@ -299,4 +288,12 @@ def could_be_record_id?(val) # (blasted admin user has id = 0!) val.is_a?(String) && (val == "0") && (arg == :user) end + + def lookup_record_by_name(type, val) + lookup = "Lookup::#{type.name.pluralize}".constantize + result = lookup.new(val).instances&.first + raise("Couldn't find an id for : #{val.inspect}") unless result + + result + end end diff --git a/test/classes/query_test.rb b/test/classes/query_test.rb index c66bf1ae6f..c49d7797c2 100644 --- a/test/classes/query_test.rb +++ b/test/classes/query_test.rb @@ -78,17 +78,16 @@ def test_validate_params_instances_by_user # assert_raises(RuntimeError) { Query.lookup(:Image) } assert_raises(RuntimeError) { Query.lookup(:Image, by_user: :bogus) } - # assert_raises(RuntimeError) { Query.lookup(:Image, by_user: "rolf") } + assert_raises(RuntimeError) { Query.lookup(:Image, by_user: "foo") } assert_raises(RuntimeError) { Query.lookup(:Image, by_user: @fungi) } assert_equal(rolf.id, Query.lookup(:Image, by_user: rolf).params[:by_user]) assert_equal(rolf.id, Query.lookup(:Image, by_user: rolf.id).params[:by_user]) assert_equal(rolf.id, - Query.lookup(:Image, by_user: rolf.id.to_s). - params[:by_user]) + Query.lookup(:Image, by_user: rolf.id.to_s).params[:by_user]) assert_equal(rolf.id, - Query.lookup(:Image, by_user: rolf.login).params[:by_user]) + Query.lookup(:Image, by_user: "rolf").params[:by_user]) end def test_validate_params_instances_users From 507651d9c7d0f4a925ab6fc0ebbc0625f2bd7bde Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 17:35:02 -0800 Subject: [PATCH 37/39] Add test to `by_user` by login --- test/classes/query/names_test.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/classes/query/names_test.rb b/test/classes/query/names_test.rb index bb432c5c75..3429cf087c 100644 --- a/test/classes/query/names_test.rb +++ b/test/classes/query/names_test.rb @@ -50,6 +50,8 @@ def test_name_in_set def test_name_by_user assert_query(Name.index_order.where(user: mary).with_correct_spelling, :Name, by_user: mary) + assert_query(Name.index_order.where(user: mary).with_correct_spelling, + :Name, by_user: "mary") assert_query(Name.index_order.where(user: dick).with_correct_spelling, :Name, by_user: dick) assert_query(Name.index_order.where(user: rolf).with_correct_spelling, From 7c15c8bdbfde6d20e09c8fbe5d0ea24dca676191 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 17:44:30 -0800 Subject: [PATCH 38/39] More thorough ids tests --- test/classes/query/names_test.rb | 35 +++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/test/classes/query/names_test.rb b/test/classes/query/names_test.rb index 3429cf087c..c793af0cc8 100644 --- a/test/classes/query/names_test.rb +++ b/test/classes/query/names_test.rb @@ -33,18 +33,29 @@ def test_name_by_rss_log assert_query(expects, :Name, by: :rss_log) end - def test_name_in_set - assert_query([names(:fungi).id, - names(:coprinus_comatus).id, - names(:conocybe_filaris).id, - names(:lepiota_rhacodes).id, - names(:lactarius_subalpinus).id], - :Name, - ids: [names(:fungi).id, - names(:coprinus_comatus).id, - names(:conocybe_filaris).id, - names(:lepiota_rhacodes).id, - names(:lactarius_subalpinus).id]) + def names_set_for_ids + [ + names(:fungi), + names(:coprinus_comatus), + names(:conocybe_filaris), + names(:lepiota_rhacodes), + names(:lactarius_subalpinus) + ] + end + + def test_name_ids_with_ids + assert_query(names_set_for_ids.map(&:id), + :Name, ids: names_set_for_ids.map(&:id)) + end + + def test_name_ids_with_instances + assert_query(names_set_for_ids.map(&:id), + :Name, ids: names_set_for_ids) + end + + def test_name_ids_with_search_names + assert_query(names_set_for_ids.map(&:id), + :Name, ids: names_set_for_ids.map(&:search_name)) end def test_name_by_user From defe252d1e9a9fbe2a9c6e2b4c8a8a9d2a3a1f5e Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Mon, 27 Jan 2025 17:46:33 -0800 Subject: [PATCH 39/39] More thorough tests for name ids param (name lookup) --- test/classes/query/names_test.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/classes/query/names_test.rb b/test/classes/query/names_test.rb index c793af0cc8..8a84c33400 100644 --- a/test/classes/query/names_test.rb +++ b/test/classes/query/names_test.rb @@ -33,7 +33,7 @@ def test_name_by_rss_log assert_query(expects, :Name, by: :rss_log) end - def names_set_for_ids + def names_set [ names(:fungi), names(:coprinus_comatus), @@ -43,19 +43,19 @@ def names_set_for_ids ] end - def test_name_ids_with_ids - assert_query(names_set_for_ids.map(&:id), - :Name, ids: names_set_for_ids.map(&:id)) + def test_name_ids_with_name_ids + assert_query(names_set.map(&:id), + :Name, ids: names_set.map(&:id)) end - def test_name_ids_with_instances - assert_query(names_set_for_ids.map(&:id), - :Name, ids: names_set_for_ids) + def test_name_ids_with_name_instances + assert_query(names_set.map(&:id), + :Name, ids: names_set) end - def test_name_ids_with_search_names - assert_query(names_set_for_ids.map(&:id), - :Name, ids: names_set_for_ids.map(&:search_name)) + def test_name_ids_with_name_search_names + assert_query(names_set.map(&:id), + :Name, ids: names_set.map(&:search_name)) end def test_name_by_user