diff --git a/Gemfile b/Gemfile index fd532b6e..b3fdf753 100644 --- a/Gemfile +++ b/Gemfile @@ -103,7 +103,7 @@ gem 'figaro' gem 'active_attr' #to only display a limited number of items on an index page -gem 'will_paginate', '~> 3.1.0' +gem 'will_paginate-bootstrap' # Markdown renderer gem 'redcarpet' diff --git a/Gemfile.lock b/Gemfile.lock index 20a33f51..83280476 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -295,6 +295,8 @@ GEM websocket-extensions (>= 0.1.0) websocket-extensions (0.1.2) will_paginate (3.1.5) + will_paginate-bootstrap (1.0.1) + will_paginate (>= 3.0.3) xpath (2.0.0) nokogiri (~> 1.3) @@ -351,7 +353,7 @@ DEPENDENCIES turbolinks twitter-bootstrap-rails web-console (~> 2.0) - will_paginate (~> 3.1.0) + will_paginate-bootstrap RUBY VERSION ruby 2.2.2p95 diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index d4ee08a8..cd88cf81 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -1,7 +1,7 @@ class ProfilesController < ApplicationController load_and_authorize_resource - before_action :set_profile, only: [:show, :edit, :update] + before_action :set_profile, only: [:show, :edit, :update, :destroy] # GET /profiles/1 def show diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 987ade58..09271c64 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -4,14 +4,20 @@ class UsersController < ApplicationController # GET /users def index - @users = User.all.paginate(:page => params[:page], :per_page => 5) + authorize! :index, User + @users = User.with_profiles.paginate(:page => params[:page], :per_page => 20) if params[:search] - @users = User.search(params[:search]).paginate(:page => params[:page], :per_page => 5) + @users = User.search(params[:search]).paginate(:page => params[:page], :per_page => 20) end end - # PATCH/PUT /users/1 + # PATCH/PUT /users/1/role def update_role + authorize! :update_role, @user + if user_params[:role] == "admin" + authorize! :update_role_to_admin, @user + end + if @user.update(user_params) redirect_to :back, notice: I18n.t('users.successful_role_update') end diff --git a/app/models/ability.rb b/app/models/ability.rb index 9a31a041..3d5fc3dc 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -60,6 +60,10 @@ def initialize(user) can [:view_applicants, :edit_applicants, :view_participants, :print_applications, :manage, :view_material, :upload_material, :print_agreement_letters, :download_material, :view_unpublished], Event can :send_email, Email can :manage, Request + # Organizers can update user roles of pupil, coach and organizer, but cannot manage admins and cannot update a role to admin + can :manage, User, role: ["pupil", "coach", "organizer"] + cannot :update_role, User, role: "admin" + cannot :update_role_to_admin, User end if user.role? :admin can :manage, :all diff --git a/app/models/user.rb b/app/models/user.rb index 14a739a8..e8874b63 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -111,7 +111,15 @@ def rejected_applications_count(event) # @param pattern to search for # @return [Array] all users with pattern in their name def self.search(pattern) - joins(:profile).where("profiles.first_name LIKE ? or profiles.last_name LIKE ?", - "%#{pattern}%", "%#{pattern}%") + with_profiles.where("profiles.first_name LIKE ? or profiles.last_name LIKE ?", "%#{pattern}%", "%#{pattern}%") + end + + # Provides access to profile information + # and orders users by first, last name and email (if user has no profile) + # + # @return [Array] all users including their profile information + def self.with_profiles() + joins("LEFT JOIN profiles ON users.id = profiles.user_id") + .order('profiles.first_name, profiles.last_name, users.email ASC') end end diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index bab1e8ab..d79be315 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -1,10 +1,14 @@ <%- model_class = User -%> @@ -27,10 +31,14 @@ <%= user.name %> <% end %> - <%= user.created_at %> + <%= I18n.l(user.created_at) %> <%= form_for user, url: update_user_role_path(user) do |f| %> - <%= f.collection_select(:role, User::ROLES, :to_s, lambda{|i| t("users.roles." + i.to_s)}, {:selected => user.role.to_s }) %> + <% if can? :update_role_to_admin, @user %> + <%= f.collection_select(:role, User::ROLES, :to_s, lambda{|i| t("users.roles." + i.to_s)}, {:selected => user.role.to_s }) %> + <% else %> + <%= f.collection_select(:role, User::ROLES.reject{|r| r == :admin}, :to_s, lambda{|i| t("users.roles." + i.to_s)}, {:selected => user.role.to_s }) %> + <% end %> <%= f.submit t('.save', :default => "Save"), class: "btn btn-xs btn-primary" %> <% end %> @@ -39,4 +47,4 @@ <% end %> -<%= will_paginate @users %> +<%= will_paginate @users, renderer: BootstrapPagination::Rails %> diff --git a/config/locales/de.will_paginate.yml b/config/locales/de.will_paginate.yml index 959b2792..0f9b519e 100644 --- a/config/locales/de.will_paginate.yml +++ b/config/locales/de.will_paginate.yml @@ -1,5 +1,5 @@ de: will_paginate: - previous_label: "← Vorherige" - next_label: "Nächste →" + previous_label: "«" + next_label: "»" page_gap: "…" \ No newline at end of file diff --git a/db/sample_data/users.rb b/db/sample_data/users.rb index 995716bc..c84b9c16 100644 --- a/db/sample_data/users.rb +++ b/db/sample_data/users.rb @@ -14,7 +14,7 @@ def user_teacher User.new( email: "lehrer@example.com", password: user_password, - role: :teacher + role: :pupil ) end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb new file mode 100644 index 00000000..5142442f --- /dev/null +++ b/spec/controllers/users_controller_spec.rb @@ -0,0 +1,71 @@ +require 'rails_helper' + +RSpec.describe UsersController, type: :controller do + + let(:valid_attributes) { FactoryGirl.attributes_for(:user) } + + describe "PUT #update" do + + before :each do + request.env['HTTP_REFERER'] = root_url + end + + context "as organizer" do + + before :each do + sign_in FactoryGirl.create(:user, role: :organizer) + end + + it "can update the role of a pupil to coach" do + @test_user = FactoryGirl.create(:user, role: :pupil) + put :update_role, id: @test_user.id, user: {role: "coach"} + @test_user.reload + expect(@test_user.role).to eq("coach") + end + + it "cannot update the role of an admin" do + @test_user = FactoryGirl.create(:user, role: :admin) + put :update_role, id: @test_user.id, user: {role: "coach"} + @test_user.reload + expect(@test_user.role).to eq("admin") + end + + it "cannot update role to admin" do + @test_user = FactoryGirl.create(:user, role: :coach) + @test_user.role = "pupil" + put :update_role, id: @test_user.id, user: {role: "admin"} + @test_user.reload + expect(@test_user.role).to eq("coach") + end + end + + context "as admin" do + + before :each do + sign_in FactoryGirl.create(:user, role: :admin) + end + + it "can update the role of a pupil to coach" do + @test_user = FactoryGirl.create(:user, role: :pupil) + put :update_role, id: @test_user.id, user: {role: "coach"} + @test_user.reload + expect(@test_user.role).to eq("coach") + end + + it "can update the role of an admin" do + @test_user = FactoryGirl.create(:user, role: :admin) + put :update_role, id: @test_user.id, user: {role: "coach"} + @test_user.reload + expect(@test_user.role).to eq("coach") + end + + it "can update role to admin" do + @test_user = FactoryGirl.create(:user, role: :coach) + @test_user.role = "pupil" + put :update_role, id: @test_user.id, user: {role: "admin"} + @test_user.reload + expect(@test_user.role).to eq("admin") + end + end + end +end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index a038e333..74166293 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -72,14 +72,54 @@ RSpec.feature "Role management page", :type => :feature do - it "can change to role of a user" do - pupil = FactoryGirl.create(:user) - login(:admin) - visit users_path - expect(page).to have_select('user_role', selected: I18n.t("users.roles.pupil")) - first('#user_role').find(:xpath, 'option[2]').select_option - expect(page).to have_select('user_role', selected: I18n.t("users.roles.coach")) - first('input[value="%s"]' % I18n.t("users.index.save") ).click + %i[pupil coach].each do |role| + context "as a #{role}" do + it "shouldn't display the role managment page and show an error" do + login(role) + visit users_path + expect(page).not_to have_text(I18n.t("activerecord.models.user.other")) + expect(page).to have_text(I18n.t('unauthorized.manage.all')) + end + end + end + + %i[admin organizer].each do |role| + context "as an #{role}" do + it "should display the role managment page" do + login(role) + visit users_path + expect(page).to have_text(I18n.t("activerecord.models.user.other")) + end + + it "can change to role of a user to coach" do + pupil = FactoryGirl.create(:user) + login(:admin) + visit users_path + expect(page).to have_select('user_role', selected: I18n.t("users.roles.pupil")) + first('#user_role').find(:xpath, 'option[2]').select_option + expect(page).to have_select('user_role', selected: I18n.t("users.roles.coach")) + first('input[value="%s"]' % I18n.t("users.index.save")).click + end + end + end + + context "as an organizer" do + it "should not display admin role in drop down" do + login(:organizer) + visit users_path + role_options = [I18n.t("users.roles.pupil"),I18n.t("users.roles.coach"),I18n.t("users.roles.organizer")] + expect(page).to have_select('user_role', options: role_options) + expect(page).to_not have_select('user_role', options: [I18n.t("users.roles.admin")]) + end + end + + context "as an admin" do + it "should display admin role in drop down" do + login(:admin) + visit users_path + role_options = [I18n.t("users.roles.pupil"),I18n.t("users.roles.coach"),I18n.t("users.roles.organizer"),I18n.t("users.roles.admin")] + expect(page).to have_select('user_role', options: role_options) + end end it "can search for users" do @@ -96,7 +136,7 @@ expect(page).to have_text(user3.profile.last_name) fill_in :search, with: "Max" - click_button I18n.t('users.index.search') + find('#search-button').click expect(page).to have_text(max1.profile.last_name) expect(page).to have_text(max2.profile.last_name) diff --git a/spec/views/navbar_spec.rb b/spec/views/navbar_spec.rb index 186aed34..e3b575e0 100644 --- a/spec/views/navbar_spec.rb +++ b/spec/views/navbar_spec.rb @@ -41,16 +41,18 @@ end end - context "logged in as an admin or organizer" do - it "shows Einstellungen, Mein Profil, Benutzerverwaltung, Ausloggen" do - profile = FactoryGirl.create(:profile, user: (FactoryGirl.create :user, role: :admin)) - sign_in profile.user - render template: 'application/index', layout: 'layouts/application' - expect(rendered).to have_css(".nav .dropdown-menu a", text: I18n.t('navbar.settings')) - expect(rendered).to have_css(".nav .dropdown-menu a", text: I18n.t('navbar.profile')) - expect(rendered).to have_css(".nav .dropdown-menu a", text: I18n.t('navbar.user_management')) - expect(rendered).to have_css(".nav .dropdown-menu a", text: I18n.t('navbar.logout')) - expect(rendered).to have_css(".nav .dropdown-menu a", count: 4) + %i[admin organizer].each do |role| + context "logged in as an #{role}" do + it "shows Einstellungen, Mein Profil, Benutzerverwaltung, Ausloggen" do + profile = FactoryGirl.create(:profile, user: (FactoryGirl.create :user, role: role)) + sign_in profile.user + render template: 'application/index', layout: 'layouts/application' + expect(rendered).to have_css(".nav .dropdown-menu a", text: I18n.t('navbar.settings')) + expect(rendered).to have_css(".nav .dropdown-menu a", text: I18n.t('navbar.profile')) + expect(rendered).to have_css(".nav .dropdown-menu a", text: I18n.t('navbar.user_management')) + expect(rendered).to have_css(".nav .dropdown-menu a", text: I18n.t('navbar.logout')) + expect(rendered).to have_css(".nav .dropdown-menu a", count: 4) + end end end @@ -90,4 +92,4 @@ end end -end +end \ No newline at end of file