test and fix Semester service

app/controllers/semester_process_volunteers_controller.rb

 class SemesterProcessVolunteersController < ApplicationController
   before_action :set_semester_process_volunteer, only: [:show, :edit, :update]
+  before_action :set_semester, only: [:index]
 
   def index
     authorize SemesterProcessVolunteer
-    @semester_process = SemesterProcess.find(params[:semester_process_id])
-    @semester_process_volunteers = @semester_process.semester_process_volunteers
+
+    @spvs = SemesterProcessVolunteer.index(Semester.parse(params[:semester])).page(params[:page])
   end
 
   def show; end
@@ -12,8 +13,8 @@ class SemesterProcessVolunteersController < ApplicationController
   def edit; end
 
   def update
-    if @semester_process_volunteer.update(semester_process_params)
-      redirect_to @semester_process_volunteer, notice: 'Semester process was successfully updated.'
+    if @spv.update(semester_process_params)
+      redirect_to @spv, notice: 'Semester process was successfully updated.'
     else
       render :edit
     end
@@ -22,10 +23,20 @@ class SemesterProcessVolunteersController < ApplicationController
   private
 
   def set_semester_process_volunteer
-    @semester_process_volunteer = SemesterProcess.find(params[:id])
-    authorize @semester_process_volunteer
-    @semester_process = @semester_process_volunteer.semester_process
-    @volunteer = @semester_process_volunteer.volunteer
+    @spv = SemesterProcessVolunteer.find(params[:id])
+    authorize @spv
+    @semester_process = @spv.semester_process
+    @volunteer = @spv.volunteer
+  end
+
+  def set_semester
+    @semester = Semester.new
+    if params[:semester]
+      @selected_semester = Semester.parse(params[:semester])
+    else
+      @selected_semester = @semester.previous
+      params[:semester] = Semester.to_s(@selected_semester)
+    end
   end
 
   def semester_process_volunteer_params

app/controllers/semester_processes_controller.rb

 class SemesterProcessesController < ApplicationController
-  before_action :set_semester_process, only: [:show, :edit, :update, :destroy]
+  before_action :set_semester_process, only: [:show, :edit, :update]
   before_action :set_semester, only: [:new, :create]
 
   def index
@@ -26,7 +26,7 @@ class SemesterProcessesController < ApplicationController
     @semester_process.build_volunteers_hours_feedbacks_and_mails
 
     if @semester_process.save
-      redirect_to @semester_process, notice: 'Semester process was successfully created.'
+      redirect_to semester_process_volunteers_path, notice: 'Semester process was successfully created.'
     else
       render :new
     end
@@ -40,14 +40,6 @@ class SemesterProcessesController < ApplicationController
     end
   end
 
-  def destroy
-    if @semester_process.destroy
-      redirect_to semester_processes_url, notice: 'Semester process was successfully destroyed.'
-    else
-      redirect_to semester_processes_url, notice: 'Failure notice'
-    end
-  end
-
   private
 
   def set_semester_process
@@ -57,9 +49,15 @@ class SemesterProcessesController < ApplicationController
 
   def set_semester
     @semester = Semester.new
-    params[:semester] = @semester.year_number unless params[:semester]
-    @selected_semester = Semester.new(*params[:semester].split(',').map(&:to_i)).current
-    @volunteers = Volunteer.semester_process_eligible(@selected_semester)
+    if params[:semester]
+      @selected_semester = Semester.parse(params[:semester])
+    else
+      @selected_semester = @semester.previous
+      params[:semester] = Semester.to_s(@selected_semester)
+    end
+
+    semester_form_param = Semester.parse(params[:semester_process]&.fetch(:semester))
+    @volunteers = Volunteer.semester_process_eligible(semester_form_param || @selected_semester)
   end
 
   def selected_volunteers

app/models/semester_process.rb

@@ -28,12 +28,16 @@ class SemesterProcess < ApplicationRecord
   delegate :missions, to: :semester_process_volunteers
 
   # creates semester date range from string '[year],[semester_number]' e.g. '2018,2'
-  def semester=(semester)
-    if semester.is_a?(String)
-      super(Semester.new(*semester.split(',').map(&:to_i)).current)
-    else
-      super(semester)
-    end
+  def semester=(set_semester)
+    set_semester = Semester.parse(set_semester) if set_semester.is_a?(String)
+
+    # for very strange reason the end of the range is shifted one day after save
+    # possibly a bug in Active Directory
+    super(set_semester.begin..set_semester.end.advance(days: -1))
+  end
+
+  def semester_t(short: true)
+    Semester.i18n_t(semester, short: short)
   end
 
   def build_semester_volunteers(volunteers, selected = nil)
