sreeni, brady: introduced methods in Child for getting primary photo,…

… removed most of the references to current_photo_key and attachments outside child

app/models/child.rb

@@ -23,7 +23,7 @@ class Child < CouchRestRails::Document
              }
           }"
 
-  validates_with_method :validate_file_name
+  validates_with_method :validate_photo_file_name
   validates_with_method :validate_audio_file_name
   validates_fields_of_type Field::NUMERIC_FIELD
   validates_fields_of_type Field::TEXT_FIELD
@@ -53,8 +53,8 @@ def validate_age
     [false, "Age must be between 1 and 99"]
   end
 
-  def validate_file_name
-    return true if @file_name == nil || /([^\s]+(\.(?i)(jpg|jpeg|png))$)/ =~ @file_name
+  def validate_photo_file_name
+    return true if @file_names.blank? || @file_names.all?{|f| f =~ /([^\s]+(\.(?i)(jpg|jpeg|png))$)/ }
     [false, "Please upload a valid photo file (jpg or png) for this child record"]
   end
 
@@ -123,8 +123,6 @@ def rotate_photo(angle)
 
     name = FileAttachment.generate_name
     attachment = FileAttachment.new(name, exisiting_photo.content_type, image.to_blob)
-    @photo_keys ||= []
-    @photo_keys << attachment.name
     attach(attachment)
   end
 
@@ -134,24 +132,27 @@ def photo=(photo_file)
     unless photo_file.is_a? Hash
       photo_file = {'0' => photo_file}
     end
-    photo_file.values.select {|photo| photo.respond_to? :content_type}.each do |photo|
-        @file_name = photo.original_path
+
+    @file_names = []
+    @photo_keys = photo_file.values.select {|photo| photo.respond_to? :content_type}.collect do |photo|
+        @file_names <<  photo.original_path
         attachment = FileAttachment.from_uploadable_file(photo, "photo-#{photo.path.hash}")
-        @photo_keys ||= []
-        @photo_keys << attachment.name
         attach(attachment)
+        attachment.name
     end
   end
-
+  
   def photos
     return [] if self['photo_keys'].blank?
     self['photo_keys'].collect do |key|
-      data = read_attachment key
-      content_type = self['_attachments'][key]['content_type']
-      FileAttachment.new key, content_type, data      
+      attachment(key)
     end
   end
 
+  def primary_photo
+    attachment(self['current_photo_key'])
+  end
+
   def photo
     photos.first
   end
@@ -240,11 +241,17 @@ def is_filled_in? field
   end
 
   private
+  def attachment(key)
+    data = read_attachment key
+    content_type = self['_attachments'][key]['content_type']
+    FileAttachment.new key, content_type, data      
+  end
+
   def update_photo_keys
-    return unless @photo_keys
+    return if @photo_keys.blank?
     self['photo_keys'] ||= [] 
+    self['current_photo_key'] = @photo_keys.first
     self['photo_keys'].concat @photo_keys
-    self['current_photo_key'] = @photo_keys.last
   end
 
   def attach(attachment)

app/views/children/_picture.html.erb

@@ -11,7 +11,7 @@
 </div>
 
 <div class="thumbnails">
-  <% @child.photos[1..-1].each do |photo| %>
+  <% @child.photos.each do |photo| %>
       <div class="thumbnail"><%= thumbnail_tag(@child, photo.name) %></div>
   <% end %>
 </div>

features/step_definitions/web_steps.rb

@@ -134,7 +134,7 @@
       type = "image/png"
     when "gif"
       type = "image/gif"
-  end
+   end
 
   attach_file(field, path, type)
 end

features/upload_a_childs_photo.feature

@@ -32,16 +32,16 @@ Feature:
     And I press "Save"
     Then I should see "Child record successfully created"
     And I should see the photo of "John"
-    Then I should see "1" thumbnails    
-
+    Then I should see "2" thumbnails    
+        
     When I follow "Edit"
     Then I should see "2" thumbnails
 
   Scenario: Uploading an invalid file in the image field
 
     Given I am on the new child page
-
-    When I fill in the basic details of a child
+    When I fill in "Haiti" for "Last known location"
+    And I fill in "John" for "Name"
     And I attach a photo "features/resources/textfile.txt"
     And I press "Save"
 

spec/controllers/attachments_controller_spec.rb

@@ -15,7 +15,7 @@
     Time.stub!(:now).and_return Time.parse("Jan 20 2010 12:04:15")
     child = Child.create('last_known_location' => "New York", 'photo' => uploadable_photo_jeff)
 
