Merge pull request #381 from kimardenmiller/spokenvote-380

Code Clean Up Reviewed, Ruby comments removed, Rebased & Resolved Merge Conflicts for PR #369

app/admin/dashboards.rb

@@ -1,10 +1,10 @@
 ActiveAdmin.register_page "Dashboard" do
 
-  menu :priority => 1, :label => proc{ I18n.t("active_admin.dashboard") }
+  menu priority: 1, label: proc{ I18n.t("active_admin.dashboard") }
 
-  content :title => proc{ I18n.t("active_admin.dashboard") } do
-    div :class => "blank_slate_container", :id => "dashboard_default_message" do
-      span :class => "blank_slate" do
+  content title: proc{ I18n.t("active_admin.dashboard") } do
+    div class: "blank_slate_container", id: "dashboard_default_message" do
+      span class: "blank_slate" do
         span I18n.t("active_admin.dashboard_welcome.welcome")
         small I18n.t("active_admin.dashboard_welcome.call_to_action")
       end

app/assets/javascripts/angular/services/api.coffee

@@ -82,7 +82,7 @@ CurrentUserLoader = (CurrentUser, $route, $q) ->
     )
     delay.promise
 
-SelectHubLoader = ($http, $q) ->
+SelectHubLoader = ['$http', '$q', ($http, $q) ->
   (hub_filter) ->
     delay = $q.defer()
     if hub_filter
@@ -97,7 +97,7 @@ SelectHubLoader = ($http, $q) ->
     else
       delay.reject 'No Hub ID found to locate'
     delay.promise
-
+]
 #
 #SelectHubLoader = (Hub, $route, $q) ->          # ui-select does not seem to like ngResource
 #  (params) ->                                   # Jul 13, 2015 Believe it's {object} vs [array] returns

app/assets/javascripts/angular/services/utility.coffee

@@ -1,25 +1,11 @@
-#Focus = ($timeout) ->
-#  (id) ->
-#    $timeout ->
-##      console.log 'Utility id arg: ', id
-#      element = angular.element( document.querySelector id )
-##      console.log 'Focus Utility element: ', element
-#      element[0].focus()  if element
-#
-## Register
-#App.Services.factory 'Focus', Focus
 
-
-svUtility = ($timeout) ->
+svUtility = ['$timeout', ($timeout) ->
   focus: (id) ->
-#    console.log 'hit: '
     $timeout ->
-#      console.log 'Utility id arg: ', id
       element = angular.element( document.querySelector id )
-#      console.log 'Focus Utility element: ', element
       element[0].focus()  if element
     ,
       10
-
+]
 # Register
 App.Services.factory 'svUtility', svUtility

app/assets/stylesheets/partials/_structural.scss

