Improve channel, user and api_key models with basic scopes, correct references and protect attributes.

Simplify channels & user controllers and models, moving code to the models.
Remove catch-all route and ensure all actions are specified, fixing url paths.
Change clone to dup in feed_controller as using clone seems to affect the cloned object (problem only in ruby 1.9.3?)

app/controllers/api_keys_controller.rb

@@ -1,23 +1,22 @@
 class ApiKeysController < ApplicationController
+  include KeyUtilities
+
 	before_filter :require_user, :set_channels_menu
 
 	def index
-		get_channel_data
-		@read_keys = ApiKey.find(:all, :conditions => { :channel_id => @channel.id, :user_id => current_user.id, :write_flag => 0 })
+		@channel = current_user.channels.find(params[:channel_id])
+		@write_key = @channel.api_keys.write_keys.first
+		@read_keys = @channel.api_keys.read_keys
 	end
 
 	def destroy
-		@api_key = ApiKey.find_by_api_key(params[:api_key])
-		@api_key.delete if @api_key.user_id == current_user.id
+		current_user.api_keys.find_by_api_key(params[:id]).try(:destroy)
 		redirect_to :back
 	end
 
 	def create
-		@channel = Channel.find(params[:channel_id])
-		# make sure channel belongs to current user
-		check_permissions(@channel)
-
-		@api_key = ApiKey.find(:first, :conditions => { :channel_id => @channel.id, :user_id => current_user.id, :write_flag => 1 } )
+		@channel = current_user.channels.find(params[:channel_id])
+		@api_key = @channel.api_keys.write_keys.first
 
 		# if no api key found or read api key
 		if (@api_key.nil? or params[:write] == '0')
@@ -32,14 +31,12 @@ class ApiKeysController < ApplicationController
 		@api_key.save
 
 		# redirect
-		redirect_to channel_api_keys_path(@channel.id) and return
+		redirect_to channel_api_keys_path(@channel)
 	end
 
 	def update
-		@api_key = ApiKey.find_by_api_key(params[:api_key][:api_key])
-
-		@api_key.note = params[:api_key][:note]
-		@api_key.save if current_user.id == @api_key.user_id
-		redirect_to channel_api_keys_path(@api_key.channel)
+		@api_key = current_user.api_keys.find_by_api_key(params[:id])
+		@api_key.update_attributes(params[:api_key])
+		redirect_to :back
 	end
 end

app/controllers/application_controller.rb

@@ -94,14 +94,9 @@ class ApplicationController < ActionController::Base
 
     # gets the same data for showing or editing
     def get_channel_data
-			@channel = Channel.find(params[:channel_id]) if params[:channel_id]
-			@channel = Channel.find(params[:id]) if @channel.nil? and params[:id]
-			@key = ''
-			# make sure channel belongs to current user
-			check_permissions(@channel)
-	
-			@api_key = ApiKey.find(:first, :conditions => { :channel_id => @channel.id, :user_id => current_user.id, :write_flag => 1 } )
-			@key = @api_key.api_key if @api_key
+      @channel = current_user.channels.find(params[:channel_id]) if params[:channel_id]
+      @channel = current_user.channels.find(params[:id]) if @channel.nil? and params[:id]
+      @key = @channel.api_keys.write_keys.first.try(:api_key) || ""
     end
 
 		def check_permissions(channel)
@@ -131,18 +126,6 @@ class ApplicationController < ActionController::Base
 			return feed_unauthorized.to_xml(:only => :entry_id)
 		end
 
-		# generates a database unique api key
-		def generate_api_key(size = 16)
-			alphanumerics = ('0'..'9').to_a + ('A'..'Z').to_a
-			k = (0..size).map {alphanumerics[Kernel.rand(36)]}.join
-	
-			# if key exists in database, regenerate key
-			k = generate_api_key if ApiKey.find_by_api_key(k)
-	
-			# output the key
-			return k
-		end
-
 		# options: days = how many days ago, start = start date, end = end date, offset = timezone offset
 		def get_date_range(params)
 			# set timezone correctly

