Merge pull request #596 from DMPRoadmap/development

bugfixes from development

app/controllers/annotations_controller.rb

@@ -16,6 +16,7 @@ def admin_create
     # if they dont exist, no requirement for them to be saved
     ex_save = example_answer.present? ? example_answer.save : true
     guid_save = guidance.present? ? guidance.save : true
+    @question.section.phase.template.dirty = true
 
     if ex_save && guid_save
       redirect_to admin_show_phase_path(id: @question.section.phase_id, section_id: @question.section_id, question_id: @question.id, edit: 'true'), notice: _('Information was successfully created.')
@@ -73,6 +74,8 @@ def admin_update
 
     @section = @question.section
     @phase = @section.phase
+    @phase.template.dirty = true
+
     if ex_save && guid_save
       redirect_to admin_show_phase_path(id: @phase.id, section_id: @section.id, question_id: @question.id, edit: 'true'), notice: _('Information was successfully updated.')
     else
@@ -95,6 +98,7 @@ def admin_destroy
     @question = @example_answer.question
     @section = @question.section
     @phase = @section.phase
+    @phase.template.dirty = true
     if @example_answer.destroy
       redirect_to admin_show_phase_path(id: @phase.id, section_id: @section.id, edit: 'true'), notice: _('Information was successfully deleted.')
     else
@@ -113,4 +117,4 @@ def init_annotation(text, question, org, type)
     return annotation
   end
 
-end
+end

app/controllers/plans_controller.rb

@@ -370,10 +370,10 @@ def rollup(plan, src_plan_key, super_id, obj_plan_key)
   def template_options(org_id, funder_id)
     @templates = []
 
-    if !org_id.blank? || !funder_id.blank?
+    if org_id.present? || funder_id.present?
       if funder_id.blank?
         # Load the org's template(s)
-        unless org_id.nil?
+        if org_id.present?
           org = Org.find(org_id)
           @templates = Template.valid.where(published: true, org: org, customization_of: nil).to_a
           @msg = _("We found multiple DMP templates corresponding to the research organisation.") if @templates.count > 1
@@ -384,20 +384,20 @@ def template_options(org_id, funder_id)
         # Load the funder's template(s)
         @templates = Template.valid.where(published: true, org: funder).to_a
 
-        unless org_id.blank?
+        if org_id.present?
           org = Org.find(org_id)
 
           # Swap out any organisational cusotmizations of a funder template
           @templates.each do |tmplt|
             customization = Template.valid.find_by(published: true, org: org, customization_of: tmplt.dmptemplate_id)
-            unless customization.nil?
+            if customization.present? && tmplt.updated_at < customization.created_at
               @templates.delete(tmplt)
               @templates << customization
             end
           end
         end
 
-        msg = _("We found multiple DMP templates corresponding to the funder.") if @templates.count > 1
+        @msg = _("We found multiple DMP templates corresponding to the funder.") if @templates.count > 1
       end
     end
 

app/controllers/templates_controller.rb

@@ -55,6 +55,7 @@ def admin_customize
       random = rand 2147483647
       break random unless Template.exists?(dmptemplate_id: random)
     end
+    customisation.dirty = true
     customisation.save
 
     customisation.phases.includes(:sections, :questions).each do |phase|

app/views/templates/admin_index.html.erb

@@ -46,10 +46,10 @@
             <td class="dmp_td_small">
               <% if hash[:live].nil? %>
 								<%= _('Unpublished') %>
-								
+
               <% elsif hash[:current].dirty? %>
                 <%= _('Unpublished changes') %>
-								
+
 							<% else %>
 								<%= _('Published') %>
               <% end %>
@@ -58,7 +58,7 @@
             	<% last_temp_updated = hash[:current].updated_at %>
            	 	<%= l last_temp_updated.to_date, formats: :short %>
           	</td>
-						
+
             <td class="dmp_td_small">
             	<%= link_to _('Edit'), admin_template_template_path(id: hash[:current].id, edit: "true"), class: "dmp_table_link" %>
             	<%= link_to _('History'), admin_template_history_template_path(id: hash[:current].id), class: "dmp_table_link" %>
@@ -119,7 +119,7 @@
 								<% elsif hash[:live].nil? %>
 									<!-- The template does not have a live version -->
 									<%= b_label = _('Un-published') %>
-								<% elsif !hash[:current].published? %>
+								<% elsif hash[:current].dirty? %>
 									<%= _('You have un-published changes') %>
 								<% else %>
 									<%= _('Published') %>
@@ -142,14 +142,14 @@
                   <% b_label = _('Edit customisation') %>
                   <%= link_to b_label, admin_template_template_path(hash[:current]), class: "dmp_table_link" %>
                 <% end %>
-                
+
               <% end %>
 
-							<% if !hash[:current].customization_of.nil? %>
+							<% if !hash[:current].customization_of.nil? && !hash[:stale] %>
 								<% if hash[:live].nil? || hash[:current].dirty? %>
 									<%= link_to _('Publish'), admin_publish_template_path(hash[:current]), method: :put, class: "dmp_table_link" %>
 								<% end %>
-								<% if !hash[:live].nil? %>
+								<% if hash[:live].present? %>
 									<%= link_to _('Unpublish'), admin_unpublish_template_path(hash[:current]), method: :put, class: "dmp_table_link" %>
 								<% end %>
 							<% end %>

lib/tasks/migrate.rake

@@ -258,8 +258,11 @@ namespace :migrate do
   task remove_duplicate_annotations: :environment do
     questions = Question.joins(:annotations).group("questions.id").having("count(annotations.id) > count(DISTINCT annotations.text)")
     questions.each do |q|
+      # store already de-duplicated id's so we dont remove them in later iterations
+      removed = []
       q.annotations.each do |a|
-        conflicts = Annotation.where(question_id: a.question_id, text: a.text).where.not(id: a.id)
+        removed << a.id
+        conflicts = Annotation.where(question_id: a.question_id, text: a.text).where.not(id: removed)
         conflicts.each {|c| c.destroy }
       end
     end

test/functional/templates_controller_test.rb

@@ -238,7 +238,7 @@ class TemplatesControllerTest < ActionDispatch::IntegrationTest
 
     assert_equal 0, customization.version
     assert_not customization.published?
-    assert_not customization.dirty?
+    assert customization.dirty?
 
     # Make sure the funder templates data is not modifiable!
     customization.phases.each do |p|