collection item column changes

rename the mutable description field to user_comment , and add immutable
title and description fields.

attempt to pull the title and description from embedly, if they're
blank.

add image, audio and video item types, and pull that from embedly as
well.

test plan: from the api, create an item with no description or title,
verify that they are auto-filled (after the background job runs, they
can't be auto-filled in the immediate response unfortunately). verify
that adding an item for a vimeo video sets the item_type to "video", for
example.

Change-Id: Ic485962d11d9fbce0ef0455982d60780d7a06800
Reviewed-on: https://gerrit.instructure.com/11171
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Jon Jensen <jon@instructure.com>

app/controllers/collection_items_controller.rb

@@ -49,9 +49,15 @@
 #       user_id: 37,
 #
 #       // The type of the item.
-#       // The only type currently defined is "url", but api consumers should
-#       // expect new types to be returned in the future and handle that
-#       // appropriately (even if it means just ignoring the item).
+#       // Currently defined types are: "url", "image", "audio", and "video".
+#       //
+#       // Canvas may define new item types at any time. "url" is the most
+#       // generic type, and just means any sort of web link. If an api consumer
+#       // sees an item_type that it doesn't yet know how to handle, treating it
+#       // as a "url" is a safe bet.
+#       //
+#       // "image", "audio" and "video" are URLs either directly to a file of that mime type, or
+#       // to a web page that was determined to contain that type as its main content.
 #       item_type: "url",
 #
 #       // The link to the item. For item type of "url", this is the entire
@@ -88,9 +94,16 @@
 #       // If image_url is null but image_pending is false, the item has no image.
 #       image_pending: false,
 #
-#       // The user-provided description of the item. This is plain text.
+#       // The title of the item.
+#       title: "My Image",
+#
+#       // The description of the item. This is plain text.
 #       description: "some block of plain text",
 #
+#       // Any user-provided comments on the item. A user can add their own
+#       // comments when cloning an existing item. This is plain text.
+#       user_comment: "some block of plain text",
+#
 #       // A snippet of HTML that can be used as an in-line preview of the
 #       // item. For example, a link to a youtube page may have an iframe inline
 #       // embed of the video.
@@ -131,7 +144,9 @@ class CollectionItemsController < ApplicationController
   #          upvoted_by_user: false,
   #          root_item_id: 3,
   #          image_url: "https://<canvas>/files/item_image.png",
+  #          title: "my title",
   #          description: "some block of plain text",
+  #          user_comment: nil,
   #          url: "https://<canvas>/api/v1/collections/items/7"
   #          created_at: "2012-05-30T17:45:25Z",
   #       }
@@ -165,7 +180,9 @@ class CollectionItemsController < ApplicationController
   #        upvoted_by_user: false,
   #        root_item_id: 3,
   #        image_url: "https://<canvas>/files/item_image.png",
+  #        title: "my title",
   #        description: "some block of plain text",
+  #        user_comment: nil,
   #        url: "https://<canvas>/api/v1/collections/items/7"
   #        created_at: "2012-05-30T17:45:25Z",
   #     }
@@ -176,7 +193,8 @@ class CollectionItemsController < ApplicationController
     end
   end
 
-  ITEM_SETTABLE_ATTRIBUTES = %w(description)
+  NEW_ITEM_DATA_SETTABLE_ATTRIBUTES = %w(image_url title description)
+  ITEM_SETTABLE_ATTRIBUTES = %w(user_comment)
 
   # @API Create or clone a collection item
   #
@@ -191,23 +209,32 @@ class CollectionItemsController < ApplicationController
   #   To clone an existing item, pass in the url to that item as returned in
   #   the JSON response in the "url" field.
   #
+  # @argument title The title of the item.
+  #   If no title  is provided, Canvas will try to automatically
+  #   add a relevant title based on the linked content.
+  #
   # @argument description The plain-text description of the item.
+  #   If no description is provided, Canvas will try to automatically
+  #   add a relevant description based on the linked content.
   #
   # @argument image_url The URL of the image to use for this item. If no image
-  #   url is provided, canvas will try to automatically determine an image
+  #   url is provided, Canvas will try to automatically determine an image
   #   representation for the link. This parameter is ignored if the new item is
   #   a clone of an existing item.
   #
+  # @argument user_comment The user's comments on the item. This can be set
+  #   when cloning an existing item, as well.
+  #
   # @example_request
   #     curl https://<canvas>/api/v1/collections/<collection_id>/items \ 
   #          -F link_url="http://www.google.com/" \ 
-  #          -F description="lmgtfy" \ 
+  #          -F user_comment="lmgtfy" \ 
   #          -H 'Authorization: Bearer <token>'
   #
   # @example_request
   #     curl https://<canvas>/api/v1/collections/<collection_id>/items \ 
   #          -F link_url="https://<canvas>/api/v1/collections/items/3" \ 
-  #          -F description="clone of some other item" \ 
+  #          -F user_comment="clone of some other item" \ 
   #          -H 'Authorization: Bearer <token>'
   #
   def create
@@ -215,7 +242,9 @@ class CollectionItemsController < ApplicationController
     if authorized_action(@item, @current_user, :create)
       item_data = CollectionItemData.data_for_url(params[:link_url] || "", @current_user)
       return render_unauthorized_action unless item_data
-      item_data.image_url = params[:image_url] if item_data.new_record?
+      if item_data.new_record?
+        item_data.attributes = params.slice(*NEW_ITEM_DATA_SETTABLE_ATTRIBUTES)
+      end
       @item.collection_item_data = item_data
       @item.attributes = params.slice(*ITEM_SETTABLE_ATTRIBUTES)
 
@@ -234,12 +263,12 @@ class CollectionItemsController < ApplicationController
   #
   # Change a collection item's mutable attributes.
   #
-  # @argument description
+  # @argument user_comment
   #
   # @example_request
   #     curl https://<canvas>/api/v1/collections/items/<item_id> \ 
   #          -X PUT \ 
-  #          -F description='edited description' \ 
+  #          -F user_comment='edited comment' \ 
   #          -H 'Authorization: Bearer <token>'
   #
   def update

app/models/collection_item.rb

@@ -25,7 +25,7 @@ class CollectionItem < ActiveRecord::Base
   alias :data :collection_item_data
   belongs_to :user
 
-  attr_accessible :collection, :collection_item_data, :description, :user
+  attr_accessible :collection, :collection_item_data, :user_comment, :user
 
   validates_presence_of :collection, :collection_item_data, :user
   validates_associated :collection_item_data

app/models/collection_item_data.rb

@@ -23,14 +23,14 @@ class CollectionItemData < ActiveRecord::Base
   belongs_to :image_attachment, :class_name => "Attachment"
   has_many :collection_item_upvotes
 
-  VALID_ITEM_TYPES = %w(url)
+  VALID_ITEM_TYPES = %w(url image audio video)
   THUMBNAIL_SIZE = "640x>"
 
   validates_inclusion_of :item_type, :in => VALID_ITEM_TYPES
   validates_as_url :link_url
   validates_presence_of :link_url
 
-  attr_accessible :root_item, :item_type, :link_url
+  attr_accessible :root_item, :item_type, :link_url, :image_url, :description, :title
 
   before_create :prepare_to_snapshot_link_url
   after_create :snapshot_link_url
@@ -43,6 +43,12 @@ class CollectionItemData < ActiveRecord::Base
     embedly_data = Canvas::Embedly.new(link_url)
 
     self.html_preview = embedly_data.object_html
+    self.title = embedly_data.title if self.title.blank?
+    self.description = embedly_data.description if self.description.blank?
+
+    if self.item_type == "url" && VALID_ITEM_TYPES.include?(embedly_data.data_type)
+      self.item_type = embedly_data.data_type
+    end
 
     if image_url.present?
       attachment = Canvas::HTTP.clone_url_as_attachment(image_url)

db/migrate/20120531183543_update_collection_item_columns.rb

@@ -0,0 +1,17 @@
+class UpdateCollectionItemColumns < ActiveRecord::Migration
+  tag :predeploy
+
+  def self.up
+    rename_column :collection_items, :description, :user_comment
+
+    add_column :collection_item_datas, :title, :string
+    add_column :collection_item_datas, :description, :text
+  end
+
+  def self.down
+    rename_column :collection_items, :user_comment, :description
+
+    remove_column :collection_item_datas, :title, :string
+    remove_column :collection_item_datas, :description, :text
+  end
+end

lib/api/v1/collection.rb

@@ -24,11 +24,11 @@ module Api::V1::Collection
   }
 
   API_COLLECTION_ITEM_JSON_OPTS = {
-    :only => %w(id collection_id user_id description created_at),
+    :only => %w(id collection_id user_id user_comment created_at),
   }
 
   API_COLLECTION_ITEM_DATA_JSON_OPTS = {
-    :only => %w(item_type link_url root_item_id post_count upvote_count html_preview),
+    :only => %w(item_type link_url root_item_id post_count upvote_count html_preview title description),
     :methods => %w(upvoted_by_user),
   }
 

