Skip to content

Commit

Permalink
Update for deactivated_at
Browse files Browse the repository at this point in the history
We recently dropped the `active` column and want to retain the behaviour
using the `deactivated_at` value:

- if deactivated_at is nil the user is active
- if deactivated_at is present the user has been deactivated

This works creates a `active` method on User and a `active?` alias, to
replicate the old behaviour.

We update the two scopes so they work based on deactivated_at.

Finally, we adjust the specs setup to make sure we are setting a
deactivated_at date not the old active value.

At this point the application is working but the tests will fail and an
attempt to activate or deactivate a user will fail, we'll fix that next.
  • Loading branch information
mec committed Dec 13, 2024
1 parent 6452b40 commit 2e15707
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 20 deletions.
9 changes: 7 additions & 2 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@ class User < ApplicationRecord
organisation_id: :organisation
}.freeze

scope :active, -> { where(active: true) }
scope :inactive, -> { where(active: false) }
scope :active, -> { where(deactivated_at: nil) }
scope :inactive, -> { where.not(deactivated_at: nil) }

delegate :service_owner?, :partner_organisation?, to: :organisation

def active
deactivated_at.blank?
end
alias_method :active?, :active

def organisation
if Current.user_organisation
return Organisation.find(Current.user_organisation)
Expand Down
5 changes: 3 additions & 2 deletions spec/factories/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
factory :administrator, class: "User" do
name { Faker::Name.name }
email { Faker::Internet.email }
active { true }
deactivated_at { nil }
anonymised_at { nil }
password { "Ab1!#{SecureRandom.uuid}" }
mobile_number { Faker::PhoneNumber.phone_number }
mobile_number_confirmed_at { 1.day.ago }
Expand All @@ -19,7 +20,7 @@
end

factory :inactive_user do
active { false }
deactivated_at { Date.yesterday }
end

trait :new_user do
Expand Down
4 changes: 2 additions & 2 deletions spec/features/users_can_sign_in_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def log_in_via_form(user, remember_me: false)

context "when the user has been deactivated" do
scenario "the user cannot log in and sees an informative message" do
user = create(:partner_organisation_user, active: false)
user = create(:partner_organisation_user, deactivated_at: DateTime.yesterday)

visit root_path
log_in_via_form(user)
Expand All @@ -368,7 +368,7 @@ def log_in_via_form(user, remember_me: false)

expect(page.current_path).to eql home_path

user.active = false
user.deactivated_at = DateTime.now
user.save

visit home_path
Expand Down
12 changes: 6 additions & 6 deletions spec/mailers/report_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

context "when the user is inactive" do
before do
user.update!(active: false)
user.update!(deactivated_at: DateTime.yesterday)
end

it "should raise an error" do
Expand Down Expand Up @@ -136,7 +136,7 @@
end

context "when the user is inactive" do
let(:user) { create(:administrator, organisation: organisation, active: false) }
let(:user) { create(:administrator, organisation: organisation, deactivated_at: DateTime.yesterday) }

it "should raise an error" do
expect { mail.body }.to raise_error(ArgumentError, "User must be active to receive report-related emails")
Expand Down Expand Up @@ -166,7 +166,7 @@
end

context "when the user is inactive" do
before { user.update!(active: false) }
before { user.update!(deactivated_at: DateTime.yesterday) }

it "should raise an error" do
expect { mail.body }.to raise_error(ArgumentError, "User must be active to receive report-related emails")
Expand Down Expand Up @@ -239,7 +239,7 @@

context "when the user is inactive" do
before do
user.update!(active: false)
user.update!(deactivated_at: DateTime.yesterday)
end

it "should raise an error" do
Expand Down Expand Up @@ -317,7 +317,7 @@

context "when the user is inactive" do
before do
user.update!(active: false)
user.update!(deactivated_at: DateTime.yesterday)
end

it "should raise an error" do
Expand Down Expand Up @@ -379,7 +379,7 @@

context "when the user is inactive" do
before do
user.update!(active: false)
user.update!(deactivated_at: DateTime.yesterday)
end

it "should raise an error" do
Expand Down
36 changes: 30 additions & 6 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,18 @@

describe "scopes" do
it "shows active users for active scope" do
create(:administrator, active: true)
create(:administrator, active: true)
create(:administrator, active: false)
create(:administrator, deactivated_at: nil)
create(:administrator, deactivated_at: nil)
create(:administrator, deactivated_at: DateTime.yesterday)

expect(User.active.size).to eq(2)
expect(User.active.last.active).to eq(true)
end

it "shows inactive users for inactive scope" do
create(:administrator, active: true)
create(:administrator, active: true)
create(:administrator, active: false)
create(:administrator, deactivated_at: nil)
create(:administrator, deactivated_at: nil)
create(:administrator, deactivated_at: DateTime.yesterday)

expect(User.inactive.size).to eq(1)
expect(User.inactive.last.active).to eq(false)
Expand Down Expand Up @@ -148,4 +148,28 @@
expect(user.valid?).to be_truthy
end
end

describe "#active" do
it "is aliased as active?" do
user = build(:partner_organisation_user)

expect(user.active?).to be true
end

context "when the user has no deactivated date" do
it "is active" do
user = build(:partner_organisation_user, deactivated_at: nil)

expect(user.active?).to be true
end
end

context "when the user has a deactivated date" do
it "is not active" do
user = build(:partner_organisation_user, deactivated_at: DateTime.yesterday)

expect(user.active?).to be false
end
end
end
end
4 changes: 2 additions & 2 deletions spec/services/report/send_state_change_emails_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

let!(:report) { create(:report, state: state) }
let!(:partner_organisation_users) { create_list(:administrator, 5, organisation: report.organisation) }
let!(:inactive_po_user) { create(:administrator, organisation: report.organisation, active: false) }
let!(:inactive_po_user) { create(:administrator, organisation: report.organisation, deactivated_at: DateTime.yesterday) }
let!(:service_owners) { create_list(:beis_user, 2) }
let!(:inactive_service_owner) { create(:beis_user, active: false) }
let!(:inactive_service_owner) { create(:beis_user, deactivated_at: DateTime.yesterday) }

let(:recipients) { ActionMailer::Base.deliveries.map { |delivery| delivery.to }.flatten }

Expand Down

0 comments on commit 2e15707

Please sign in to comment.