app/controllers/channels_controller.rb

@@ -21,72 +21,33 @@ class ChannelsController < ApplicationController
 	end
 
 	def update
-		@channel = Channel.find(params[:id])
-		# make sure channel belongs to current user
-		check_permissions(@channel)
-		# protect against bots
-		render :text => '' and return if params[:userlogin].length > 0
-
+		@channel = current_user.channels.find(params[:id])
 		@channel.update_attributes(params[:channel])
-		@channel.name = "#{t(:channel_default_name)} #{@channel.id}" if params[:channel][:name].empty?
-		@channel.save
 
-		redirect_to channel_path(@channel.id) and return
+		redirect_to channel_path(@channel.id)
 	end
 
   def create
-		# protect against bots
-		render :text => '' and return if params[:userlogin].length > 0
-
-		# get default name for field
-		@d = t(:channel_default_field)
-
-		# add channel with defaults
-		@channel = Channel.new(:field1 => "#{@d}1")
-		@channel.user_id = current_user.id
-		@channel.save
-		
-		# now that the channel is saved, we can create the default name
-		@channel.name = "#{t(:channel_default_name)} #{@channel.id}"
-		@channel.save
-
-		# create an api key for this channel
-		@api_key = ApiKey.new
-		@api_key.channel_id = @channel.id
-		@api_key.user_id = current_user.id
-		@api_key.write_flag = 1
-		@api_key.api_key = generate_api_key
-		@api_key.save
+    channel = current_user.channels.create(:field1 => "#{t(:channel_default_field)} 1")
+    channel.add_write_api_key
    
     # redirect to edit the newly created channel 
-		redirect_to edit_channel_path(@channel.id)
+    redirect_to edit_channel_path(channel)
   end
 
   # clear all data from a channel
   def clear
-		channel = Channel.find(params[:id])
-		# make sure channel belongs to current user
-		check_permissions(channel)
-		
-		# do the delete
-		channel.feeds.each do |f|
-			f.delete
-		end
-
-		# set the channel's last_entry_id to nil
-		channel.last_entry_id = nil
-		channel.save
+    channel = current_user.channels.find(params[:id])
+    channel.feeds.delete_all
+    channel.update_attribute(:last_entry_id, nil)
 
     redirect_to channels_path
 	end
 
 	def destroy
-		@channel = Channel.find(params[:id])
-		# make sure channel belongs to current user
-		check_permissions(@channel)
+		channel = current_user.channels.find(params[:id])
+		channel.destroy
 
-		# do the delete
-		@channel.delete
 		redirect_to channels_path
 	end
 
@@ -255,6 +216,9 @@ class ChannelsController < ApplicationController
 		redirect_to channel_path(channel.id)
 	end
 
+
+private
+
 	# determine if the date can be parsed
 	def date_parsable?(date)
 		return !is_a_number?(date)

app/controllers/feed_controller.rb

@@ -556,7 +556,7 @@ class FeedController < ApplicationController
 
 		# creates an empty clone of an object
 		def create_empty_clone(object)
-			empty_clone = object.clone
+			empty_clone = object.dup
 			empty_clone.attribute_names.each { |attr| empty_clone[attr] = nil }
 			return empty_clone
 		end

app/controllers/mailer_controller.rb

@@ -1,23 +1,20 @@
 class MailerController < ApplicationController
 
 	def resetpassword
-		# protect against bots
-		render :text => '' and return if params[:userlogin].length > 0
-
 		@user = User.find_by_login_or_email(params[:user][:login])
+
 		if @user.nil?
-			sleep 2
 			session[:mail_message] = t(:account_not_found)
 		else
 			begin
 				@user.reset_perishable_token!
-				#Mailer.password_reset(@user, "https://www.thingspeak.com/users/reset_password/#{@user.id}?token=#{@user.perishable_token}").deliver
+				# Mailer.password_reset(@user, "https://www.thingspeak.com/users/#{@user.id}/reset_password?token=#{@user.perishable_token}").deliver
 				session[:mail_message] = t(:password_reset_mailed)
 			rescue
 				session[:mail_message] = t(:password_reset_error)
 			end
 		end
