Add explicit #start_on/#end_on to Role, remove FutureRole, Role#delet…

…e_on (#150)

app/controllers/history_roles_controller.rb

@@ -25,7 +25,7 @@ def create # rubocop:disable Metrics/AbcSize
   end
 
   def destroy
-    role = Role.with_deleted.find(params[:id])
+    role = Role.with_inactive.find(params[:id])
     person = role.person
     authorize!(:destroy, role)
 
@@ -56,8 +56,8 @@ def build_role(group)
       group: group,
       person_id: params[:role][:person_id],
       label: params[:role][:label],
-      created_at: params[:role][:start_date],
-      deleted_at: params[:role][:end_date],
+      start_on: params[:role][:start_date],
+      end_on: params[:role][:end_date],
       historic_membership: true
     )
   end

app/models/invoice_item/membership_fee.rb

@@ -36,12 +36,11 @@ def member_count_for_dynamic_cost(recipient_id, cutoff_date)
     if layer.uses_manually_counted_members?
       layer.manual_member_count
     else
-      roles_scope = Role.with_deleted.joins(:group)
+      roles_scope = Role.with_inactive.joins(:group)
         .where(type: Group::VereinMitglieder::Mitglied.sti_name,
           group: {layer_group_id: layer.id})
       if cutoff_date.present?
-        roles_scope.where("roles.deleted_at IS NULL OR roles.deleted_at > ?", cutoff_date)
-          .where(created_at: ...cutoff_date)
+        roles_scope.active(cutoff_date)
       else
         roles_scope
       end.count

app/models/person/active_years.rb

@@ -49,8 +49,8 @@ def active_member_role?
   end
 
   def calculate_active_years(end_date = Time.zone.now)
-    roles.with_deleted.where(veteran_role_condition).map do |role|
-      VeteranYears.new(role.created_at.year, (role.deleted_at || end_date).year)
+    roles.with_inactive.where(veteran_role_condition).map do |role|
+      VeteranYears.new((role.start_on || role.created_at).year, (role.end_on || end_date).year)
     end.sort.sum.years.to_i
   end
 end

app/models/role/mitglieder_mitglied.rb

@@ -27,7 +27,10 @@ class Role::MitgliederMitglied < Role
   after_destroy :update_active_years_on_person
   after_save :update_active_years_on_person
 
-  validates_date :deleted_at,
+  validates_date :start_on,
+    allow_nil: true
+
+  validates_date :end_on,
     if: :historic_membership,
     allow_nil: false,
     on_or_before: -> { Time.zone.today },

config/locales/models.sbv.de.yml

@@ -330,8 +330,8 @@ de:
         suisa_status: SUISA Status
       role:
         dates: Daten
-        created_at: Eintritt
-        deleted_at: Austritt
+        start_on: Eintritt
+        end_on: Austritt
         correspondence_language: Korrespondenz Sprache
         class:
           permission:

db/migrate/20180915094905_add_active_years_to_people.rb

@@ -9,8 +9,8 @@ def change
           end_date = Time.zone.now
 
           Person.find_each do |person|
-            active_years = person.roles.with_deleted.where("type LIKE '%Mitglied'").map do |role|
-              VeteranYears.new(role.created_at.year, (role.deleted_at || end_date).year)
+            active_years = person.roles.with_inactive.where("type LIKE '%Mitglied'").map do |role|
+              VeteranYears.new((role.start_on || role.created_at).year, (role.end_on || end_date).year)
             end.sort.sum.years.to_i
             active_roles = person.roles.where("type LIKE '%Mitglied'").any?
 

lib/tasks/reactivate.rake

@@ -65,10 +65,12 @@ class Reactivator
   end
 
   def restore_roles(group)
-    group.roles.with_deleted.where(
-      deleted_at: (@start_date.prev_day..@end_date.next_day)
+    group.roles.with_inactive.where(
+      end_on: (@start_date.prev_day..@end_date.next_day)
     ).find_each do |role|
-      role.restore!
+      if (end_on_change = role.versions.last.changeset["end_on"])
+        role.update(end_on: end_on_change.first)
+      end
     end
   end
 end

spec/controllers/history_roles_controller_spec.rb

@@ -61,9 +61,9 @@
     }
     expect do
       post :create, params: {group_id: group.id, role: role_params}