@@ -46,6 +50,6 @@ class SemesterProcess < ApplicationRecord
   end
 
   def build_volunteers_hours_feedbacks_and_mails
-    semester_process_volunteers.map(&:build_hours_feedbacks_and_mails)
+    semester_process_volunteers.map(&:build_hours_and_mails)
   end
 end

app/models/semester_process_volunteer.rb

@@ -4,6 +4,7 @@ class SemesterProcessVolunteer < ApplicationRecord
   belongs_to :volunteer
   belongs_to :semester_process
   delegate :semester, to: :semester_process
+  delegate :semester_t, to: :semester_process
   delegate :creator, to: :semester_process
 
   belongs_to :responsible, -> { with_deleted }, class_name: 'User',
@@ -26,6 +27,18 @@ class SemesterProcessVolunteer < ApplicationRecord
   has_many :reminders, -> { where(kind: 'reminder') }, class_name: 'SemesterProcessMail',
     foreign_key: 'semester_process_volunteer_id', inverse_of: 'semester_process_volunteer'
 
+  scope :index_joins, lambda {
+    joins(:semester_process).joins(volunteer: [:contact]).joins(:semester_process_volunteer_missions)
+  }
+
+  scope :index, lambda { |semester = nil|
+    if semester
+      index_joins.where('semester_processes.semester && daterange(?,?)', semester.begin, semester.end)
+    else
+      index_joins
+    end
+  }
+
   # will only return an array, not a AD-result
   def missions
     semester_process_volunteer_missions.map(&:mission)
@@ -40,10 +53,9 @@ class SemesterProcessVolunteer < ApplicationRecord
     end
   end
 
-  def build_hours_feedbacks_and_mails
+  def build_hours_and_mails
     missions.each do |mission|
       hours << mission.hours.date_between_inclusion(:meeting_date, semester.begin, semester.end)
-      semester_feedbacks << SemesterFeedback.new(mission: mission, volunteer: mission.volunteer)
     end
     semester_process_mails << SemesterProcessMail.new(kind: :mail, sent_by: creator)
   end

app/policies/semester_process_policy.rb

@@ -14,5 +14,4 @@ class SemesterProcessPolicy < ApplicationPolicy
   alias_method :edit?,    :superadmin?
   alias_method :create?,  :superadmin?
   alias_method :update?,  :superadmin?
-  alias_method :destroy?, :superadmin?
 end

app/services/semester.rb

 class Semester
+  attr_reader :context
+
+  MONTH_NUMBER_MAP = {
+    12 => 1,
+    1 => 1,
+    2 => 1,
+    3 => 1,
+    4 => 1,
+    5 => 1,
+    6 => 2,
+    7 => 2,
+    8 => 2,
+    9 => 2,
+    10 => 2,
+    11 => 2
+  }.freeze
+
+  class << self
+    def parse(string)
+      return unless string
+
+      year, number = string.split(',').map(&:to_i)
+      year -= 1 if number == 1
+      start_date = Time.zone.local(year, (18 - number * 6), 1)
+      semester_range_from_start(start_date)
+    end
+
+    def to_s(date = nil)
+      date = to_process_date(date)
+      "#{year(date)},#{number(date)}"
+    end
+
+    def number(date = nil)
+      date = to_process_date(date)
+      MONTH_NUMBER_MAP[date.month]
+    end
+
+    def year(date = nil)
+      date = to_process_date(date)
+      date.month == 12 ? date.year + 1 : date.year
+    end
+
+    def to_process_date(date)
+      if date.blank?
+        Time.zone.now
+      elsif date.is_a?(Range)
+        date.end
+      else
+        date
+      end
+    end
+
+    def i18n_t(semester, short: true)
+      if short
+        I18n.t(:semester_short, number: number(semester), year: year(semester))
+      else
+        I18n.t(:semester_long, number: number(semester), year: year(semester), begin: I18n.l(semester.begin.to_date),
+          end: I18n.l(semester.end.to_date))
+      end
+    end
+
+    def semester_start_time(date_time)
+      date_time = date_time.to_date
+      if (6..11).cover?(date_time.month)
+        Time.zone.local(date_time.year, 6, 1)
+      elsif date_time.month == 12
+        Time.zone.local(date_time.year, 12, 1)
+      else
+        Time.zone.local(date_time.year - 1, 12, 1)
+      end
+    end
+
+    def semester_range_from_start(date_time)
+      date_time = date_time.to_date
+      date_time..date_time.advance(months: 5).end_of_month
+    end
+  end
+
   # params:
