diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/auth/passwords_controller.rb similarity index 94% rename from app/controllers/users/passwords_controller.rb rename to app/controllers/auth/passwords_controller.rb index 6517b6581..0f6e1c9b0 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/auth/passwords_controller.rb @@ -1,4 +1,4 @@ -class Users::PasswordsController < Devise::PasswordsController +class Auth::PasswordsController < Devise::PasswordsController include Helpers::Email def reset_confirmation diff --git a/app/controllers/users/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb similarity index 90% rename from app/controllers/users/sessions_controller.rb rename to app/controllers/auth/sessions_controller.rb index f81f8fb05..a117aecff 100644 --- a/app/controllers/users/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -1,4 +1,4 @@ -class Users::SessionsController < Devise::SessionsController +class Auth::SessionsController < Devise::SessionsController include Helpers::Email def create diff --git a/app/controllers/users/account_controller.rb b/app/controllers/users/account_controller.rb deleted file mode 100644 index ccbc9cc31..000000000 --- a/app/controllers/users/account_controller.rb +++ /dev/null @@ -1,25 +0,0 @@ -class Users::AccountController < ApplicationController - def check_logged_in - if current_user.nil? - redirect_to(new_user_session_path) - end - end - - def index - check_logged_in - end - - def personal_details - check_logged_in - end - - def update - if current_user.update(user_params) - redirect_to(users_account_path) - end - end - - def user_params - params.require(:user).permit(:email, :name, :password) - end -end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb deleted file mode 100644 index 985099900..000000000 --- a/app/controllers/users/registrations_controller.rb +++ /dev/null @@ -1,7 +0,0 @@ -class Users::RegistrationsController < Devise::RegistrationsController -protected - - def after_update_path_for(_resource) - users_account_path - end -end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb new file mode 100644 index 000000000..2f6da5342 --- /dev/null +++ b/app/controllers/users_controller.rb @@ -0,0 +1,50 @@ +class UsersController < ApplicationController + include Devise::Controllers::SignInOut + include Helpers::Email + before_action :authenticate_user! + + def update + if current_user.update(user_params) + bypass_sign_in current_user + redirect_to user_path(current_user) + end + end + + def new + @resource = User.new + end + + def create + @resource = User.new + if user_params["email"].empty? + @resource.errors.add :email, "Enter an email address" + elsif !email_valid?(user_params["email"]) + @resource.errors.add :email, "Enter an email address in the correct format, like name@example.com" + end + if @resource.errors.present? + render :new, status: :unprocessable_entity + else + @user = User.create!(user_params.merge(org_params).merge(password_params)) + @user.send_reset_password_instructions + redirect_to users_organisation_path(current_user.organisation) + end + end + + def edit_password + render :edit_password + end + +private + + def password_params + { password: SecureRandom.hex(8) } + end + + def org_params + { organisation: current_user.organisation } + end + + def user_params + params.require(:user).permit(:email, :name, :password) + end +end diff --git a/app/views/devise/mailer/_password_change_forgotten.html.erb b/app/views/devise/mailer/_password_change_forgotten.html.erb new file mode 100644 index 000000000..894cbda1d --- /dev/null +++ b/app/views/devise/mailer/_password_change_forgotten.html.erb @@ -0,0 +1,8 @@ +

Hello <%= @resource.email %>!

+ +

Someone has requested a link to change your password. You can do this through the link below.

+ +

<%= govuk_link_to 'Change my password', edit_password_url(@resource, reset_password_token: @token) %>

+ +

If you didn't request this, please ignore this email.

+

Your password won't change until you access the link above and create a new one.

diff --git a/app/views/devise/mailer/_password_change_initial.html.erb b/app/views/devise/mailer/_password_change_initial.html.erb new file mode 100644 index 000000000..6645c7c77 --- /dev/null +++ b/app/views/devise/mailer/_password_change_initial.html.erb @@ -0,0 +1,6 @@ +

Hello <%= @resource.name %>!

+ +

