Ensure collections/lps with deleted items can be still be updated

Author: fbacall

app/assets/javascripts/templates/autocompleter/collection_item.hbs

@@ -9,7 +9,11 @@
         <input type="hidden" name="{{ prefix }}[][id]" value="{{ item.id }}" />
         <input type="hidden" name="{{ prefix }}[][resource_id]" value="{{ item.resource_id }}" />
         <input type="hidden" name="{{ prefix }}[][resource_type]" value="{{ item.resource_type }}" />
+        {{#if item.url}}
         <a href="{{ item.url }}" target="_blank">{{ item.title }}</a>
+        {{ else }}
+        <span class="muted">{{ item.title }}</span>
+        {{/if}}
         <input type="text" class="form-control input-sm" name="{{ prefix }}[][comment]" placeholder="Enter a comment" value="{{ item.comment }}">
     </div>
 

app/models/collection_item.rb

@@ -14,10 +14,12 @@ def log_activity
     self.collection.create_activity(:add_item, owner: User.current_user,
                                     parameters: { resource_id: self.resource_id,
                                                   resource_type: self.resource_type,
-                                                  resource_title: self.resource.title })
-    self.resource.create_activity(:add_to_collection, owner: User.current_user,
-                                  parameters: { collection_id: self.collection_id,
-                                                collection_title: self.collection.title })
+                                                  resource_title: self.resource&.title })
+    if self.resource
+      self.resource.create_activity(:add_to_collection, owner: User.current_user,
+                                    parameters: { collection_id: self.collection_id,
+                                                  collection_title: self.collection.title })
+    end
   end
 
   def reindex_resource

app/models/learning_path_topic_item.rb

@@ -13,10 +13,12 @@ def log_activity
     self.topic.create_activity(:add_item, owner: User.current_user,
                                parameters: { resource_id: self.resource_id,
                                              resource_type: self.resource_type,
-                                             resource_title: self.resource.title })
-    self.resource.create_activity(:add_to_topic, owner: User.current_user,
-                                  parameters: { topic_id: self.topic_id,
-                                                topic_title: self.topic.title })
+                                             resource_title: self.resource&.title })
+    if self.resource
+      self.resource.create_activity(:add_to_topic, owner: User.current_user,
+                                    parameters: { topic_id: self.topic_id,
+                                                  topic_title: self.topic.title })
+    end
   end
 
   def previous_item

app/views/collections/_collection_items_form.erb

@@ -15,7 +15,7 @@
   json = items.map do |item|
     { item: { id: item.id, order: item.order, comment: item.comment,
               resource_id: item.resource_id, resource_type: item.resource_type,
-              title: item.resource.title, url: polymorphic_path(item.resource) },
+              title: item.resource&.title || t('deleted_resource'), url: item.resource ? polymorphic_path(item.resource) : nil },
       prefix: form_field_name }
   end.to_json
 

app/views/public_activity/common/_add_item.html.erb

@@ -1,7 +1,7 @@
 <% type = activity.parameters[:resource_type].constantize %>
 <i class="fa fa-pencil-square-o"></i>
 <%= activity_owner(activity) %>
-<%= t('activity.actions.add') %> <%= type.model_name.human.downcase %> <%= link_to activity.parameters[:resource_title],
+<%= t('activity.actions.add') %> <%= type.model_name.human.downcase %> <%= link_to activity.parameters[:resource_title] || t('deleted_resource'),
                                                                                    polymorphic_path(type.model_name.singular.to_sym,
                                                                                                     id: activity.parameters[:resource_id]) %>
 <%= t('activity.target_html', resource: activity_resource(activity)) %>

test/controllers/collections_controller_test.rb

@@ -750,4 +750,42 @@ class CollectionsControllerTest < ActionController::TestCase
       assert_select 'h4', text: 'Deleted resource'
     end
   end
+
+  test 'can edit collection with deleted resource' do
+    sign_in @collection.user
+
+    m1 = materials(:biojs)
+    m2 = materials(:interpro)
+    ci1 = @collection.items.create!(resource: materials(:biojs), order: 2, comment: 'hello')
+    ci2 = @collection.items.create!(resource: materials(:interpro), order: 1, comment: 'hello!!')
+    assert_no_difference('CollectionItem.count') do
+      m1.destroy!
+    end
+
+    get :edit, params: { id: @collection }
+    assert_response :success
+  end
+
+  test 'can update collection with deleted resource' do
+    sign_in @collection.user
+
+    m1 = materials(:biojs)
+    m2 = materials(:interpro)
+    ci1 = @collection.items.create!(resource: materials(:biojs), order: 2, comment: 'hello')
+    ci2 = @collection.items.create!(resource: materials(:interpro), order: 1, comment: 'hello!!')
+    assert_no_difference('CollectionItem.count') do
+      m1.destroy!
+    end
+
+    params = { id: ci1.id, resource_type: ci1.resource_type, resource_id: ci1.resource_id }
+
+    patch :update, params: { id: @collection.id,
+                             collection: {
+                               items_attributes: { '1': params.merge(comment: 'Some comment') }
+                             }
+    }
+
+    assert_redirected_to collection_path(assigns(:collection))
+    assert_equal 'Some comment', ci1.reload.comment
+  end
 end

test/controllers/learning_path_topics_controller_test.rb

@@ -566,4 +566,36 @@ class LearningPathTopicsControllerTest < ActionController::TestCase
       assert_select 'h4', text: 'Deleted resource'
     end
   end
+
+  test 'can edit learning_path_topic with deleted resource' do
+    sign_in @learning_path_topic.user
+
+    topic_item = @learning_path_topic.items.first
+    assert_no_difference('LearningPathTopicItem.count') do
+      topic_item.resource.destroy!
+    end
+
+    get :edit, params: { id: @learning_path_topic }
+    assert_response :success
+  end
+
+  test 'can update learning_path_topic with deleted resource' do
+    sign_in @learning_path_topic.user
+
+    topic_item = @learning_path_topic.items.first
+    params = { id: topic_item.id, resource_type: topic_item.resource_type, resource_id: topic_item.resource_id }
+    assert_no_difference('LearningPathTopicItem.count') do
+      topic_item.resource.destroy!
+    end
+
+    get :edit, params: { id: @learning_path_topic }
+    patch :update, params: { id: @learning_path_topic.id,
+                             learning_path_topic: {
+                               items_attributes: { '1': params.merge(comment: 'Some comment') }
+                             }
+    }
+
+    assert_redirected_to learning_path_topic_path(assigns(:learning_path_topic))
+    assert_equal 'Some comment', topic_item.reload.comment
+  end
 end