lib/canvas/embedly.rb

@@ -16,7 +16,7 @@
 # with this program. If not, see <http://www.gnu.org/licenses/>.
 #
 
-class Canvas::Embedly < Struct.new(:title, :description, :images, :object_html)
+class Canvas::Embedly < Struct.new(:title, :description, :images, :object_html, :data_type)
 
   class Image < Struct.new(:url)
     def as_json(*a)
@@ -34,7 +34,7 @@ class Canvas::Embedly < Struct.new(:title, :description, :images, :object_html)
   MAXWIDTH = 640
 
   def as_json(*a)
-    { 'title' => self.title, 'description' => self.description, 'images' => self.images.map { |i| i.as_json(*a) }, 'object_html' => self.object_html }
+    { 'title' => self.title, 'description' => self.description, 'images' => self.images.map { |i| i.as_json(*a) }, 'object_html' => self.object_html, 'data_type' => self.data_type }
   end
 
   protected
@@ -48,6 +48,7 @@ class Canvas::Embedly < Struct.new(:title, :description, :images, :object_html)
       return
     end
 
+    self.data_type = data.type
     self.title = data.title
     self.description = data.description
     if data.images
@@ -64,6 +65,14 @@ class Canvas::Embedly < Struct.new(:title, :description, :images, :object_html)
       self.object_html = data.html
     end
 
