diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index eec728e..d8fa38e 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -39,6 +39,8 @@ def create # PATCH/PUT /users/1 def update + return redirect_to(user, alert: "Only admins can edit users") unless current_user.admin? + if user.update(user_params) redirect_to user, notice: "User was successfully updated.", status: :see_other else @@ -48,8 +50,11 @@ def update # DELETE /users/1 def destroy + return redirect_to(user, alert: "Only admins can remove users") unless current_user.admin? + return redirect_to(user, alert: "You cannot remove yourself") if current_user == user + user.destroy! - redirect_to users_path, notice: "User was successfully destroyed.", status: :see_other + redirect_to users_path, notice: "User was successfully removed.", status: :see_other end private @@ -61,7 +66,7 @@ def user # Only allow a list of trusted parameters through. def user_params - params.expect(user: [:email]) + params.expect(user: %i[email admin]) end def auto_generate_password diff --git a/app/views/users/_form.html.erb b/app/views/users/_form.html.erb index d67b597..267dc00 100644 --- a/app/views/users/_form.html.erb +++ b/app/views/users/_form.html.erb @@ -16,6 +16,10 @@ <%= form.email_field :email %> +
+ <%= form.label :admin, "Is admin?" %> + <%= form.checkbox :admin %> +
<%= form.submit class: "govuk-button" %>
diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 87dc174..b3daef3 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -12,6 +12,7 @@ Email + Role Link @@ -21,6 +22,8 @@ <%= user.email %> + + <%= user.admin? ? "Admin" : "User" %> <%= link_to "Show this user", user %> diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index b727d60..f5e0273 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -2,14 +2,29 @@

User: <%= @user.email %>

-

- Remove this user if you wish to disable their ability to log into this application. -

+ <%= content_tag(:p, "This user is an admin", class: "govuk-body") if @user.admin? %> -

- <%= govuk_button_to "Remove this user", @user, method: :delete %> -

+ <% if current_user.admin? %> +

+ Remove this user if you wish to disable their ability to log into this application. +

+

+ <%= govuk_link_to "Edit this user", edit_user_path(@user) %> +

+ + <% if current_user == @user %> +

You cannot remove yourself.

+ <% else %> +

+ <%= govuk_button_to "Remove this user", @user, method: :delete %> +

+ <% end %> + <% else %> +

+ Ask an admin to remove this user if you wish to disable their ability to log into this application. +

+ <% end %>

<%= govuk_link_to "Back to users", users_path %>

diff --git a/db/migrate/20241205113814_add_admin_to_users.rb b/db/migrate/20241205113814_add_admin_to_users.rb new file mode 100644 index 0000000..b1eb2b7 --- /dev/null +++ b/db/migrate/20241205113814_add_admin_to_users.rb @@ -0,0 +1,5 @@ +class AddAdminToUsers < ActiveRecord::Migration[8.0] + def change + add_column :users, :admin, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 4a639d3..5e9426a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2024_11_28_122145) do +ActiveRecord::Schema[8.0].define(version: 2024_12_05_113814) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -31,6 +31,7 @@ t.datetime "remember_created_at" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "admin" t.index ["email"], name: "index_users_on_email", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true end diff --git a/db/seeds.rb b/db/seeds.rb index ed67b15..16c41d4 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -4,4 +4,5 @@ # User.find_or_create_by!(email: "robert.nichols@digital.cabinet-office.gov.uk") do |user| user.password = Devise.friendly_token + user.admin = true end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 6c0e360..678bb50 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -2,5 +2,9 @@ factory :user do email { Faker::Internet.email(domain: "example.com") } password { Devise.friendly_token } + + trait :is_admin do + admin { true } + end end end diff --git a/spec/requests/users_spec.rb b/spec/requests/users_spec.rb index d426020..d363e16 100644 --- a/spec/requests/users_spec.rb +++ b/spec/requests/users_spec.rb @@ -2,6 +2,7 @@ RSpec.describe "/users", type: :request do let(:user) { create :user } + let(:admin) { create :user, :is_admin } let(:logged_in_user) { create :user } let(:valid_attributes) { attributes_for :user } @@ -69,7 +70,8 @@ end describe "PATCH /update" do - context "with valid parameters" do + context "as admin with valid parameters" do + let(:logged_in_user) { admin } it "updates the requested user" do patch user_url(user), params: { user: valid_attributes } user.reload @@ -83,7 +85,22 @@ end end + context "with valid parameters" do + it "updates the requested user" do + patch user_url(user), params: { user: valid_attributes } + user.reload + expect(user.email).not_to eq(valid_attributes[:email]) + end + + it "redirects to the user" do + patch user_url(user), params: { user: valid_attributes } + user.reload + expect(response).to redirect_to(user_url(user)) + end + end + context "with invalid parameters" do + let(:logged_in_user) { admin } it "renders a response with 422 status (i.e. to display the 'edit' template)" do patch user_url(user), params: { user: invalid_attributes } expect(response).to have_http_status(:unprocessable_entity) @@ -92,16 +109,48 @@ end describe "DELETE /destroy" do - it "destroys the requested user" do + it "does not destroy the requested user" do user # Ensure user created before count expect { delete user_url(user) - }.to change(User, :count).by(-1) + }.not_to change(User, :count) end - it "redirects to the users list" do + it "redirects to the user" do delete user_url(user) - expect(response).to redirect_to(users_url) + expect(response).to redirect_to(user) + end + + context "when logged in as admin" do + let(:logged_in_user) { admin } + + it "destroys the requested user" do + user # Ensure user created before count + expect { + delete user_url(user) + }.to change(User, :count).by(-1) + end + + it "redirects to the users list" do + delete user_url(user) + expect(response).to redirect_to(users_url) + end + end + + context "when user is current user" do + let(:user) { admin } + let(:logged_in_user) { admin } + it "does not destroy the requested user" do + user # Ensure user created before count + expect { + delete user_url(user) + }.not_to change(User, :count) + end + + it "redirects to the user" do + delete user_url(user) + expect(response).to redirect_to(user) + end end end end