An account has been created for you to submit CORE data on behalf of @resource.organisation.

+ +

Your username is <% @resource.email %>, use the link below to set your password. +

<%= govuk_link_to 'Change my password', edit_password_url(@resource, reset_password_token: @token) %>

diff --git a/app/views/devise/mailer/reset_password_instructions.html.erb b/app/views/devise/mailer/reset_password_instructions.html.erb index 894cbda1d..dd4412b35 100644 --- a/app/views/devise/mailer/reset_password_instructions.html.erb +++ b/app/views/devise/mailer/reset_password_instructions.html.erb @@ -1,8 +1,5 @@ -

Hello <%= @resource.email %>!

- -

Someone has requested a link to change your password. You can do this through the link below.

- -

<%= govuk_link_to 'Change my password', edit_password_url(@resource, reset_password_token: @token) %>

- -

If you didn't request this, please ignore this email.

-

Your password won't change until you access the link above and create a new one.

+<% if @resource.last_sign_in_at.nil? %> + <%= render partial: "password_change_initial" %> +<% else %> + <%= render partial: "password_change_forgotten" %> +<% end %> diff --git a/app/views/devise/passwords/edit.html.erb b/app/views/devise/passwords/edit.html.erb deleted file mode 100644 index be85834c2..000000000 --- a/app/views/devise/passwords/edit.html.erb +++ /dev/null @@ -1,18 +0,0 @@ -<%= form_for(resource, as: resource_name, url: password_path(resource_name), html: { method: :put }) do |f| %> -
-
-

Reset your password

- <%= render "devise/shared/error_messages", resource: resource %> - - <%= f.hidden_field :reset_password_token %> - - <%= f.govuk_password_field :password, - label: { text: "New password" }, - hint: @minimum_password_length ? { text: "Your password must be at least #{@minimum_password_length} characters and hard to guess." } : nil, - autocomplete: "new-password" - %> - - <%= f.govuk_submit "Reset password" %> -
-
-<% end %> diff --git a/app/views/devise/registrations/new.html.erb b/app/views/devise/registrations/new.html.erb deleted file mode 100644 index ec331e9df..000000000 --- a/app/views/devise/registrations/new.html.erb +++ /dev/null @@ -1,23 +0,0 @@ -

Sign up

- -<%= form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f| %> - <%= render "devise/shared/error_messages", resource: resource %> - - <%= f.govuk_email_field :email, - label: { text: "Email address" }, - autocomplete: "email" - %> - - <%= f.govuk_password_field :password, - hint: @minimum_password_length ? { text: "#{@minimum_password_length} characters minimum" } : nil, - autocomplete: "new-password" - %> - - <%= f.govuk_password_field :password_confirmation, - autocomplete: "new-password" - %> - - <%= f.govuk_submit "Sign up" %> -<% end %> - -<%= render "devise/shared/links" %> diff --git a/app/views/devise/shared/_error_messages.html.erb b/app/views/devise/shared/_error_messages.html.erb deleted file mode 100644 index ba7ab8870..000000000 --- a/app/views/devise/shared/_error_messages.html.erb +++ /dev/null @@ -1,15 +0,0 @@ -<% if resource.errors.any? %> -
-

- <%= I18n.t("errors.messages.not_saved", - count: resource.errors.count, - resource: resource.class.model_name.human.downcase) - %> -

