Update uers_contoller and add almost working spec.

Gemfile

@@ -104,7 +104,11 @@ group :test do
   gem 'test-unit'
   gem 'timecop'
 
+  # FOr JS test
+  gem 'mime-types'
+
   # Use dev versions because of rspec bug
+  
   gem 'rspec-core', git: 'https://github.com/rspec/rspec-core'
   gem 'rspec-expectations', git: 'https://github.com/rspec/rspec-expectations'
   gem 'rspec-mocks', git: 'https://github.com/rspec/rspec-mocks'

Gemfile.lock

@@ -464,6 +464,7 @@ DEPENDENCIES
   i18n-js
   i18n_country_select
   jquery-rails
+  mime-types
   mysql2
   neat (~> 1.6.0)
   newrelic_rpm

app/controllers/application_controller.rb

@@ -45,7 +45,7 @@ class ApplicationController < ActionController::Base
     end
 
     rescue_from Error do |exception|
-      render text: exception.message, layout: true
+      render text: exception.message, layout: true, status: 500
     end
 
     rescue_from ActiveRecord::StaleObjectError do |exception|

app/controllers/users_controller.rb

@@ -2,9 +2,11 @@ class UsersController < ApplicationController
   before_action :get_user, only: [:show, :history, :popup, :agenda, :edit, :update, :destroy]
   respond_to :html, :js
 
+  PAGES = ["general", "favorites", "computer", "articles", "movies", "teams", "matches", "predictions", "comments"]
+
   def index
     search = params[:search]
-    if search && search.match(/^ip:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/) && cuser && cuser.admin?
+    if search && search.match(/^ip:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/) && cuser&.admin?
       @users = User.where(lastip: $1).paginate(per_page: 40, page: params[:page])
     else
       if params[:filter] == 'lately'
@@ -19,26 +21,25 @@ class UsersController < ApplicationController
     @page = "general"
     respond_to do |format|
       format.js do
-        pages = ["general", "favorites", "computer", "articles", "movies", "teams", "matches", "predictions", "comments"]
-        if pages.include?(params[:page])
-          @page = params[:page]
-        end
+        @page = params[:page] if self.PAGES.include?(params[:page])
       end
       format.html {}
     end
   end
 
+  # FIXME: consider merging
+  def popup
+    render layout: false
+  end
+
   def agenda
+    raise AccessError unless @user == cuser or cuser&.admin?
     @teamer = Teamer.new
     @teamer.user = @user
   end
 
   def history
-    raise AccessError unless cuser and cuser.admin?
-  end
-
-  def popup
-    render layout: false
+    raise AccessError unless cuser&.admin?
   end
 
   def new
@@ -72,7 +73,7 @@ class UsersController < ApplicationController
   end
 
   def update
-    raise AccessError unless @user.can_update? cuser
+    raise AccessError unless @user.can_update? cuser  
     # FIXME: use permit
     params[:user].delete(:username) unless @user.can_change_name? cuser
     if @user.update_attributes(User.params(params, cuser, "update"))
@@ -89,37 +90,35 @@ class UsersController < ApplicationController
     redirect_to users_url
   end
 
+  # FIXME: maybe move to session controller
   def login
-    return unless request.post?
-
-    if u = User.authenticate(params[:login][:username].downcase, params[:login][:password])
-      raise Error, t(:accounts_locked) if u.banned? Ban::TYPE_SITE
-
-      flash[:notice] = t(:login_successful)
-      save_session u
-
-      if session[:return_to]
-        return_to
+    if params[:login] && (u = User.authenticate(params[:login]))
+      if u.banned? Ban::TYPE_SITE
+        flash[:notice] = t(:accounts_locked)
       else
-        redirect_to_back
+        flash[:notice] = t(:login_successful)
+        save_session u
       end
     else
       flash[:error] = t(:login_unsuccessful)
+    end
+    # FIXME: check return on rails 6
+    if session[:return_to]
+      return_to
+    else
       redirect_to_back
     end
   end
 
   def logout
-    if request.post?
-      session[:user] = nil
-      flash[:notice] = t(:login_out)
-      redirect_to :root
-    end
+    session[:user] = nil
+    flash[:notice] = t(:login_out)
+    redirect_to :root
   end
 
   def forgot
     if request.post?
-      if u = User.first(:conditions => {:username => params[:username], :email => params[:email]}) and u.send_new_password
+      if (user1 = User.where(username: params[:username], email: params[:email]).first) && user1.send_new_password
         flash[:notice] = t(:passwords_sent)
       else
         flash[:error] = t(:incorrect_information)
@@ -137,6 +136,6 @@ class UsersController < ApplicationController
     session[:user] = user.id
     user.lastip = request.ip
     user.lastvisit = DateTime.now
-    user.save()
+    user.save
   end
 end

app/models/user.rb

@@ -299,6 +299,7 @@ class User < ActiveRecord::Base
     self.time_zone = "Amsterdam"
   end
 