-    end.to change { leader.roles.with_deleted.count }.by(1)
+    end.to change { leader.roles.with_inactive.count }.by(1)
     expect(leader.reload.active_years).to eq 2
-    expect(leader.roles.with_deleted).to be_any { |role| role.label === "1. Sax" }
+    expect(leader.roles.with_inactive).to be_any { |role| role.label === "1. Sax" }
   end
 
   it "POST#create creates new role and deleted mitglieder verein in hidden verein group" do
@@ -81,7 +81,11 @@
     expect do
       post :create, params: {group_id: leader.primary_group_id, role: role_params}
       expect(response).to redirect_to(history_group_person_path(leader.primary_group, leader))
-    end.to change { leader.roles.count }.by(0)
+    end.to change { leader.roles.count }.by(1)
+
+    new_role = leader.roles.last
+    expect(new_role.group.name).to eq "Dummy"
+    expect(new_role.end_on).to eq Time.zone.today
 
     expect(leader.reload.active_years).to eq 2
 
@@ -92,15 +96,15 @@
 
   it "DELETE#destroy hard destroys role and updates active_years" do
     role = roles(:suisa_admin)
-    role.update(created_at: 3.years.ago)
+    role.update(start_on: 3.years.ago)
 
     person = role.person
     person.update_active_years
 
     sign_in(people(:leader))
     expect do
       delete :destroy, params: {group_id: role.group_id, id: role.id}
-    end.to change { role.person.roles.with_deleted.count }.by(-1)
+    end.to change { role.person.roles.with_inactive.count }.by(-1)
 
     expect(role.person.active_years).to eq 0
 
@@ -110,19 +114,19 @@
 
   it "DELETE#destroy hard destroys deleted role and updates active_years" do
     role = roles(:suisa_admin)
-    role.update(created_at: 3.years.ago, deleted_at: 10.days.ago)
+    role.update(start_on: 3.years.ago, end_on: 10.days.ago)
 
     person = role.person
     person.update_active_years
 
     sign_in(people(:leader))
     expect do
       delete :destroy, params: {group_id: role.group_id, id: role.id}
-    end.to change { role.person.roles.with_deleted.count }.by(-1)
+    end.to change { role.person.roles.with_inactive.count }.by(-1)
 
     expect(role.person.active_years).to eq 0
 
-    expect(flash[:notice]).to eq "Verantwortlicher SUISA wurde erfolgreich gelöscht."
+    expect(flash[:notice]).to match(/Verantwortlicher SUISA \(bis \d{2}\.\d{2}\.\d{4}\) wurde erfolgreich gelöscht./)
     expect(response).to redirect_to(group_path(role.group))
   end
 end

spec/domain/people/merger_spec.rb

@@ -6,16 +6,16 @@
   let(:person) { Fabricate(:person) }
   let(:duplicate) { Fabricate(:person_with_address_and_phone) }
   let(:actor) { people(:admin) }
-  let(:person_roles) { person.roles.with_deleted }
+  let(:person_roles) { person.roles.with_inactive }
 
   let(:merger) { described_class.new(@source.reload, @target.reload, actor) }
 
   before do
     Fabricate("Group::RootMusikkommission::Mitglied",
       group: groups(:musikkommission_4),
       person: duplicate,
-      created_at: 5.seconds.ago,
-      deleted_at: Time.zone.now)
+      start_on: 5.seconds.ago,
+      end_on: Time.zone.now)
   end
 
   context "merge people" do

spec/fixtures/roles.yml

@@ -36,7 +36,7 @@ member:
   person: member
   group: musikverband_hastdutoene
   type: Group::VereinMitglieder::Mitglied
-  created_at: <%= 1.year.ago %>
+  start_on: <%= 1.year.ago %>
 
 suisa_admin:
   person: suisa_admin

spec/models/invoice_item/membership_fee_spec.rb

