From e089054028604260c33f8a7a0320596849d08af4 Mon Sep 17 00:00:00 2001 From: vangberg Date: Fri, 28 Aug 2026 09:41:14 +0200 Subject: [PATCH 01/11] Test existing parent and missing-parent quality checks --- test/models/name/quality_checks_test.rb | 60 +++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/models/name/quality_checks_test.rb b/test/models/name/quality_checks_test.rb index 0011ddb3..0c802f4e 100644 --- a/test/models/name/quality_checks_test.rb +++ b/test/models/name/quality_checks_test.rb @@ -5,6 +5,10 @@ def evaluate(name, check) Name::QualityChecks::QcWarningSet.new(name).evaluate(check) end + def warning(name, check) + Name::QualityChecks::QcWarning.new(check, name: name) + end + test 'malformed_subspecies_name does not raise and flags a malformed name' do name = Name.new( name: 'Testimonas exampleensis testus', rank: 'subspecies' @@ -42,6 +46,62 @@ def evaluate(name, check) assert_not(evaluate(name, :inconsistent_language)) end + test 'inconsistent_parent_rank does not flag a parent one rank above' do + name = names(:escherichia_coli) + qc = warning(name, :inconsistent_parent_rank) + + assert_predicate(qc, :scope) + assert_not_predicate(qc, :failure) + end + + test 'inconsistent_parent_rank flags a parent more than one rank above' do + name = names(:escherichia_coli).dup + name.parent = names(:nanobdellaceae) + qc = warning(name, :inconsistent_parent_rank) + + assert_predicate(qc, :scope) + assert_predicate(qc, :failure) + end + + test 'inconsistent_parent_rank flags a parent at the same rank' do + name = names(:escherichia_coli).dup + name.parent = names(:bacillus_subtilis) + qc = warning(name, :inconsistent_parent_rank) + + assert_predicate(qc, :scope) + assert_predicate(qc, :failure) + end + + test 'inconsistent_parent_rank is out of scope without a parent rank' do + name = names(:escherichia_coli).dup + name.parent = names(:escherichia).dup + name.parent.rank = nil + qc = warning(name, :inconsistent_parent_rank) + + assert_not_predicate(qc, :scope) + end + + test 'missing_parent flags a non-top-rank name without a placement' do + name = names(:bacillus_subtilis) + qc = warning(name, :missing_parent) + + assert_predicate(qc, :scope) + assert_predicate(qc, :failure) + end + + test 'missing_parent accepts an incertae sedis placement' do + name = names(:luteria_ianthellae) + Placement.create!( + name: name, preferred: true, + incertae_sedis: 'incertae sedis (Bacteria)', + incertae_sedis_text: 'Its precise placement within Bacteria is unknown.' + ) + qc = warning(name, :missing_parent) + + assert_predicate(qc, :scope) + assert_not_predicate(qc, :failure) + end + test 'binary_name_above_species flags a multi-word genus name' do name = Name.new(name: 'Testimonas exampleensis', rank: 'genus') From 6c8d039bf63458f99f1959585d017c1532d0906c Mon Sep 17 00:00:00 2001 From: vangberg Date: Fri, 28 Aug 2026 09:16:01 +0200 Subject: [PATCH 02/11] Store placement incertae sedis status as a boolean --- app/models/name/quality_checks.rb | 18 ++++++- app/models/placement.rb | 9 ++-- ...name_placement_incertae_sedis_to_legacy.rb | 26 ++++++++++ db/schema.rb | 5 +- test/fixtures/names.yml | 28 +++++++++++ test/fixtures/placements.yml | 26 +++++++++- test/models/name/quality_checks_test.rb | 38 +++++++++++++++ test/models/name_test.rb | 2 +- test/models/placement_test.rb | 47 +++++++++++++++++-- test/models/tutorial_test.rb | 2 +- 10 files changed, 186 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20260828100000_rename_placement_incertae_sedis_to_legacy.rb diff --git a/app/models/name/quality_checks.rb b/app/models/name/quality_checks.rb index 03268929..e2346361 100644 --- a/app/models/name/quality_checks.rb +++ b/app/models/name/quality_checks.rb @@ -118,11 +118,26 @@ class QcWarning }, area: :nomenclature, rules: %w[7a 7b], - scope: ->(_w, n) { n.rank? && n.parent&.rank? }, + scope: ->(_w, n) { + !n.incertae_sedis? && n.rank? && n.parent&.rank? + }, failure: ->(_w, n) { n.class.ranks.index(n.rank) != n.class.ranks.index(n.parent.rank) + 1 } }.merge(@@link_to_edit_parent), + inconsistent_incertae_sedis_parent_rank: { + message: 'An incertae sedis parent must be at least two ranks above ' \ + 'the name', + area: :nomenclature, + scope: ->(_w, n) { + n.incertae_sedis? && n.rank? && n.parent&.rank? + }, + failure: ->(_w, n) { + name_index = n.class.ranks.index(n.rank) + parent_index = n.class.ranks.index(n.parent.rank) + name_index - parent_index < 2 + } + }.merge(@@link_to_edit_parent), # - Rules 7c and 7d are implied by the structure of the SeqCode Registry # - Recommendation 7 missing_rank: { @@ -1117,6 +1132,7 @@ def qc_warnings large_contig_count low_n50 short_largest_contig missing_source_data inconsistent_type_rank missing_parent inconsistent_type_species inconsistent_parent_rank + inconsistent_incertae_sedis_parent_rank inconsistent_syllabification inconsistent_language binary_name_above_species inconsistent_species_name malformed_subspecies_name reserved_suffix diff --git a/app/models/placement.rb b/app/models/placement.rb index 8856977d..fdfa0f37 100644 --- a/app/models/placement.rb +++ b/app/models/placement.rb @@ -9,12 +9,8 @@ class Placement < ApplicationRecord has_rich_text(:incertae_sedis_text) validates(:incertae_sedis_text, presence: true, if: :incertae_sedis?) - validates(:incertae_sedis, presence: true, allow_nil: true) validates( - :incertae_sedis, absence: { - if: :parent, - message: 'cannot be declared if the parent taxon is set' - } + :incertae_sedis, inclusion: { in: [true, false] } ) validates(:preferred, uniqueness: { scope: :name_id, if: :preferred? }) @@ -23,7 +19,8 @@ class Placement < ApplicationRecord def incertae_sedis_html return '' unless incertae_sedis? - incertae_sedis.gsub(/(incertae sedis)/i, '\\1').html_safe + qualifier = " (#{parent.name})" if parent + "incertae sedis#{qualifier}".html_safe end def downwards? diff --git a/db/migrate/20260828100000_rename_placement_incertae_sedis_to_legacy.rb b/db/migrate/20260828100000_rename_placement_incertae_sedis_to_legacy.rb new file mode 100644 index 00000000..2a83ac43 --- /dev/null +++ b/db/migrate/20260828100000_rename_placement_incertae_sedis_to_legacy.rb @@ -0,0 +1,26 @@ +class RenamePlacementIncertaeSedisToLegacy < ActiveRecord::Migration[6.1] + def up + rename_column :placements, :incertae_sedis, :incertae_sedis_legacy + add_column :placements, :incertae_sedis, :boolean, null: false, default: false + + execute <<~SQL + UPDATE placements + SET incertae_sedis = TRUE + WHERE LOWER(TRIM(incertae_sedis_legacy::text)) NOT IN ('', 'false', 'f', '0') + SQL + + %w[Bacteria Archaea].each do |domain| + execute <<~SQL + UPDATE placements + SET parent_id = (SELECT id FROM names WHERE name = '#{domain}' LIMIT 1) + WHERE LOWER(TRIM(incertae_sedis_legacy::text)) = + 'incertae sedis (#{domain.downcase})' + SQL + end + end + + def down + remove_column :placements, :incertae_sedis + rename_column :placements, :incertae_sedis_legacy, :incertae_sedis + end +end diff --git a/db/schema.rb b/db/schema.rb index 6f900aa0..247174a2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2026_08_19_100000) do +ActiveRecord::Schema.define(version: 2026_08_28_100000) do # These are extensions that must be enabled in order to support this database enable_extension "fuzzystrmatch" @@ -331,9 +331,10 @@ t.boolean "preferred", default: false t.datetime "created_at", precision: 6, null: false t.datetime "updated_at", precision: 6, null: false - t.string "incertae_sedis" + t.string "incertae_sedis_legacy" t.boolean "gtdb_taxonomy" t.boolean "ncbi_taxonomy" + t.boolean "incertae_sedis", default: false, null: false t.index ["name_id"], name: "index_placements_on_name_id" t.index ["publication_id"], name: "index_placements_on_publication_id" end diff --git a/test/fixtures/names.yml b/test/fixtures/names.yml index 147fc169..dc5b4252 100644 --- a/test/fixtures/names.yml +++ b/test/fixtures/names.yml @@ -24,6 +24,34 @@ escherichia_colie: rank: species status: 5 # draft +bacteria: + name: Bacteria + rank: domain + status: 15 # SeqCode + +incertae_sedis_with_distant_parent: + name: Luteria + rank: genus + status: 15 # SeqCode + parent: bacteria + +incertae_sedis_with_immediate_parent: + name: Escherichia fergusonii + rank: species + status: 15 # SeqCode + parent: escherichia + +incertae_sedis_with_same_rank_parent: + name: Escherichia albertii + rank: species + status: 15 # SeqCode + parent: escherichia_coli + +incertae_sedis_without_parent: + name: Nanoclepta + rank: genus + status: 15 # SeqCode + bacillus: name: Bacillus rank: genus diff --git a/test/fixtures/placements.yml b/test/fixtures/placements.yml index 354db503..7a11c621 100644 --- a/test/fixtures/placements.yml +++ b/test/fixtures/placements.yml @@ -17,5 +17,29 @@ escherichia_coli: incertae_sedis: name: incertae_sedis + parent: bacteria preferred: true - incertae_sedis: Incertae sedis (Bacteria) + incertae_sedis: true + +incertae_sedis_with_distant_parent: + name: incertae_sedis_with_distant_parent + parent: bacteria + preferred: true + incertae_sedis: true + +incertae_sedis_with_immediate_parent: + name: incertae_sedis_with_immediate_parent + parent: escherichia + preferred: true + incertae_sedis: true + +incertae_sedis_with_same_rank_parent: + name: incertae_sedis_with_same_rank_parent + parent: escherichia_coli + preferred: true + incertae_sedis: true + +incertae_sedis_without_parent: + name: incertae_sedis_without_parent + preferred: true + incertae_sedis: true diff --git a/test/models/name/quality_checks_test.rb b/test/models/name/quality_checks_test.rb index 0c802f4e..be4768ed 100644 --- a/test/models/name/quality_checks_test.rb +++ b/test/models/name/quality_checks_test.rb @@ -81,6 +81,44 @@ def warning(name, check) assert_not_predicate(qc, :scope) end + test 'inconsistent_parent_rank is out of scope for incertae sedis' do + name = names(:incertae_sedis_with_distant_parent) + qc = warning(name, :inconsistent_parent_rank) + + assert_not_predicate(qc, :scope) + end + + test 'incertae sedis parent rank accepts a parent two or more ranks above' do + name = names(:incertae_sedis_with_distant_parent) + qc = warning(name, :inconsistent_incertae_sedis_parent_rank) + + assert_predicate(qc, :scope) + assert_not_predicate(qc, :failure) + end + + test 'incertae sedis parent rank flags a parent one rank above' do + name = names(:incertae_sedis_with_immediate_parent) + qc = warning(name, :inconsistent_incertae_sedis_parent_rank) + + assert_predicate(qc, :scope) + assert_predicate(qc, :failure) + end + + test 'incertae sedis parent rank flags a parent at the same rank' do + name = names(:incertae_sedis_with_same_rank_parent) + qc = warning(name, :inconsistent_incertae_sedis_parent_rank) + + assert_predicate(qc, :scope) + assert_predicate(qc, :failure) + end + + test 'incertae sedis parent rank is out of scope without a parent' do + name = names(:incertae_sedis_without_parent) + qc = warning(name, :inconsistent_incertae_sedis_parent_rank) + + assert_not_predicate(qc, :scope) + end + test 'missing_parent flags a non-top-rank name without a placement' do name = names(:bacillus_subtilis) qc = warning(name, :missing_parent) diff --git a/test/models/name_test.rb b/test/models/name_test.rb index f023e230..f2e4597a 100644 --- a/test/models/name_test.rb +++ b/test/models/name_test.rb @@ -24,7 +24,7 @@ class NameTest < ActiveSupport::TestCase name = names(:incertae_sedis) assert_equal( - 'Incertae sedis (Bacteria)', name.incertae_sedis_html + 'incertae sedis (Bacteria)', name.incertae_sedis_html ) end end diff --git a/test/models/placement_test.rb b/test/models/placement_test.rb index 4c0aadf1..ea5042a7 100644 --- a/test/models/placement_test.rb +++ b/test/models/placement_test.rb @@ -1,7 +1,48 @@ require 'test_helper' class PlacementTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end + test 'placement has a parent' do + placement = Placement.new( + name: names(:escherichia_coli), parent: names(:escherichia), + incertae_sedis: false + ) + + assert_predicate placement, :valid? + end + + test 'incertae sedis placement can have a parent' do + placement = Placement.new( + name: names(:escherichia), parent: names(:bacteria), incertae_sedis: true, + incertae_sedis_text: 'Its placement within Bacteria is unresolved.' + ) + + assert_predicate placement, :valid? + end + + test 'incertae sedis placement can have no parent' do + placement = Placement.new( + name: names(:escherichia), incertae_sedis: true, + incertae_sedis_text: 'Its placement is unresolved.' + ) + + assert_predicate placement, :valid? + end + + test 'incertae sedis HTML includes its parent' do + placement = Placement.new( + name: names(:escherichia), parent: names(:bacteria), incertae_sedis: true + ) + + assert_equal( + 'incertae sedis (Bacteria)', placement.incertae_sedis_html + ) + end + + test 'incertae sedis HTML without a parent omits the qualifier' do + placement = Placement.new( + name: names(:escherichia), incertae_sedis: true + ) + + assert_equal 'incertae sedis', placement.incertae_sedis_html + end end diff --git a/test/models/tutorial_test.rb b/test/models/tutorial_test.rb index 6146544f..3d4e62ac 100644 --- a/test/models/tutorial_test.rb +++ b/test/models/tutorial_test.rb @@ -133,7 +133,7 @@ class TutorialTest < ActiveSupport::TestCase end name = Name.find(name.id) - assert_equal 'incertae sedis (Bacteria)', name.placement.incertae_sedis + assert_predicate name.placement, :incertae_sedis? assert_not_predicate old_placement.reload, :preferred? assert_includes name.alt_placements, old_placement assert_nil name[:incertae_sedis] From 5fb3cb404d5a12721b1cb1cca1b277316260bb9a Mon Sep 17 00:00:00 2001 From: vangberg Date: Fri, 28 Aug 2026 11:27:29 +0200 Subject: [PATCH 03/11] Require a parent for incertae sedis placements --- app/controllers/placements_controller.rb | 6 ++- app/models/name.rb | 12 ++--- app/models/name/quality_checks.rb | 9 ++-- app/models/placement.rb | 4 +- app/models/tutorial/batch.rb | 41 ++++++++-------- .../controllers/placements_controller_test.rb | 48 ++++++++++++++++++- test/fixtures/names.yml | 10 +--- test/fixtures/placements.yml | 5 -- test/models/name/quality_checks_test.rb | 13 ++--- test/models/placement_test.rb | 24 ++++++---- test/models/tutorial_test.rb | 12 ++--- 11 files changed, 111 insertions(+), 73 deletions(-) diff --git a/app/controllers/placements_controller.rb b/app/controllers/placements_controller.rb index 5977e723..51b40852 100644 --- a/app/controllers/placements_controller.rb +++ b/app/controllers/placements_controller.rb @@ -103,7 +103,8 @@ def create_or_update(back_action) old_placement.update!(preferred: false) end @name.update!( - parent: @placement.parent, assigned_in: @placement.publication, + parent: @placement.incertae_sedis? ? nil : @placement.parent, + assigned_in: @placement.publication, ) end rescue @@ -125,7 +126,8 @@ def prefer @name.placement.try(:update!, preferred: false) @placement.update!(preferred: true) @name.update!( - parent: @placement.parent, assigned_in: @placement.publication + parent: @placement.incertae_sedis? ? nil : @placement.parent, + assigned_in: @placement.publication ) ok = true end diff --git a/app/models/name.rb b/app/models/name.rb index ce71f12d..449aac48 100644 --- a/app/models/name.rb +++ b/app/models/name.rb @@ -128,7 +128,7 @@ class Name < ApplicationRecord message: 'can only contain letters, dashes, dots, and apostrophe' } ) - validates(:incertae_sedis, presence: true, allow_nil: true) + validates(:incertae_sedis, inclusion: { in: [true, false] }, allow_nil: true) validates( :incertae_sedis, absence: { if: :parent, @@ -1466,11 +1466,9 @@ def harmonize_register_and_status end def ensure_consistent_placement - placement_incertae_sedis = incertae_sedis - - if parent_id.present? || placement_incertae_sedis.present? + if parent_id.present? pp = placements.where( - parent_id: parent_id, incertae_sedis: placement_incertae_sedis + parent_id: parent_id, incertae_sedis: false ).first if pp.present? if pp.preferred? @@ -1483,10 +1481,12 @@ def ensure_consistent_placement Placement.new( name_id: id, parent_id: parent_id, - incertae_sedis: placement_incertae_sedis, + incertae_sedis: false, preferred: true ).save end + elsif placements.where(preferred: true, incertae_sedis: true).exists? + true else # Conservatively preserve as alternative placement placements.update(preferred: false) diff --git a/app/models/name/quality_checks.rb b/app/models/name/quality_checks.rb index e2346361..d49460a2 100644 --- a/app/models/name/quality_checks.rb +++ b/app/models/name/quality_checks.rb @@ -130,11 +130,11 @@ class QcWarning 'the name', area: :nomenclature, scope: ->(_w, n) { - n.incertae_sedis? && n.rank? && n.parent&.rank? + n.incertae_sedis? && n.rank? && n.placement.parent&.rank? }, failure: ->(_w, n) { name_index = n.class.ranks.index(n.rank) - parent_index = n.class.ranks.index(n.parent.rank) + parent_index = n.class.ranks.index(n.placement.parent.rank) name_index - parent_index < 2 } }.merge(@@link_to_edit_parent), @@ -156,7 +156,10 @@ class QcWarning link_to: ->(_w, n) { [:edit_parent, n] }, recommendations: %w[7], scope: ->(_w, n) { n.rank? && !n.top_rank? }, - failure: ->(_w, n) { !n.incertae_sedis? && !n.parent.present? } + failure: ->(_w, n) { + parent = n.incertae_sedis? ? n.placement.parent : n.parent + !parent.present? + } }, # Section 3. Naming of Taxa diff --git a/app/models/placement.rb b/app/models/placement.rb index fdfa0f37..d9bc9a61 100644 --- a/app/models/placement.rb +++ b/app/models/placement.rb @@ -5,7 +5,7 @@ class Placement < ApplicationRecord ) belongs_to(:publication, optional: true) validates(:name, presence: true) - validates(:parent, presence: true, unless: :incertae_sedis?) + validates(:parent, presence: true) has_rich_text(:incertae_sedis_text) validates(:incertae_sedis_text, presence: true, if: :incertae_sedis?) @@ -33,6 +33,6 @@ def downwards? private def harmonize_name_parent - name.update(parent: parent) if preferred + name.update(parent: incertae_sedis? ? nil : parent) if preferred end end diff --git a/app/models/tutorial/batch.rb b/app/models/tutorial/batch.rb index 09fb983e..7ce6993b 100644 --- a/app/models/tutorial/batch.rb +++ b/app/models/tutorial/batch.rb @@ -54,9 +54,11 @@ def param_names # Deal with foreign keys if j['parent'] && - j['parent'] =~ /^incertae sedis( \((Archaea|Bacteria)\))?/ - j['incertae_sedis'] = j['parent'] - j['parent'] = nil + match = j['parent'].match( + /^incertae sedis( \((Archaea|Bacteria)\))?/ + ) + j['incertae_sedis'] = true + j['parent'] = match[2] end j['parent'] &&= Name.new(name: j['parent']) if j['nomenclatural_type_type'].to_s.downcase == 'name' && @@ -348,7 +350,6 @@ def batch_step_01(params, user) default_pars = { status: 0, created_by: user } param_names.each do |par| new_par = {} - placement_par = nil # Parents if par['parent'] @@ -357,13 +358,11 @@ def batch_step_01(params, user) parent = Name.new(default_pars.merge(name: par['parent'].name)) parent.save! end - placement_par = { parent: parent, incertae_sedis: nil } - elsif par['incertae_sedis'] - placement_par = { - parent: nil, - incertae_sedis: par['incertae_sedis'], + save_batch_placement( + Name.find_by_variants(par['name']), + parent: parent, incertae_sedis: par['incertae_sedis'] || false, incertae_sedis_text: par['description'] - } + ) end # Nomenclatural types @@ -384,16 +383,7 @@ def batch_step_01(params, user) end end - name = Name.find_by_variants(par['name']) - name.update!(new_par) - if placement_par - placement = name.placements.find_or_initialize_by( - parent: placement_par[:parent], - incertae_sedis: placement_par[:incertae_sedis] - ) - name.placements.where.not(id: placement.id).update_all(preferred: false) - placement.update!(placement_par.merge(preferred: true)) - end + Name.find_by_variants(par['name']).update!(new_par) end # If all is good, go to next step @@ -411,4 +401,15 @@ def batch_step_02(params, user) @next_action = [:new_register, tutorial: self] end + private + + def save_batch_placement(name, attributes) + placement = name.placements.find_or_initialize_by( + attributes.slice(:parent) + ) + name.placements.where(preferred: true).where.not(id: placement.id) + .find_each { |current| current.update!(preferred: false) } + placement.update!(attributes.merge(preferred: true)) + end + end diff --git a/test/controllers/placements_controller_test.rb b/test/controllers/placements_controller_test.rb index 3e4ba8f6..077b50db 100644 --- a/test/controllers/placements_controller_test.rb +++ b/test/controllers/placements_controller_test.rb @@ -1,8 +1,54 @@ require 'test_helper' class PlacementsControllerTest < ActionDispatch::IntegrationTest + include Devise::Test::IntegrationHelpers + setup do - @placement = placements(:one) + @curator = users(:curator) + @name = names(:draft_by_contributor) + sign_in(@curator) + end + + test 'creates a preferred incertae sedis placement' do + assert_difference('Placement.count', 1) do + post placements_url, params: { + placement: { + name_id: @name.id, + parent: names(:bacteria).name, + incertae_sedis: '1', + incertae_sedis_text: 'No reliable higher placement is known.' + } + } + end + + placement = @name.reload.placement + assert_redirected_to name_url(@name) + assert_predicate placement, :preferred? + assert_equal names(:bacteria), placement.parent + assert_predicate placement, :incertae_sedis? + assert_equal( + 'No reliable higher placement is known.', + placement.incertae_sedis_text.to_plain_text + ) + assert_nil @name.parent end + test 'creates a preferred non-incertae sedis placement' do + assert_difference('Placement.count', 1) do + post placements_url, params: { + placement: { + name_id: @name.id, + parent: names(:escherichia).name, + incertae_sedis: '0' + } + } + end + + placement = @name.reload.placement + assert_redirected_to name_url(@name) + assert_predicate placement, :preferred? + assert_equal names(:escherichia), placement.parent + assert_not_predicate placement, :incertae_sedis? + assert_equal names(:escherichia), @name.parent + end end diff --git a/test/fixtures/names.yml b/test/fixtures/names.yml index dc5b4252..32c85686 100644 --- a/test/fixtures/names.yml +++ b/test/fixtures/names.yml @@ -30,27 +30,19 @@ bacteria: status: 15 # SeqCode incertae_sedis_with_distant_parent: - name: Luteria + name: Testimonas rank: genus status: 15 # SeqCode - parent: bacteria incertae_sedis_with_immediate_parent: name: Escherichia fergusonii rank: species status: 15 # SeqCode - parent: escherichia incertae_sedis_with_same_rank_parent: name: Escherichia albertii rank: species status: 15 # SeqCode - parent: escherichia_coli - -incertae_sedis_without_parent: - name: Nanoclepta - rank: genus - status: 15 # SeqCode bacillus: name: Bacillus diff --git a/test/fixtures/placements.yml b/test/fixtures/placements.yml index 7a11c621..30b22d42 100644 --- a/test/fixtures/placements.yml +++ b/test/fixtures/placements.yml @@ -38,8 +38,3 @@ incertae_sedis_with_same_rank_parent: parent: escherichia_coli preferred: true incertae_sedis: true - -incertae_sedis_without_parent: - name: incertae_sedis_without_parent - preferred: true - incertae_sedis: true diff --git a/test/models/name/quality_checks_test.rb b/test/models/name/quality_checks_test.rb index be4768ed..19a75174 100644 --- a/test/models/name/quality_checks_test.rb +++ b/test/models/name/quality_checks_test.rb @@ -112,13 +112,6 @@ def warning(name, check) assert_predicate(qc, :failure) end - test 'incertae sedis parent rank is out of scope without a parent' do - name = names(:incertae_sedis_without_parent) - qc = warning(name, :inconsistent_incertae_sedis_parent_rank) - - assert_not_predicate(qc, :scope) - end - test 'missing_parent flags a non-top-rank name without a placement' do name = names(:bacillus_subtilis) qc = warning(name, :missing_parent) @@ -127,11 +120,11 @@ def warning(name, check) assert_predicate(qc, :failure) end - test 'missing_parent accepts an incertae sedis placement' do + test 'missing_parent accepts an incertae sedis placement with a parent' do name = names(:luteria_ianthellae) Placement.create!( - name: name, preferred: true, - incertae_sedis: 'incertae sedis (Bacteria)', + name: name, parent: names(:bacteria), preferred: true, + incertae_sedis: true, incertae_sedis_text: 'Its precise placement within Bacteria is unknown.' ) qc = warning(name, :missing_parent) diff --git a/test/models/placement_test.rb b/test/models/placement_test.rb index ea5042a7..4a542930 100644 --- a/test/models/placement_test.rb +++ b/test/models/placement_test.rb @@ -19,13 +19,26 @@ class PlacementTest < ActiveSupport::TestCase assert_predicate placement, :valid? end - test 'incertae sedis placement can have no parent' do + test 'preferred incertae sedis parent is not synced to the name' do + name = names(:escherichia) + placement = Placement.create!( + name: name, parent: names(:bacteria), incertae_sedis: true, + incertae_sedis_text: 'Its placement within Bacteria is unresolved.', + preferred: true + ) + + assert_equal names(:bacteria), placement.parent + assert_nil name.reload.parent + end + + test 'incertae sedis placement must have a parent' do placement = Placement.new( name: names(:escherichia), incertae_sedis: true, incertae_sedis_text: 'Its placement is unresolved.' ) - assert_predicate placement, :valid? + assert_not_predicate placement, :valid? + assert_includes placement.errors[:parent], "can't be blank" end test 'incertae sedis HTML includes its parent' do @@ -38,11 +51,4 @@ class PlacementTest < ActiveSupport::TestCase ) end - test 'incertae sedis HTML without a parent omits the qualifier' do - placement = Placement.new( - name: names(:escherichia), incertae_sedis: true - ) - - assert_equal 'incertae sedis', placement.incertae_sedis_html - end end diff --git a/test/models/tutorial_test.rb b/test/models/tutorial_test.rb index 3d4e62ac..757cac5f 100644 --- a/test/models/tutorial_test.rb +++ b/test/models/tutorial_test.rb @@ -15,7 +15,7 @@ class TutorialTest < ActiveSupport::TestCase assert_nil name[:incertae_sedis] assert_same name, placement.name assert_equal 'Nanobdellaceae', placement.parent.name - assert_nil placement.incertae_sedis + assert_not_predicate placement, :incertae_sedis? assert_empty placement.incertae_sedis_text.to_plain_text end @@ -35,8 +35,8 @@ class TutorialTest < ActiveSupport::TestCase ) assert_nil name.parent assert_nil name[:incertae_sedis] - assert_nil placement.parent - assert_equal 'incertae sedis (Bacteria)', placement.incertae_sedis + assert_equal names(:bacteria).name, placement.parent.name + assert_predicate placement, :incertae_sedis? assert_equal( tutorial.value(:names).first['description'], placement.incertae_sedis_text.to_plain_text @@ -56,7 +56,7 @@ class TutorialTest < ActiveSupport::TestCase assert_predicate placement, :preferred? assert_equal parent, placement.parent assert_equal parent, name.parent - assert_nil placement.incertae_sedis + assert_not_predicate placement, :incertae_sedis? end test 'batch updates a claimable name with a preferred placement' do @@ -110,9 +110,9 @@ class TutorialTest < ActiveSupport::TestCase assert_not_nil placement assert_predicate placement, :preferred? - assert_nil placement.parent + assert_equal names(:bacteria), placement.parent assert_nil name.parent - assert_equal 'incertae sedis (Bacteria)', placement.incertae_sedis + assert_predicate placement, :incertae_sedis? assert_equal explanation, placement.incertae_sedis_text.to_plain_text assert_nil name[:incertae_sedis] end From 86c44cb2ef8c4ac6d157a315fe1eaa71927d44dc Mon Sep 17 00:00:00 2001 From: vangberg Date: Wed, 2 Sep 2026 14:33:36 +0200 Subject: [PATCH 04/11] Add fixed and incertae sedis placement selector --- app/assets/stylesheets/custom.scss | 21 ++- app/controllers/placements_controller.rb | 3 +- app/javascript/packs/application.js | 1 - app/javascript/packs/autocomplete.js | 1 + app/models/placement.rb | 5 + app/views/placements/_form.html.erb | 128 ++++++++++++------ .../controllers/placements_controller_test.rb | 2 +- test/models/placement_test.rb | 12 ++ test/system/placements_test.rb | 18 ++- test/system/tutorials_test.rb | 7 +- 10 files changed, 140 insertions(+), 58 deletions(-) diff --git a/app/assets/stylesheets/custom.scss b/app/assets/stylesheets/custom.scss index eaaa3f8c..2d7721c5 100644 --- a/app/assets/stylesheets/custom.scss +++ b/app/assets/stylesheets/custom.scss @@ -60,6 +60,26 @@ input[type=checkbox] { accent-color: theme-color-level(primary, 0); } +.easy-autocomplete { + width: 100%; +} + +.placement-form { + #incertae-sedis { + display: none; + } + + &:has([name="placement[incertae_sedis]"][value="true"]:checked) { + #standard-placement { + display: none; + } + + #incertae-sedis { + display: block; + } + } +} + #dashboard-actions, .btn { .fa, .fas, .far, .fal, .fad, .fab { min-width: 1.5em; @@ -378,4 +398,3 @@ nav.navbar { [data-rank=g] { color: hsl(280deg, 80%, 30%); } [data-rank=s] { color: hsl(320deg, 80%, 30%); } } - diff --git a/app/controllers/placements_controller.rb b/app/controllers/placements_controller.rb index 51b40852..9cc8fc36 100644 --- a/app/controllers/placements_controller.rb +++ b/app/controllers/placements_controller.rb @@ -46,10 +46,11 @@ def update def create_or_update(back_action) par = params.require(:placement).permit( :incertae_sedis, :incertae_sedis_text, - :parent, :preferred, :publication + :parent, :incertae_sedis_parent, :preferred, :publication ) @placement.incertae_sedis = par[:incertae_sedis].presence @placement.incertae_sedis_text = par[:incertae_sedis_text] + par[:parent] = par[:incertae_sedis_parent] if @placement.incertae_sedis? if @name.placement @placement.preferred = par[:preferred] if current_user.curator? else diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index 82a89a43..ead3615e 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -29,4 +29,3 @@ import "packs/network"; import "packs/styling"; import "packs/copyable"; import "channels/index.js"; - diff --git a/app/javascript/packs/autocomplete.js b/app/javascript/packs/autocomplete.js index 7490ca85..1490b561 100644 --- a/app/javascript/packs/autocomplete.js +++ b/app/javascript/packs/autocomplete.js @@ -1,6 +1,7 @@ $(document).on("turbolinks:load", function() { var eac_options = { + adjustWidth: false, url: function(phrase) { var data = $($(event)[0]["srcElement"]).data(); var what = data["autocomplete"]; diff --git a/app/models/placement.rb b/app/models/placement.rb index d9bc9a61..a2f81c88 100644 --- a/app/models/placement.rb +++ b/app/models/placement.rb @@ -23,6 +23,11 @@ def incertae_sedis_html "incertae sedis#{qualifier}".html_safe end + def incertae_sedis_parent_rank + rank_index = name&.rank_index + Name.ranks[rank_index - 2] if rank_index && rank_index >= 2 + end + def downwards? return false unless parent.present? # e.g., incertae sedis diff --git a/app/views/placements/_form.html.erb b/app/views/placements/_form.html.erb index 5725a901..e57e31a7 100644 --- a/app/views/placements/_form.html.erb +++ b/app/views/placements/_form.html.erb @@ -14,29 +14,95 @@ <%= simple_form_for( @placement, url: @placement.persisted? ? @placement : placements_url, - html: { method: @placement.persisted? ? :patch : :post } + html: { + method: @placement.persisted? ? :patch : :post, + class: 'placement-form' + } ) do |f| %> <%= f.input(:name_id, as: :hidden) %> -

