Move suspensions from access_rights to dedicated table

app/controllers/manage/users_controller.rb

@@ -122,6 +122,9 @@ def edit
     @accessible_roles = get_accessible_roles_for_current_user
     @db_auth = AuthenticationSystemUser.find_by_user_id(@user.id)
     @access_right = @user.access_right_for current_inventory_pool
+    @suspendsion = @user.suspensions.find_by(
+      inventory_pool: current_inventory_pool
+    )
   end
 
   ### update / put ############################################################

app/models/access_right.rb

@@ -1,10 +1,47 @@
 class AccessRight < ApplicationRecord
   audited
 
+  ### suspension virtual attributes ##################################
+  # the point of these is to avoid changing all the frontendcode
+  # after moving the suspensions to their own table
+
+  attribute :suspended_until, :date
+  attribute :suspended_reason, :text
+
+  after_save do |access_right|
+    if access_right.suspended_until.present?
+      Suspension.find_or_initialize_by(
+        user_id: access_right.user_id,
+        inventory_pool_id: access_right.inventory_pool_id
+      ).update_attributes(
+        suspended_until: access_right.suspended_until,
+        suspended_reason: access_right.suspended_reason
+      )
+    else
+      Suspension.find_or_initialize_by(
+        user_id: access_right.user_id,
+        inventory_pool_id: access_right.inventory_pool_id
+      ).try(&:destroy)
+    end
+  end
+
+  after_initialize :set_suspension_attributes
+  after_find :set_suspension_attributes
+  after_touch :set_suspension_attributes
+
+  def set_suspension_attributes
+    self.suspended_until = self.suspension.try(:suspended_until)
+    self.suspended_reason = self.suspension.try(:suspended_reason)
+  end
+
+  ####################################################################
+
   belongs_to :user, inverse_of: :access_rights
   belongs_to :inventory_pool, inverse_of: :access_rights
 
-  ####################################################################
+  def suspension
+    Suspension.find_by(user: user, inventory_pool: inventory_pool)
+  end
 
   # NOTE the elements have to be sorted in ascending order
   ROLES_HIERARCHY = [:customer,
@@ -13,7 +50,7 @@ class AccessRight < ApplicationRecord
                      :inventory_manager]
   AVAILABLE_ROLES = ROLES_HIERARCHY
 
-  AUTOMATIC_SUSPENSION_DATE = Date.new(2099, 1, 1)
+  AUTOMATIC_SUSPENSION_DATE = Date.today + 1000.years
 
   def role
     read_attribute(:role).to_sym
@@ -31,8 +68,6 @@ def role=(v)
 
   ####################################################################
 
-  validates_presence_of :suspended_reason, if: :suspended_until?
-
   before_validation(on: :create) do
     if user
       unless user.access_rights.active.empty?
@@ -53,10 +88,21 @@ def role=(v)
   ####################################################################
 
   scope :active, (lambda do
-    joins(:inventory_pool).where('inventory_pools.is_active': true)
+    joins(<<-SQL)
+      LEFT JOIN inventory_pools
+      ON inventory_pools.id = access_rights.inventory_pool_id
+    SQL
+    .where(<<-SQL)
+      inventory_pools.is_active = 't'
+    SQL
   end)
 
   scope :suspended, (lambda do
+    joins(<<-SQL)
+      LEFT JOIN suspensions
+      ON (suspensions.user_id = access_rights.user_id
+        AND  suspensions.inventory_pool_id = access_rights.inventory_pool_id)
+    SQL
     where
       .not(suspended_until: nil)
       .where('suspended_until >= ?', Time.zone.today)
@@ -75,7 +121,11 @@ def label_for_audits
   end
 
   def suspended?
-    !suspended_until.nil? and suspended_until >= Time.zone.today
+    suspension = Suspension.find_by(
+      user_id: self.user_id,
+      inventory_pool_id: self.inventory_pool_id
+    )
+    (suspension and suspension.suspended_until >= Time.zone.today) or false
   end
 
   ####################################################################

app/models/inventory_pool.rb

@@ -29,14 +29,18 @@ class InventoryPool < ApplicationRecord
                                 reject_if: proc { |holiday| holiday[:id] })
 
   has_many :access_rights, dependent: :delete_all
+
   has_many(:users, -> { distinct }, through: :access_rights)
+
+  has_many :suspensions, dependent: :delete_all
+
   has_many(:suspended_users,
            (lambda do
-               where.not(access_rights: { suspended_until: nil })
-               .where('access_rights.suspended_until >= ?', Time.zone.today)
-               .distinct
-           end),
-           through: :access_rights, source: :user)
+            where.not(suspensions: { suspended_until: nil })
+              .where('suspensions.suspended_until >= ?', Time.zone.today)
+              .distinct
+            end),
+           through: :suspensions, source: :user)
 
   has_many :rooms, -> { distinct }, through: :items
   has_many :items, dependent: :restrict_with_exception

