[refactor] datagroup.

app/controllers/datagroups_controller.rb

@@ -39,7 +39,7 @@ def update
 
   def destroy
     unless @datagroup.datasets.empty?
-      flash[:error] = "Datagroup has associated datasets and cann't be deleted"
+      flash[:error] = "Datagroup has associated datasets thus can't be deleted"
       redirect_to :back
     end
     if @datagroup.destroy
@@ -62,15 +62,14 @@ def update_categories
     end
     f = params[:csvfile][:file].path
 
-    @changes = @datagroup.update_categories_with_csv(f, current_user)
+    changes = @datagroup.update_and_merge_categories_with_csv(f, current_user)
 
     unless @datagroup.errors.empty?
       flash[:error] = @datagroup.errors.full_messages.to_sentence
       redirect_to :back and return
     else
-      flash[:notice] = "Categories successfully updated"
-      flash[:updates] = @changes
-      redirect_to datagroup_path @datagroup
+      flash[:notice] = "#{changes[:u]} categories are updated and #{changes[:m]} categories are merged"
+      redirect_to datagroup_path(@datagroup)
     end
   end
 

app/models/category.rb

@@ -71,6 +71,15 @@ def update_sheetcells_with_csv(file, user)
     update_overview
   end
 
+  def self.merge(from_category, to_category, user)
+    return unless from_category.datagroup_id == to_category.datagroup_id
+
+    comment_string = "Merged #{from_category.short} by #{user.lastname} at #{Time.now.to_s} via CSV; "
+    from_category.sheetcells.update_all(:category_id => to_category.id)
+    to_category.update_attributes(:comment => "#{to_category.comment} #{comment_string}".strip) # this triggers regeneration of datasets
+    from_category.delete
+  end
+
 private
 
   def validate_and_reduce_sheetcells_csv(csv_lines)

app/models/datagroup.rb

@@ -1,10 +1,7 @@
-## The Datagroup class models the Datagroup table.
-##
-## Datagroups define the type of data that has been recorded in terms of what was measured, how it was measured and any information source or reference used.
+## Datagroups define the type of data that has been recorded in terms of what was measured
 ## They can be reused across multiple "Datacolumn"s and "Dataset"s.
 ## A Helper (system) "Datagroup" is the default "Datagroup" when no specific measurement information is included in the "Dataworkbook".
 ##
-## A "Datagroup" can contain one or more "Datatype"s.
 ## A "Category" must belong to a "Datagroup" and will be unique within that "Datagroup".
 require 'csv'
 
@@ -14,8 +11,6 @@ class Datagroup < ActiveRecord::Base
   has_many :categories, :dependent => :destroy
   has_many :datasets, :through => :datacolumns
 
-  #acts_as_taggable
-
   validates_presence_of :title, :description
   validates_uniqueness_of :title, :case_sensitive => false
 
@@ -53,52 +48,30 @@ def self.delete_orphan_datagroups
     Datagroup.delete(to_be_deleted)
   end
 
-  def update_categories_with_csv (file, user)
+  def update_and_merge_categories_with_csv (file, user)
     begin
-      lines = CSV.read file
+      lines = CSV.read(file, CsvData::OPTS)
     rescue
-      errors.add :file, 'can not be read' and return false
-    end
-    lines = validate_and_reduce_categories_csv(lines)
-    return if !lines || !errors.blank?
-
-    cats = Category.find lines.collect{|l| l[0]}
-    comment_string = "Updated via CVS by #{user.lastname}, #{Time.now.to_s}."
-    changes = []
-
-    lines.each do |l|
-      c = cats.detect{|c| c.id == l[0].to_i}
-      if c.short != l[1] || c.long != l[2] || c.description != l[3]
-        changes << c
-        c.short = l[1]
-        c.long = l[2]
-        c.description = l[3]
-        c.comment = "#{c.comment} #{comment_string}".strip
-      end
-    end
-
-    unless categories_remain_unique?(changes)
-      errors.add :categories, 'need to remain unique for datagroup' and return false
+      errors.add :file, 'can not be read' and return
     end