-  #   context - default is now, but optionally any other point in time
-  def initialize(year = nil, semester_number = nil)
-    @context = if year && semester_number == 2
-                 Time.zone.local(year, 6, 1)
-               elsif year && semester_number == 1
-                 Time.zone.local(year - 1, 12, 1)
-               else
+  #   year - integer of year
+  #   semester_number - required if year integer
+  def initialize(year = nil, number = 1)
+    @context = if year.nil?
                  Time.zone.now
+               else
+                 year -= 1 if number == 1
+                 Time.zone.local(year, (18 - number * 6), 1)
                end
   end
 
-  def number(date_time = nil)
-    if (6..11).cover?(date_time&.month || @context.month)
-      2
-    else
-      1
-    end
+  def year(date = nil)
+    Semester.year(date || @context)
   end
 
-  def year_number(count: 1, direction: :previous, set_semester: nil)
-    string_semester = set_semester || public_send(direction, count)
-    "#{string_semester.end.year},#{number(string_semester.end)}"
+  def to_s(date = nil)
+    Semester.to_s(date || @context)
+  end
+
+  def number(date = nil)
+    Semester.number(date || @context)
   end
 
   # running semester where in now
   def current
-    @current ||= semester_range_from_start(semester_start_time(@context))
+    @current ||= Semester.semester_range_from_start(
+      Semester.semester_start_time(@context)
+    )
   end
 
   # params:
@@ -34,24 +113,32 @@ class Semester
   def previous(count = 1)
     return current if count == 0
 
-    current.begin.advance(months: -(6 * count))..current.end.advance(months: -(6 * count))
+    Semester.semester_range_from_start(current.begin.advance(months: -6 * count))
+  end
+
+  def previous_s(count = 1)
+    to_s(previous(count))
   end
 
   # params:
   #   count - shift of semesters - only positive integers
-  def next(count = 1)
+  def next_semester(count = 1)
     return current if count == 0
 
-    current.begin.advance(months: 6 * count)..current.end.advance(months: 6 * count)
+    Semester.semester_range_from_start(current.begin.advance(months: 6 * count))
+  end
+
+  def next_s(count = 1)
+    to_s(next_semester(count))
   end
 
   # list of semester datetime ranges
   #
   # params:
   #   count - amount of semesters
-  #   direction - [:next || :previous]
+  #   direction - [:next_semester || :previous]
   #   with_current - include current semester as first in array
-  def list(count = 3, direction = :previous, with_current: true)
+  def list(count = 3, direction: :previous, with_current: true)
     list = (1..count).to_a.map do |step|
       public_send(direction, step)
     end
@@ -62,30 +149,9 @@ class Semester
     end
   end
 
-  def collection(count = 3, direction = :previous, with_current: true)
-    list(count, direction, with_current: with_current).map do |semester|
-      semester_number = number(semester.end)
-      semester_year = semester.end.year
-      [
-        "#{semester_number}. Semester #{semester_year} (#{I18n.l(semester.begin.to_date)} - #{I18n.l(semester.end.to_date)})",
-        year_number(set_semester: semester)
-      ]
+  def collection(count = 3, direction: :previous, with_current: true)
+    list(count, direction: direction, with_current: with_current).map do |semester|
+      [Semester.i18n_t(semester, short: false), to_s(semester)]
     end
   end
-
-  private
-
-  def semester_start_time(date_time)
-    if (6..11).cover?(date_time.month)
-      Time.zone.local(date_time.year, 6, 1)
-    elsif date_time.month == 12
-      Time.zone.local(date_time.year, 12, 1)
-    else
-      Time.zone.local(date_time.year - 1, 12, 1)
-    end
-  end
-
-  def semester_range_from_start(date_time)
-    date_time..date_time.advance(months: 5).end_of_month
-  end
 end

app/views/application/_navigation.html.slim

@@ -22,7 +22,7 @@ nav.navbar.navbar-top.hidden-print
             - if policy(GroupOffer).index?
               li= link_to 'Gruppenangebote', group_offers_path
             - if policy(SemesterProcess).index?
