Special rights for organizers for user managment (#87) (#347)

* Failing tests for #87

* Inserted dropdown and user name with link to profile

* added simple pagination

* Moved Role managment to users#index and implmented set roles

* travis needs Gemfile.lock

* Implemented search bar

* update_role not needed here

* remove unneccessary elemetns

* display notice on role update

* remove profiles index page, user administration is handled by users index page

* remove role selection for users

* add translation for role adiminstration

* add missing translation for will_paginate

* remove index routing spec

* add and correct tests

* pupil is called Nutzer

* cleanup unneccessary functions

* adapt search to use first/last_name from profile of user

* add missing feature test for search bar

* use bootstrap extension of will_paginate

* localize created_at

* order users by profile.first_name

* increase users per page to 100

* there is no role teacher

* adapt tests and create user controller spec

* implement special organizer permissions for user managment

* change pagination to 20

* finish user controller test and adapt roles for organizers

* use bootstrap for search bar

* fix test for search button

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'

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

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

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

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

app/models/user.rb

@@ -111,7 +111,15 @@ def rejected_applications_count(event)
   # @param pattern to search for
   # @return [Array<User>] 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<User>] 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

app/views/users/index.html.erb

@@ -1,10 +1,14 @@
 <%- model_class = User -%>
 <div class="page-header">
-  <h1><%=t '.title', :default => model_class.model_name.human(count: 2).titleize %></h1>
+  <h1><%= t '.title', :default => model_class.model_name.human(count: 2).titleize %></h1>
 
   <%= form_tag(users_path, :method => "get", id: "search-form") do %>
-  <%= text_field_tag :search, params[:search], placeholder: t(".search_users", :default => "Search users") %>
-  <%= submit_tag t('.search', :default => "Search"), class: "btn btn-xs btn-primary" %>
+      <div class="input-group col-md-4">
+        <%= text_field_tag :search, params[:search], class: "form-control", placeholder: t(".search_users", :default => "Search users") %>
+        <div class="input-group-btn">
+          <button class="btn btn-primary" type="submit" id="search-button"><i class="glyphicon glyphicon-search"></i></button>
+        </div>
+      </div>
   <% end %>
 </div>
 
@@ -27,10 +31,14 @@
                 <%= user.name %>
             <% end %>
           </td>
-          <td><%= user.created_at %></td>
+          <td><%= I18n.l(user.created_at) %></td>
           <td>
             <%= 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 %>
           </td>
@@ -39,4 +47,4 @@
     <% end %>
   </tbody>
 </table>
-<%= will_paginate @users  %>
+<%= will_paginate @users, renderer: BootstrapPagination::Rails %>

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: "…"

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
 

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

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)

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