Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions app/models/name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ class Name < ApplicationRecord
alias :correspondences :name_correspondences
has_many(:checks, dependent: :destroy)
has_many(:check_users, -> { distinct }, through: :checks, source: :user)
has_many(:placements, -> { includes(:parent) }, dependent: :destroy)
has_many(
:placements, -> { includes(:parent) },
dependent: :destroy, inverse_of: :name
)
has_many(
:child_placements, -> { includes(:name) },
class_name: 'Placement', foreign_key: 'parent_id', dependent: :destroy
Expand All @@ -39,7 +42,7 @@ class Name < ApplicationRecord
:combinational_derivatives, class_name: 'Name', foreign_key: 'basonym_id',
dependent: :nullify # Inverse of basonym
)
has_many(:curations)
has_many(:curations, dependent: :destroy)
has_many(:name_paratypes, dependent: :destroy)
has_many(
:paratype_publications, through: :name_paratypes, source: :publication
Expand Down Expand Up @@ -1478,9 +1481,16 @@ def harmonize_register_and_status
end

def ensure_consistent_placement
if parent_id.present? || incertae_sedis.present?
placement_incertae_sedis =
if saved_change_to_incertae_sedis?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will go away in #337

self[:incertae_sedis]
else
incertae_sedis
end

if parent_id.present? || placement_incertae_sedis.present?
pp = placements.where(
parent_id: parent_id, incertae_sedis: incertae_sedis
parent_id: parent_id, incertae_sedis: placement_incertae_sedis
).first
if pp.present?
if pp.preferred?
Expand All @@ -1491,8 +1501,11 @@ def ensure_consistent_placement
else
placements.update(preferred: false) &&
Placement.new(
name_id: id, parent_id: parent_id, incertae_sedis: incertae_sedis,
incertae_sedis_text: incertae_sedis_text, preferred: true
name_id: id,
parent_id: parent_id,
incertae_sedis: placement_incertae_sedis,
incertae_sedis_text: incertae_sedis_text,
preferred: true
).save
end
else
Expand Down
47 changes: 36 additions & 11 deletions app/models/tutorial/batch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,19 @@ def param_names
def ephemeral_names
@ephemeral_names ||=
param_names.map do |i|
Name.new(i)
name_attributes = i.except('parent', 'incertae_sedis')
placement_attributes = {
parent: i['parent'],
incertae_sedis: i['incertae_sedis'],
preferred: true
}
if i['incertae_sedis'].present?
placement_attributes[:incertae_sedis_text] = i['description']
end

Name.new(name_attributes).tap do |name|
name.placements.build(placement_attributes)
end
rescue => e
$stderr.puts e
nil
Expand Down Expand Up @@ -113,15 +125,20 @@ def check_ephemeral_names(user)
@check_ephemeral_names ||= :pending
return ephemeral_names if @check_ephemeral_names == :done

name_names = ephemeral_names.map(&:name)
ephemeral_names.each do |name|
placement = name.placements.first

# Is the information complete?
Tutorial::Batch.name_keys_base.each do |i|
next if i == :parent

unless name.send(i).present?
name.errors.add(i, :missing, message: 'is missing')
end
end

placement.validate # Populate placement.errors for the review step

# Is the etymology present?
unless name.etymology?
name.errors.add(
Expand All @@ -139,8 +156,8 @@ def check_ephemeral_names(user)
end

# Is the parent already registered?
if name.parent && !name_exists?(name.parent.name)
name.errors.add(
if placement.parent && !name_exists?(placement.parent.name)
placement.errors.add(
:parent, :does_not_exist,
message: 'does not exist and is not proposed in this list'
)
Expand Down Expand Up @@ -305,7 +322,8 @@ def batch_step_01(params, user)
# Remove foreign keys in first pass
par = par_ori.dup
%w[
parent nomenclatural_type_entry nomenclatural_type_type
parent incertae_sedis
nomenclatural_type_entry nomenclatural_type_type
nomenclatural_type_id nomenclatural_type
].each { |i| par.delete(i) }

Expand Down Expand Up @@ -333,12 +351,17 @@ def batch_step_01(params, user)

# Parents
if par['parent']
new_par[:parent] = Name.find_by_variants(par['parent'].name)
unless new_par[:parent]
name = Name.new(default_pars.merge(name: par['parent'].name))
name.save!
new_par[:parent_id] = name.id
parent = Name.find_by_variants(par['parent'].name)
unless parent
parent = Name.new(default_pars.merge(name: par['parent'].name))
parent.save!
end
new_par[:parent] = parent
new_par[:incertae_sedis] = nil
elsif par['incertae_sedis']
new_par[:parent] = nil
new_par[:incertae_sedis] = par['incertae_sedis']
new_par[:incertae_sedis_text] = par['description']
end

# Nomenclatural types
Expand All @@ -359,7 +382,9 @@ def batch_step_01(params, user)
end
end

Name.find_by_variants(par['name']).update!(new_par)
name = Name.find_by_variants(par['name'])
name.update!(new_par)
Comment thread
vangberg marked this conversation as resolved.
Dismissed
name.update_column(:incertae_sedis, nil) if par['incertae_sedis']
end

# If all is good, go to next step
Expand Down
12 changes: 7 additions & 5 deletions app/views/tutorials/batch/_01.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<div class="accordion" id="batch-names-accordion">
<% @tutorial.check_ephemeral_names(current_user) %>
<% @tutorial.ephemeral_names.each_with_index do |name, idx| %>
<% placement = name&.placements&.first %>
<div class="card">
<% id = "batch-names-#{idx}" %>
<% if name.nil? || name&.name.nil? %>
Expand All @@ -32,13 +33,14 @@
<button class="btn btn-link btn-block text-left" type="button"
data-toggle="collapse" data-target="#<%= id %>"
aria-expanded="false" aria-controls="<%= id %>">
<% if name.errors.any? %>
<% if name.errors.any? || placement.errors.any? %>
<%= fa_icon('exclamation-circle', class: 'text-danger') %>
<% end %>
<%= name.name_html %>
<span class="text-muted">
- <%= name.rank %> of
<%= name.parent.try(:name_html) || 'unknown' %>
<%= placement.parent.try(:name_html) ||
placement.incertae_sedis_html.presence || 'unknown' %>
</span>
</button>
</h2>
Expand Down Expand Up @@ -66,11 +68,12 @@
<% end %>
<%= modal_button(id, as_anchor: true) do %>Record details<% end %>

<% if name.errors.any? %>
<% if name.errors.any? || placement.errors.any? %>
<div class="alert alert-danger">
<h4>Issues detected in preliminary check:</h4>
<ul>
<% name.errors.full_messages.each do |error| %>
<% (name.errors.full_messages +
placement.errors.full_messages).each do |error| %>
<li><%= error %></li>
<% end %>
</ul>
Expand Down Expand Up @@ -166,4 +169,3 @@
Please note that any errors saved will have to be individually resolved.
This process may take up to a couple of minutes, please be patient.
</p>

Binary file not shown.
Binary file not shown.
15 changes: 15 additions & 0 deletions test/fixtures/names.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,21 @@ bacillus_subtilis:
rank: species
status: 20 # ICNP

nanobdellaceae:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel some tension between using fixtures with names that describe the fixture (e.g. valid_name or unpublished_name) and then just using the name like here. Not sure what direction to go in.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think mixing them up is ok, depending on the aim of the test(s) using the fixture

name: Nanobdellaceae
rank: family
status: 15 # SeqCode

nanoclepta_minutus:
name: Nanoclepta minutus
rank: species
status: 15 # SeqCode

luteria_ianthellae:
name: Luteria ianthellae
rank: species
status: 15 # SeqCode

type_genome_with_locations:
name: Testimonas mappensis
rank: species
Expand Down
152 changes: 149 additions & 3 deletions test/models/tutorial_test.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,153 @@
require 'test_helper'

class TutorialTest < ActiveSupport::TestCase
# test "the truth" do
# assert true
# end
test 'batch parses a name' do
tutorial = upload_batch_spreadsheet('SCR_UploadBatch-Nanoclepta.xlsx')
name = tutorial.ephemeral_names.first
placement = name.placements.first

assert_equal 1, tutorial.step
assert_empty tutorial.value(:genomes)
assert_equal 1, tutorial.value(:names).size
assert_equal 'Nanoclepta', tutorial.value(:names).first['name']
assert_equal 'Nanobdellaceae', tutorial.value(:names).first['parent']
assert_nil name.parent
assert_nil name[:incertae_sedis]
assert_same name, placement.name
assert_equal 'Nanobdellaceae', placement.parent.name
assert_nil placement.incertae_sedis
assert_empty placement.incertae_sedis_text.to_plain_text
end

test 'batch parses an incertae sedis name' do
tutorial = upload_batch_spreadsheet(
'SCR_UploadBatch-Luteria-Incertae_sedis.xlsx'
)
name = tutorial.ephemeral_names.first
placement = name.placements.first

assert_equal 1, tutorial.step
assert_empty tutorial.value(:genomes)
assert_equal 1, tutorial.value(:names).size
assert_equal 'Luteria', tutorial.value(:names).first['name']
assert_equal(
'incertae sedis (Bacteria)', tutorial.value(:names).first['parent']
)
assert_nil name.parent
assert_nil name[:incertae_sedis]
assert_nil placement.parent
assert_equal 'incertae sedis (Bacteria)', placement.incertae_sedis
assert_equal(
tutorial.value(:names).first['description'],
placement.incertae_sedis_text.to_plain_text
)
end

test 'batch creates a parent placement' do
tutorial = upload_batch_spreadsheet('SCR_UploadBatch-Nanoclepta.xlsx')
parent = names(:nanobdellaceae)

assert tutorial.batch_step_01({}, users(:contributor))

name = Name.find_by!(name: 'Nanoclepta')
placement = name.placement

assert_not_nil placement
assert_predicate placement, :preferred?
assert_equal parent, placement.parent
assert_equal parent, name.parent
assert_nil placement.incertae_sedis
end

test 'batch updates a claimable name with a preferred placement' do
parent = names(:nanobdellaceae)
name = Name.create!(
name: 'Nanoclepta', rank: 'genus', status: 5,
created_by: users(:contributor), parent: parent
)
placement = name.placement
tutorial = upload_batch_spreadsheet('SCR_UploadBatch-Nanoclepta.xlsx')

assert_no_difference('Placement.count') do
assert tutorial.batch_step_01({}, users(:contributor))
end

assert_equal placement, name.reload.placement
assert_equal parent, name.parent
end

test 'batch replaces a claimable name preferred placement' do
old_parent = Name.create!(
name: 'Nanoarchaeaceae', rank: 'family', status: 15
)
name = Name.create!(
name: 'Nanoclepta', rank: 'genus', status: 5,
created_by: users(:contributor), parent: old_parent
)
old_placement = name.placement
tutorial = upload_batch_spreadsheet('SCR_UploadBatch-Nanoclepta.xlsx')

assert_difference('Placement.count', 1) do
assert tutorial.batch_step_01({}, users(:contributor))
end

name = Name.find(name.id)
assert_equal names(:nanobdellaceae), name.placement.parent
assert_not old_placement.reload.preferred?
assert_includes name.alt_placements, old_placement
end

test 'batch creates an incertae sedis placement' do
tutorial = upload_batch_spreadsheet(
'SCR_UploadBatch-Luteria-Incertae_sedis.xlsx'
)
explanation = tutorial.value(:names).first['description']

assert tutorial.batch_step_01({}, users(:contributor))

name = Name.find_by!(name: 'Luteria')
placement = name.placement

assert_not_nil placement
assert_predicate placement, :preferred?
assert_nil placement.parent
assert_nil name.parent
assert_equal 'incertae sedis (Bacteria)', placement.incertae_sedis
assert_equal explanation, placement.incertae_sedis_text.to_plain_text
assert_nil name[:incertae_sedis]
end

test 'batch replaces a claimable name placement with incertae sedis' do
old_parent = names(:escherichia)
name = Name.create!(
name: 'Luteria', rank: 'genus', status: 5,
created_by: users(:contributor), parent: old_parent
)
old_placement = name.placement
tutorial = upload_batch_spreadsheet(
'SCR_UploadBatch-Luteria-Incertae_sedis.xlsx'
)

assert_difference('Placement.count', 1) do
assert tutorial.batch_step_01({}, users(:contributor))
end

name = Name.find(name.id)
assert_equal 'incertae sedis (Bacteria)', name.placement.incertae_sedis
assert_not_predicate old_placement.reload, :preferred?
assert_includes name.alt_placements, old_placement
assert_nil name[:incertae_sedis]
end

private

def upload_batch_spreadsheet(filename)
tutorial = Tutorial.create!(
pipeline: 'batch', user: users(:contributor), step: 0, ongoing: true
)
File.open(Rails.root.join('test/fixtures/files', filename)) do |file|
assert tutorial.batch_step_00({ file: file }, users(:contributor))
end
tutorial
end
end
Loading
Loading