diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb index 3cb02b769..c389942dc 100644 --- a/app/helpers/tag_helper.rb +++ b/app/helpers/tag_helper.rb @@ -30,7 +30,7 @@ module TagHelper deactivated: "grey", deleted: "red", merged: "orange", - unconfirmed: "red", + unconfirmed: "blue", }.freeze def status_tag(status, classes = []) diff --git a/app/views/users/_user_list.html.erb b/app/views/users/_user_list.html.erb index 19e206b03..436c0def2 100644 --- a/app/views/users/_user_list.html.erb +++ b/app/views/users/_user_list.html.erb @@ -18,6 +18,9 @@ <% row.with_cell(header: true, text: "Last logged in", html_attributes: { scope: "col", }) %> + <% row.with_cell(header: true, text: "Status", html_attributes: { + scope: "col", + }) %> <% end %> <% end %> <% users.each do |user| %> @@ -50,7 +53,8 @@ <% end %> <% end %> <% row.with_cell(text: simple_format(org_cell(user), {}, wrapper_tag: "div")) %> - <% row.with_cell(text: user.active? ? user.last_sign_in_at&.to_formatted_s(:govuk_date) : status_tag(user.status)) %> + <% row.with_cell(text: user.last_sign_in_at&.to_formatted_s(:govuk_date)) %> + <% row.with_cell(text: status_tag(user.status)) %> <%= govuk_link_to users_path(user) do %> User <%= user.id %> <% end %> diff --git a/app/views/users/delete_confirmation.html.erb b/app/views/users/delete_confirmation.html.erb index 46b94d54d..4c8653a78 100644 --- a/app/views/users/delete_confirmation.html.erb +++ b/app/views/users/delete_confirmation.html.erb @@ -21,4 +21,4 @@ <%= govuk_button_link_to "Cancel", user_path(@user), html: { method: :get }, secondary: true %> - \ No newline at end of file + diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index bfb9c9c98..a4e5e7910 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -112,11 +112,18 @@ row.with_action end end %> - + <%= summary_list.with_row do |row| %> + <% row.with_key { "Status" } %> + <% row.with_value do %> + <%= details_html({ name: "Status", value: status_tag(@user.status), id: "status" }) %> + <% if @user.status == :deactivated && current_user.support? && !UserPolicy.new(current_user, @user).delete? %> + This user was active in an open or editable collection year, and cannot be deleted. + <% end %> + <% end %> + <% end %> <%= summary_list.with_row do |row| - row.with_key { "Status" } - row.with_value { status_tag(@user.status) } - row.with_action + row.with_key { "Last logged in" } + row.with_value { @user.last_sign_in_at&.to_formatted_s(:govuk_date) } end %> <% end %> diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb index 180a92aec..e3335c762 100644 --- a/spec/features/user_spec.rb +++ b/spec/features/user_spec.rb @@ -480,14 +480,14 @@ RSpec.describe "User Features" do it "allows to cancel user deactivation" do click_link("No – I’ve changed my mind") expect(page).to have_current_path("/users/#{other_user.id}") - expect(page).to have_no_content("This user has been deactivated.") + assert_selector ".govuk-tag", text: /Deactivated/, count: 0 expect(page).to have_no_css(".govuk-notification-banner.govuk-notification-banner--success") end it "allows to deactivate the user" do click_button("I’m sure – deactivate this user") expect(page).to have_current_path("/users/#{other_user.id}") - expect(page).to have_content("Deactivated") + assert_selector ".govuk-tag", text: /Deactivated/, count: 1 expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") end end @@ -517,7 +517,7 @@ RSpec.describe "User Features" do it "allows to cancel user reactivation" do click_link("No – I’ve changed my mind") expect(page).to have_current_path("/users/#{other_user.id}") - expect(page).to have_content("Deactivated") + assert_selector ".govuk-tag", text: /Deactivated/, count: 1 expect(page).to have_no_css(".govuk-notification-banner.govuk-notification-banner--success") end @@ -525,7 +525,7 @@ RSpec.describe "User Features" do expect(notify_client).to receive(:send_email).with(email_address: other_user.email, template_id: User::USER_REACTIVATED_TEMPLATE_ID, personalisation:).once click_button("I’m sure – reactivate this user") expect(page).to have_current_path("/users/#{other_user.id}") - expect(page).to have_no_content("This user has been deactivated.") + assert_selector ".govuk-tag", text: /Deactivated/, count: 0 expect(page).to have_css(".govuk-notification-banner.govuk-notification-banner--success") end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 9afcdc727..f73136948 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -646,7 +646,7 @@ RSpec.describe UsersController, type: :request do end it "shows if user is not active" do - expect(page).to have_content("Deactivated") + assert_select ".govuk-tag", text: /Deactivated/, count: 1 end it "allows reactivating the user" do @@ -1187,7 +1187,7 @@ RSpec.describe UsersController, type: :request do describe "#index" do let!(:other_user) { create(:user, organisation: user.organisation, name: "User 2", email: "other@example.com") } - let!(:inactive_user) { create(:user, organisation: user.organisation, active: false, name: "User 3", email: "inactive@example.com") } + let!(:inactive_user) { create(:user, organisation: user.organisation, active: false, name: "User 3", email: "inactive@example.com", last_sign_in_at: Time.zone.local(2022, 10, 10)) } let!(:other_org_user) { create(:user, name: "User 4", email: "otherorg@otherexample.com", organisation: create(:organisation, :without_dpc)) } before do @@ -1201,7 +1201,11 @@ RSpec.describe UsersController, type: :request do expect(page).to have_content(other_org_user.name) end - it "shows last logged in as deactivated for inactive users" do + it "shows last logged in date for all users" do + expect(page).to have_content("10 October 2022") + end + + it "shows status tag as deactivated for inactive users" do expect(page).to have_content("Deactivated") end @@ -1484,7 +1488,7 @@ RSpec.describe UsersController, type: :request do end it "shows if user is not active" do - expect(page).to have_content("Deactivated") + assert_select ".govuk-tag", text: /Deactivated/, count: 1 end it "allows reactivating the user" do @@ -1494,6 +1498,29 @@ RSpec.describe UsersController, type: :request do it "allows deleting the the user" do expect(page).to have_link("Delete this user", href: "/users/#{other_user.id}/delete-confirmation") end + + it "does not render informative text about deleting the user" do + expect(response).to have_http_status(:ok) + expect(page).not_to have_content("This user was active in an open or editable collection year, and cannot be deleted.") + end + + context "and has associated logs in editable collection period" do + before do + create(:data_protection_confirmation, organisation: other_user.organisation, confirmed: true) + create(:lettings_log, owning_organisation: other_user.organisation, created_by: other_user) + get "/users/#{other_user.id}" + end + + it "does not render delete this user" do + expect(response).to have_http_status(:ok) + expect(page).not_to have_link("Delete this user", href: "/users/#{user.id}/delete-confirmation") + end + + it "adds informative text about deleting the user" do + expect(response).to have_http_status(:ok) + expect(page).to have_content("This user was active in an open or editable collection year, and cannot be deleted.") + end + end end end