+    # reject non-iframe html embeds
+    if self.object_html.present?
+      doc = Nokogiri::HTML::DocumentFragment.parse(self.object_html)
+      if doc.children.map { |c| c.name.downcase } != ['iframe']
+        self.object_html = nil
+      end
+    end
+
     @raw_response = data
   end
 

spec/apis/v1/collections_spec.rb

@@ -154,9 +154,9 @@ describe "Collections API", :type => :integration do
   end
 
   def create_collection_items(user)
-    @i1 = collection_item_model(:description => "item 1", :user => user, :collection => @c1, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/one"))
-    @i2 = collection_item_model(:description => "item 2", :user => user, :collection => @c1, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/two"))
-    @i3 = collection_item_model(:description => "item 3", :user => user, :collection => @c2, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/three"))
+    @i1 = collection_item_model(:user_comment => "item 1", :user => user, :collection => @c1, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/one"))
+    @i2 = collection_item_model(:user_comment => "item 2", :user => user, :collection => @c1, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/two"))
+    @i3 = collection_item_model(:user_comment => "item 3", :user => user, :collection => @c2, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/three"))
     @unscoped_items_path = "/api/v1/collections/items"
     @c1_items_path = "/api/v1/collections/#{@c1.id}/items"
     @c2_items_path = "/api/v1/collections/#{@c2.id}/items"
@@ -179,9 +179,11 @@ describe "Collections API", :type => :integration do
       'image_url' => item.data.image_attachment && "http://www.example.com/images/thumbnails/#{item.data.image_attachment.id}/#{item.data.image_attachment.uuid}?size=640x%3E",
       'image_pending' => item.data.image_pending,
       'html_preview' => item.data.html_preview,
-      'description' => item.description,
+      'user_comment' => item.user_comment,
       'url' => "http://www.example.com/api/v1/collections/items/#{item.id}",
       'created_at' => item.created_at.iso8601,
+      'description' => item.data.description,
+      'title' => item.data.title,
     }
   end
 
@@ -214,7 +216,7 @@ describe "Collections API", :type => :integration do
         @user2 = @user
         @user = @user1
         @c3 = @user2.collections.create!(:name => 'user2', :visibility => 'public')
-        @i4 = collection_item_model(:description => "cloned item 3", :user => @c3.context, :collection => @c3, :collection_item_data => @i3.collection_item_data); @i3.reload
+        @i4 = collection_item_model(:user_comment => "cloned item 3", :user => @c3.context, :collection => @c3, :collection_item_data => @i3.collection_item_data); @i3.reload
         @items3_path = "/api/v1/collections/#{@c3.id}/items"
         @items3_path_options = { :controller => "collection_items", :action => "index", :format => "json", :collection_id => @c3.to_param }
       end
@@ -227,14 +229,14 @@ describe "Collections API", :type => :integration do
 
       describe "item creation" do
         it "should allow creating from a http url" do
-          json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :description => 'new item' })
+          json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
           new_item = @c1.collection_items.last(:order => :id)
           new_item.collection_item_data.link_url.should == "http://www.example.com/a/b/c"
           new_item.user.should == @user
         end
 
         it "should allow cloning an existing item" do
-          json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://localhost/api/v1/collections/items/#{@i3.id}", :description => 'cloned' })
+          json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://localhost/api/v1/collections/items/#{@i3.id}", :user_comment => 'cloned' })
           json['post_count'].should == 3
           new_item = @c1.collection_items.last(:order => :id)
           new_item.collection_item_data.should == @i3.collection_item_data
@@ -244,19 +246,19 @@ describe "Collections API", :type => :integration do
         it "should not allow cloning an item the user can't access" do
           @user = @user2
           expect {
-            json = api_call(:post, @items3_path, @items3_path_options.merge(:action => "create"), { :link_url => "http://localhost/api/v1/collections/items/#{@i1.id}", :description => 'cloned' }, {}, :expected_status => 401)
+            json = api_call(:post, @items3_path, @items3_path_options.merge(:action => "create"), { :link_url => "http://localhost/api/v1/collections/items/#{@i1.id}", :user_comment => 'cloned' }, {}, :expected_status => 401)
           }.to change(CollectionItem, :count).by(0)
         end
 
         it "should reject non-http urls" do
           expect {
-            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "javascript:alert(1)", :description => 'new item' }, {}, :expected_status => 400)
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "javascript:alert(1)", :user_comment => 'new item' }, {}, :expected_status => 400)
           }.to change(CollectionItem, :count).by(0)
         end
 
         describe "images" do
           it "should take a snapshot of the link url if no image is provided and there is no embedly image" do