+    return unless validate_categories_csv?(lines)
 
     merges = collect_merges(lines)
-    validate_merges(merges, lines)
-    return if !errors.blank?
+    updates = collect_updates(lines, user)
+    # categories that are to be merged don't need to be updated
+    merge_sources = merges.collect{|m| m["ID"].to_i}
+    updates.reject! {|c| merge_sources.include?(c.id)}
 
-    merge_sources = merges.collect{|m| m[0]}
-    changes.reject! {|c| merge_sources.include?(c.id)}
+    unless categories_remain_unique?(updates, merges)
+      errors.add :categories, 'need to remain unique for datagroup' and return
+    end
 
-    changes.each {|c| c.save}
-    execute_merges(merges,user)
+    # begin updating and merging
+    # merging should be ahead of updating
+    execute_merges(merges, user)
+    execute_updates(updates)
 
-    update_overview = Hash.new
-    changes.each do |c|
-      update_overview[c.id] = 'u'
-    end
-    merges.each do |m|
-      update_overview[m[1]] = update_overview[m[1]].to_s + 'm'
-    end
-    update_overview
+    return {u: updates.length, m: merges.length}
   end
 
   def self.search(q)
@@ -111,90 +84,74 @@ def self.search(q)
 
 private
 
-  def validate_and_reduce_categories_csv (csv_lines)
-    if csv_lines[0].nil?
-      errors.add :csv, 'seems to be empty' and return false
-    end
-
-    unless csv_lines[0][0] == 'ID'
+  def validate_categories_csv?(csv_lines)
+    errors.add :csv, 'seems to be empty' and return false if csv_lines.empty?
+    unless (['ID', 'SHORT', 'LONG', 'DESCRIPTION', 'MERGE ID'] - csv_lines.headers).empty?
       errors.add :csv, 'header does not match' and return false
     end
+    errors.add :csv, 'IDs must be unique' and return false unless csv_lines["ID"].uniq!.nil?
+    errors.add :csv, 'ID must not be empty' and return false if csv_lines["ID"].any?(&:blank?)
+    errors.add :csv, 'SHORT must not be empty' and return false if csv_lines["SHORT"].any?(&:blank?)
 
-    unless csv_lines[0].length == 5
-      errors.add :csv, 'has wrong number of columns' and return false
-    end
+    dg_cats_ids = self.category_ids
 
-    # clean header from csv / allow empty lines
-    csv_lines.delete_at 0
-    csv_lines.delete_if {|l| l.compact.empty?}
-
-    if csv_lines.empty?
-      errors.add :csv, 'no categories given' and return false
-    end
-
-    csv_cats_ids = csv_lines.collect{|l| l[0].to_i}
-
-    unless csv_cats_ids.uniq!.nil?
-      errors.add :csv, 'IDs must be unique' and return false
-    end
-
-    dg_cats_ids = self.categories.collect{|c| c.id}
-    cats_no_match = csv_cats_ids - dg_cats_ids
+    cats_no_match = csv_lines["ID"].map(&:to_i) - dg_cats_ids
     unless cats_no_match.empty?
-      errors.add :csv, "category #{cats_no_match} not matching the categories of this datagroup" and return false
+      errors.add :csv, "ID out of range: #{cats_no_match.to_sentence}" and return false
     end
-
-    csv_lines.each do |l|
-      if l[0].blank?
-        errors.add :csv, 'ID must not be empty' and return false
-      end
-      if l[1].blank?
-        errors.add :csv, 'SHORT must not be empty' and return false
-      end
+    merge_ids_no_match = csv_lines["MERGE ID"].compact.map(&:to_i) - dg_cats_ids
+    unless merge_ids_no_match.empty?
+      errors.add :csv, "MERGE ID out of range: #{merge_ids_no_match.to_sentence}" and return false
     end
 