@@ -45,8 +45,8 @@
           expect(invoice_item.dynamic_cost).to eq(600) # 3 * 20 * 10
         end
 
-        it "excludes members with roles deleted_at before cutoff_date" do
-          member_roles.sample(5).each { |r| r.update(deleted_at: 5.days.from_now.to_s.to_date) }
+        it "excludes members with roles end_on before cutoff_date" do
+          member_roles.sample(5).each { |r| r.update(end_on: 5.days.from_now.to_s.to_date) }
           expect(invoice_item.dynamic_cost).to eq(550) # (3 * 20 - 5) * 10
         end
       end
@@ -71,8 +71,8 @@
           expect(invoice_item.dynamic_cost).to eq(600) # 3 * 20 * 10
         end
 
-        it "excludes members with roles deleted_at before cutoff_date" do
-          member_roles.sample(5).each { |r| r.update(deleted_at: 5.days.from_now.to_s.to_date) }
+        it "excludes members with roles end_on before cutoff_date" do
+          member_roles.sample(5).each { |r| r.update(end_on: 5.days.from_now.to_s.to_date) }
           expect(invoice_item.dynamic_cost).to eq(550) # (3 * 20 - 5) * 10
         end
       end
@@ -97,8 +97,8 @@
           expect(invoice_item.dynamic_cost).to eq(600) # 3 * 20 * 10
         end
 
-        it "excludes members with roles deleted_at before cutoff_date" do
-          member_roles.sample(5).each { |r| r.update(deleted_at: 5.days.from_now.to_s.to_date) }
+        it "excludes members with roles end_on before cutoff_date" do
+          member_roles.sample(5).each { |r| r.update(end_on: 5.days.from_now.to_s.to_date) }
           expect(invoice_item.dynamic_cost).to eq(550) # (3 * 20 - 5) * 10
         end
       end

spec/models/person_spec.rb

@@ -33,13 +33,13 @@
 
     def create_role(role_class, years: 10, start_date: false, end_date: false)
       start_year = 2005
-      created_at = begin
+      start_on = begin
         start_date.presence || Date.current.change(year: start_year)
       rescue Date::Error
         Date.current.change(day: 25, year: start_year)
       end
 
-      deleted_at = if end_date == false
+      end_on = if end_date == false
         begin
           Date.current.change(year: start_year + years)
         rescue Date::Error
@@ -53,8 +53,8 @@ def create_role(role_class, years: 10, start_date: false, end_date: false)
         role_class.name.to_sym,
         person: subject,
         group: groups(:mitglieder_hastdutoene),
-        created_at: created_at,
-        deleted_at: deleted_at
+        start_on: start_on,
+        end_on: end_on
       )
     end
 
@@ -88,8 +88,8 @@ def create_role(role_class, years: 10, start_date: false, end_date: false)
         :"Role::MitgliederMitglied",
         person: subject,
         group: groups(:mitglieder_hastdutoene),
-        created_at: Date.current.change(year: 2005, day: 25),
-        deleted_at: Date.current.change(year: 2012)
+        start_on: Date.current.change(year: 2005, day: 25),
+        end_on: Date.current.change(year: 2012)
       ).save(validate: false)
 
       subject.update_active_years
@@ -111,8 +111,8 @@ def create_role(role_class, years: 10, start_date: false, end_date: false)
         :"Role::MitgliederPassivmitglied",
         person: subject,
         group: groups(:mitglieder_hastdutoene),
-        created_at: Date.current.change(year: 2005, day: 25),
-        deleted_at: Date.current.change(year: 2015, day: 25)
+        start_on: Date.current.change(year: 2005, day: 25),
+        end_on: Date.current.change(year: 2015, day: 25)
       ).save(validate: false)
 
       subject.update_active_years
@@ -181,7 +181,7 @@ def create_role(role_class, years: 10, start_date: false, end_date: false)
 
     it "handles active_years not being cached" do
       travel_to("2020-03-16") do