-		redirect_to :controller => 'user_session', :action => 'new'
+		redirect_to login_path
 	end
 
 end

app/controllers/users_controller.rb

@@ -8,15 +8,12 @@ class UsersController < ApplicationController
   end
   
   def create
-		# protect against bots
-		render :text => '' and return if params[:userlogin].length > 0
-
 		@user = User.new(params[:user])
 
 		# save user
 		if @user.valid?
 			if @user.save
-				redirect_back_or_default account_path and return
+				redirect_back_or_default account_path
 			end	
 		else
 			render :action => :new
@@ -26,17 +23,16 @@ class UsersController < ApplicationController
   
   def show
 		@menu = 'account'
-    @user = @current_user
+    @user = current_user
   end
  
   def edit
 		@menu = 'account'
-    @user = @current_user
+    @user = current_user
   end
 
 	# displays forgot password page
 	def forgot_password
-		@user = User.new
 	end
 
 	# this action is called from an email link when a password reset is requested
@@ -44,7 +40,7 @@ class UsersController < ApplicationController
 		# if user has been logged in (due to previous form submission)
 		if !current_user.nil?
 			@user = current_user
-			@user.errors.add_to_base(t(:password_problem))
+			@user.errors.add(t(:password_problem))
 			@valid_link = true
 		else
 			@user = User.find_by_id(params[:id])
@@ -76,13 +72,13 @@ class UsersController < ApplicationController
 
   def update
 		@menu = 'account'
-    @user = @current_user # makes our views "cleaner" and more consistent
+    @user = current_user # makes our views "cleaner" and more consistent
 		# check current password and update
 		if @user.valid_password?(params[:password_current]) && @user.update_attributes(params[:user])
       redirect_to account_path
     else
-			@user.errors.add_to_base(t(:password_incorrect))
-      render :action => :edit
+      @user.errors.add :base, t(:password_incorrect)
+      render :edit      
     end
   end
 

app/models/api_key.rb

@@ -1,7 +1,22 @@
 class ApiKey < ActiveRecord::Base
   belongs_to :channel
+  belongs_to :user
 
   validates_uniqueness_of :api_key
+
+  scope :write_keys, :conditions => { :write_flag => true }
+  scope :read_keys, :conditions => { :write_flag => false }
+
+  attr_readonly :created_at
+  attr_accessible :note
+
+  def to_s
+    api_key
+  end
+
+  def to_param
+    api_key
+  end
 end
 
 

app/models/channel.rb

@@ -1,7 +1,40 @@
 class Channel < ActiveRecord::Base
+  include KeyUtilities
+  
   belongs_to :user
 	has_many :feeds
 	has_many :api_keys
+
+  attr_readonly :created_at
+  attr_protected :user_id, :last_entry_id
+
+  after_create :set_initial_default_name
+  before_validation :set_default_name
+  
+  validates :name, :presence => true, :on => :update
+
+  def add_write_api_key
+    write_key = self.api_keys.new
+    write_key.user = self.user
+    write_key.write_flag = true
+    write_key.api_key = generate_api_key
+    write_key.save
+  end
+
+  def field_label(field_number)
+    self["field#{field_number}"]
+  end
+
+private
+
+  def set_default_name    
+    self.name = "#{I18n.t(:channel_default_name)} #{self.id}" if self.name.blank?
+  end
+
+  def set_initial_default_name
+    update_attribute(:name, "#{I18n.t(:channel_default_name)} #{self.id}")
+  end
+
 end
 
 

app/models/feed.rb

@@ -2,6 +2,9 @@ class Feed < ActiveRecord::Base
 	belongs_to :channel
 
   self.include_root_in_json = false
+  
+  attr_readonly :created_at
+  attr_protected :channel_id
 end
 
 

app/models/user.rb

@@ -1,5 +1,6 @@
 class User < ActiveRecord::Base
   has_many :channels
+  has_many :api_keys
 
   acts_as_authentic
 

app/views/api_keys/index.html.erb