@@ -96,16 +96,6 @@ textarea {
 
 // TODO depreciate some of below?
 
-//.full-width-tall {
-//  width: 94%;
-//  height: 80px;
-//}
-
-//.full-width-short {
-//  width: 94%;
-//  height: 40px;
-//}
-
 .length_counter {
   text-align: right;
   font-size: 10px;

app/controllers/authentications_controller.rb

@@ -1,18 +1,10 @@
 class AuthenticationsController < Devise::SessionsController
   def create
     @new_user_saved = false
-
     # Try to find authentication first
     authentication = Authentication.find_by_provider_and_uid(auth_params[:provider], auth_params[:uid] )
     try_existing_user = User.find_by_id(authentication.try(:user_id)) || User.find_by_email(auth_params[:email])
 
-      #Code we might use if we ever auth with email again:
-        #if auth_params[:email] =~ /\A[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]+\z/
-        #  User.find_by_email(auth_params[:email])
-        #else
-        #  User.find_by_id(authentication.try(:user_id))
-        #end
-
     # Create user or update the user info as needed
     user = User.from_omniauth(try_existing_user.try(:id), auth_params)
     user.authentications.create(provider: auth_params[:provider], uid: auth_params[:uid], token: auth_params[:token]) unless authentication

app/controllers/hubs_controller.rb

@@ -6,77 +6,66 @@ class HubsController < ApplicationController
   def index
     hub_filter, location_id_filter = params[:hub_filter], params[:location_id_filter]
 
-    if hub_filter.presence && location_id_filter.presence
-      @hubs = Hub.where('group_name ilike ? AND formatted_location = ?', "%#{hub_filter}%", location_id_filter)
-    elsif hub_filter.presence
-      @hubs = Hub.where('formatted_location ilike ? or group_name ilike ?', "%#{hub_filter}%", "%#{hub_filter}%")
-    elsif location_id_filter.presence
-      @hubs = Hub.where('location_id = ?', location_id_filter)
-    else
-      @hubs = Hub.all
-    end
+    filter_hubs(hub_filter, location_id_filter)
 
-    found_hubs = @hubs.map { |h| "#{h.group_name} #{h.formatted_location}" }
+    found_hubs = @hubs.map(&:attributes).map { |h| h.slice(:group_name, :formatted_location) }
 
     # Add google place matches to list of hubs if they dont already exist in our DB
-    if hub_filter.presence 
-      begin 
-        service = GooglePlacesAutocompleteService.new
-        service.find_regions(hub_filter).each do |l|
-          if !found_hubs.include?("#{l[:type]} #{l[:description]}")
+    if hub_filter.present?
+      begin
+        GooglePlacesAutocompleteService.new.find_regions(hub_filter).each do |l|
+          unless found_hubs.include?(group_name: l[:type], formatted_location: l[:description])
             new_hub = Hub.new(group_name: l[:type], location_id: l[:id], formatted_location: l[:description], description: l[:reference])
             new_hub.id = 0
             @hubs << new_hub
           end
         end
-      rescue ArgumentError 
-        pp "WARNING: Could not use google service to find hubs"
+      rescue ArgumentError => e
+        Rails.logger.error "WARNING: Could not use google service to find hubs. Error: #{e.message}"
       end
     end
 
     respond_to do |format|
       format.html # index.html.erb
-      format.json { render json: @hubs.to_json(:methods => [:full_hub, :short_hub, :select_id]) }
+      format.json { render json: @hubs.to_json(methods: [:full_hub, :short_hub, :select_id]) }
     end
   end
 
   # GET /hubs/1
   # GET /hubs/1.json
   def show
-    if params[:id].starts_with?(GooglePlacesAutocompleteService.prefix) 
-      hub_id = params[:id][3..-1]
+    if params[:id].starts_with?(GooglePlacesAutocompleteService.prefix)
+      hub_id = params[:id].sub(GooglePlacesAutocompleteService.prefix, '')
       @hub = Hub.find_by_description(hub_id)
-      if !@hub 
-        begin 
-          service = GooglePlacesAutocompleteService.new
-          found_hub = service.get_place_details(hub_id)
+      unless hub.present?
+        begin
+          found_hub = GooglePlacesAutocompleteService.new.get_place_details(hub_id)
           if found_hub
             @hub = Hub.new(group_name: found_hub[:type], location_id: found_hub[:id], formatted_location: found_hub[:description], description: found_hub[:reference])
             @hub.id = 0
           else
             @hub = {}
           end
-        rescue ArgumentError 
-          pp "WARNING: Could not use google service to find hub details"
+        rescue ArgumentError => e
+          flash[:warning] = "WARNING: Could not use google service to find hub details. Error: #{e.message}"
         end
       end
-    else 
+    else
       @hub = Hub.find(params[:id])
     end
 
     respond_to do |format|
       format.html # show.html.erb
-      format.json { render json: @hub.to_json(:methods => [:full_hub, :short_hub, :select_id]) }
+      format.json { render json: @hub.to_json(methods: [:full_hub, :short_hub, :select_id]) }
     end
   end
 
   # GET /hubs/new
   # GET /hubs/new.json
   def new
     @hub = Hub.new
-    if params[:requested_group].presence
-      @hub.group_name = params[:requested_group]
-    end
+
+    @hub.group_name = params[:requested_group] if params[:requested_group].present?
 
     respond_to do |format|
       format.html # new.html.erb
@@ -112,7 +101,7 @@ def update
 
     respond_to do |format|
       if @hub.update_attributes(params[:hub])
-        format.html { redirect_to @hub, notice: 'Hub was successfully updated.' }
+        format.html { redirect_to @hub, noutice: 'Hub was successfully updated.' }
         format.json { head :no_content }
       else
         format.html { render action: "edit" }
@@ -132,4 +121,19 @@ def destroy
       format.json { head :no_content }
     end
   end
+
+  private
+
+  def filter_hubs(hub_filter, location_id_filter)
+    @hubs = Hub.all
+    if hub_filter.present? && location_id_filter.present?
+      @hubs = @hubs.where('group_name ilike ?', "%#{hub_filter}%").where(formatted_location: location_id_filter)
+    elsif hub_filter.present?
+      @hubs = @hubs.where('formatted_location ilike ? OR group_name ilike ?', "%#{hub_filter}%", "%#{hub_filter}%")
+    elsif location_id_filter.present?
+      @hubs = @hubs.where(location_id: location_id_filter)
+    end
+
+    @hubs
+  end
 end

app/controllers/omniauth_callbacks_controller.rb

@@ -9,24 +9,17 @@ def all
 
     if user.nil?
       user = User.new(:email => email)
-      user.authentications.build(:provider => provider, :uid => uid, :token => token)
-      #user.skip_confirmation!
-
+      user.authentications.build(provider: provider, uid: uid, token: token)
       if user.save!
-        user.create_avatar(:remote_image_url => avatar_url) rescue nil  # Dont fail if we're unable to save avatar
+        user.create_avatar(remote_image_url: avatar_url) rescue nil  # Dont fail if we're unable to save avatar
         render json: {success: true, redirect: new_user_registration_url}
-        #flash.notice = 'Thanks for joining Spokenvote!'
-        #custom_sign_in_and_redirect(user)
       else
         session["devise.user_attributes"] = user.attributes
         render json: {success: false, redirect: new_user_registration_url}
-        #redirect_to new_user_registration_url
       end
     else
-      # If we ever use this code we'll want to verify that the authentication record belongs to this user only
 
-      user.authentications.create(:provider => provider, :uid => uid, :token => token) if !authentication # Regular signed up user, allow him this omniauth signup also
-      #flash.notice = 'Signed in successfully!'
+      user.authentications.create(provider: provider, uid: uid, token: token) unless authentication # Regular signed up user, allow him this omniauth signup also
       render json: {success: true, status: 'signed_in'}
       custom_sign_in_and_redirect user
     end

app/controllers/proposals_controller.rb

@@ -1,33 +1,33 @@
 class ProposalsController < ApplicationController
   include ApplicationHelper
   before_action :authenticate_user!, :except => [:show, :index, :related_vote_in_tree, :related_proposals]
-  before_action :find_hub, :find_user, only: :index
 
   # GET /proposals.json
   def index
+    if hub_id_google?(params[:hub])
+      @proposals = []
+      return render 'index'
+    end
+
+    @hub = Hub.find_by(id: params[:hub])
+    @user = User.find_by(id: params[:user])
+
+    proposals = Proposal.by_hub(@hub).by_user(@user).includes(:hub)
+
+    unless params[:filter] == 'my'
+      proposals = proposals.where(id: Proposal.top_voted_proposal_in_tree.map(&:id))
+    end
 
-    top_voted_proposal_ids = Proposal.top_voted_proposal_in_tree.map(&:id)
-    proposals = Proposal.where(id: top_voted_proposal_ids)
-    proposals = proposals.where(hub_id: @hub.id) if @hub
-    proposals = proposals.where(user_id: @user.id) if @user
-    @proposals = proposals
-    @proposals = proposals.includes(:hub)
-
-    filter = params[:filter] || 'active'
-    if filter == 'active'
-      @proposals.to_a.sort! { |a, b| b.votes_in_tree <=> a.votes_in_tree }
-    elsif filter == 'recent'
-      @proposals = proposals.order('updated_at DESC')
-    elsif current_user
-      user_id = filter == 'my' ? current_user.try(:id) : params[:user_id]
-      user = User.find(user_id) if user_id
+    @proposals = filter_proposals(proposals, params[:filter].presence)
+
+    if provided_user_id.present?
+      user = User.find(provided_user_id)
 
       user_voted_proposal_root_ids = user.voted_proposals.map(&:root_id)
-      @proposals.to_a.delete_if { |proposal| !user_voted_proposal_root_ids.include? proposal.root_id }
-      @proposals = @proposals.sort { |a, b| b.votes_in_tree <=> a.votes_in_tree }
-    else  # Default to 'active' list
-      @proposals.to_a.sort! { |a, b| b.votes_in_tree <=> a.votes_in_tree }
+      @proposals = @proposals.reject { |proposal| !user_voted_proposal_root_ids.include? proposal.root_id }
     end
+
+    @proposals = @proposals.sort { |a, b| b.votes_in_tree <=> a.votes_in_tree }
   end
 
   # GET /proposals/1.json
@@ -76,10 +76,6 @@ def create
   # PUT /proposals/1.json
   def update
     @proposal = Proposal.find(params[:id])
-    #if params[:proposal][:votes_attributes][:comment].match(/\n/)
-    #  params[:proposal][:votes_attributes][:comment].gsub!(/\n\n/, '<br><br>').gsub!(/\n/, '<br>')
-    #end
-
     respond_to do |format|
       if @proposal.update_attributes(params[:proposal])
         format.json { render json: @proposal.to_json(methods: 'supporting_statement'), status: :ok }
@@ -94,7 +90,7 @@ def destroy
     @proposal = Proposal.find(params[:id])
     @proposal.destroy
 
-    redirect_to action: :index, status: 200
+    redirect_to action: :index, status: :ok
   end
 
   # GET /proposals/:id/related_proposals.json
@@ -123,13 +119,30 @@ def find_hub
     end
   end
 
-  def find_user
-    @user = User.find(params[:user]) if params[:user]
+  def filter_proposals(proposals, filter = 'active')
+    case filter
+    when 'active'
+      proposals.sort { |a, b| b.votes_in_tree <=> a.votes_in_tree }
+    when 'recent'
+      proposals.by_recency
+    when 'my'
+      proposals.by_voting_user(current_user)
+    else
+      proposals
+    end
+  end
+
+  def hub_id_google?(hub_id)
+    hub_id.try(:starts_with?, GooglePlacesAutocompleteService.prefix) || false
+  end
+
+  def provided_user_id
+    current_user.try(:id) || params[:user_id]
   end
 
   def proposal_params
     if parent_proposal
-      params[:proposal].merge!(parent_id: @parent.id, hub_id: @parent.hub.id)
+      params[:proposal].merge!(parent_id: parent_proposal.id, hub_id: parent_proposal.hub.id)
       params.require(:proposal).permit(:statement, :parent_id, :hub_id)
     else
       if existing_hub
@@ -148,21 +161,11 @@ def parent_proposal
   end
 
   def existing_hub
-    if params[:proposal][:hub_id].is_a?(String) && params[:proposal][:hub_id].starts_with?(GooglePlacesAutocompleteService.prefix) # no need to hit db if hub doesn't exist in our DB yet
-      nil
-    else
+    if !hub_id_google?(params[:proposal][:hub_id]) # no need to hit db if hub doesn't exist in our DB yet
       @hub ||= Hub.find_by_id(params[:proposal][:hub_id])
+    else
+      nil
     end
   end
 
-  #def fetch_more(proposal_id, page, offset)
-  #  records_limit = 10
-  #  page_number = (params[:page].presence || 0).to_i
-  #  proposal_id = (params[:proposal_id].presence || params[:id]).to_i
-  #
-  #  if params[:proposal_id].presence
-  #    offset_by = (page_number * records_limit) + 2
-  #  end
-  #end
-
 end