-              li= link_to 'Semester Prozesse', semester_processes_path
+              li= link_to 'Semester Prozesse', semester_process_volunteers_path
             - if policy(ReminderMailing).index?
               li = link_to 'Erinnerungs-Emails', reminder_mailings_path
             - if policy(Event).index?

app/views/semester_process_volunteers/_index_nav.html.slim

+nav.navbar.section-navigation
+  hr
+  ul.list-inline
+    li= button_link 'Neuen Semester Prozess erstellen', new_semester_process_path, dimension: 'sm'
+  hr

app/views/semester_process_volunteers/edit.html.slim

 h1= t_title(:edit)
 
-= simple_form_for(@semester_process_volunteer) do |f|
+= simple_form_for(@spv) do |f|
 
-
-= form_navigation_btn :show
+.row
+  .col-xs-12
+    = button_link(t_action(:show), @spv)

app/views/semester_process_volunteers/index.html.slim

 h1= t_title(:index)
+
+= render 'index_nav'
+
+= bootstrap_paginate(@spvs)
+
+.table-responsive
+  table.table.table-striped
+    thead
+      tr
+        th.hidden-print Aktionen
+        th= t_attr(:creator)
+        th= t_attr(:semester)
+        th= t_attr(:created_at)
+
+    tbody
+      - @spvs.each do |spv|
+        tr
+          td.index-action-cell.hidden-print
+            = link_to t_action(:show), spv
+            = link_to t_action(:edit), edit_semester_process_volunteer_path(spv)
+          td= spv.creator
+          td= spv.semester_t
+          td= l(spv.created_at)
+
+= bootstrap_paginate(@spvs)
+
+= render 'index_nav'

app/views/semester_process_volunteers/show.html.slim

-h1= t_title(:show)
+h1= t('.title', semester: @spv.semester_t, name: @spv.volunteer.contact.natural_name)
 
 = form_navigation_btn :edit

config/locales/de.yml

@@ -185,6 +185,11 @@ de:
     social_worker: Sozialarbeiter/in
     superadmin: Superadmin
     volunteer: Freiwillige/r
+  semester_long: '%{number}. Semester %{year} (%{begin} - %{end})'
+  semester_short: '%{number}. Semester %{year}'
+  semester_process_volunteers:
+    show:
+      title: 'Semester Prozess %{semester} von %{name}'
   salutation: &id-salutations
     all: Alle
     family: Familie

config/routes.rb

@@ -96,9 +96,8 @@ Rails.application.routes.draw do
     get :send_half_year, on: :member
   end
 
-  resources :semester_processes do
-    resources :semester_process_volunteers, only: [:index, :show, :update, :edit]
-  end
+  resources :semester_process_volunteers
+  resources :semester_processes, except: [:destroy]
 
   resources :volunteer_applications, only: [:new, :create] do
     get :thanks, on: :collection

test/helpers/semester_service_test.rb

