Merge pull request #530 from berkmancenter/13176_url_kludge_2

Notify users when works were submitted without URLs

app/serializers/notice_serializer.rb

@@ -3,20 +3,40 @@ class NoticeSerializer < ActiveModel::Serializer
              :topics, :sender_name, :principal_name, :recipient_name, :works,
              :tags, :jurisdictions, :action_taken, :language
 
+  # Data to be returned when a Work has no associated URL.
+  FALLBACK = [{ url: 'No URL submitted' }].freeze
+
   # TODO: serialize additional entities
 
   def topics
     object.topics.map(&:name)
   end
 
+  # It would be cleaner to define a WorkSerializer and follow
+  # active-model-serializer conventions. However, we rename fields on Work in
+  # our API responses, conditionally upon the type of Notice associated. That
+  # is not a use case anticipated by active-model-serializer, and it turns out
+  # to be much easier to define the work attribute here, and then have it
+  # accessible as a hash (rather than as an object to be serialized) so we can
+  # use hash operations on it within the hooks supported by
+  # active-model-serializer.
   def works
-    object.works.as_json(
+    base_works = object.works.as_json(
       only: [:description],
       include: {
         infringing_urls: { only: %i[url url_original] },
         copyrighted_urls: { only: %i[url url_original] }
       }
     )
+    cleaned_works(base_works)
+  end
+
+  def cleaned_works(base_works)
+    base_works.each do |work|
+      work['infringing_urls'] = FALLBACK if work['infringing_urls'].empty?
+      work['copyrighted_urls'] = FALLBACK if work['copyrighted_urls'].empty?
+    end
+    base_works
   end
 
   def tags

app/views/notices/_works_urls.html.erb

@@ -2,8 +2,11 @@
   <div class="row">
     <span class="label original-title"><%= original_title %></span>
     <ol class="list original-urls">
-      <%# Don't do code style changes here, it's intentional to avoid additional spaces in the response %>
-      <% work.copyrighted_urls.each do |url| %><%= content_tag_for(:li, url) do %><%= url.url %><% end %><% end %>
+      <% if work.copyrighted_urls.each do |url| %>
+        <%= content_tag_for(:li, url) do %><%= url.url %><% end %>
+      <% end.empty? %>
+        No copyrighted URLs were submitted.
+      <% end %>
     </ol>
   </div>
 <% end %>
@@ -12,8 +15,11 @@
   <div class="row">
     <span class="label infringing-title"><%= infringing_title %></span>
     <ol class="list infringing-urls">
-      <%# Don't do code style changes here, it's intentional to avoid additional spaces in the response %>
-      <% work.infringing_urls.each do |url| %><%= content_tag_for(:li, url) do %><%= url.url %><% end %><% end %>
+      <% if work.infringing_urls.each do |url| %>
+        <%= content_tag_for(:li, url) do %><%= url.url %><% end %>
+      <% end.empty? %>
+        No infringing URLs were submitted.
+      <% end %>
     </ol>
   </div>
 <% end %>

spec/support/elasticsearch.rb

@@ -30,7 +30,7 @@
   # the connection pool. (This may be a multithreading issue, with Capybara
   # and webrick running in different threads; in theory the connection pool
   # handling should be threadsafe but in practice maybe it isn't.)
-  config.before :all, type: :feature do
+  config.before :all, type: :integration do
     searchable_models.each do |model|
       begin
         model.__elasticsearch__.client.transport.reload_connections!

spec/views/notices/show.html.erb_spec.rb

@@ -199,6 +199,140 @@
     end
   end
 
+  it 'displays a warning about infringing urls when expected but absent' do
+    params = {
+      notice: {
+        title: 'A title',
+        type: 'DMCA',
+        subject: 'Infringement Notfication via Blogger Complaint',
+        date_sent: '2013-05-22',
+        date_received: '2013-05-23',
+        works_attributes: [
+          {
+            description: 'The Avengers',
+            copyrighted_urls_attributes: [
+              { url: 'http://example.com/test_url_1' },
+              { url: 'http://example.com/test_url_2' },
+              { url: 'http://example.com/test_url_3' }
+            ]
+          }
+        ],
+        entity_notice_roles_attributes: [
+          {
+            name: 'recipient',
+            entity_attributes: {
+              name: 'Google',
+              kind: 'organization',
+              address_line_1: '1600 Amphitheatre Parkway',
+              city: 'Mountain View',
+              state: 'CA',
+              zip: '94043',
+              country_code: 'US'
+            }
+          },
+          {
+            name: 'sender',
+            entity_attributes: {
+              name: 'Joe Lawyer',
+              kind: 'individual',
+              address_line_1: '1234 Anystreet St.',
+              city: 'Anytown',
+              state: 'CA',
+              zip: '94044',
+              country_code: 'US'
+            }
+          }
+        ]
+      }
+    }
+
+    notice = Notice.new(params[:notice])
+    notice.save
+
+    assign(:notice, notice)
+
+    render
+
+    notice.works.each do |work|
+      expect(rendered).to have_css("#work_#{work.id} .description",
+                                   text: work.description)
+
+      work.copyrighted_urls.each do |url|
+        expect(rendered).to have_css("#work_#{work.id} li.copyrighted_url",
+                                     text: url.url)
+      end
+
+      expect(rendered).to have_text 'No infringing URLs were submitted.'
+    end
+  end
+
+  it 'displays a warning about copyrighted urls when expected but absent' do
+    params = {
+      notice: {
+        title: 'A title',
+        type: 'DMCA',
+        subject: 'Infringement Notfication via Blogger Complaint',
+        date_sent: '2013-05-22',
+        date_received: '2013-05-23',
+        works_attributes: [
+          {
+            description: 'The Avengers',
+            infringing_urls_attributes: [
+              { url: 'http://example.com/test_url_1' },
+              { url: 'http://example.com/test_url_2' },
+              { url: 'http://example.com/test_url_3' }
+            ]
+          }
+        ],
+        entity_notice_roles_attributes: [
+          {
+            name: 'recipient',
+            entity_attributes: {
+              name: 'Google',
+              kind: 'organization',
+              address_line_1: '1600 Amphitheatre Parkway',
+              city: 'Mountain View',
+              state: 'CA',
+              zip: '94043',
+              country_code: 'US'
+            }
+          },
+          {
+            name: 'sender',
+            entity_attributes: {
+              name: 'Joe Lawyer',
+              kind: 'individual',
+              address_line_1: '1234 Anystreet St.',
+              city: 'Anytown',
+              state: 'CA',
+              zip: '94044',
+              country_code: 'US'
+            }
+          }
+        ]
+      }
+    }
+
+    notice = Notice.new(params[:notice])
+    notice.save
+
+    assign(:notice, notice)
+
+    render
+
+    notice.works.each do |work|
+      expect(rendered).to have_css("#work_#{work.id} .description",
+                                   text: work.description)
+
+      work.infringing_urls.each do |url|
+        expect(rendered).to have_css("#work_#{work.id} li.infringing_url",
+                                     text: url.url)
+      end
+
+      expect(rendered).to have_text 'No copyrighted URLs were submitted.'
+    end
+  end
+
   it 'displays the notice source' do
     assign(:notice, build(:dmca, source: 'Arbitrary source'))