Simplify the subject column name used in CSV exports and imports

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

app/models/institution.rb

@@ -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

app/models/institution_subject.rb

@@ -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

app/services/csv_export/models/user.rb

@@ -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.)

app/services/csv_import/user_importer.rb

@@ -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
 
@@ -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)

spec/models/institution_spec.rb

@@ -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

spec/models/institution_subject_spec.rb

@@ -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

spec/services/csv_export_service_spec.rb

@@ -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,[email protected],0123456789,User Role,Intervention criteria
         CSV
         is_expected.to eq csv