+require 'test_helper'
+
+class SemesterServiceTest < ActionView::TestCase
+  test '#initialize' do
+    subject = Semester.new
+    assert_equal Time.zone.now.to_date, subject.context.to_date
+
+    subject = Semester.new(2018, 2)
+    assert_equal time_z(2018, 6, 1), subject.context
+
+    subject = Semester.new(2018, 1)
+    assert_equal time_z(2017, 12, 1), subject.context
+  end
+
+  test '#to_s' do
+    travel_to time_z(2018, 8, 10)
+    subject = Semester.new
+    assert_equal '2018,2', subject.to_s
+
+    travel_to time_z(2017, 12, 10)
+    subject = Semester.new
+    assert_equal '2018,1', subject.to_s
+
+    travel_to time_z(2018, 2, 10)
+    subject = Semester.new
+    assert_equal '2018,1', subject.to_s
+  end
+
+  test '#number' do
+    travel_to time_z(2018, 8, 10)
+    subject = Semester.new
+    assert_equal 2, subject.number
+    travel_to time_z(2018, 4, 10)
+    subject = Semester.new
+    assert_equal 1, subject.number
+    travel_to time_z(2017, 12, 10)
+    subject = Semester.new
+    assert_equal 1, subject.number
+
+    assert_equal 1, subject.number(time_z(2010, 12, 23))
+    assert_equal 1, subject.number(time_z(2010, 1, 23))
+    assert_equal 2, subject.number(time_z(2010, 7, 30))
+
+    assert_equal 1, subject.number(time_z(2010, 12, 1).to_date..time_z(2010, 5, 1).end_of_month.to_date)
+    assert_equal 2, subject.number(time_z(2010, 6, 1).to_date..time_z(2010, 11, 1).end_of_month.to_date)
+  end
+
+  test '#current' do
+    travel_to time_z(2018, 8, 10)
+    subject = Semester.new
+    assert_equal time_z(2018, 6, 1).to_date..time_z(2018, 11, 1).end_of_month.to_date, subject.current
+
+    subject = Semester.new(2017, 2)
+    assert_equal time_z(2017, 6, 1).to_date..time_z(2017, 11, 1).end_of_month.to_date, subject.current
+
+    subject = Semester.new(2017, 1)
+    assert_equal time_z(2016, 12, 1).to_date..time_z(2017, 5, 1).end_of_month.to_date, subject.current
+  end
+
+  test '#previous' do
+    travel_to time_z(2018, 8, 10)
+
+    subject = Semester.new
+    assert_equal time_z(2017, 12, 1).to_date..time_z(2018, 5, 1).end_of_month.to_date, subject.previous
+    assert_equal time_z(2017, 6, 1).to_date..time_z(2017, 11, 1).end_of_month.to_date, subject.previous(2)
+    assert_equal time_z(2016, 12, 1).to_date..time_z(2017, 5, 1).end_of_month.to_date, subject.previous(3)
+  end
+
+  test '#next_semester' do
+    travel_to time_z(2018, 8, 10)
+
+    subject = Semester.new
+    assert_equal time_z(2018, 12, 1).to_date..time_z(2019, 5, 1).end_of_month.to_date, subject.next_semester
+    assert_equal time_z(2019, 6, 1).to_date..time_z(2019, 11, 1).end_of_month.to_date, subject.next_semester(2)
+    assert_equal time_z(2019, 12, 1).to_date..time_z(2020, 5, 1).end_of_month.to_date, subject.next_semester(3)
+  end
+
+  test '#previous_s' do
+    travel_to time_z(2018, 8, 10)
+
+    subject = Semester.new
+    assert_equal '2018,1', subject.previous_s
+    assert_equal '2017,2', subject.previous_s(2)
+    assert_equal '2017,1', subject.previous_s(3)
+  end
+
+  test '#next_s' do
+    travel_to time_z(2018, 8, 10)
+
+    subject = Semester.new
+    assert_equal '2019,1', subject.next_s
+    assert_equal '2019,2', subject.next_s(2)
+    assert_equal '2020,1', subject.next_s(3)
+  end
+
+  test '#list' do
+    travel_to time_z(2018, 8, 10)
+
+    subject = Semester.new
+    assert_equal [
+      time_z(2018, 6, 1).to_date..time_z(2018, 11, 1).end_of_month.to_date,
+      time_z(2017, 12, 1).to_date..time_z(2018, 5, 1).end_of_month.to_date,
+      time_z(2017, 6, 1).to_date..time_z(2017, 11, 1).end_of_month.to_date,
+      time_z(2016, 12, 1).to_date..time_z(2017, 5, 1).end_of_month.to_date
+    ], subject.list
+
+    assert_equal [
+      time_z(2018, 6, 1).to_date..time_z(2018, 11, 1).end_of_month.to_date,
+      time_z(2017, 12, 1).to_date..time_z(2018, 5, 1).end_of_month.to_date,
+      time_z(2017, 6, 1).to_date..time_z(2017, 11, 1).end_of_month.to_date
+    ], subject.list(2)
+
+    assert_equal [
+      time_z(2017, 12, 1).to_date..time_z(2018, 5, 1).end_of_month.to_date,
+      time_z(2017, 6, 1).to_date..time_z(2017, 11, 1).end_of_month.to_date,
+      time_z(2016, 12, 1).to_date..time_z(2017, 5, 1).end_of_month.to_date
+    ], subject.list(with_current: false)
+
+    assert_equal [
+      time_z(2018, 6, 1).to_date..time_z(2018, 11, 1).end_of_month.to_date,
+      time_z(2018, 12, 1).to_date..time_z(2019, 5, 1).end_of_month.to_date,
+      time_z(2019, 6, 1).to_date..time_z(2019, 11, 1).end_of_month.to_date,
+      time_z(2019, 12, 1).to_date..time_z(2020, 5, 1).end_of_month.to_date
+    ], subject.list(direction: :next_semester)
+  end
+
+  test '#collection' do
+    travel_to time_z(2018, 8, 10)
+