app/models/suspension.rb

@@ -0,0 +1,6 @@
+class Suspension < ApplicationRecord
+  audited
+
+  belongs_to :user
+  belongs_to :inventory_pool
+end

app/models/user.rb

@@ -26,7 +26,10 @@ class User < ApplicationRecord
 
   has_many :orders
 
+  has_many :suspensions
+
   has_many :access_rights, dependent: :restrict_with_exception
+
   has_many(:inventory_pools,
            -> { distinct }, through: :access_rights) do
     def with_borrowable_items
@@ -289,9 +292,11 @@ def self.send_deadline_soon_reminder_to_everybody
 
   def automatic_suspend(inventory_pool)
     if inventory_pool.automatic_suspension? and not suspended?(inventory_pool)
-      access_right_for(inventory_pool).update_attributes \
-        suspended_until: AccessRight::AUTOMATIC_SUSPENSION_DATE,
-        suspended_reason: inventory_pool.automatic_suspension_reason
+      Suspension.find_or_initialize_by(
+        inventory_pool: inventory_pool,
+        user: self).update_attributes(
+          suspended_until: AccessRight::AUTOMATIC_SUSPENSION_DATE,
+          suspended_reason: inventory_pool.automatic_suspension_reason)
       puts "Suspended: #{self.name} on #{inventory_pool} for overdue take back"
     end
   end
@@ -349,7 +354,8 @@ def access_right_for(ip)
   end
 
   def suspended?(ip)
-    access_rights.active.suspended.where(inventory_pool_id: ip).exists?
+    suspensions.where(inventory_pool_id: ip)
+      .where('suspended_until >= ?', Date.today).exists?
   end
 
   #################### End role_requirement

factories/access_right_factory.rb

@@ -1,9 +1,28 @@
 FactoryGirl.define do
 
+  factory :suspension do
+  end
+
   factory :access_right do
     role { :customer }
     user
     inventory_pool
+
+    transient do
+      suspended_until nil
+      suspended_reason nil
+    end
+
+    after :create do |ac, trans|
+      if trans.suspended_until
+        FactoryGirl.create :suspension,
+                           suspended_until: trans.suspended_until,
+                           suspended_reason: trans.suspended_reason,
+                           inventory_pool_id: ac.inventory_pool_id,
+                           user_id: ac.user_id
+      end
+    end
+
   end
 
 end

features/manage/inventory_pool/inventory_pool_basic_information.feature

@@ -131,7 +131,7 @@ Feature: Basic information for inventory pools
     When I save
     Then this configuration is saved
     When a user is suspended automatically due to late contracts
-    Then they are suspended for this inventory pool until '1/1/2099'
+    Then they are suspended for this inventory pool
     And the reason for suspension is the one specified for this inventory pool
     When I disable "Automatic suspension"
     And I save

features/step_definitions/manage/pool_information_steps.rb

@@ -152,13 +152,13 @@
   @user.automatic_suspend(@current_inventory_pool)
 end
 
-Then(/^they are suspended for this inventory pool until '(\d+)\/(\d+)\/(\d+)'$/) do |day, month, year|
-  @access_right = @user.access_right_for(@current_inventory_pool)
-  expect(@access_right.suspended_until).to eq Date.new(year.to_i, month.to_i, day.to_i)
+Then(/^they are suspended for this inventory pool/) do
+  @suspension= Suspension.find_by(user: @user, inventory_pool: @current_inventory_pool)
+  expect(@suspension.suspended_until).to be> Date.today
 end
 
 Then(/^the reason for suspension is the one specified for this inventory pool$/) do
-  expect(@access_right.suspended_reason).to eq @reason
+  expect(@suspension.suspended_reason).to eq @reason
 end
 
 When(/^I (enable|disable) automatic access$/) do |arg1|
@@ -259,10 +259,10 @@
 
 Then(/^the existing suspension motivation and the suspended time for this user are not overwritten$/) do
   def checks_suspension
-    ar = @user.access_right_for(@current_inventory_pool)
-    expect(ar.suspended_until).to eq @suspended_until
-    expect(ar.suspended_reason).to eq @suspended_reason
-    expect(ar.suspended_reason).not_to eq @current_inventory_pool.automatic_suspension_reason
+    sus = Suspension.find_by(user: @user, inventory_pool: @current_inventory_pool)
+    expect(sus.suspended_until).to eq @suspended_until
+    expect(sus.suspended_reason).to eq @suspended_reason
+    expect(sus.suspended_reason).not_to eq @current_inventory_pool.automatic_suspension_reason
   end
 
   checks_suspension

features/step_definitions/manage/user_management_steps.rb

