From b601fe40b3952649f0ffe4b9fd9f31ea09a5a6cc Mon Sep 17 00:00:00 2001
From: Mackenzie Morgan <macoafi@gmail.com>
Date: Tue, 24 May 2016 16:55:37 -0400
Subject: [PATCH] refactor photo controller create for readability

---
 app/controllers/photos_controller.rb | 53 ++++++++++++++--------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb
index 8130ef4e1..73d47ec5a 100644
--- a/app/controllers/photos_controller.rb
+++ b/app/controllers/photos_controller.rb
@@ -63,35 +63,20 @@ class PhotosController < ApplicationController
     @photo.owner_id = current_member.id
     @photo.set_flickr_metadata
 
-    # several models can have photos. we need to know what model and the id
-    # for the entry to attach the photo to
-    valid_models = ["planting", "harvest", "garden"]
-    if params[:type]
-      if valid_models.include?(params[:type])
-        if params[:id]
-          item = params[:type].camelcase.constantize.find_by_id(params[:id])
-          if item
-            if item.owner.id == current_member.id
-              #  This syntax is weird, so just know that it means this:
-              #  @photo.harvests << item unless @photo.harvests.include?(item)
-              #  but with the correct many-to-many relationship automatically referenced
-              (@photo.send "#{params[:type]}s") << item unless (@photo.send "#{params[:type]}s").include?(item)
-            else
-              flash[:alert] = "You must own both the #{params[:type]} and the photo."
-            end
-          else
-            flash[:alert] = "Couldn't find #{params[:type]} to connect to photo."
-          end
-        else
-          flash[:alert] = "Missing id parameter"
-        end
+    if has_valid_key && has_item_id
+      item = params[:type].camelcase.constantize.find_by_id(params[:id])
+      if item && member_owns_item(item)
+        #  This syntax is weird, so just know that it means this:
+        #  @photo.harvests << item unless @photo.harvests.include?(item)
+        #  but with the correct many-to-many relationship automatically referenced
+        (@photo.send "#{params[:type]}s") << item unless (@photo.send "#{params[:type]}s").include?(item)
       else
-        flash[:alert] = "Cannot attach photos to #{params[:type]}"
+        flash[:alert] = "Could not find this item owned by you"
       end
-    else    
-      flash[:alert] = "Missing type parameter"
+    else
+      flash[:alert] = "Missing or invalid type or id parameter"
     end
-  
+
     respond_to do |format|
       if @photo.save
         format.html { redirect_to @photo, notice: 'Photo was successfully added.' }
@@ -134,6 +119,22 @@ class PhotosController < ApplicationController
 
   private
 
+  def valid_models
+    ["planting", "harvest", "garden"]
+  end
+
+  def has_valid_key
+    (params.key? :type) && valid_models.include?(params[:type])
+  end
+
+  def has_item_id
+    params.key? :id
+  end
+
+  def member_owns_item(item)
+    item.owner.id == current_member.id
+  end
+
   def photo_params
     params.require(:photo).permit(:flickr_photo_id, :owner_id, :title, :license_name,
     :license_url, :thumbnail_url, :fullsize_url, :link_url)