Merge pull request #865 from mssola/user-management

Allow admins to edit users

app/controllers/admin/users_controller.rb

@@ -1,5 +1,6 @@
 class Admin::UsersController < Admin::BaseController
   respond_to :html, :js
+  before_action :another_user_access, only: [:edit, :update]
 
   def index
     @users = User.not_portus.page(params[:page])
@@ -22,6 +23,23 @@ def create
     end
   end
 
+  # GET /admin/user/1/edit
+  def edit
+  end
+
+  # PATCH/PUT /admin/user/1
+  def update
+    return if @user.nil?
+
+    attr = params.require(:user).permit([:email])
+
+    if @user.update_attributes(attr)
+      redirect_to admin_users_path, notice: "User updated successfully"
+    else
+      redirect_to edit_admin_user_path(@user), alert: @user.errors.full_messages
+    end
+  end
+
   # PATCH/PUT /admin/user/1/toggle_admin
   def toggle_admin
     user = User.find(params[:id])
@@ -40,4 +58,15 @@ def user_create_params
     permitted = [:username, :email, :password, :password_confirmation]
     params.require(:user).permit(permitted)
   end
+
+  # Sets the @user instance variable if the current user is different from the
+  # one specified in params[:id]. Moreover, if the current user is the same as
+  # the targeted one, then a 403 response is rendered.
+  def another_user_access
+    @user = User.find(params[:id])
+    return if !@user.nil? && @user != current_user
+
+    @user = nil
+    render nothing: true, status: 403
+  end
 end

app/views/admin/users/edit.html.slim

@@ -0,0 +1,14 @@
+.panel.panel-default
+  .panel-heading
+    h5 Edit User
+  .panel-body
+    = form_for [:admin, @user], html: {class: 'form-horizontal', role: 'form'} do |f|
+      .form-group
+        = f.label :email, {class: 'control-label col-md-2'}
+        .col-md-7
+          = f.email_field(:email, class: 'form-control', required: true, autofocus: true)
+
+      .form-group
+        .col-md-offset-2.col-md-7
+          = f.submit('Update', class: 'btn btn-primary')
+

app/views/admin/users/index.html.slim

@@ -26,7 +26,11 @@
         tbody
           - @users.each do |user|
             tr[id="user_#{user.id}"]
-              td= user.username
+              - if user == current_user
+                td= user.username
+              - else
+                td
+                  = link_to user.username, edit_admin_user_path(user), { title: "Edit user '#{user.username}'" }
               td= user.email
               td.admin-btn
                 - if current_user.id == user.id

config/routes.rb

@@ -47,7 +47,7 @@
     resources :registries, except: [:show, :destroy]
     resources :namespaces, only: [:index]
     resources :teams, only: [:index]
-    resources :users, only: [:index, :create, :new] do
+    resources :users, except: [:destroy] do
       put "toggle_admin", on: :member
     end
   end

spec/controllers/admin/users_controller_spec.rb

@@ -1,7 +1,6 @@
 require "rails_helper"
 
 RSpec.describe Admin::UsersController, type: :controller do
-
   let(:admin) { create(:admin) }
   let(:user) { create(:user) }
 
@@ -109,4 +108,51 @@
       end.not_to change(User, :count)
     end
   end
+
+  describe "GET #edit" do
+    before :each do
+      create(:registry)
+      sign_in admin
+    end
+
+    it "returns with a failure if the current user tries to edit himself" do
+      get :edit, id: admin.id
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it "returns success when editing another user" do
+      get :edit, id: user.id
+      expect(response).to have_http_status(:success)
+    end
+  end
+
+  describe "PUT/PATCH #update" do
+    before :each do
+      create(:registry)
+      sign_in admin
+    end
+
+    it "returns with a failure if the current user tries to update himself" do
+      put :update, id: admin.id
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it "returns with a failure if users pass a bad parameter" do
+      original = user.username
+
+      put :update, id: user.id, user: { email: admin.email }
+      expect(response).to have_http_status(302)
+
+      expect(user.reload.username).to eq(original)
+    end
+
+    it "succeeds if everything was ok" do
+      original = user.email
+
+      put :update, id: user.id, user: { email: user.email + "o" }
+      expect(response).to have_http_status(302)
+
+      expect(user.reload.email).to eq(original + "o")
+    end
+  end
 end

spec/features/admin/users_spec.rb

@@ -65,4 +65,27 @@
       expect(page).to have_content("User '#{user.username}' is no longer an admin")
     end
   end
+
+  describe "Edit user" do
+    scenario "allows the admin to update a user", js: true do
+      visit edit_admin_user_path(user)
+
+      fill_in "Email", with: "[email protected]"
+      click_button "Update"
+
+      wait_for_effect_on("#alert")
+      expect(page).to have_content("[email protected]")
+      expect(page).to have_content("User updated successfully")
+    end
+
+    scenario "disallows the admin to update a user with a wrong name", js: true do
+      visit edit_admin_user_path(user)
+
+      fill_in "Email", with: admin.email
+      click_button "Update"
+
+      wait_for_effect_on("#alert")
+      expect(page).to have_content("has already been taken")
+    end
+  end
 end