+  # Password  should store password and password_hash shoulds store
   def update_password
     self.password = Digest::MD5.hexdigest(raw_password) if raw_password and raw_password.length > 0
   end
@@ -329,8 +330,8 @@ class User < ActiveRecord::Base
     cuser and cuser.admin?
   end
 
-  def self.authenticate(username, password)
-    where("LOWER(username) = LOWER(?)", username).where(:password => Digest::MD5.hexdigest(password)).first
+  def self.authenticate(login)
+    where("LOWER(username) = LOWER(?)", login[:username]).where(password: Digest::MD5.hexdigest(login[:password])).first
   end
 
   def self.get id

config/routes.rb

@@ -32,6 +32,8 @@ Ensl::Application.routes.draw do
   resources :categories
   resources :options
   resources :polls
+  resources :custom_urls, only: [:create, :update, :destroy]
+  resources :brackets
 
   get 'comments/quote'
 
@@ -51,7 +53,12 @@ Ensl::Application.routes.draw do
   get 'forums/down'
 
   resources :forums
-  resources :users
+  resources :users do
+    collection do
+      get 'forgot'
+      post 'forgot'
+    end
+  end
   resources :locks
   resources :contesters
 
@@ -84,8 +91,6 @@ Ensl::Application.routes.draw do
     get :quote
   end
 
-  resources :brackets
-
   get 'about/action'
   get 'about/staff'
   get 'about/adminpanel'
@@ -133,13 +138,10 @@ Ensl::Application.routes.draw do
   get 'users/login'
   get 'users/logout'
   get 'users/popup'
-  get 'users/forgot', to: "users#forgot"
 
   get 'votes/create'
   get "polls/showvotes/:id", to: "polls#showvotes", as: "polls_showvotes"
 
-  resources :custom_urls, only: [:create, :update, :destroy]
-
   get "custom_urls", to: "custom_urls#administrate"
 
   get ':controller/:action', requirements: { action: /A-Za-z/ }

spec/controllers/users_controller_spec.rb

@@ -1,3 +1,263 @@
-RSpec.describe UsersController, type: :controller do
+require 'rails_helper'
+require 'mime/types'
 
