From fa0bcfeb42c4ad8c1fc92f455cae22d17955940d Mon Sep 17 00:00:00 2001 From: Vasiliy Ermolovich Date: Wed, 15 Feb 2012 21:05:55 +0300 Subject: [PATCH] refactor controllers --- app/controllers/auth_controller.rb | 13 ++++---- app/controllers/feeds_controller.rb | 2 +- app/controllers/sessions_controller.rb | 6 ++-- app/controllers/subscriptions_controller.rb | 32 ++++++++++---------- app/controllers/users_controller.rb | 33 ++++++++++----------- app/models/author.rb | 4 +++ app/models/user.rb | 4 +++ 7 files changed, 50 insertions(+), 44 deletions(-) diff --git a/app/controllers/auth_controller.rb b/app/controllers/auth_controller.rb index a711dce1..689ba7b5 100644 --- a/app/controllers/auth_controller.rb +++ b/app/controllers/auth_controller.rb @@ -18,7 +18,7 @@ def auth unless @auth = Authorization.find_from_hash(auth) if logged_in? Authorization.create_from_hash!(auth, root_url, current_user) - redirect_to "/users/#{current_user.username}/edit" and return + redirect_to [:edit, current_user] and return else # This situation here really sucks. I'd like to do something better, @@ -43,7 +43,7 @@ def auth flash[:error] = "Sorry, someone else has that username. Please pick another." end - redirect_to '/users/new' + redirect_to new_user_path return end @@ -61,7 +61,8 @@ def auth session[:user_id] = @auth.user.id flash[:notice] = "You're now logged in." - redirect_to '/' + + redirect_to root_path end def failure @@ -82,13 +83,13 @@ def failure # This lets someone remove a particular Authorization from their account. def destroy - user = User.first(:username => params[:username]) - if user + if user = User.first(:username => params[:username]) auth = Authorization.first(:provider => params[:provider], :user_id => user.id) auth.destroy if auth # Without re-setting the session[:user_id] we're logged out session[:user_id] = user.id end - redirect_to "/users/#{params[:username]}/edit" + + redirect_to [:edit, user] end end diff --git a/app/controllers/feeds_controller.rb b/app/controllers/feeds_controller.rb index 480f9b03..4a7ff5ca 100644 --- a/app/controllers/feeds_controller.rb +++ b/app/controllers/feeds_controller.rb @@ -9,7 +9,7 @@ def show feed = Feed.first :id => params[:id] if feed.local? # Redirect to the local profile page - redirect_to "/users/#{feed.author.username}" + redirect_to user_path(feed.author) else # Why not... # While weird, to render the view for this model, one diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index a2fa617f..0b555d58 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -27,7 +27,7 @@ def create @user.save session[:user_id] = @user.id flash[:notice] = "Thanks for signing up!" - redirect_to "/" + redirect_to root_path return else @user.errors.add(:password, "can't be empty") @@ -39,7 +39,7 @@ def create if user = User.authenticate(params[:username], params[:password]) session[:user_id] = user.id flash[:notice] = "Login successful." - redirect_to "/" + redirect_to root_path return end flash[:error] = "The password given for username \"#{params[:username]}\" is incorrect. @@ -52,7 +52,7 @@ def create def destroy session[:user_id] = nil flash[:notice] = "You've been logged out." - redirect_to '/' + redirect_to root_path end end diff --git a/app/controllers/subscriptions_controller.rb b/app/controllers/subscriptions_controller.rb index a2908e7b..1c527db8 100644 --- a/app/controllers/subscriptions_controller.rb +++ b/app/controllers/subscriptions_controller.rb @@ -1,10 +1,9 @@ class SubscriptionsController < ApplicationController - def show - - feed = Feed.first :id => params[:id] + before_filter :find_feed, :except => :create + def show if params['hub.challenge'] - sub = OSub::Subscription.new(request.url, feed.url, nil, feed.verify_token) + sub = OSub::Subscription.new(request.url, @feed.url, nil, @feed.verify_token) # perform the hub's challenge respond = sub.perform_challenge(params['hub.challenge']) @@ -12,8 +11,8 @@ def show # verify that the random token is the same as when we # subscribed with the hub initially and that the topic # url matches what we expect - verified = params['hub.topic'] == feed.url - if verified and sub.verify_subscription(params['hub.verify_token']) + verified = params['hub.topic'] == @feed.url + if verified && sub.verify_subscription(params['hub.verify_token']) render :text => respond[:body], :status => respond[:status] else # if the verification fails, the specification forces us to @@ -30,19 +29,17 @@ def show def destroy require_login! :return => request.referrer - feed = Feed.first :id => params[:id] - - @author = feed.author + @author = @feed.author if @author.user == current_user # You're not allowed to follow yourself. redirect_to request.referrer - elsif !current_user.following_url? feed.url + elsif !current_user.following_url? @feed.url # If we're not following them, noop. flash[:notice] = "You're not following #{@author.username}." redirect_to request.referrer else - current_user.unfollow! feed + current_user.unfollow! @feed flash[:notice] = "No longer following #{@author.username}." redirect_to request.referrer @@ -52,12 +49,9 @@ def destroy # subscriber receives updates # should be 'put', PuSH sucks at REST def post_update - feed = Feed.first :id => params[:id] - if feed.nil? - raise ActionController::RoutingError.new('Not Found') - end + raise ActionController::RoutingError.new('Not Found') if @feed.nil? - feed.update_entries(request.body.read, request.url, feed.url, request.env['HTTP_X_HUB_SIGNATURE']) + @feed.update_entries(request.body.read, request.url, @feed.url, request.env['HTTP_X_HUB_SIGNATURE']) render :nothing => true end @@ -96,4 +90,10 @@ def create flash[:notice] = "Now following #{f.author.username}." redirect_to request.referrer end + + private + + def find_feed + @feed = Feed.first :id => params[:id] + end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 90d43200..30e35c4e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,4 +1,5 @@ class UsersController < ApplicationController + before_filter :find_user, :only => [:show, :edit, :update, :feed, :following, :followers] def index set_params_page @@ -11,28 +12,24 @@ def index end def show - user = User.find_by_case_insensitive_username(params[:id]) - - if user.nil? + if @user.nil? render :file => "#{Rails.root}/public/404.html", :status => 404 - elsif user.username != params[:id] # case difference - redirect_to "/users/#{user.username}" + elsif @user.username != params[:id] # case difference + redirect_to user_path(@user) else set_params_page - @author = user.author - @updates = user.updates + @author = @user.author + @updates = @user.updates @updates = @updates.paginate(:page => params[:page], :per_page => params[:per_page]) set_pagination_buttons(@updates) - headers['Link'] = "<#{user_xrd_path(user.author.username)}>; rel=\"lrdd\"; type=\"application/xrd+xml\"" + headers['Link'] = "<#{user_xrd_path(@user.author)}>; rel=\"lrdd\"; type=\"application/xrd+xml\"" end end def edit - @user = User.find_by_case_insensitive_username(params[:id]) - # While it might be cool to edit other people's profiles, we probably # shouldn't let you do that. We're no fun. if @user == current_user @@ -43,7 +40,6 @@ def edit end def update - @user = User.find_by_case_insensitive_username(params[:id]) if @user == current_user response = @user.edit_user_profile(params) if response == true @@ -110,9 +106,8 @@ def create_from_email # Whatevs. # Except we ARE doing a redirect??? /me shakes fist at steve def feed - user = User.find_by_case_insensitive_username(params[:id]) - if user - redirect_to "/feeds/#{user.feed.id}.atom" + if @user + redirect_to "/feeds/#{@user.feed.id}.atom" else render :file => "#{Rails.root}/public/404.html", :status => 404 end @@ -121,8 +116,6 @@ def feed # Who do you think is a really neat person? This page will show it to the # world, so pick wisely! def following - @user = User.find_by_case_insensitive_username(params[:id]) - if @user.nil? render :file => "#{Rails.root}/public/404.html", :status => 404 elsif @user.username != params[:id] # case difference @@ -154,8 +147,6 @@ def following # This shows off how cool you are: I hope you've got the biggest number of # followers. Only one way to find out... def followers - @user = User.find_by_case_insensitive_username(params[:id]) - if @user.nil? render :file => "#{Rails.root}/public/404.html", :status => 404 elsif @user.username != params[:id] # case difference @@ -297,4 +288,10 @@ def reset_password_with_token render "login/password_reset" end end + + private + + def find_user + @user = User.find_by_case_insensitive_username(params[:id]) + end end diff --git a/app/models/author.rb b/app/models/author.rb index 6c3f3dcc..6a667bed 100644 --- a/app/models/author.rb +++ b/app/models/author.rb @@ -206,6 +206,10 @@ def normalize_domain self.domain = norm end + def to_param + username + end + def self.search(params = {}) if params[:search] && !params[:search].empty? Author.where(:username => /#{params[:search]}/i) diff --git a/app/models/user.rb b/app/models/user.rb index b6ff2064..612be3a2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -330,6 +330,10 @@ def self.find_by_case_insensitive_username(username) User.first(:username => /^#{Regexp.escape(username)}$/i) end + def to_param + username + end + private def create_feed