Skip to content

Commit

Permalink
Simplify the subject column name used in CSV exports and imports
Browse files Browse the repository at this point in the history
On export, use just the subject as much as possible, and add the description if needed.
On import, allow different possible combinations, as long as it’s unambiguous.

refs #1253
  • Loading branch information
n-b committed Oct 6, 2020
1 parent f8ca38e commit 56b7266
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 64 deletions.
16 changes: 0 additions & 16 deletions app/models/institution.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,6 @@ def available_subjects
.group_by { |is| is.theme } # Enumerable#group_by maintains ordering
end

# Find the one subject that matches the passed label
# return nil if there’s an ambiguity
def find_institution_subject(label)
return nil if label.nil?

clean_label = label.downcase.strip
matches = institutions_subjects.filter do |is|
label == is.csv_identifier || clean_label.in?([
is.description.downcase.strip,
is.subject.label.downcase.strip,
is.theme.label.downcase.strip
])
end
matches.first if matches.count == 1
end

##
#
def to_param
Expand Down
31 changes: 29 additions & 2 deletions app/models/institution_subject.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,34 @@ def validate_description_presence

## used for serialization in advisors csv
#
def csv_identifier
[theme, subject, description.presence].compact.to_csv(col_sep: ':').strip
def unique_name
if similar_institutions_subjects.present?
"#{subject.label}:#{description}" # We know description isn‘t blank, see :validate_description_presence
else
subject.label
end
end

def self.find_with_name(institution, name)
return nil if name.nil?

clean_name = name.downcase.strip

matches = institution.institutions_subjects.preload(:subject, :theme).filter do |institution_subject|
institution_subject.possible_names.include? clean_name
end

# return nil if there’s an ambiguity
matches.first if matches.count == 1
end

def possible_names
[
"#{theme.label}:#{subject.label}:#{description}".downcase.strip,
"#{subject.label}:#{description}".downcase.strip,
description&.downcase.strip,
subject.label.downcase.strip,
theme.label.downcase.strip
]
end
end
6 changes: 3 additions & 3 deletions app/services/csv_export/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ def csv_fields_for_relevant_expert_team
}
end

def csv_fields_for_relevant_expert_subjects(subjects)
subjects.map do |institution_subject|
def csv_fields_for_relevant_expert_subjects(institutions_subjects)
institutions_subjects.map do |institution_subject|
# We build a hash of <institution subject>: <expert subject>
# * There can be only one expert_subject for an (expert, institution_subject) pair.
title = institution_subject.csv_identifier
title = institution_subject.unique_name
lambda = -> {
# This block is executed in the context of a User
# (`self` is a User; See `object.instance_exec(&lambda)` in CsvExportService.)
Expand Down
8 changes: 4 additions & 4 deletions app/services/csv_import/user_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def build_several_subjects_mapping(headers, other_known_headers)
@several_subjects_mapping =
headers
.without(other_known_headers)
.index_with { |header| @institution.find_institution_subject(header) }
.index_with { |header| InstitutionSubject.find_with_name(@institution, header) }
.compact
end

Expand Down Expand Up @@ -105,10 +105,10 @@ def one_subject_mapping
end

def import_one_subject(expert, all_attributes)
identifier = all_attributes[Expert.human_attribute_name('subject')]
return if identifier.blank?
name = all_attributes[Expert.human_attribute_name('subject')]
return if name.blank?

institution_subject = expert.institution.institutions_subjects.find{ |is| is.csv_identifier == identifier }
institution_subject = InstitutionSubject.find_with_name(expert.institution, name)

expert.experts_subjects.clear
expert.experts_subjects.new(institution_subject: institution_subject)
Expand Down
28 changes: 0 additions & 28 deletions spec/models/institution_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,32 +33,4 @@
end
end
end

describe 'find_institution_subject' do
subject { institution.find_institution_subject(label) }

let(:institution) { create :institution, name: 'The Institution' }
let(:theme) { create :theme, label: 'The Theme' }
let(:the_subject) { create :subject, label: 'The Subject', theme: theme }
let!(:is1) { create :institution_subject, institution: institution, subject: the_subject, description: 'First IS' }
let!(:is2) { create :institution_subject, institution: institution, subject: the_subject, description: 'Second IS' }

context 'label is not found' do
let(:label) { 'other' }

it{ is_expected.to be_nil }
end

context 'label is found and unique' do
let(:label) { 'First IS' }

it{ is_expected.to eq is1 }
end

context 'label is found but not unique' do
let(:label) { 'The Subject' }

it{ is_expected.to be_nil }
end
end
end
50 changes: 40 additions & 10 deletions spec/models/institution_subject_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,53 @@
end
end

describe 'csv_identifier' do
subject { institution_subject.csv_identifier }
describe 'unique_name' do
subject { institution_subject.unique_name }

let(:institution_subject) { create :institution_subject, subject: the_subject, description: description }
let(:the_subject) { create :subject, theme: theme, label: 'Label of the subject' }
let(:institution_subject) { create :institution_subject, institution: institution, subject: the_subject, description: 'First IS' }
let!(:other_institution_subject) { create :institution_subject, institution: institution, subject: other_subject, description: 'Second IS' }
let(:the_subject) { create :subject, theme: theme, label: 'The Subject' }
let(:institution) { create :institution }
let(:theme) { create :theme, label: 'Label of the theme' }

context 'with a description' do
let(:description) { 'Description détaillée' }
context 'with no other similar institution_subject' do
let(:other_subject) { create :subject, theme: theme, label: 'Other subject' }

it{ is_expected.to eq 'Label of the theme:Label of the subject:Description détaillée' }
it{ is_expected.to eq 'The Subject' }
end

context 'with no description' do
let(:description) { '' }
context 'with a similar institution_subject' do
let(:other_subject) { the_subject }

it{ is_expected.to eq 'Label of the theme:Label of the subject' }
it{ is_expected.to eq 'The Subject:First IS' }
end
end

describe 'find_with_name' do
subject { described_class.find_with_name(institution, label) }

let(:institution) { create :institution, name: 'The Institution' }
let(:theme) { create :theme, label: 'The Theme' }
let(:the_subject) { create :subject, label: 'The Subject', theme: theme }
let!(:is1) { create :institution_subject, institution: institution, subject: the_subject, description: 'First IS' }
let!(:is2) { create :institution_subject, institution: institution, subject: the_subject, description: 'Second IS' }

context 'label is not found' do
let(:label) { 'other' }

it{ is_expected.to be_nil }
end

context 'label is found and unique' do
let(:label) { 'First IS' }

it{ is_expected.to eq is1 }
end

context 'label is found but not unique' do
let(:label) { 'The Subject' }

it{ is_expected.to be_nil }
end
end
end
2 changes: 1 addition & 1 deletion spec/services/csv_export_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@

it do
csv = <<~CSV
Institution,Antenne,Prénom et nom,E-mail,Téléphone,Fonction,Test Theme:Test Subject:Description for institution
Institution,Antenne,Prénom et nom,E-mail,Téléphone,Fonction,Test Subject
Test Institution,Test Antenne,User 1,user@user.com,0123456789,User Role,Intervention criteria
CSV
is_expected.to eq csv
Expand Down

0 comments on commit 56b7266

Please sign in to comment.