-    get :show, :child_id => child.id, :id => child['current_photo_key']
+    get :show, :child_id => child.id, :id => child.primary_photo.name
 
     response.content_type.should == uploadable_photo_jeff.content_type
     response.body.should == uploadable_photo_jeff.read
@@ -29,7 +29,7 @@
     Time.stub!(:now).and_return Time.parse("Feb 20 2010 12:04")
     created_child.update_attributes :photo => uploadable_photo
 
-    get :show, :child_id => child.id, :id => child['current_photo_key']
+    get :show, :child_id => child.id, :id => child.primary_photo.name
 
     response.content_type.should == uploadable_photo_jeff.content_type
     response.body.should == uploadable_photo_jeff.read

spec/integration/children_spec.rb

@@ -10,29 +10,23 @@
         "age" => "28",
         "last_known_location" => "London",
         "photo" => photo})
-    child.save.should be_true, display_child_errors(child.errors)
-
-    data = get_object(:child, child.id)
-
-    data['name'].should == "Dave"
-    data['age'].should == "28"
-    data['last_known_location'].should == "London"
-    data['_attachments'].should_not be_empty
-    data['_attachments'][data['current_photo_key']]['content_type'].should == photo.content_type
+    child.save.should be_true
   end
 
   it "should load an existing child record from the database" do
-    child_fixture = Child.new_with_user_name("jdoe", {
+    photo = uploadable_photo
+    child = Child.new_with_user_name("jdoe", {
         "name" => "Paul",
         "age" => "10",
-        "last_known_location" => "New York"})
-    child_id = post_object(:child, child_fixture)
-
-    child = Child.get(child_id)
+        "last_known_location" => "New York", "photo" => photo})
+    child.save
+    
+    child_from_db = Child.get(child.id)
 
-    child['name'].should == "Paul"
-    child['age'].should == "10"
-    child['last_known_location'].should == "New York"
+    child_from_db['name'].should == "Paul"
+    child_from_db['age'].should == "10"
+    child_from_db['last_known_location'].should == "New York"
+    child_from_db.primary_photo.data.size.should eql uploadable_photo.data.size
   end
 
   it "should persist multiple photo attachments" do

spec/models/child_spec.rb

@@ -109,20 +109,10 @@
       child['last_updated_at'].should == "2010-01-17 19:05:00UTC"
     end
 
-    it "should update attachments when there is a photo update" do
-      current_time = Time.parse("Jan 17 2010 14:05:32")
-      Time.stub!(:now).and_return current_time
-      child = Child.new
-      child.update_properties_with_user_name "jdoe", uploadable_photo, nil, {}
-      key = child['_attachments'].keys.select {|key| key =~ /.*2010-01-17T140532/}.first
-      child['_attachments'][key]['data'].should_not be_blank
-    end
-
     it "should not update attachments when the photo value is nil" do
       child = Child.new
       child.update_properties_with_user_name "jdoe", nil, nil, {}
-      child['_attachments'].should be_blank
-      child['current_photo_key'].should be_nil
+      child.photos.should be_empty
     end
 
     it "should update attachment when there is audio update" do
@@ -137,13 +127,7 @@
       child = Child.new
       child.photo.should == nil
     end
-
-    it "should do nothing when a zero byte photo is associated with the child" do
-      child = Child.new
-      child['current_photo_key'] = ""
-      child.photo.should == nil
     end
-  end
 
   describe "validation of custom fields" do
     it "should fail to validate if all fields are nil" do      
@@ -153,14 +137,15 @@
       child.errors[:validate_has_at_least_one_field_value].should == ["Please fill in at least one field or upload a file"]
     end
     it "should fail to validate if all fields on child record are the default values" do      
-      child = Child.new({:height=>"",:reunite_with_mother=>"No", :current_photo_key=>nil})
+      child = Child.new({:height=>"",:reunite_with_mother=>"No"})
       FormSection.stub!(:all_enabled_child_fields).and_return [
                 Field.new(:type => Field::NUMERIC_FIELD, :name => 'height'),
                 Field.new(:type => Field::CHECK_BOX, :name => 'reunite_with_mother'),
                 Field.new(:type => Field::PHOTO_UPLOAD_BOX, :name => 'current_photo_key') ]
       child.should_not be_valid
       child.errors[:validate_has_at_least_one_field_value].should == ["Please fill in at least one field or upload a file"]
     end