Parent taxon

-

- Indicate the name of the parent taxon in the rank of - <%= @name.expected_parent_rank %> -

- <%= - f.input( - :parent, autofocus: true, - label: @name.expected_parent_rank.try(:capitalize), - input_html: { - value: @placement.parent.try(:name), - data: { - behavior: 'autocomplete', autocomplete: 'names', - rank: @name.expected_parent_rank +

Placement type

+

Choose how precisely this taxon can be placed.

+
+
+ +
+
+ +
+
+ +
+ Placement details +
+
+ <%= + f.input( + :parent, autofocus: !@placement.incertae_sedis?, + label: "Parent #{@name.expected_parent_rank}", + input_html: { + id: 'standard-placement-parent', + value: @placement.parent.try(:name), + data: { + behavior: 'autocomplete', autocomplete: 'names', + rank: @name.expected_parent_rank + } } - } - ) - %> + ) + %> +
+ +
+ <%= + f.input( + :parent, autofocus: @placement.incertae_sedis?, + label: 'Uncertain within', + hint: "Select a taxon at least two ranks above #{@name.rank}.", + input_html: { + id: 'incertae-sedis-parent', + name: 'placement[incertae_sedis_parent]', + value: @placement.parent.try(:name), + data: { + behavior: 'autocomplete', autocomplete: 'names', + minimum_rank: @placement.incertae_sedis_parent_rank + } + } + ) + %> + <%= + f.input( + :incertae_sedis_text, + label: 'Description of the classification problems', + as: :rich_text_area + ) + %> +
+
+
+ <%= f.input( :publication, @@ -56,32 +122,6 @@ <% end %>
-
-

Or declare incertae sedis

-

- Alternatively, you can indicate that the classification of this taxon is - uncertain and why -

- <%= - f.input( - :incertae_sedis, - collection: [ - 'Incertae sedis', - 'Incertae sedis (Bacteria)', - 'Incertae sedis (Archaea)' - ] - ) - %> - <%= - f.input( - :incertae_sedis_text, - label: 'Description of the classification problems', - as: :rich_text_area - ) - %> -
-
- <%= f.button(:submit, 'Submit') %> <%= link_to('Cancel', @name, role: 'button', class: 'btn btn-secondary') %>

diff --git a/test/controllers/placements_controller_test.rb b/test/controllers/placements_controller_test.rb index 077b50db..4231ac99 100644 --- a/test/controllers/placements_controller_test.rb +++ b/test/controllers/placements_controller_test.rb @@ -14,7 +14,7 @@ class PlacementsControllerTest < ActionDispatch::IntegrationTest post placements_url, params: { placement: { name_id: @name.id, - parent: names(:bacteria).name, + incertae_sedis_parent: names(:bacteria).name, incertae_sedis: '1', incertae_sedis_text: 'No reliable higher placement is known.' } diff --git a/test/models/placement_test.rb b/test/models/placement_test.rb index 4a542930..10d57aea 100644 --- a/test/models/placement_test.rb +++ b/test/models/placement_test.rb @@ -19,6 +19,18 @@ class PlacementTest < ActiveSupport::TestCase assert_predicate placement, :valid? end + test 'incertae sedis parent rank is at least two ranks above the name' do + placement = Placement.new(name: names(:escherichia_coli)) + + assert_equal 'family', placement.incertae_sedis_parent_rank + end + + test 'incertae sedis parent rank is nil without two higher ranks' do + placement = Placement.new(name: names(:bacteria)) + + assert_nil placement.incertae_sedis_parent_rank + end + test 'preferred incertae sedis parent is not synced to the name' do name = names(:escherichia) placement = Placement.create!( diff --git a/test/system/placements_test.rb b/test/system/placements_test.rb index 49284cd9..b4ac6cc2 100644 --- a/test/system/placements_test.rb +++ b/test/system/placements_test.rb @@ -16,14 +16,15 @@ class PlacementsTest < ApplicationSystemTestCase test 'creating the first placement for a name' do name = Name.create!( - name: 'Escherichia fergusonii', rank: 'species', status: 5, + name: 'Escherichia vulneris', rank: 'species', status: 5, created_by: @user ) assert_nil(name.placement) visit new_placement_path(name) - fill_in 'Genus', with: @parent.name + choose 'Fixed placement' + fill_in 'Parent genus', with: @parent.name click_button 'Submit' assert_text 'Placement successfully updated' @@ -37,13 +38,14 @@ class PlacementsTest < ApplicationSystemTestCase test 'declaring incertae sedis in Bacteria' do name = Name.create!( - name: 'Escherichia fergusonii', rank: 'species', status: 5, + name: 'Escherichia ruysiae', rank: 'species', status: 5, created_by: @user ) explanation = 'Its placement within Bacteria is unresolved.' visit new_placement_path(name) - select 'Incertae sedis (Bacteria)', from: 'Incertae sedis' + choose 'Incertae sedis' + fill_in 'Uncertain within', with: names(:bacteria).name fill_in_rich_text_area( 'Description of the classification problems', with: explanation ) @@ -55,9 +57,10 @@ class PlacementsTest < ApplicationSystemTestCase name.reload placement = name.placement - assert_equal('Incertae sedis (Bacteria)', placement.incertae_sedis) + assert_predicate placement, :incertae_sedis? + assert_equal names(:bacteria), placement.parent assert_equal(explanation, placement.incertae_sedis_text.to_plain_text) - assert_nil(placement.parent) + assert_nil name.parent assert_predicate(placement, :preferred?) end @@ -69,7 +72,8 @@ class PlacementsTest < ApplicationSystemTestCase visit name_path(name) click_link 'Report alternative placement' - fill_in 'Genus', with: alternative_parent.name + choose 'Fixed placement' + fill_in 'Parent genus', with: alternative_parent.name click_button 'Submit' assert_text 'Placement successfully updated' diff --git a/test/system/tutorials_test.rb b/test/system/tutorials_test.rb index cd521986..d0a79f30 100644 --- a/test/system/tutorials_test.rb +++ b/test/system/tutorials_test.rb @@ -44,17 +44,18 @@ class TutorialsTest < ApplicationSystemTestCase visit tutorial_url(@tutorial) upload_batch_spreadsheet('SCR_UploadBatch-Luteria-Incertae_sedis.xlsx') - incertae_sedis = 'incertae sedis (Bacteria)' - assert_batch_review(name: 'Luteria', parent: incertae_sedis) + assert_batch_review(name: 'Luteria', parent: 'Bacteria') create_batch_entries genus = Name.find_by!(name: 'Luteria') + placement = genus.placement @tutorial.reload assert_equal 'genus', genus.rank assert_nil genus.parent - assert_equal incertae_sedis, genus.incertae_sedis + assert_equal names(:bacteria), placement.parent + assert_predicate placement, :incertae_sedis? assert_equal type_species, genus.nomenclatural_type assert_equal @tutorial, genus.tutorial assert_equal ['Luteria'], @tutorial.names.pluck(:name) From ca87607a800c6b1d70b7889c490f547c518b303f Mon Sep 17 00:00:00 2001 From: vangberg Date: Tue, 8 Sep 2026 11:04:05 +0200 Subject: [PATCH 05/11] Inline batch tutorial placement saving --- app/models/tutorial/batch.rb | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/app/models/tutorial/batch.rb b/app/models/tutorial/batch.rb index 7ce6993b..527a344c 100644 --- a/app/models/tutorial/batch.rb +++ b/app/models/tutorial/batch.rb @@ -358,10 +358,13 @@ def batch_step_01(params, user) parent = Name.new(default_pars.merge(name: par['parent'].name)) parent.save! end - save_batch_placement( - Name.find_by_variants(par['name']), - parent: parent, incertae_sedis: par['incertae_sedis'] || false, - incertae_sedis_text: par['description'] + name = Name.find_by_variants(par['name']) + placement = name.placements.find_or_initialize_by(parent: parent) + name.placements.where(preferred: true).where.not(id: placement.id) + .find_each { |current| current.update!(preferred: false) } + placement.update!( + incertae_sedis: par['incertae_sedis'] || false, + incertae_sedis_text: par['description'], preferred: true ) end @@ -401,15 +404,4 @@ def batch_step_02(params, user) @next_action = [:new_register, tutorial: self] end - private - - def save_batch_placement(name, attributes) - placement = name.placements.find_or_initialize_by( - attributes.slice(:parent) - ) - name.placements.where(preferred: true).where.not(id: placement.id) - .find_each { |current| current.update!(preferred: false) } - placement.update!(attributes.merge(preferred: true)) - end - end From 3938050ad3ac25a9a59aa03e6b9e94724a92870c Mon Sep 17 00:00:00 2001 From: vangberg Date: Tue, 8 Sep 2026 11:15:07 +0200 Subject: [PATCH 06/11] Unify parent rank restrictions with allowed ranks autocomplete --- app/controllers/names_controller.rb | 11 ++++----- app/javascript/packs/autocomplete.js | 5 ++-- app/models/placement.rb | 10 ++++++-- app/views/dev/autocomplete.html.erb | 2 +- app/views/placements/_form.html.erb | 4 ++-- test/controllers/names_controller_test.rb | 21 +++++++++++++--- test/models/placement_test.rb | 29 ++++++++++++++++++----- 7 files changed, 58 insertions(+), 24 deletions(-) diff --git a/app/controllers/names_controller.rb b/app/controllers/names_controller.rb index 217258d1..8252c947 100644 --- a/app/controllers/names_controller.rb +++ b/app/controllers/names_controller.rb @@ -52,19 +52,16 @@ class NamesController < ApplicationController before_action(:authenticate_can_edit_validated!, only: %i[update edit_links]) # GET /names/autocomplete.json?q=Maco - # GET /names/autocomplete.json?q=Allo&rank=genus - # GET /names/autocomplete.json?q=Pseu&minimum_rank=class + # GET /names/autocomplete.json?q=Allo&ranks=genus + # GET /names/autocomplete.json?q=Pseu&ranks=domain,phylum,class def autocomplete name = params[:q].downcase - rank = params[:rank]&.downcase - minimum_rank = params[:minimum_rank]&.downcase @names = Name.where('LOWER(name) LIKE ?', "#{name}%") .or(Name.where('LOWER(name) LIKE ?', "% #{name}%")) .limit(20) - @names = @names.where(rank: rank) if rank - if minimum_rank && (rank_index = Name.ranks.index(minimum_rank)) - @names = @names.where(rank: Name.ranks.take(rank_index + 1)) + if params.key?(:ranks) + @names = @names.where(rank: params[:ranks].downcase.split(',')) end @names = @names.where(redirect: nil) end diff --git a/app/javascript/packs/autocomplete.js b/app/javascript/packs/autocomplete.js index 1490b561..cba1cbcf 100644 --- a/app/javascript/packs/autocomplete.js +++ b/app/javascript/packs/autocomplete.js @@ -7,9 +7,8 @@ $(document).on("turbolinks:load", function() { var what = data["autocomplete"]; var url = new URL(what + "/autocomplete.json", ROOT_PATH); url.searchParams.set("q", phrase); - if(data["rank"]) { url.searchParams.set("rank", data["rank"]); } - if(data["minimumRank"]) { - url.searchParams.set("minimum_rank", data["minimumRank"]); + if(data["ranks"] !== undefined) { + url.searchParams.set("ranks", data["ranks"].join(",")); } return url.toString(); }, diff --git a/app/models/placement.rb b/app/models/placement.rb index a2f81c88..4cb6c1a1 100644 --- a/app/models/placement.rb +++ b/app/models/placement.rb @@ -23,9 +23,15 @@ def incertae_sedis_html "incertae sedis#{qualifier}".html_safe end - def incertae_sedis_parent_rank + def allowed_parent_ranks(incertae_sedis: incertae_sedis?) rank_index = name&.rank_index - Name.ranks[rank_index - 2] if rank_index && rank_index >= 2 + return [] unless rank_index && rank_index.positive? + + if incertae_sedis + Name.ranks.take(rank_index - 1) + else + [Name.ranks[rank_index - 1]] + end end def downwards? diff --git a/app/views/dev/autocomplete.html.erb b/app/views/dev/autocomplete.html.erb index d02e6577..ff0fd50e 100644 --- a/app/views/dev/autocomplete.html.erb +++ b/app/views/dev/autocomplete.html.erb @@ -26,7 +26,7 @@ + data-autocomplete="names" data-ranks='["phylum"]'> diff --git a/app/views/placements/_form.html.erb b/app/views/placements/_form.html.erb index e57e31a7..29486cab 100644 --- a/app/views/placements/_form.html.erb +++ b/app/views/placements/_form.html.erb @@ -68,7 +68,7 @@ value: @placement.parent.try(:name), data: { behavior: 'autocomplete', autocomplete: 'names', - rank: @name.expected_parent_rank + ranks: @placement.allowed_parent_ranks(incertae_sedis: false) } } ) @@ -87,7 +87,7 @@ value: @placement.parent.try(:name), data: { behavior: 'autocomplete', autocomplete: 'names', - minimum_rank: @placement.incertae_sedis_parent_rank + ranks: @placement.allowed_parent_ranks(incertae_sedis: true) } } ) diff --git a/test/controllers/names_controller_test.rb b/test/controllers/names_controller_test.rb index 4a63d187..d0dbd8ec 100644 --- a/test/controllers/names_controller_test.rb +++ b/test/controllers/names_controller_test.rb @@ -44,7 +44,7 @@ class NamesControllerTest < ActionDispatch::IntegrationTest test 'autocomplete filters names to an exact rank' do get autocomplete_names_url( - format: :json, q: 'Bacill', rank: 'phylum' + format: :json, q: 'Bacill', ranks: 'phylum' ) assert_response :success @@ -54,15 +54,30 @@ class NamesControllerTest < ActionDispatch::IntegrationTest assert_includes entry['display'], '>phylum' end - test 'autocomplete filters names to a minimum rank level' do + test 'autocomplete filters names to allowed ranks' do get autocomplete_names_url( - format: :json, q: 'Bacill', minimum_rank: 'class' + format: :json, q: 'Bacill', ranks: 'domain,phylum,class' ) assert_response :success assert_equal %w[Bacilli Bacillota], autocomplete_values.sort end + test 'autocomplete returns no names for an empty ranks list' do + get autocomplete_names_url(format: :json, q: 'Escherichia', ranks: '') + + assert_response :success + assert_empty autocomplete_values + end + + test 'autocomplete without ranks includes all matching ranks' do + get autocomplete_names_url(format: :json, q: 'Escherichia') + + assert_response :success + assert_includes autocomplete_values, names(:escherichia).name + assert_includes autocomplete_values, names(:escherichia_coli).name + end + test 'add_paratype_strain_commit ignores a tampered name_id' do sign_in(users(:curator)) target_name = names(:unregistered) diff --git a/test/models/placement_test.rb b/test/models/placement_test.rb index 10d57aea..87fbf43d 100644 --- a/test/models/placement_test.rb +++ b/test/models/placement_test.rb @@ -19,16 +19,33 @@ class PlacementTest < ActiveSupport::TestCase assert_predicate placement, :valid? end - test 'incertae sedis parent rank is at least two ranks above the name' do - placement = Placement.new(name: names(:escherichia_coli)) + test 'fixed placement allows only the immediate parent rank' do + placement = Placement.new(name: names(:escherichia_coli), incertae_sedis: false) - assert_equal 'family', placement.incertae_sedis_parent_rank + assert_equal ['genus'], placement.allowed_parent_ranks end - test 'incertae sedis parent rank is nil without two higher ranks' do - placement = Placement.new(name: names(:bacteria)) + test 'incertae sedis allows all ranks at least two above the name' do + placement = Placement.new(name: names(:escherichia_coli), incertae_sedis: true) - assert_nil placement.incertae_sedis_parent_rank + assert_equal %w[domain phylum class order family], placement.allowed_parent_ranks + assert_equal ['genus'], placement.allowed_parent_ranks(incertae_sedis: false) + end + + test 'placement allows no parent ranks without higher ranks or a name' do + [names(:bacteria), nil].each do |name| + placement = Placement.new(name: name) + + assert_empty placement.allowed_parent_ranks(incertae_sedis: false) + assert_empty placement.allowed_parent_ranks(incertae_sedis: true) + end + end + + test 'phylum allows a fixed domain parent but no incertae sedis parent' do + placement = Placement.new(name: Name.new(name: 'Bacillota', rank: 'phylum')) + + assert_equal ['domain'], placement.allowed_parent_ranks(incertae_sedis: false) + assert_empty placement.allowed_parent_ranks(incertae_sedis: true) end test 'preferred incertae sedis parent is not synced to the name' do From 33154e7aa770b8b336ea21fab2c48f2fd2d4afc1 Mon Sep 17 00:00:00 2001 From: vangberg Date: Tue, 8 Sep 2026 11:28:43 +0200 Subject: [PATCH 07/11] Unify parent rank quality checks using Placement allowed ranks --- app/models/name/quality_checks.rb | 20 +++----------- test/models/name/quality_checks_test.rb | 35 +++++++++++++++---------- 2 files changed, 24 insertions(+), 31 deletions(-) diff --git a/app/models/name/quality_checks.rb b/app/models/name/quality_checks.rb index d49460a2..76f60aa8 100644 --- a/app/models/name/quality_checks.rb +++ b/app/models/name/quality_checks.rb @@ -112,30 +112,17 @@ class QcWarning inconsistent_parent_rank: { message: ->(_w, n) { <<~MSG - The parent rank (#{n.parent.inferred_rank}) is inconsistent + The parent rank (#{n.placement.parent.inferred_rank}) is inconsistent with the rank of this name (#{n.inferred_rank}) MSG }, area: :nomenclature, rules: %w[7a 7b], scope: ->(_w, n) { - !n.incertae_sedis? && n.rank? && n.parent&.rank? + n.rank? && n.placement&.parent&.rank? }, failure: ->(_w, n) { - n.class.ranks.index(n.rank) != n.class.ranks.index(n.parent.rank) + 1 - } - }.merge(@@link_to_edit_parent), - inconsistent_incertae_sedis_parent_rank: { - message: 'An incertae sedis parent must be at least two ranks above ' \ - 'the name', - area: :nomenclature, - scope: ->(_w, n) { - n.incertae_sedis? && n.rank? && n.placement.parent&.rank? - }, - failure: ->(_w, n) { - name_index = n.class.ranks.index(n.rank) - parent_index = n.class.ranks.index(n.placement.parent.rank) - name_index - parent_index < 2 + !n.placement.allowed_parent_ranks.include?(n.placement.parent.rank) } }.merge(@@link_to_edit_parent), # - Rules 7c and 7d are implied by the structure of the SeqCode Registry @@ -1135,7 +1122,6 @@ def qc_warnings large_contig_count low_n50 short_largest_contig missing_source_data inconsistent_type_rank missing_parent inconsistent_type_species inconsistent_parent_rank - inconsistent_incertae_sedis_parent_rank inconsistent_syllabification inconsistent_language binary_name_above_species inconsistent_species_name malformed_subspecies_name reserved_suffix diff --git a/test/models/name/quality_checks_test.rb b/test/models/name/quality_checks_test.rb index 19a75174..f2be34a9 100644 --- a/test/models/name/quality_checks_test.rb +++ b/test/models/name/quality_checks_test.rb @@ -55,8 +55,8 @@ def warning(name, check) end test 'inconsistent_parent_rank flags a parent more than one rank above' do - name = names(:escherichia_coli).dup - name.parent = names(:nanobdellaceae) + name = names(:escherichia_coli) + name.placement.parent = Name.new(name: 'Enterobacteriaceae', rank: 'family') qc = warning(name, :inconsistent_parent_rank) assert_predicate(qc, :scope) @@ -64,8 +64,8 @@ def warning(name, check) end test 'inconsistent_parent_rank flags a parent at the same rank' do - name = names(:escherichia_coli).dup - name.parent = names(:bacillus_subtilis) + name = names(:escherichia_coli) + name.placement.parent = names(:bacillus_subtilis) qc = warning(name, :inconsistent_parent_rank) assert_predicate(qc, :scope) @@ -73,24 +73,29 @@ def warning(name, check) end test 'inconsistent_parent_rank is out of scope without a parent rank' do - name = names(:escherichia_coli).dup - name.parent = names(:escherichia).dup - name.parent.rank = nil + name = names(:escherichia_coli) + name.placement.parent.rank = nil qc = warning(name, :inconsistent_parent_rank) assert_not_predicate(qc, :scope) end - test 'inconsistent_parent_rank is out of scope for incertae sedis' do - name = names(:incertae_sedis_with_distant_parent) - qc = warning(name, :inconsistent_parent_rank) + test 'inconsistent_parent_rank is out of scope without a placement' do + name = Name.new(name: 'Escherichia coli', rank: 'species') - assert_not_predicate(qc, :scope) + assert_not_predicate(warning(name, :inconsistent_parent_rank), :scope) + end + + test 'inconsistent_parent_rank is out of scope without a name rank' do + name = names(:escherichia_coli) + name.rank = nil + + assert_not_predicate(warning(name, :inconsistent_parent_rank), :scope) end test 'incertae sedis parent rank accepts a parent two or more ranks above' do name = names(:incertae_sedis_with_distant_parent) - qc = warning(name, :inconsistent_incertae_sedis_parent_rank) + qc = warning(name, :inconsistent_parent_rank) assert_predicate(qc, :scope) assert_not_predicate(qc, :failure) @@ -98,15 +103,17 @@ def warning(name, check) test 'incertae sedis parent rank flags a parent one rank above' do name = names(:incertae_sedis_with_immediate_parent) - qc = warning(name, :inconsistent_incertae_sedis_parent_rank) + assert_nil name.parent + qc = warning(name, :inconsistent_parent_rank) assert_predicate(qc, :scope) assert_predicate(qc, :failure) + assert_includes(qc.message, 'parent rank (genus)') end test 'incertae sedis parent rank flags a parent at the same rank' do name = names(:incertae_sedis_with_same_rank_parent) - qc = warning(name, :inconsistent_incertae_sedis_parent_rank) + qc = warning(name, :inconsistent_parent_rank) assert_predicate(qc, :scope) assert_predicate(qc, :failure) From 022ea283475dea101a63b513e11f49dbd9e271e0 Mon Sep 17 00:00:00 2001 From: vangberg Date: Tue, 8 Sep 2026 11:51:43 +0200 Subject: [PATCH 08/11] Fix incertae sedis lineage lookup to use placement parent --- app/models/name.rb | 4 +--- test/models/name_test.rb | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/app/models/name.rb b/app/models/name.rb index 449aac48..85873eff 100644 --- a/app/models/name.rb +++ b/app/models/name.rb @@ -992,9 +992,7 @@ def lineage(with_self: false) def lineage_parent return parent if parent - if incertae_sedis? && incertae_sedis =~ /Incertae sedis \((.+)\)/ - self.class.find_by_variants($1) - end + placement.parent if incertae_sedis? end def type_name_alt_placement diff --git a/test/models/name_test.rb b/test/models/name_test.rb index f2e4597a..2620a292 100644 --- a/test/models/name_test.rb +++ b/test/models/name_test.rb @@ -1,6 +1,30 @@ require 'test_helper' class NameTest < ActiveSupport::TestCase + test 'lineage follows the parent for a normally placed species' do + name = names(:escherichia_coli) + + assert_equal names(:escherichia), name.lineage_parent + assert_equal [names(:escherichia), name], name.lineage(with_self: true) + end + + test 'incertae sedis lineage follows the preferred placement parent' do + name = names(:escherichia_coli) + name.parent = nil + name.placement.incertae_sedis = true + name.placement.parent = names(:bacteria) + + assert_equal names(:bacteria), name.lineage_parent + assert_equal [names(:bacteria), name], name.lineage(with_self: true) + end + + test 'lineage without a parent or preferred placement contains only itself' do + name = Name.new(name: 'Escherichia coli', rank: 'species') + + assert_nil name.lineage_parent + assert_equal [name], name.lineage(with_self: true) + end + test 'add_to_register adds name to a draft register' do name = names(:unregistered) register = registers(:draft) From 23a50ca0d5a21d349e63d97234416a508c3d862f Mon Sep 17 00:00:00 2001 From: vangberg Date: Tue, 8 Sep 2026 11:56:56 +0200 Subject: [PATCH 09/11] Fix incertae sedis validation and display in batch previews --- app/models/tutorial/batch.rb | 2 +- app/views/tutorials/batch/_01.html.erb | 4 ++-- test/models/tutorial_test.rb | 3 +++ test/system/tutorials_test.rb | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/models/tutorial/batch.rb b/app/models/tutorial/batch.rb index 527a344c..bca322f3 100644 --- a/app/models/tutorial/batch.rb +++ b/app/models/tutorial/batch.rb @@ -82,7 +82,7 @@ def ephemeral_names name_attributes = i.except('parent', 'incertae_sedis') placement_attributes = { parent: i['parent'], - incertae_sedis: i['incertae_sedis'], + incertae_sedis: i['incertae_sedis'] || false, preferred: true } if i['incertae_sedis'].present? diff --git a/app/views/tutorials/batch/_01.html.erb b/app/views/tutorials/batch/_01.html.erb index 2867f341..ea5c8c66 100644 --- a/app/views/tutorials/batch/_01.html.erb +++ b/app/views/tutorials/batch/_01.html.erb @@ -39,8 +39,8 @@ <%= name.name_html %> - <%= name.rank %> of - <%= placement.parent.try(:name_html) || - placement.incertae_sedis_html.presence || 'unknown' %> + <%= placement.incertae_sedis_html.presence || + placement.parent.try(:name_html) || 'unknown' %> diff --git a/test/models/tutorial_test.rb b/test/models/tutorial_test.rb index 757cac5f..789df3fc 100644 --- a/test/models/tutorial_test.rb +++ b/test/models/tutorial_test.rb @@ -16,6 +16,9 @@ class TutorialTest < ActiveSupport::TestCase assert_same name, placement.name assert_equal 'Nanobdellaceae', placement.parent.name assert_not_predicate placement, :incertae_sedis? + assert_equal false, placement.incertae_sedis + placement.validate + assert_empty placement.errors[:incertae_sedis] assert_empty placement.incertae_sedis_text.to_plain_text end diff --git a/test/system/tutorials_test.rb b/test/system/tutorials_test.rb index d0a79f30..1499c92a 100644 --- a/test/system/tutorials_test.rb +++ b/test/system/tutorials_test.rb @@ -44,7 +44,7 @@ class TutorialsTest < ApplicationSystemTestCase visit tutorial_url(@tutorial) upload_batch_spreadsheet('SCR_UploadBatch-Luteria-Incertae_sedis.xlsx') - assert_batch_review(name: 'Luteria', parent: 'Bacteria') + assert_batch_review(name: 'Luteria', parent: 'incertae sedis (Bacteria)') create_batch_entries From 849d9aafb301b7a5ba6556660e83cdfd67e96793 Mon Sep 17 00:00:00 2001 From: vangberg Date: Wed, 9 Sep 2026 10:02:09 +0200 Subject: [PATCH 10/11] Update autocomplete dev page for ranks API --- app/views/dev/autocomplete.html.erb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/dev/autocomplete.html.erb b/app/views/dev/autocomplete.html.erb index ff0fd50e..cf87709c 100644 --- a/app/views/dev/autocomplete.html.erb +++ b/app/views/dev/autocomplete.html.erb @@ -35,7 +35,8 @@ + data-autocomplete="names" + data-ranks='<%= Name.ranks_at_or_above('class').to_json %>'> Domain, phylum, or class. @@ -43,7 +44,8 @@ + data-autocomplete="names" + data-ranks='<%= Name.ranks_at_or_above('genus').to_json %>'> Domain through genus; excludes species and subspecies. From e6f002bbe65da952f856df3f20bfb028f6e26c45 Mon Sep 17 00:00:00 2001 From: vangberg Date: Wed, 9 Sep 2026 10:12:41 +0200 Subject: [PATCH 11/11] Escape incertae sedis parent name --- app/models/placement.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/placement.rb b/app/models/placement.rb index 4cb6c1a1..45592fa6 100644 --- a/app/models/placement.rb +++ b/app/models/placement.rb @@ -19,8 +19,9 @@ class Placement < ApplicationRecord def incertae_sedis_html return '' unless incertae_sedis? - qualifier = " (#{parent.name})" if parent - "incertae sedis#{qualifier}".html_safe + ActionController::Base.helpers.safe_join( + ['incertae sedis'.html_safe, (" (#{parent.name})" if parent)].compact + ) end def allowed_parent_ranks(incertae_sedis: incertae_sedis?)