- -
-<% end %> diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 960a4ecbd..ce2ec84db 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -41,7 +41,7 @@ elsif component.navigation_item(text: 'Case logs', href: case_logs_path) component.navigation_item(text: 'Your organisation', href: "/organisations/#{current_user.organisation.id}") - component.navigation_item(text: 'Your account', href: users_account_path) + component.navigation_item(text: 'Your account', href: user_path(current_user)) component.navigation_item(text: 'Sign out', href: destroy_user_session_path, options: {:method => :delete}) end end diff --git a/app/views/organisations/users.html.erb b/app/views/organisations/users.html.erb index 390d9c0ff..dbf53b484 100644 --- a/app/views/organisations/users.html.erb +++ b/app/views/organisations/users.html.erb @@ -3,7 +3,7 @@ <%= "Users" %> <% end %> -<%= govuk_button_link_to "Invite user", new_user_path, method: :post %> +<%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %> <%= govuk_table do |table| %> <%= table.head do |head| %> <%= head.row do |row| diff --git a/app/views/users/account/personal_details.html.erb b/app/views/users/edit.html.erb similarity index 83% rename from app/views/users/account/personal_details.html.erb rename to app/views/users/edit.html.erb index 0db97f6b7..1d12d702b 100644 --- a/app/views/users/account/personal_details.html.erb +++ b/app/views/users/edit.html.erb @@ -5,7 +5,7 @@ ) %> <% end %> -<%= form_for(current_user, as: :user, url: account_update_path(), html: { method: :patch }) do |f| %> +<%= form_for(current_user, as: :user, html: { method: :patch }) do |f| %>

Change your personal details

diff --git a/app/views/devise/registrations/edit.html.erb b/app/views/users/edit_password.html.erb similarity index 86% rename from app/views/devise/registrations/edit.html.erb rename to app/views/users/edit_password.html.erb index d8fd2d9ce..b2b008f3b 100644 --- a/app/views/devise/registrations/edit.html.erb +++ b/app/views/users/edit_password.html.erb @@ -5,7 +5,7 @@ ) %> <% end %> -<%= form_for(resource, as: resource_name, url: user_registration_path(), html: { method: :patch }) do |f| %> +<%= form_for(current_user, as: :user, html: { method: :patch }) do |f| %>

Change your password

diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb new file mode 100644 index 000000000..4aacdd6a9 --- /dev/null +++ b/app/views/users/new.html.erb @@ -0,0 +1,28 @@ +<% content_for :before_content do %> + <%= govuk_back_link( + text: 'Back', + href: :back, + ) %> +<% end %> + +<%= form_for(@resource, as: :user, html: { method: :post }) do |f| %> +
+
+ <%= f.govuk_error_summary %> + +

Invite user to submit CORE data