+
     it "should validate numeric types" do
       fields = [{:type => 'numeric_field', :name => 'height', :display_name => "height"}]
       child = Child.new
@@ -414,86 +399,36 @@
 
     context "with a single new photo" do
       let(:child) {Child.create('photo' => uploadable_photo, 'last_known_location' => 'London')}    
-      it "should create a field with current_photo_key on creation" do
-        child['photo_keys'].size.should == 1
-      end
-
-      it "should create a field with current_photo_key on creation" do
-        child['current_photo_key'].should_not be_empty
-      end
-
-      it "should have current_photo_key as photo attachment key on creation" do
-        child['_attachments'].should have_key(child['current_photo_key'])
-      end
-
-      it "should only have one attachment on creation" do
-        child['_attachments'].size.should == 1
-      end
-
       it "should only have one photo on creation" do
         child.photos.size.should eql 1
       end
 
-      it "should have data after creation" do
-        Child.get(child.id)['_attachments'].values.first['length'].should be > 0
-      end
-
-      it "should be able to read attachment after a photo change" do
-        attachment = child.media_for_key(child['current_photo_key'])
-        attachment.data.read.should == File.read(uploadable_photo.original_path)
+      it "should be the primary photo" do
+        child.primary_photo.data.size.should eql uploadable_photo.data.size
       end
     end
 
     context "with multiple new photos" do
       let(:child) {Child.create('photo' => {'0' => uploadable_photo_jeff, '1' => uploadable_photo_jorge}, 'last_known_location' => 'London')}    
-      it "should add photo key for each uploaded photo" do
-        child['photo_keys'].uniq.size.should == 2
-      end
-
-      it "should only the right number of attachments on creation" do
-        child['_attachments'].size.should == 2
-      end
-
-      it "should have current_photo_key as photo attachment key on creation" do
-        child['photo_keys'].each do |key|
-          child['_attachments'].should have_key(key)
-        end
-      end
-
-      it "should have data after creation" do
-        Child.get(child.id)['_attachments'].values.each do |value|
-          value['length'].should be > 0
-        end
-      end
-
       it "should have corrent number of photos after creation" do
         child.photos.size.should eql 2
       end
 
-      it "should have the last photo as the current photo key" do
-        child['current_photo_key'].should eql(child['photo_keys'].last)
+      it "should return the first photo as a primary photo" do
+        child.primary_photo.data.size.should eql uploadable_photo_jeff.size
       end
     end
 
     context "when updating a photo" do
+      let(:child) {Child.create('photo' => uploadable_photo, 'last_known_location' => 'London')}  
       before(:each) do
         updated_at_time = Time.parse("Feb 20 2010 12:04:32")
         Time.stub!(:now).and_return updated_at_time
         child.update_attributes :photo => uploadable_photo_jeff
       end
 
-      let(:child) {Child.create('photo' => uploadable_photo, 'last_known_location' => 'London')}    
-      it "should update current_photo_key on a photo change" do
-        child['current_photo_key'] =~ /photo.*?-2010-02-20T120432/
-      end
-
-      it "should have updated current_photo_key as photo attachment key on a photo change" do
-        child['_attachments'].keys.join(' ').should =~  /photo.*-2010-02-20T120432/
-      end
-
-      it "should have photo data after a photo change" do
-        key = Child.get(child.id)['_attachments'].keys.select {|key| key =~ /photo.*-2010-02-20T120432/}.first
-        Child.get(child.id)['_attachments'][key]['length'].should be > 0
+      it "should become the primary photo" do
+        child.primary_photo.data.size.should eql uploadable_photo_jeff.size
       end
     end
   end
@@ -654,6 +589,8 @@
       child.update_attributes :photo => uploadable_photo_jeff
 
       changes = child['histories'].first['changes']
+
+      #TODO: this should be instead child.photo_history.first.from or something like that
       changes['current_photo_key']['from'].should =~ /photo.*?-2010-01-20T120424/
     end
 
@@ -667,6 +604,7 @@
       child.update_attributes :photo => uploadable_photo_jeff
 
       changes = child['histories'].first['changes']
+      #TODO: this should be instead child.photo_history.first.to or something like that
       changes['current_photo_key']['to'].should =~ /photo.*?-2010-02-20T120424/
     end
 
@@ -777,7 +715,7 @@
       child.audio.should be_nil
     end
   end
-  
+
   private
 
   def create_child(name)