-            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :description => 'new item' })
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
             @item = CollectionItem.find(json['id'])
             @item.data.image_pending.should == true
             @att = Attachment.new(:uploaded_data => stub_png_data)
@@ -274,7 +276,7 @@ describe "Collections API", :type => :integration do
           end
 
           it "should clone and use the image if provided" do
-            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :image_url => "http://www.example.com/my/image.png", :description => 'new item' })
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :image_url => "http://www.example.com/my/image.png", :user_comment => 'new item' })
             @item = CollectionItem.find(json['id'])
             @item.data.image_pending.should == true
             http_res = mock('Net::HTTPOK', :body => File.read(Rails.root+"public/images/cancel.png"), :code => 200)
@@ -292,7 +294,7 @@ describe "Collections API", :type => :integration do
           end
 
           it "should use the embedly image if no image is provided" do
-            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :description => 'new item' })
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
             @item = CollectionItem.find(json['id'])
             @item.data.image_pending.should == true
             Canvas::Embedly.any_instance.expects(:get_embedly_data).with("http://www.example.com/a/b/c").returns(stub_everything('embedly api', :type => 'test', :images => [{'url' => 'http://www.example.com/image1'}], :html => "<iframe>test</iframe>"))
@@ -312,17 +314,61 @@ describe "Collections API", :type => :integration do
             json['image_url'].should == "http://www.example.com/images/thumbnails/#{@att.id}/#{@att.uuid}?size=640x%3E"
           end
         end