@@ -146,10 +146,10 @@ def check_user_list(users)
 end
 
 Given /^a (.*?)user (with|without) assigned role appears in the user list$/ do |suspended, with_or_without|
-  user = User.where(login: 'normin').first
+  @user = User.where(login: 'normin').first
   case suspended
     when 'suspended '
-      user
+      @user
         .access_rights
         .where(inventory_pool: @current_inventory_pool)
         .active
@@ -158,22 +158,23 @@ def check_user_list(users)
   end
   case with_or_without
     when 'with'
-      expect(user.access_rights.active.empty?).to be false
+      expect(@user.access_rights.active.empty?).to be false
     when 'without'
-      user.access_rights.active.delete_all
-      expect(user.access_rights.active.empty?).to be true
+      @user.access_rights.active.delete_all
+      expect(@user.access_rights.active.empty?).to be true
   end
   step %Q(I can find the user administration features in the "Manage" area under "Users")
   within '#user-list' do
     find('.line', match: :first)
     step 'I scroll loading all pages'
-    @el = find('.line', match: :prefer_exact, text: user.name)
+    @el = find('.line', match: :prefer_exact, text: @user.name)
   end
 end
 
 Then /^I see the following information, in order:$/ do |table|
   user = User.find @el.find('[data-id]')['data-id']
   access_right = user.access_right_for(@current_inventory_pool)
+  suspension = Suspension.find_by(user: user, inventory_pool: @current_inventory_pool)
 
   strings = table.hashes.map do |x|
     case x[:attr]
@@ -185,7 +186,7 @@ def check_user_list(users)
         role = access_right.try(:role) || 'no access'
         _(role.to_s.humanize)
       when 'Suspended until dd.mm.yyyy'
-        "#{_("Suspended until")} %s" % I18n.l(access_right.suspended_until)
+        "#{_("Suspended until")} %s" % I18n.l(suspension.suspended_until)
     end
   end
   expect(@el.text).to match Regexp.new(strings.join('.*'))
@@ -305,9 +306,9 @@ def check_user_list(users)
 
 Then(/^I can create and suspend users$/) do
   step 'I can create a new user for the inventory_pool'
-  expect(@user.access_right_for(@inventory_pool).suspended?).to be false
+	expect(@user.suspended?(@inventory_pool)).to be false
   expect(page.driver.browser.process(:put, manage_update_inventory_pool_user_path(@inventory_pool, @user, format: :json), access_right: {suspended_until: Date.today + 1.year, suspended_reason: 'suspended reason'}).successful?).to be true
-  expect(@user.reload.access_right_for(@inventory_pool).suspended?).to be true
+	expect(@user.suspended?(@inventory_pool)).to be true
 end
 
 Then(/^I can assign and remove roles to and from users as specified in the following table, but only in the inventory pool for which I am manager$/) do |table|

features/step_definitions/technical/user_steps.rb

@@ -7,12 +7,13 @@
   User.remind_and_suspend_all
 end
 
-Then(/^every such user is suspended until '(\d+)\.(\d+)\.(\d+)' in the corresponding inventory pool$/) do |day, month, year|
+Then(/^every such user is suspended in the corresponding inventory pool$/) do
   @reservations.each do |c|
     ip = c.inventory_pool
     u = c.user
-    ar = u.access_right_for(ip)
-    expect(ar.suspended_until).to eq Date.new(year.to_i, month.to_i, day.to_i)
+    suspension = Suspension.find_by(user: u, inventory_pool: ip)
+    expect(suspension).to be
+    expect(suspension.suspended_until).to be>= Date.today
   end
 end
 

features/technical/user/suspend.feature

@@ -6,5 +6,5 @@ Feature: Suspend all users
   Scenario: Suspend all users with late take backs
     Given there are at least 2 users with late take backs from at least 2 inventory pools where automatic suspension is activated
     When the cronjob executes the rake task for reminding and suspending all late users
-    Then every such user is suspended until '1.1.2099' in the corresponding inventory pool
+    Then every such user is suspended in the corresponding inventory pool
     And the suspended reason is the one configured for the corresponding inventory pool

spec/steps/borrow/submit_order_steps.rb

@@ -10,9 +10,11 @@ module SubmitOrderSteps
       include ::Spec::PersonasDumpSteps
 
       step 'I am suspended for a pool I am a customer of' do
-        ar = AccessRight.find_by(inventory_pool: @inventory_pool, role: :customer)
-        ar.update_attributes(suspended_until: Date.tomorrow,
-                             suspended_reason: Faker::Lorem.sentence)
+        FactoryGirl.create :suspension,
+                           user_id: @current_user.id,
+                           inventory_pool_id: @inventory_pool.id,
+                           suspended_until: Date.tomorrow,
+                           suspended_reason: Faker::Lorem.sentence
       end
 
       step 'there is a borrowable item in this pool' do