+ + <%= f.govuk_text_field :name, + autocomplete: "name" + %> + + <%= f.govuk_email_field :email, + label: { text: "Email address" }, + autocomplete: "email", + value: @resource.email + %> + + <%= f.govuk_submit "Continue" %> +
+
+<% end %> diff --git a/app/views/users/account/index.html.erb b/app/views/users/show.html.erb similarity index 75% rename from app/views/users/account/index.html.erb rename to app/views/users/show.html.erb index bc9ae8a89..e90d35cdc 100644 --- a/app/views/users/account/index.html.erb +++ b/app/views/users/show.html.erb @@ -11,19 +11,19 @@ <%= summary_list.row do |row| row.key { 'Name' } row.value { current_user.name } - row.action(visually_hidden_text: 'name', href: '/users/account/personal-details', html_attributes: { 'data-qa': 'change-name' }) + row.action(visually_hidden_text: 'name', href: edit_user_path, html_attributes: { 'data-qa': 'change-name' }) end %> <%= summary_list.row() do |row| row.key { 'Email address' } row.value { current_user.email } - row.action(visually_hidden_text: 'email address', href: '/users/account/personal-details', html_attributes: { 'data-qa': 'change-email' }) + row.action(visually_hidden_text: 'email address', href: edit_user_path, html_attributes: { 'data-qa': 'change-email' }) end %> <%= summary_list.row do |row| row.key { 'Password' } row.value { '••••••••' } - row.action(visually_hidden_text: 'password', href: edit_user_registration_path, html_attributes: { 'data-qa': 'change-password' }) + row.action(visually_hidden_text: 'password', href: password_edit_user_path, html_attributes: { 'data-qa': 'change-password' }) end %> <%= summary_list.row do |row| diff --git a/config/routes.rb b/config/routes.rb index 3327f8917..c7be56592 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,25 +1,22 @@ Rails.application.routes.draw do devise_for :admin_users, ActiveAdmin::Devise.config - devise_for :users, controllers: { passwords: "users/passwords", sessions: "users/sessions" }, path_names: { sign_in: "sign-in", sign_out: "sign-out" }, skip: [:registrations] + devise_for :users, controllers: { + passwords: "auth/passwords", + sessions: "auth/sessions", + }, path_names: { sign_in: "sign-in", sign_out: "sign-out" } + devise_scope :user do - get "confirmations/reset", to: "users/passwords#reset_confirmation" - get "users/edit" => "devise/registrations#edit", :as => "edit_user_registration" - patch "users" => "users/registrations#update", :as => "user_registration" - patch "details" => "users/account#update", :as => "account_update" + get "confirmations/reset", to: "auth/passwords#reset_confirmation" end # For details on the DSL available within this file, see https://guides.rubyonrails.org/routing.html ActiveAdmin.routes(self) root to: "test#index" get "about", to: "about#index" - get "/users/account", to: "users/account#index" - - form_handler = FormHandler.instance - form = form_handler.get_form("2021_2022") resources :users do - collection do - get "account/personal-details", to: "users/account#personal_details" + member do + get "password/edit", to: "users#edit_password" end end @@ -27,17 +24,21 @@ Rails.application.routes.draw do member do get "details", to: "organisations#show" get "users", to: "organisations#users" + get "users/invite", to: "users/account#new" end end + form_handler = FormHandler.instance + form = form_handler.get_form("2021_2022") + resources :case_logs, path: "/case-logs" do collection do - post "/bulk-upload", to: "bulk_upload#bulk_upload" - get "/bulk-upload", to: "bulk_upload#show" + post "bulk-upload", to: "bulk_upload#bulk_upload" + get "bulk-upload", to: "bulk_upload#show" end member do - post "/form", to: "case_logs#submit_form" + post "form", to: "case_logs#submit_form" end form.pages.map do |page| diff --git a/db/schema.rb b/db/schema.rb index c9570f994..8b85217f5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -163,9 +163,9 @@ ActiveRecord::Schema.define(version: 2021_12_01_144335) do t.string "why_dont_you_know_la" t.integer "unitletas" t.integer "builtype" - t.datetime "property_void_date" t.bigint "owning_organisation_id" t.bigint "managing_organisation_id" + t.datetime "property_void_date" t.integer "renttype" t.integer "needstype" t.integer "lettype" diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 63a02b5e3..3a3d97b7e 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -26,4 +26,18 @@ RSpec.describe "User Features" do expect(page).to have_current_path("/organisations/#{org_id}/details") end end + + context "Organisation users" do + it "users can be added" do + visit("/organisations/#{org_id}") + click_link("Users") + click_link("Invite user") + expect(page).to have_current_path("/users/new") + expect(page).to have_content("Invite user to submit CORE data") + fill_in("user[name]", with: "New User") + fill_in("user[email]", with: "new_user@example.com") + expect { click_button("Continue") }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect(page).to have_current_path("/organisations/#{org_id}/users") + end + end end diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index d70e33ec8..d8d2d1e9b 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -96,7 +96,7 @@ RSpec.describe "User Features" do end it "tries to access account page, redirects to log in page" do - visit("/users/account") + visit("/users/#{user.id}") expect(page).to have_content("Sign in to your account to submit CORE data") end end @@ -141,42 +141,46 @@ RSpec.describe "User Features" do visit("/case-logs") expect(page).to have_link("Your account") click_link("Your account") - expect(page).to have_current_path("/users/account") - end - - it "main page is present and accessible" do - visit("/users/account") - expect(page).to have_content("Your account") - end - - it "personal details page is present and accessible" do - visit("/users/account/personal-details") - expect(page).to have_content("Change your personal details") - end - - it "edit password page present and accessible" do - visit("users/edit") - expect(page).to have_content("Change your password") + expect(page).to have_current_path("/users/#{user.id}") end it "can navigate to change your password page from main account page" do - visit("/users/account") + visit("/users/#{user.id}") find('[data-qa="change-password"]').click expect(page).to have_content("Change your password") fill_in("user[current_password]", with: "pAssword1") fill_in("user[password]", with: "Password123!") click_button("Update") - expect(page).to have_current_path("/users/account") + expect(page).to have_current_path("/users/#{user.id}") end it "allow user to change name" do - visit("/users/account") + visit("/users/#{user.id}") find('[data-qa="change-name"]').click expect(page).to have_content("Change your personal details") fill_in("user[name]", with: "Test New") click_button("Save changes") - expect(page).to have_current_path("/users/account") + expect(page).to have_current_path("/users/#{user.id}") expect(page).to have_content("Test New") end end + + context "Adding a new user" do + before(:each) do + visit("/case-logs") + fill_in("user[email]", with: user.email) + fill_in("user[password]", with: "pAssword1") + click_button("Sign in") + end + + it "validates email" do + visit("users/new") + fill_in("user[name]", with: "New User") + fill_in("user[email]", with: "thisis'tanemail") + click_button("Continue") + expect(page).to have_selector("#error-summary-title") + expect(page).to have_selector("#user-email-field-error") + expect(page).to have_content(/Enter an email address in the correct format, like name@example.com/) + end + end end diff --git a/spec/requests/users/passwords_controller_spec.rb b/spec/requests/auth/passwords_controller_spec.rb similarity index 84% rename from spec/requests/users/passwords_controller_spec.rb rename to spec/requests/auth/passwords_controller_spec.rb index ac48332e1..7bb617f78 100644 --- a/spec/requests/users/passwords_controller_spec.rb +++ b/spec/requests/auth/passwords_controller_spec.rb @@ -1,7 +1,7 @@ require "rails_helper" require_relative "../../support/devise" -RSpec.describe Users::PasswordsController, type: :request do +RSpec.describe Auth::PasswordsController, type: :request do let(:params) { { user: { email: email } } } context "when a password reset is requested for a valid email" do @@ -18,7 +18,7 @@ RSpec.describe Users::PasswordsController, type: :request do context "when a password reset is requested with an email that doesn't exist in the system" do before do - allow_any_instance_of(Users::PasswordsController).to receive(:is_navigational_format?).and_return(false) + allow_any_instance_of(Auth::PasswordsController).to receive(:is_navigational_format?).and_return(false) end let(:email) { "madeup_email@test.com" } @@ -32,7 +32,7 @@ RSpec.describe Users::PasswordsController, type: :request do end context "when a password reset is requested the email" do - let(:user) { FactoryBot.create(:user) } + let(:user) { FactoryBot.create(:user, last_sign_in_at: Time.zone.now) } let(:email) { user.email } it "should contain the correct email" do diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 3239dafbf..cd07845ac 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -4,6 +4,7 @@ RSpec.describe OrganisationsController, type: :request do let(:user) { FactoryBot.create(:user) } let(:organisation) { user.organisation } let(:headers) { { "Accept" => "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } context "details tab" do before do @@ -40,9 +41,7 @@ RSpec.describe OrganisationsController, type: :request do end it "shows a new user button" do - expected_html = " "text/html" } } + let(:page) { Capybara::Node::Simple.new(response.body) } + + describe "#show" do + before do + sign_in user + get "/users/#{user.id}", headers: headers, params: {} + end + + it "show the user details" do + expect(page).to have_content("Your account") + end + end + + describe "#edit" do + before do + sign_in user + get "/users/#{user.id}/edit", headers: headers, params: {} + end + + it "show the edit personal details page" do + expect(page).to have_content("Change your personal details") + end + end + + describe "#edit_password" do + before do + sign_in user + get "/users/#{user.id}/password/edit", headers: headers, params: {} + end + + it "show the edit password page" do + expect(page).to have_content("Change your password") + end + end +end