+RSpec.describe UsersController, type: :controller do
+  let!(:params) { FactoryBot.attributes_for(:user) }
+  let!(:invalid_params) { params.merge(:steamid => (50..150).map { (65 + rand(26)).chr }.join) }
+  let!(:admin) { create(:user, :admin) } 
+  let!(:user) { create(:user) }
+
+  before :all do
+    create(:user)
+  end
+
+  # TODO: check flash
+
+  describe 'GET #index' do
+    it "renders the template" do
+      get :index
+      expect(response).to render_template("index")
+    end
+
+    it "assigns users" do
+      get :index
+      # TODO
+      expect(assigns(:users))
+    end
+
+    # TODO Test pagination + search
+  end
+
+  describe 'GET #popup' do
+    context 'with valid user' do
+      it "renders the template" do
+        login_admin
+        get :popup, params: {:id => user.id}, xhr: true
+        expect(response).to render_template("popup")
+      end
+      # Check for pages TODO
+    end
+  end
+
+  describe 'GET #agenda' do
+    context 'with valid user' do
+      it "renders the template" do
+        login user.username
+        get :agenda, params: {:id => user.id}
+        expect(response).to render_template("agenda")
+      end
+    end
+
+    context 'with admin access' do
+      it "renders the template" do
+        login_admin
+        get :agenda, params: {:id => user.id}
+        expect(response).to render_template("agenda")
+      end
+      # Check for pages TODO
+    end
+
+    context 'with valid user access another user' do
+      it "respond with 403" do
+        user2 = create(:user)
+        login user.username
+        get :agenda, params: {:id => user2.id}
+        expect(response).to have_http_status(:forbidden)
+      end
+    end
+  end
+
+  describe 'GET #history' do
+    context 'with admin access' do
+      it "renders the template" do
+        login_admin
+        get :history, params: {:id => user.id}
+        expect(response).to render_template("history")
+      end
+      # Check for pages TODO
+    end
+
+    context 'without admin access' do
+      it "respond with 403" do
+        user2 = create(:user)
+        login user.username
+        get :history, params: {:id => user2.id}
+        expect(response).to have_http_status(:forbidden)
+      end
+    end
+  end
+
+  describe 'GET #edit' do
+    let!(:user) { create(:user) }
+
+    it "renders the template" do
+      login_admin
+      get :edit, params: {id: user.id}
+      expect(response).to render_template("edit")
+    end
+  end
+
+  context 'POST' do
+    before(:each) do
+      expect(:params).not_to eq(:invalid_params)
+    end
+
+    describe 'with valid values' do
+      it "creates the model" do
+        login_admin
+        post :create, params: {:user => params}
+        # user.any_instance.should_receive(:update_attributes).with(params)
+        # FIXME: ignore lastvisit and raw_password
+        expect(User.last).to have_attributes(params.except(:raw_password,))
+      end
+
+      it "redirects correctly" do
+        login_admin
+        post :create, params: {:user => params}
+        expect(response).to redirect_to(user_path(User.last))
+      end
+    end
+
+    describe 'with invalid values' do
+      it "does not create the model" do
+        login_admin
+        count = User.count
+        post :create, params: {:user => invalid_params}
+        # user.any_instance.should_receive(:update_attributes).with(params)
+        expect(User.count).to eq(count)
+      end
+
+      it "renders :new" do
+        login_admin
+        post :create, params: {:user => invalid_params}
+        expect(response).to render_template("new")
+      end
+    end
+  end
+
+  context 'PUT' do
+    describe 'with valid values' do
+      it "updates the model" do
+        login_admin
+        params = FactoryBot.attributes_for(:user)
+        put :update, params: {:id => user.id, :user => params}
+        # user.any_instance.should_receive(:update_attributes).with(params)
+        expect(User.find(user.id).attributes).not_to eq(user.attributes)
+      end
+
+      it "redirects correctly" do
+        login_admin
+        request.env["HTTP_REFERER"] = "where_i_came_from"
+        put :update, params: {:id => user.id, :user => params}
+
+        expect(response).to redirect_to("where_i_came_from")
+      end
+    end
+
+    describe 'with invalid values' do
+      it "does not update the model" do
+        login_admin
+        put :update, params: {:id => user.id, :user => invalid_params}
+        expect(User.find(user.id).attributes).to eq(user.attributes)
+      end
+
+      it "renders :edit" do
+        login_admin
+        put :update, params: {:id => user.id, :user => invalid_params}
+        expect(response).to render_template("edit")
+      end
+    end
+  end
+
+  context 'DELETE' do
+    describe 'with valid parameters' do
+      it "deletes the model" do
+        login_admin
+        count = User.count
+        delete :destroy, params: {:id => user.id}
+
+        expect(User.where(id: user.id).count).to eq(0)
+        expect(User.count).to eq(count - 1)
+        # user.any_instance.should_receive(:update_attributes).with(params)
+      end
+
+      it "redirects correctly" do
+        login_admin
+        request.env["HTTP_REFERER"] = "where_i_came_from"
+        delete :destroy, params: {:id => user.id}
+
+        expect(response).to redirect_to(users_path())
+      end
+    end
+
+    describe 'without access' do
+      it "does not delete the model" do
+        login(user.username)
+        count = User.all.count
+        delete :destroy, params: {:id => user.id}
+
+        expect(User.count).to eq(count)
+      end
+    end
+  end
+
+  context 'POST #login' do
+    describe 'with valid values' do
+      it "set the session ID (logs in)" do
+        post :login, params: {login: {username: user.username, password: user.raw_password}}
+        expect(session[:user]).to eq(user.id)
+      end
+
+      # TODO
+      #expect(User).to have_received(:authenticate).with(params[:login])
+
+      it "redirects correctly" do
+        request.env["HTTP_REFERER"] = "where_i_came_from"
+        post :login, params: {login: {username: user.username, password: user.raw_password}}
+        expect(response).to redirect_to("where_i_came_from")
+      end
+    end
+
+    describe 'with invalid values' do
+      it "fails to set the session ID" do
+        post :login, params: {login: {username: user.username, password: user.raw_password + "foo"}}
+        expect(session[:user]).not_to eq(user.id)
+      end
+    end
+
+    describe 'banned accounts cannot log in' do
+      it "fails to set the session ID" do
+        ban = create(:ban, user: user)
+        post :login, params: {login: {username: user.username, password: user.raw_password}}
+        expect(session[:user]).not_to eq(user.id)
+      end
+    end
+  end
+
+  context 'GET #forgot' do
+    describe 'with valid values' do
+      it "renders the template" do
+        get :forgot
+        expect(response).to render_template("forgot")
+      end
+    end
+  end
+
+  context 'POST #forgot' do
+    describe 'with valid values' do
+      it "renders the template" do
+        #TODO: mock this function
+        post :forgot, params: {username: user.username, email: user.email}
+        expect(response).to render_template("forgot")
+      end
+
+      it "calls the function" do
+        # User.any_instance.stub(:send_new_password).and_return(true)
+        allow_any_instance_of(User).to receive(:send_new_password).and_return(true)
+        post :forgot, params: {username: user.username, email: user.email}
+        # FIXME
+        # expect_any_instance_of(User).to receive(:send_new_password)
+      end
+    end
+  end
 end

spec/factories/user.rb

@@ -8,7 +8,7 @@ FactoryBot.define do
     lastname "Player"
     country "EU"
     raw_password "PasswordABC123"
-    lastvisit "Sun, 15 Mar 2020 13:31:06 +0000"
+    # lastvisit "Sun, 15 Mar 2020 13:31:06 +0000"
 
     after(:create) do |user|
       create(:profile, user: user)