+
+        describe "embedly data" do
+          it "should use the embeldy description and title if none are given" do
+            Canvas::Embedly.any_instance.stubs(:get_embedly_data).with("http://www.example.com/a/b/c").returns(stub_everything('embedly api', :type => 'html', :description => 'e desc', :title => 'e title'))
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
+            run_job()
+            CollectionItem.find(json['id']).data.attributes.slice('title', 'description').should == { 'title' => "e title", 'description' => "e desc" }
+
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item', :title => "custom title" })
+            run_job()
+            CollectionItem.find(json['id']).data.attributes.slice('title', 'description').should == { 'title' => "custom title", 'description' => "e desc" }
+
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item', :description => "custom description" })
+            run_job()
+            CollectionItem.find(json['id']).data.attributes.slice('title', 'description').should == { 'title' => "e title", 'description' => "custom description" }
+          end
+
+          it "should use the embedly item type if valid" do
+            Canvas::Embedly.any_instance.stubs(:get_embedly_data).with("http://www.example.com/a/b/c").returns(stub_everything('embedly api', :type => 'video'))
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
+            run_job()
+            CollectionItem.find(json['id']).data.item_type.should == 'video'
+
+            Canvas::Embedly.any_instance.stubs(:get_embedly_data).with("http://www.example.com/a/b/c").returns(stub_everything('embedly api', :type => 'rtf'))
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
+            run_job()
+            CollectionItem.find(json['id']).data.item_type.should == 'url'
+          end
+
+          it "should only allow iframe embeds" do
+            iframe_html =  "<iframe src='http://example.com/'></iframe>"
+            div_html = "<div class='blah'><p>text</p></div>"
+
+            Canvas::Embedly.any_instance.expects(:get_embedly_data).with("http://www.example.com/a/b/c").returns(stub_everything('embedly api', :type => 'html', :html => iframe_html))
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
+            run_job()
+            CollectionItem.find(json['id']).data.html_preview.should == iframe_html
+
+            Canvas::Embedly.any_instance.expects(:get_embedly_data).with("http://www.example.com/a/b/c").returns(stub_everything('embedly api', :type => 'html', :html => div_html))
+            json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
+            run_job()
+            CollectionItem.find(json['id']).data.html_preview.should be_blank
+          end
+        end
       end
 
       it "should allow editing mutable fields" do
         json = api_call(:put, "#{@unscoped_items_path}/#{@i1.id}", @unscoped_items_path_options.merge(:item_id => @i1.to_param, :action => "update"), {
-          :description => "modified",
+          :user_comment => "modified",
           :link_url => 'cant change',
           :item_type => 'cant change',
           :image_url => "http://www.example.com/cant_change"
         })
         json.should == item_json(@i1.reload)
-        @i1.description.should == "modified"
+        @i1.user_comment.should == "modified"
         @i1.collection_item_data.item_type.should == "url"
         @i1.data.image_pending.should == false
       end
@@ -528,7 +574,7 @@ describe "Collections API", :type => :integration do
         @user2 = user_with_pseudonym
         @group_membership2 = @group.add_user(@user2)
 
-        @i4 = collection_item_model(:description => "item 4", :user => @user2, :collection => @c2, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/three"))
+        @i4 = collection_item_model(:user_comment => "item 4", :user => @user2, :collection => @c2, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/three"))
       end
 
       context "as a group member" do
@@ -539,18 +585,18 @@ describe "Collections API", :type => :integration do
         end
 
         it "should allow creating a new item" do
-          json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :description => 'new item' })
+          json = api_call(:post, @c1_items_path, @c1_items_path_options.merge(:action => "create"), { :link_url => "http://www.example.com/a/b/c", :user_comment => 'new item' })
           new_item = @c1.collection_items.last(:order => :id)
           new_item.collection_item_data.link_url.should == "http://www.example.com/a/b/c"
           new_item.user.should == @user
         end
 
         it "should not allow updating an item created by someone else" do