-    csv_lines
-  end
+    merges = collect_merges(csv_lines)
+    merge_source_ids = merges.map{|l| l["ID"]}
+    merge_target_ids = merges.map{|l| l["MERGE ID"]}
+    errors.add :csv, "Recursive merges are not allowed" and return false unless (merge_source_ids & merge_target_ids).empty?
 
-  def categories_remain_unique? (changed_categories)
-    unchanged_categories = self.categories.select(:short).where("id NOT IN (?)", changed_categories.collect{|c| c.id})
-    all_shorts = unchanged_categories.collect{|c| c.short} + changed_categories.collect{|c| c.short}
-    all_shorts.uniq! == nil ? true : false
+    return true
   end
 
-  def collect_merges (csv_lines)
-    csv_lines.reject {|l| l[4].blank?}.collect {|l| [l[0].to_i, l[4].to_i]}
+  def collect_merges(csv_lines)
+    csv_lines.select {|l| l['MERGE ID'].present? && l['ID'] != l['MERGE ID']}
   end
 
-  def validate_merges(merge_pairs, csv_lines)
-    csv_ids = csv_lines.collect {|l| l[0].to_i}
-    merge_source_ids = merge_pairs.collect {|mp| mp[0]}
-    merge_target_ids = merge_pairs.collect {|mp| mp[1]}
+  def collect_updates(lines, user)
+    cats = self.categories.select([:id, :short, :long, :description, :comment])
+    comment_string = "Updated via CVS by #{user.lastname}, #{Time.now.to_s}."
+    changes = []
 
-    unless (merge_target_ids - csv_ids).empty?
-      errors.add :categories, 'can not merge with categories which are not present'
+    lines.each do |l|
+      c = cats.detect{|c| c.id == l["ID"].to_i}
+      if c.short != l['SHORT'] || c.long != l['LONG'] || c.description != l['DESCRIPTION']
+        c.short = l['SHORT']
+        c.long = l['LONG']
+        c.description = l['DESCRIPTION']
+        c.comment = "#{c.comment} #{comment_string}".strip
+        changes << c
+      end
     end
+    return changes
+  end
 
-    unless (merge_target_ids & merge_source_ids).empty?
-      errors.add :categories, 'recursive merges are not allowed'
-    end
+  def categories_remain_unique?(updates, merges)
+    changed_categories = updates.collect {|u| u.id} | merges.collect{|m| m['ID'].to_i}
+    short_of_unchanged_categories = self.categories.where("id NOT IN (?)", changed_categories).pluck(:short)
+    all_shorts = updates.collect{|u| u.short} + short_of_unchanged_categories
+    all_shorts.uniq!.nil? ? true : false
   end
 
   def execute_merges(merge_pairs, user)
     merge_pairs.each do |mp|
-      source_cat = Category.find mp[0]
-      target_cat = Category.find mp[1]
-
-      cells = source_cat.sheetcells
-      cells.each do |cell|
-        cell.category = target_cat
-        cell.save
-      end
-      comment_string = "Merged #{source_cat.id} via CVS by #{user.lastname}, #{Time.now.to_s}."
-      target_cat.comment = "#{target_cat.comment} #{comment_string}".strip
-      target_cat.save
-      source_cat.destroy
+      source_cat = Category.find mp["ID"]
+      target_cat = Category.find mp["MERGE ID"]
+      Category.merge(source_cat, target_cat, user)
     end
   end
 
+  def execute_updates(updates)
+    ActiveRecord::Base.transaction do
+      updates.each { |c| c.save }
+    end
+  end
 end

app/models/sheetcell.rb

@@ -6,7 +6,7 @@
 class Sheetcell < ActiveRecord::Base
 
   belongs_to :datacolumn
-  belongs_to :category, :dependent => :destroy
+  belongs_to :category
   after_initialize :set_default_status
 
   def set_default_status