Further work on playlist item testing

app/models/ability.rb

@@ -150,9 +150,11 @@ def playlist_permissions
       # can :create, Playlist
     end
     can :read, Playlist, visibility: Playlist::PUBLIC
+
+    playlist_item_permissions
   end
 
-  def playlist_items
+  def playlist_item_permissions
     if @user.id.present?
       can :manage, PlaylistItem do |playlist_item|
         can? :manage, playlist_item.playlist

app/models/avalon_annotation.rb

@@ -5,8 +5,6 @@ class AvalonAnnotation < ActiveAnnotations::Annotation
   after_save :post_to_solr
   after_destroy :delete_from_solr
 
-  attr_accessor :master_file
-
   alias_method :comment, :content
   alias_method :comment=, :content=
 
@@ -15,10 +13,9 @@ class AvalonAnnotation < ActiveAnnotations::Annotation
 
   validates :master_file, :title, :start_time, presence: true
   validates :start_time, numericality: { greater_than_or_equal_to: 0, message: "must be greater than or equal to 0"}
-  validates :end_time, numericality: { greater_than: Proc.new {|a| Float(a.start_time) rescue 0}, less_than_or_equal_to: Proc.new {|a| a.master_file.duration.to_f }, message: "must be between start time and end of section"}
+  validates :end_time, numericality: { greater_than: Proc.new {|a| Float(a.start_time) rescue 0}, less_than_or_equal_to: Proc.new {|a| a.master_file.duration.to_f rescue -1}, message: "must be between start time and end of section"}
 
   after_initialize do
-    self.source = master_file unless master_file.nil?
     selector_default!
     title_default!
   end
@@ -88,11 +85,13 @@ def title_default!
 
   # Sets the class variable @master_file by finding the master referenced in the source uri
   def master_file
-    @master_file ||= MasterFile.find(source.split('/').last) if source
+    @master_file ||= MasterFile.find(self.source.split('/').last) if self.source
   end
 
   def master_file=(value)
     @master_file = value
+    self.source = @master_file
+    @master_file
   end
 
   # Calcuates the mediafragment_uri based on either the internal fragment value or start and end times

spec/factories/avalon_annotation.rb

@@ -14,7 +14,7 @@
 
 FactoryGirl.define do
   factory :avalon_annotation do
-    master_file
+    master_file { FactoryGirl.create(:master_file) }
     title { Faker::Lorem.word }
     comment { Faker::Lorem.sentence }
     start_time { 0.0 }

spec/factories/master_files.rb

@@ -19,7 +19,7 @@
     percent_complete {"#{rand(100)}"}
     workflow_name 'avalon'
     duration {'100'}
-    mediaobject {FactoryGirl.create(:media_object)}
+    association :mediaobject, factory: :media_object
 
     factory :master_file_with_derivative do
       workflow_name 'avalon'

spec/factories/playlist_item.rb

@@ -15,7 +15,7 @@
 FactoryGirl.define do
   factory :playlist_item do
     playlist
-    avalon_annotation
+    association :annotation, factory: :avalon_annotation
     #title { Faker::Lorem.word }
     #comment { Faker::Lorem.sentence }
     #start_time { 0.0 }

spec/models/playlist_item_spec.rb

@@ -1,4 +1,5 @@
 require 'spec_helper'
+require 'cancan/matchers'
 
 RSpec.describe PlaylistItem, type: :model do
   describe 'validations' do
@@ -10,28 +11,46 @@
     subject{ ability }
     let(:ability){ Ability.new(user) }
     let(:user){ FactoryGirl.create(:user) }
+    let(:master_file) { FactoryGirl.create(:master_file) }
+    let(:avalon_annotation) { FactoryGirl.create(:avalon_annotation, master_file: master_file) }
+    let(:playlist_item) { FactoryGirl.create(:playlist_item, playlist: playlist, annotation: avalon_annotation) }
 
     context 'when owner' do
       let(:playlist) { FactoryGirl.create(:playlist, user: user) }
-      let(:playlist_item) { FactoryGirl.create(:playlist_item, playlist: playlist) }
 
       it{ is_expected.to be_able_to(:manage, playlist_item) }
       it{ is_expected.to be_able_to(:create, playlist_item) }
-      it{ is_expected.to be_able_to(:read, playlist_item) }
       it{ is_expected.to be_able_to(:update, playlist_item) }
       it{ is_expected.to be_able_to(:delete, playlist_item) }
+
+      context 'when master file is NOT readable by user' do
+        it{ is_expected.not_to be_able_to(:read, playlist_item) }
+      end
+
+      context 'when master file is readable by user' do
+        let(:media_object) { FactoryGirl.create(:media_object, visibility: 'public') }
+        let(:master_file) { FactoryGirl.create(:master_file, mediaobject: media_object) }
+        it{ is_expected.to be_able_to(:read, playlist_item) }
+      end
     end
 
     context 'when other user' do
       let(:playlist) { FactoryGirl.create(:playlist, visibility: Playlist::PUBLIC) }
-      let(:playlist_item) { FactoryGirl.create(:playlist_item, playlist: playlist) }
 
       it{ is_expected.not_to be_able_to(:manage, playlist_item) }
       it{ is_expected.not_to be_able_to(:create, playlist_item) }
-      it{ is_expected.to be_able_to(:read, playlist_item) }
       it{ is_expected.not_to be_able_to(:update, playlist_item) }
       it{ is_expected.not_to be_able_to(:delete, playlist_item) }
+
+      context 'when master file is NOT readable by user' do
+        it{ is_expected.not_to be_able_to(:read, playlist_item) }
+      end
+
+      context 'when master file is readable by user' do
+        let(:media_object) { FactoryGirl.create(:media_object, visibility: 'public') }
+        let(:master_file) { FactoryGirl.create(:master_file, mediaobject: media_object) }
+        it{ is_expected.to be_able_to(:read, playlist_item) }
+      end
     end
   end
-
 end