-          orig_description = @i1.description
+          orig_user_comment = @i1.user_comment
           json = api_call(:put, "#{@unscoped_items_path}/#{@i1.id}", @unscoped_items_path_options.merge(:item_id => @i1.to_param, :action => "update"), {
-            :description => "item1 description edited",
+            :user_comment => "item1 user_comment edited",
           }, {}, :expected_status => 401)
-          @i1.reload.description.should == orig_description
+          @i1.reload.user_comment.should == orig_user_comment
         end
 
         it "should not allow deleting an item created by someone else" do
@@ -569,9 +615,9 @@ describe "Collections API", :type => :integration do
         end
 
         it "should allow updating an item created by someone else" do
-          json = api_call(:put, "#{@unscoped_items_path}/#{@i4.id}", @unscoped_items_path_options.merge(:item_id => @i4.to_param, :action => "update"), { :description => "modified" })
+          json = api_call(:put, "#{@unscoped_items_path}/#{@i4.id}", @unscoped_items_path_options.merge(:item_id => @i4.to_param, :action => "update"), { :user_comment => "modified" })
           json.should == item_json(@i4.reload)
-          @i4.description.should == "modified"
+          @i4.user_comment.should == "modified"
         end
 
         it "should allow deleting an item created by someone else" do

spec/apis/v1/discussion_topics_api_spec.rb

@@ -1156,7 +1156,7 @@ describe DiscussionTopicsController, :type => :integration do
   context "collection items" do
     before(:each) do
       @collection = @user.collections.create!(:name => 'test1', :visibility => 'private')
-      @item = collection_item_model(:description => "item 1", :user => @collection.context, :collection => @collection, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/one"))
+      @item = collection_item_model(:user_comment => "item 1", :user => @collection.context, :collection => @collection, :collection_item_data => collection_item_data_model(:link_url => "http://www.example.com/one"))
     end
 
     it "should return a discussion topic for an item" do

spec/factories/collections_factory.rb

@@ -17,7 +17,7 @@ end
 
 def valid_collection_item_attributes
   {
-    :description => "test item",
+    :user_comment => "test item",
   }
 end
 

spec/integration/collection_items_controller_spec.rb

@@ -38,6 +38,7 @@ describe CollectionItemsController do
         'description' => nil,
         'images' => [],
         'object_html' => nil,
+        'data_type' => nil,
       }
     end
 
@@ -53,12 +54,13 @@ describe CollectionItemsController do
         'description' => data.description,
         'images' => [{ 'url' => data.thumbnail_url }],
         'object_html' => nil,
+        'data_type' => nil,
       }
     end
 
     it "should return extended data for paid embedly accounts" do
       user_session(user)
-      data = OpenObject.new(:title => "t1", :description => "d1", :images => [{'url' => 'u1'},{'url' => 'u2'}], :object => OpenObject.new(:html => "<b>hai</b>"))
+      data = OpenObject.new(:title => "t1", :description => "d1", :images => [{'url' => 'u1'},{'url' => 'u2'}], :object => OpenObject.new(:html => "<iframe src='test'></iframe>"))
       PluginSetting.expects(:settings_for_plugin).with(:embedly).returns({ :api_key => 'test', :plan_type => 'paid'})
       Embedly::API.any_instance.expects(:preview).with(:url => "http://www.example.com/", :maxwidth => Canvas::Embedly::MAXWIDTH).returns([data])
       post "/collection_items/link_data", :url => "http://www.example.com/"
@@ -67,7 +69,8 @@ describe CollectionItemsController do
         'title' => data.title,
         'description' => data.description,
         'images' => [{ 'url' => 'u1' }, { 'url' => 'u2' }],
-        'object_html' => "<b>hai</b>",
+        'object_html' => "<iframe src='test'></iframe>",
+        'data_type' => nil,
       }
     end
   end