@@ -5,7 +5,7 @@
 </h2>
    
 <h3><%= t(:api_key_write) %></h3>
-<%= @key %>
+<%= @write_key %>
 
 <br /><br />
 
@@ -18,21 +18,21 @@
         <table>
             <tr>
                 <td><%= t(:api_key_key) %>:</td>
-                <td><%= read_key.api_key %></td>
+                <td><%= read_key %></td>
             </tr>
             <tr>
                 <td class="VAT"><%= t(:note) %>:</td>
                 <td>
-                    <%= form_for read_key, :as => :api_key, :url => { :controller => 'api_keys', :action => 'update' }, :html => {:method => 'put'} do |f| %>
+                    <%= form_for read_key, :as => :api_key, :url => channel_api_key_path(@channel, read_key), :html => {:method => 'put'} do |f| %>
                     <%= f.text_area :note, :cols => 30, :rows => 4 %>
                 </td>
             </tr>
             <tr>
-                <td><%= f.hidden_field :api_key, :value => read_key.api_key %></td>
+                <td></td>
                 <td>
                     <div class="FL"><%= f.submit t(:note_save) %></div>
                     <% end %>
-                    <%= button_to t(:api_key_delete), { :controller => 'api_keys', :action => 'destroy', :api_key => read_key.api_key}, :confirm => t(:confirm_read_key_delete) %></td>
+                    <%= button_to t(:api_key_delete), channel_api_key_path(@channel, read_key) , :method => 'delete', :confirm => t(:confirm_read_key_delete) %></td>
             </tr>
         </table>
         <br /><br />

app/views/channels/edit.html.erb

@@ -78,12 +78,12 @@
 <br /><br />
 
 <h2><%= t(:channel_clear_message) %></h2>
-<%= button_to t(:channel_clear), { :controller => 'channels', :action => 'clear', :id => @channel.id }, :confirm => t(:confirm_channel_clear) %>
+<%= button_to t(:channel_clear), clear_channel_path(@channel), :confirm => t(:confirm_channel_clear) %>
 
 <br /><br />
 
 <h2><%= t(:channel_delete_message) %></h2>
-<%= button_to t(:channel_delete), channel_path(@channel.id), :method => 'delete', :confirm => t(:confirm_channel_delete) %>
+<%= button_to t(:channel_delete), channel_path(@channel), :method => 'delete', :confirm => t(:confirm_channel_delete) %>
 
 <script type="text/javascript">
 	// remember default field label

app/views/channels/import.html.erb

@@ -6,7 +6,7 @@
 
 <%= t(:upload_select) %>
 <br /><br />
-<%= form_for :upload, :url => {:controller => 'channels', :action => 'upload', :channel_id => params[:channel_id]}, :html => { :multipart => true } do |f| %>
+<%= form_for :upload, :url => upload_channel_path(@channel), :html => { :multipart => true } do |f| %>
   <%= f.file_field :csv %>
   <br /><br />
   <%= t(:time_zone) %>

app/views/charts/index.html.erb

@@ -1,4 +1,5 @@
-<%= javascript_include_tag 'rest' %>
+<script type="text/javascript" src="/javascripts/rest.js"></script>
+  
 
 <h2>
 	<%= link_to t(:channels), channels_path %> &raquo;

app/views/layouts/_header.html.erb

@@ -1,6 +1,6 @@
 <div id="options">
 	<% if current_user %>		
-		<span class="action"> <%= link_to t(:signout), logout_path %></span>	
+		<span class="action"> <%= link_to t(:signout), logout_path, :method => 'delete' %></span>	
 	<% else %>
 		<span class="action">
 			<%= link_to t(:signup), new_user_path %>

app/views/users/_login.html.erb

@@ -1,4 +1,4 @@
-<%= form_for (@user_session = UserSession.new), :url => user_session_path, :html => { :id => 'loginform' } do |f| %>
+<%= form_for @user_session, :url => user_session_path, :html => { :id => 'loginform' } do |f| %>
 	<input name='userlogin' class='userlogin' />
 	<%= f.hidden_field :remember_me, :value => false %>
 	<table id="login" class="round">