-        expect(subject.roles.with_deleted.where(type: "Group::VereinMitglieder::Mitglied").count).to eq 0
+        expect(subject.roles.with_inactive.where(type: "Group::VereinMitglieder::Mitglied").count).to eq 0
         expect(subject.active_years).to be_nil
 
         expect(subject.prognostic_active_years).to eq 0
@@ -190,11 +190,11 @@ def create_role(role_class, years: 10, start_date: false, end_date: false)
           Group::VereinMitglieder::Mitglied.name.to_sym,
           person: subject,
           group: groups(:mitglieder_hastdutoene),
-          created_at: Date.current.change(year: 2010),
-          deleted_at: nil # active Role
+          start_on: Date.current.change(year: 2010),
+          end_on: nil # active Role
         )
 
-        expect(subject.roles.without_deleted.where(type: "Group::VereinMitglieder::Mitglied").count).to eq 1
+        expect(subject.roles.where(type: "Group::VereinMitglieder::Mitglied").count).to eq 1
         expect(subject.active_years).to eq 10
 
         # simulate data not being present

spec/models/roles/mitglieder_mitglied_spec.rb

@@ -17,8 +17,8 @@
         type: "Group::VereinMitglieder::Mitglied",
         group: groups(:mitglieder_mg_aarberg),
         person: people(:member),
-        created_at: 1.year.ago,
-        deleted_at: nil,
+        start_on: 1.year.ago,
+        end_on: nil,
         historic_membership: true
       )
     end
@@ -27,24 +27,24 @@
       expect(subject.historic_membership).to be true
     end
 
-    it "is invalid without deleted_at" do
-      expect(subject.deleted_at).to be_nil
+    it "is invalid without end_on" do
+      expect(subject.end_on).to be_nil
 
       expect(subject).to_not be_valid
       expect(subject.errors.full_messages)
         .to include("Austritt ist kein gültiges Datum")
     end
 
-    it "is invalid with future deleted_at" do
-      subject.deleted_at = 1.week.from_now
+    it "is invalid with future end_on" do
+      subject.end_on = 1.week.from_now
 
       expect(subject).to_not be_valid
       expect(subject.errors.full_messages)
         .to include("Austritt kann nicht später als heute sein")
     end
 
-    it "is valid with deleted_at in the past" do
-      subject.deleted_at = 1.week.ago
+    it "is valid with end_on in the past" do
+      subject.end_on = 1.week.ago
 
       expect(subject).to be_valid
     end

spec/performance/active_year_performance_spec.rb

@@ -16,11 +16,11 @@
 
   before do
     person.roles.each { |role| role.really_destroy! }
-    Role.create!(person: person, group: group, created_at: 20.years.ago, deleted_at: 17.years.ago, type: "Group::VereinMitglieder::Mitglied")
-    Role.create!(person: person, group: group, created_at: 15.years.ago, deleted_at: 13.years.ago, type: "Group::VereinMitglieder::Mitglied")
-    Role.create!(person: person, group: group, created_at: 10.years.ago, deleted_at: 7.years.ago, type: "Group::VereinMitglieder::Mitglied")
-    Role.create!(person: person, group: group, created_at: 5.years.ago, deleted_at: 3.years.ago, type: "Group::VereinMitglieder::Mitglied")
-    Role.create!(person: person, group: group, created_at: 1.year.ago, deleted_at: nil, type: "Group::VereinMitglieder::Mitglied")
+    Role.create!(person: person, group: group, start_on: 20.years.ago, end_on: 17.years.ago, type: "Group::VereinMitglieder::Mitglied")
+    Role.create!(person: person, group: group, start_on: 15.years.ago, end_on: 13.years.ago, type: "Group::VereinMitglieder::Mitglied")
+    Role.create!(person: person, group: group, start_on: 10.years.ago, end_on: 7.years.ago, type: "Group::VereinMitglieder::Mitglied")
+    Role.create!(person: person, group: group, start_on: 5.years.ago, end_on: 3.years.ago, type: "Group::VereinMitglieder::Mitglied")
+    Role.create!(person: person, group: group, start_on: 1.year.ago, end_on: nil, type: "Group::VereinMitglieder::Mitglied")
   end
 
   def measure(max_time, &block)