From af67284995dda02ee543f7c9ce24229ddc343bcf Mon Sep 17 00:00:00 2001 From: sirMackk Date: Mon, 7 Oct 2013 11:19:45 +0100 Subject: [PATCH] User ctrl refactor - breaks up large methods, moves some logic into model Includes missing methods from backup for travis to pass fix missing code, failing specs keep params handling in the controller. --- app/controllers/users_controller.rb | 60 ++++++++++++----------- app/models/user.rb | 5 +- spec/controllers/users_controller_spec.rb | 2 +- spec/models/user_spec.rb | 6 +-- 4 files changed, 37 insertions(+), 36 deletions(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d237587b305..2f6a456a064 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -40,7 +40,7 @@ class UsersController < ApplicationController end def update - user = User.where(username_lower: params[:username].downcase).first + user = fetch_user_from_params guardian.ensure_can_edit!(user) json_result(user, serializer: UserSerializer) do |u| updater = UserUpdater.new(user) @@ -127,7 +127,6 @@ class UsersController < ApplicationController params[:for_user_id] ? User.find(params[:for_user_id]) : current_user end - def create return fake_success_response if suspicious? params @@ -157,24 +156,28 @@ class UsersController < ApplicationController if @user.blank? flash[:error] = I18n.t('password_reset.no_token') else - if request.put? && params[:password].present? - @user.password = params[:password] - if @user.save - - if Guardian.new(@user).can_access_forum? - # Log in the user - log_on_user(@user) - flash[:success] = I18n.t('password_reset.success') - else - @requires_approval = true - flash[:success] = I18n.t('password_reset.success_unapproved') - end - end - end + raise Discourse::InvalidParameters.new(:password) unless good_reset_request_format + @user.password = params[:password] + logon_after_password_reset if @user.save end render layout: 'no_js' end + def good_reset_request_format + request.put? && params[:password].present? + end + + def logon_after_password_reset + if Guardian.new(@user).can_access_forum? + # Log in the user + log_on_user(@user) + flash[:success] = I18n.t('password_reset.success') + else + @requires_approval = true + flash[:success] = I18n.t('password_reset.success_unapproved') + end + end + def change_email params.require(:email) user = fetch_user_from_params @@ -229,13 +232,15 @@ class UsersController < ApplicationController def send_activation_email @user = fetch_user_from_params @email_token = @user.email_tokens.unconfirmed.active.first - if @user - @email_token ||= @user.email_tokens.create(email: @user.email) - Jobs.enqueue(:user_email, type: :signup, user_id: @user.id, email_token: @email_token.token) - end + enqueue_activation_email if @user render nothing: true end + def enqueue_activation_email + @email_token ||= @user.email_tokens.create(email: @user.email) + Jobs.enqueue(:user_email, type: :signup, user_id: @user.id, email_token: @email_token.token) + end + def search_users term = params[:term].to_s.strip topic_id = params[:topic_id] @@ -289,21 +294,18 @@ class UsersController < ApplicationController end # check the file size (note: this might also be done in the web server) - filesize ||= File.size(file.tempfile) + filesize ||= File.size(file.tempfile) max_size_kb = SiteSetting.max_image_size_kb * 1024 - if filesize > max_size_kb - return render status: 413, - text: I18n.t("upload.images.too_large", - max_size_kb: max_size_kb) + return render status: 413, text: I18n.t("upload.images.too_large", max_size_kb: max_size_kb) + else + filesize end - unless SiteSetting.authorized_image?(file) - return render status: 422, text: I18n.t("upload.images.unknown_image_type") - end + return render status: 422, text: I18n.t("upload.images.unknown_image_type") unless SiteSetting.authorized_image?(file) upload = Upload.create_for(user.id, file, filesize) - user.update_avatar(upload) + user.upload_avatar(upload) Jobs.enqueue(:generate_avatars, user_id: user.id, upload_id: upload.id) diff --git a/app/models/user.rb b/app/models/user.rb index f2f5b30e7a1..859f1ba1b6a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -473,9 +473,9 @@ class User < ActiveRecord::Base created_at > 1.day.ago end - def update_avatar(upload) + def upload_avatar(avatar) self.uploaded_avatar_template = nil - self.uploaded_avatar = upload + self.uploaded_avatar = avatar self.use_uploaded_avatar = true self.save! end @@ -574,7 +574,6 @@ class User < ActiveRecord::Base end end - private def previous_visit_at_update_required?(timestamp) diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 46b76cdafa6..ea76e2652ce 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -249,7 +249,7 @@ describe UsersController do context 'valid token' do before do EmailToken.expects(:confirm).with('asdfasdf').returns(user) - get :password_reset, token: 'asdfasdf' + put :password_reset, token: 'asdfasdf', password: 'newpassword' end it 'returns success' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a58f39700a9..9978f79c431 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -826,12 +826,12 @@ describe User do end end - describe "#update_avatar" do + describe "#upload_avatar" do let(:upload) { Fabricate(:upload) } let(:user) { Fabricate(:user) } - it "should update use's avatar" do - expect(user.update_avatar(upload)).to be_true + it "should update user's avatar" do + expect(user.upload_avatar(upload)).to be_true end end