@@ -21,7 +21,7 @@
 		</tr>
 		<tr>
 			<td></td>
-			<td><%= link_to t(:forgot), forgot_password_path, :id => 'forgot_password' %></td>
+			<td><%= link_to t(:forgot), forgot_password_users_path, :id => 'forgot_password' %></td>
 		</tr>
 		<tr>
 			<td></td>

app/views/users/forgot_password.html.erb

@@ -1,7 +1,7 @@
 <h2><%= t(:password_forgot) %></h2>
 <%= t(:password_forgot_message) %>
 <br /><br />
-<%= form_for @user, :url => { :controller => 'mailer', :action => 'resetpassword' } do |f| %>
+<%= form_for :user, :url => resetpassword_path do |f| %>
 	<input name='userlogin' class='userlogin' />
 	<%= f.text_field :login %>
 	<%= f.submit t(:submit) %>

app/views/users/reset_password.html.erb

@@ -1,6 +1,6 @@
 <% if @valid_link %>
 	<h2><%= t(:password_new) %></h2>
-	<%= form_for @user, :url => { :controller => 'users', :action => 'change_password', :id => @user.id } do |f| %>
+	<%= form_for @user, :url => change_password_user_path(@user) do |f| %>
 		<%= error_messages_for 'user', :header_message => t(:try_again), :message => t(:password_new_error) %>
 		<input name='userlogin' class='userlogin' />
 		<table class="bigtable">

config/initializers/wrap_parameters.rb

@@ -0,0 +1,12 @@
+# This file contains settings for ActionController::ParamsWrapper which
+# is enabled by default.
+
+# Enable parameter wrapping for JSON. You can disable this by setting :format to an empty array.
+ActiveSupport.on_load(:action_controller) do
+  wrap_parameters format: [:json]
+end
+
+# # Disable root element in JSON by default.
+# ActiveSupport.on_load(:active_record) do
+#   self.include_root_in_json = false
+# end

config/routes.rb

@@ -11,7 +11,15 @@ Thingspeak::Application.routes.draw do
 
   resource :user_session
   resource 'account', :to => 'users'
-	resources :users
+  resources :users do
+    member do      
+      get :reset_password
+      put :change_password
+    end
+    collection do
+      get :forgot_password
+    end
+  end
 
   # specific feeds
   match 'channels/:channel_id/feed(s)(.:format)' => 'feed#index'
@@ -25,18 +33,20 @@ Thingspeak::Application.routes.draw do
 
   # nest feeds into channels
   resources :channels do
+    member do
+      get :import
+      post :upload
+      post :clear
+    end
     resources :feed
     resources :feeds, :to => 'feed'
-		resources :api_keys
+    resources :api_keys, :except => [:show, :edit]
     resources :status
     resources :statuses, :to => 'statuses'
     resources :charts
   end
 
-	match 'login' => 'user_sessions#new', :as => :login
-	match 'logout' => 'user_sessions#destroy', :as => :logout
-	match 'users/reset_password', :to => 'users#reset_password', :as => 'reset_password'
-	match 'forgot_password', :to => 'users#forgot_password', :as => 'forgot_password'
-
-  match ':controller(/:action(/:id(.:format)))'
+  match 'login' => 'user_sessions#new', :as => :login, :via => :get
+  match 'logout' => 'user_sessions#destroy', :as => :logout, :via => :delete
+  match 'mailer/resetpassword', :to => 'mailer#resetpassword', :as => :resetpassword, :via => :post
 end

lib/key_utilities.rb

@@ -0,0 +1,14 @@
+module KeyUtilities
+    
+  # generates a database unique api key
+  def generate_api_key(size = 16)
+    alphanumerics = ('0'..'9').to_a + ('A'..'Z').to_a
+    k = (0..size).map {alphanumerics[Kernel.rand(36)]}.join
+  
+    # if key exists in database, regenerate key
+    k = generate_api_key if ApiKey.find_by_api_key(k)
+  
+    # output the key
+    k
+  end
+end