From 95c68bf9a2753846d6140b9db570a6566455c92a Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Thu, 28 Feb 2019 17:25:37 +0100
Subject: [PATCH 01/13] fix: ensure better that semester process volunteer
 journal entries are created with the correct user and such

---
 app/controllers/journals_controller.rb        | 71 +++++++++++++------
 app/views/journals/_form.html.slim            |  2 +-
 .../index.html.slim                           |  6 +-
 config/routes.rb                              |  2 +
 4 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb
index 96b66296c..617438cad 100644
--- a/app/controllers/journals_controller.rb
+++ b/app/controllers/journals_controller.rb
@@ -1,6 +1,8 @@
 class JournalsController < ApplicationController
+  before_action :set_semester_process_volunteer
   before_action :set_journaled
   before_action :set_journal, only: [:edit, :update, :destroy]
+  before_action :set_form_url, only: [:edit, :new]
 
   def index
     authorize Journal
@@ -10,7 +12,7 @@ class JournalsController < ApplicationController
   end
 
   def new
-    @journal = Journal.new
+    @journal = Journal.new(user: current_user)
     handle_semester_feedback_quote
     authorize @journal
   end
@@ -23,7 +25,7 @@ class JournalsController < ApplicationController
     @journal.journalable = @journaled
     authorize @journal
     if @journal.save
-      redirect_to [@journaled, Journal], make_notice
+      redirect_to redirect_back_path, make_notice
     else
       render :new
     end
@@ -31,7 +33,7 @@ class JournalsController < ApplicationController
 
   def update
     if @journal.update(journal_params)
-      redirect_to [@journaled, Journal], make_notice
+      redirect_to redirect_back_path, make_notice
     else
       render :edit
     end
@@ -39,43 +41,72 @@ class JournalsController < ApplicationController
 
   def destroy
     @journal.destroy
-    redirect_to [@journaled, Journal], make_notice
+    redirect_to redirect_back_path, make_notice
   end
 
   private
 
+  def set_form_url
+    @form_url = if @semester_process_volunteer
+      polymorphic_path([@semester_process_volunteer, :journals])
+    else
+      polymorphic_path([@journaled, :journals])
+    end
+  end
+
+  def redirect_back_path
+    if @semester_process_volunteer
+      semester_process_volunteers_path
+    else
+      [@journaled, Journal]
+    end
+  end
+
   def set_journal
     @journal = Journal.find_by(id: params[:id])
     if @journal
       authorize @journal
     else
-      redirect_to @journaled, notice: t('crud.c_action.destroy', model: t_model)
+      redirect_to @semester_process_volunteer || @journaled, notice: t('crud.c_action.destroy', model: t_model)
     end
   end
 
   def set_journaled
-    return @journaled = Client.find(params[:client_id]) if params[:client_id]
-    @journaled = Volunteer.find(params[:volunteer_id])
+    @journaled = if @semester_process_volunteer
+      @semester_process_volunteer.volunteer
+    elsif params[:client_id]
+      Client.find(params[:client_id])
+    elsif params[:volunteer_id]
+      Volunteer.find(params[:volunteer_id])
+    end
+  end
+
+  def set_semester_process_volunteer
+    return unless params[:semester_process_volunteer_id]
+
+    @semester_process_volunteer = SemesterProcessVolunteer.find(params[:semester_process_volunteer_id])
   end
 
   def handle_semester_feedback_quote
-    return unless params[:sp_volunteer_id]
-
-    @journal.category = :feedback
-    @semester_process_volunteer = SemesterProcessVolunteer.find(params[:sp_volunteer_id])
-    @journal.journalable = @semester_process_volunteer.volunteer
-    @journal.title = "Semester Prozess Feedback vom #{I18n.l(@semester_process_volunteer.created_at.to_date)}: "
-    @semester_feedback = @semester_process_volunteer.semester_feedbacks&.first
-    return unless @semester_feedback
-
-    @journal.body = @semester_feedback.slice(:goals, :achievements, :future, :comments).map do |key, sfb_quote|
-      "#{I18n.t("activerecord.attributes.feedback.#{key}")}:\n«#{sfb_quote}»" if sfb_quote.present?
-    end.compact.join("\n\n")
+    return unless @semester_process_volunteer
+
+    @journal.assign_attributes(category: :feedback, journalable: @journaled,
+      title: "Semester Prozess Feedback vom #{I18n.l(@semester_process_volunteer.created_at.to_date)}: ",
+      body: create_body(@semester_process_volunteer.semester_feedbacks&.first))
+  end
+
+  def create_body(semester_feedback = nil)
+    return unless semester_feedback
+
+    semester_feedback.slice(:goals, :achievements, :future, :comments)
+      .map do |key, sfb_quote|
+        "#{I18n.t("activerecord.attributes.feedback.#{key}")}:\n«#{sfb_quote}»" if sfb_quote.present?
+      end.compact.join("\n\n")
   end
 
   def journal_params
     params.require(:journal).permit(
-      :category, :user_id, :body, :title, :client_id, :volunteer_id
+      :category, :user_id, :body, :title, :client_id, :volunteer_id, :semester_process_volunteer_id
     )
   end
 end
diff --git a/app/views/journals/_form.html.slim b/app/views/journals/_form.html.slim
index f99019720..b21af6503 100644
--- a/app/views/journals/_form.html.slim
+++ b/app/views/journals/_form.html.slim
@@ -1,4 +1,4 @@
-= simple_form_for [@journaled, @journal] do |f|
+= simple_form_for [@journaled, @journal], url: @form_url do |f|
   = simple_error_notice f
   h1= @journaled.contact.full_name
   .row
diff --git a/app/views/semester_process_volunteers/index.html.slim b/app/views/semester_process_volunteers/index.html.slim
index 9a0cb6159..6d0e92e45 100644
--- a/app/views/semester_process_volunteers/index.html.slim
+++ b/app/views/semester_process_volunteers/index.html.slim
@@ -26,9 +26,9 @@ h1 Semester Prozess
         tr*{ data: { spv_id: spv.id } }
           td.index-action-cell.hidden-print
             = button_link icon_span(:edit), review_semester_review_semester_url(spv), title: 'Bearbeiten'
-            = button_link icon_span(:journal), volunteer_journals_path(spv.volunteer), title: 'Journal'
-            = button_link icon_span(:journal_new), new_volunteer_journal_path(spv.volunteer, sp_volunteer_id: spv.id),
-              title: 'Neuen Journal eintrag mit Zitat erstellen'
+            = button_link icon_span(:journal), polymorphic_path([spv, Journal]), title: 'Journal'
+            = button_link icon_span(:journal_new), new_polymorphic_path([spv, Journal]),
+                title: 'Neuen Journal eintrag mit Zitat erstellen'
             = button_link icon_span(:hours), volunteer_hours_path(spv.volunteer), title: 'Stunden'
           td
             - if spv.commited_at.present?
diff --git a/config/routes.rb b/config/routes.rb
index 188a7dd60..849fc8c22 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -100,6 +100,8 @@ Rails.application.routes.draw do
     put :take_responsibility, on: :member
     put :mark_as_done, on: :member
     put :update_notes, on: :member
+
+    resources :journals
   end
 
   resources :review_semesters do
-- 
GitLab


From 62be8a3bbd2936a0df668b26483acf016eabc0fc Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Thu, 28 Feb 2019 17:41:59 +0100
Subject: [PATCH 02/13] fix test that was still using the old path

---
 test/controllers/journals_controller_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/controllers/journals_controller_test.rb b/test/controllers/journals_controller_test.rb
index 8ffa27be0..974ce8f3b 100644
--- a/test/controllers/journals_controller_test.rb
+++ b/test/controllers/journals_controller_test.rb
@@ -13,7 +13,7 @@ class JournalsControllerTest < ActionDispatch::IntegrationTest
     @spv = create(:semester_process_volunteer, volunteer: @volunteer)
     @semester_feedback = create(:semester_feedback, :no_mission, semester_process_volunteer: @spv, volunteer: @volunteer)
 
-    get new_volunteer_journal_path(@volunteer, sp_volunteer_id: @spv.id)
+    get new_polymorphic_path([@spv, Journal])
     @journal = @controller.instance_variable_get(:@journal)
     assert_equal @journal.category, 'feedback'
     assert_equal @journal.title, "Semester Prozess Feedback vom #{I18n.l(@semester_feedback.created_at.to_date)}: "
-- 
GitLab


From 1ec2bd0e9a61addd7cbd2d7225e9877b7bfda5ed Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 15:47:10 +0100
Subject: [PATCH 03/13] fix journal factory assigning volunteers user instead
 of admin

---
 test/factories/users.rb      | 5 +++++
 test/system/journals_test.rb | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/test/factories/users.rb b/test/factories/users.rb
index 834f32210..e6335ea3f 100644
--- a/test/factories/users.rb
+++ b/test/factories/users.rb
@@ -18,6 +18,10 @@ FactoryBot.define do
       role User::VOLUNTEER
     end
 
+    trait :admin do
+      role User::SUPERADMIN
+    end
+
     trait :department_manager do
       role User::DEPARTMENT_MANAGER
     end
@@ -38,6 +42,7 @@ FactoryBot.define do
       profile {}
     end
 
+    factory :admin, traits: [:admin]
     factory :social_worker, traits: [:social_worker]
     factory :department_manager, traits: [:department_manager, :with_department]
     factory :department_manager_without_department, traits: [:department_manager]
diff --git a/test/system/journals_test.rb b/test/system/journals_test.rb
index b09de0124..dbc02a9e2 100644
--- a/test/system/journals_test.rb
+++ b/test/system/journals_test.rb
@@ -5,7 +5,7 @@ class JournalsTest < ApplicationSystemTestCase
     @superadmin = create :user
     @department_manager = create :user, :department_manager
     @volunteer = create :volunteer, registrar: @department_manager, acceptance: :accepted
-    @volunteer.journals = [FactoryBot.create(:journal, user: @volunteer.user)]
+    @volunteer.journals = [FactoryBot.create(:journal, user: @superadmin)]
     @journal_volunteer = @volunteer.journals.reload.first
   end
 
-- 
GitLab


From 756eeaaab7afc62b89a59c137ac526ab1ea1d77f Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 15:47:54 +0100
Subject: [PATCH 04/13] fix: remove dead routes in review_semesters resources

---
 config/routes.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/routes.rb b/config/routes.rb
index 849fc8c22..30f1dc5d5 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -104,7 +104,7 @@ Rails.application.routes.draw do
     resources :journals
   end
 
-  resources :review_semesters do
+  resources :review_semesters, only: [] do
     get :review_semester, on: :member
     patch :submit_review, on: :member
   end
-- 
GitLab


From b27c178a24686a677eae708e9436ddd2f5ca047d Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 15:49:00 +0100
Subject: [PATCH 05/13] remove obsolete render_mission helper, as slim does
 this just fine

---
 app/views/semester_process_volunteers/index.html.slim | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/app/views/semester_process_volunteers/index.html.slim b/app/views/semester_process_volunteers/index.html.slim
index 6d0e92e45..08af49792 100644
--- a/app/views/semester_process_volunteers/index.html.slim
+++ b/app/views/semester_process_volunteers/index.html.slim
@@ -36,7 +36,9 @@ h1 Semester Prozess
             - else
               .label.label-warning Unbestätigt
           td= link_to_if policy(Volunteer).edit?, spv.volunteer.contact.full_name, edit_volunteer_path(spv.volunteer)
-          td= render_missions(spv)
+          td.index-action-cell
+            - spv.semester_process_volunteer_missions.each do |spvm|
+              = link_to spvm.mission.to_label, spvm.mission, target: '_blank'
           = td_truncate_content_modal(spv.render_feedback(:goals), 'Ziele', shorten_size: 300)
           = td_truncate_content_modal(spv.render_feedback(:achievements), 'Erfolge', shorten_size: 300)
           = td_truncate_content_modal(spv.render_feedback(:future), 'Zukunft', shorten_size: 300)
-- 
GitLab


From bf5c202d28ca5423a33a307fe6dfe6b03bf36136 Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 15:52:02 +0100
Subject: [PATCH 06/13] rename @semester_process_volunteer to @sp_volunteer

---
 app/controllers/journals_controller.rb         | 16 ++++++++--------
 app/controllers/review_semesters_controller.rb |  7 +++----
 app/models/journal.rb                          |  1 -
 app/models/semester_process_volunteer.rb       | 10 ++++++++++
 .../review_semesters/review_semester.html.slim | 18 +++++++++---------
 5 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb
index 617438cad..d349644b8 100644
--- a/app/controllers/journals_controller.rb
+++ b/app/controllers/journals_controller.rb
@@ -47,15 +47,15 @@ class JournalsController < ApplicationController
   private
 
   def set_form_url
-    @form_url = if @semester_process_volunteer
-      polymorphic_path([@semester_process_volunteer, :journals])
+    @form_url = if @sp_volunteer
+      polymorphic_path([@sp_volunteer, :journals])
     else
       polymorphic_path([@journaled, :journals])
     end
   end
 
   def redirect_back_path
-    if @semester_process_volunteer
+    if @sp_volunteer
       semester_process_volunteers_path
     else
       [@journaled, Journal]
@@ -67,13 +67,13 @@ class JournalsController < ApplicationController
     if @journal
       authorize @journal
     else
-      redirect_to @semester_process_volunteer || @journaled, notice: t('crud.c_action.destroy', model: t_model)
+      redirect_to @sp_volunteer || @journaled, notice: t('crud.c_action.destroy', model: t_model)
     end
   end
 
   def set_journaled
-    @journaled = if @semester_process_volunteer
-      @semester_process_volunteer.volunteer
+    @journaled = if @sp_volunteer
+      @sp_volunteer.volunteer
     elsif params[:client_id]
       Client.find(params[:client_id])
     elsif params[:volunteer_id]
@@ -84,11 +84,11 @@ class JournalsController < ApplicationController
   def set_semester_process_volunteer
     return unless params[:semester_process_volunteer_id]
 
-    @semester_process_volunteer = SemesterProcessVolunteer.find(params[:semester_process_volunteer_id])
+    @sp_volunteer = SemesterProcessVolunteer.find(params[:semester_process_volunteer_id])
   end
 
   def handle_semester_feedback_quote
-    return unless @semester_process_volunteer
+    return unless @sp_volunteer
 
     @journal.assign_attributes(category: :feedback, journalable: @journaled,
       title: "Semester Prozess Feedback vom #{I18n.l(@semester_process_volunteer.created_at.to_date)}: ",
diff --git a/app/controllers/review_semesters_controller.rb b/app/controllers/review_semesters_controller.rb
index c3c817c73..c2c4a4fdd 100644
--- a/app/controllers/review_semesters_controller.rb
+++ b/app/controllers/review_semesters_controller.rb
@@ -9,13 +9,12 @@ class ReviewSemestersController < ApplicationController
     # you shall not pass
     return if @semester_process_volunteer.commited_at
 
-    set_reviewed
-    assign_volunteer_attributes
+    @sp_volunteer.set_comitted(current_user)
+    @sp_volunteer.volunteer.assign_attributes(review_params[:volunteer_attributes].slice(:waive, :bank, :iban))
     build_nested_objects
 
     if save_feedback_data!
-      create_journals
-      redirect_to review_semester_review_semester_url(@semester_process_volunteer), notice: t('.success')
+      redirect_to review_semester_review_semester_url(@sp_volunteer), notice: t('.success')
     else
       render :review_semester
     end
diff --git a/app/models/journal.rb b/app/models/journal.rb
index ceeeb3f45..2a62ba850 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -3,7 +3,6 @@ class Journal < ApplicationRecord
 
   belongs_to :user, -> { with_deleted }, inverse_of: 'journals'
   belongs_to :assignment, optional: true
-
   belongs_to :journalable, polymorphic: true, required: false
 
   CATEGORIES = [
diff --git a/app/models/semester_process_volunteer.rb b/app/models/semester_process_volunteer.rb
index 338e68814..f3fa6a583 100644
--- a/app/models/semester_process_volunteer.rb
+++ b/app/models/semester_process_volunteer.rb
@@ -112,4 +112,14 @@ class SemesterProcessVolunteer < ApplicationRecord
     self.responsibility_taken_at = Time.zone.now
     super(responsible_user)
   end
+
+  def set_comitted(user)
+    self.commited_by = user
+    self.commited_at = Time.zone.now
+  end
+
+  def reset_comitted
+    self.commited_by = nil
+    self.commited_at = nil
+  end
 end
diff --git a/app/views/review_semesters/review_semester.html.slim b/app/views/review_semesters/review_semester.html.slim
index 7e8b843b5..5c12831c9 100644
--- a/app/views/review_semesters/review_semester.html.slim
+++ b/app/views/review_semesters/review_semester.html.slim
@@ -1,14 +1,14 @@
-- disabled = @semester_process_volunteer.commited_at.present?
-= simple_form_for(@semester_process_volunteer, html: { class: 'col-xs-12 '},
-  url: submit_review_review_semester_path(@semester_process_volunteer)) do |f|
+- disabled = @sp_volunteer.commited_at.present?
+= simple_form_for(@sp_volunteer, html: { class: 'col-xs-12 '},
+  url: submit_review_review_semester_path(@sp_volunteer)) do |f|
   = simple_error_notice f
-  h1 Halbjahres-Rapporte #{@semester_process_volunteer.semester_t} für #{@semester_process_volunteer.volunteer.contact.full_name}
+  h1 Halbjahres-Rapporte #{@sp_volunteer.semester_t} für #{@sp_volunteer.volunteer.contact.full_name}
   - if disabled
     .submit-box
       .col-xs-12
-        p.text-danger Bestätigt am #{l(@semester_process_volunteer.commited_at.to_date)} durch #{profile_link(@semester_process_volunteer.commited_by)}
+        p.text-danger Bestätigt am #{l(@sp_volunteer.commited_at.to_date)} durch #{profile_link(@sp_volunteer.commited_by)}
 
-  = f.simple_fields_for :semester_feedbacks, @semester_process_volunteer.semester_process_volunteer_missions.need_feedback do |spvm|
+  = f.simple_fields_for :semester_feedbacks, @sp_volunteer.semester_process_volunteer_missions.need_feedback do |spvm|
     - mission = spvm.object.mission
     h2.m-b-20.m-t-30 Einsatz #{mission.to_label}
 
@@ -18,13 +18,13 @@
         .row
           .col-xs-12= ff.input field, disabled: disabled
 
-    - if mission.hours.within_semester(@semester_process_volunteer.semester).any?
+    - if mission.hours.within_semester(@sp_volunteer.semester).any?
         h3 Stunden
-        = render 'hours/last_submitted_hours', hours: mission.hours.within_semester(@semester_process_volunteer.semester)
+        = render 'hours/last_submitted_hours', hours: mission.hours.within_semester(@sp_volunteer.semester)
 
     = spvm.simple_fields_for :hour, Hour.new do |hf|
       = hf.input :spv_mission_id, as: :hidden, input_html: { value: spvm.object.id}
-      h4.m-b-20.m-t-20= "Restliche Stunden in diesem Semester (#{@semester_process_volunteer.semester_period})"
+      h4.m-b-20.m-t-20= "Restliche Stunden in diesem Semester (#{@sp_volunteer.semester_period})"
       .row
         .col-xs-2= hf.input :hours, label: 'Stunden',
                                     input_html: { value: f.object.hours == 0 ? '' : f.object.hours, min: 0},
-- 
GitLab


From eb17e64dbb0e6b6c81eb230f2215c17dc7ee31e8 Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 15:54:36 +0100
Subject: [PATCH 07/13] remove ReviewSemesterHelper and
 SemesterProcessVolunteerHelper

- their methods where all only used in one place in one controller, so they basically
  only made the code less readable and unnecessary scattered over the code base
---
 .../review_semesters_controller.rb            | 72 +++++++++++++++--
 .../semester_process_volunteers_controller.rb | 35 ++++++++-
 app/helpers/review_semester_helper.rb         | 78 -------------------
 .../semester_process_volunteer_helper.rb      | 61 ---------------
 app/models/semester_process_volunteer.rb      |  5 +-
 5 files changed, 102 insertions(+), 149 deletions(-)
 delete mode 100644 app/helpers/review_semester_helper.rb
 delete mode 100644 app/helpers/semester_process_volunteer_helper.rb

diff --git a/app/controllers/review_semesters_controller.rb b/app/controllers/review_semesters_controller.rb
index c2c4a4fdd..db2c69a23 100644
--- a/app/controllers/review_semesters_controller.rb
+++ b/app/controllers/review_semesters_controller.rb
@@ -1,13 +1,12 @@
 class ReviewSemestersController < ApplicationController
-  before_action :prepare_review, :initialize_nested_objects, only: [:review_semester, :submit_review]
-
-  include ReviewSemesterHelper, SemesterProcessVolunteerHelper
+  before_action :set_sp_volunteer, :initialize_nested_objects,
+    only: [:review_semester, :submit_review]
 
   def review_semester; end
 
   def submit_review
     # you shall not pass
-    return if @semester_process_volunteer.commited_at
+    return if @sp_volunteer.commited_at
 
     @sp_volunteer.set_comitted(current_user)
     @sp_volunteer.volunteer.assign_attributes(review_params[:volunteer_attributes].slice(:waive, :bank, :iban))
@@ -22,11 +21,70 @@ class ReviewSemestersController < ApplicationController
 
   private
 
+  def build_nested_objects
+    review_params[:semester_feedbacks_attributes].each do |_key, hash|
+      spv_mission = SemesterProcessVolunteerMission.find(hash[:semester_feedback][:spv_mission_id])
+      @nested_objects[spv_mission.id.to_s][:feedback] = SemesterFeedback.new(
+        hash[:semester_feedback].merge(author: current_user, semester_process_volunteer: @sp_volunteer)
+      )
+      next unless hash[:hour][:hours].to_i.positive?
+
+      @nested_objects[spv_mission.id.to_s][:hours] = Hour.new(
+        hash[:hour].merge(volunteer: spv_mission.volunteer,
+          meeting_date: spv_mission.semester_process_volunteer.semester.last.to_date,
+          hourable: spv_mission.mission.group_assignment? ? spv_mission.mission.group_offer : spv_mission.mission)
+      )
+    end
+  end
+
+  def save_feedback_data!
+    @sp_volunteer.volunteer.validate_waive_and_bank = true
+
+    ActiveRecord::Base.transaction do
+      @sp_volunteer.save!
+      @volunteer.save!
+
+      @nested_objects.values.each do |missions|
+        missions.values.map(&:save!)
+      end
+    end
+    # reload the variable
+    @sp_volunteer.reload
+    true
+  rescue ActiveRecord::RecordInvalid => exception
+    @sp_volunteer.reset_comitted
+    flash[:alert] = exception.message
+    false
+  end
+
+  def set_sp_volunteer
+    @sp_volunteer = SemesterProcessVolunteer.find(params[:id])
+    authorize @sp_volunteer
+    @volunteer = @sp_volunteer.volunteer
+    @missions = @sp_volunteer.missions
+  end
+
+  def initialize_nested_objects
+    @nested_objects = @sp_volunteer.semester_process_volunteer_missions.map do |spvm|
+      [spvm.id.to_s, { feedback: @sp_volunteer.semester_feedback_with_mission(spvm.mission) || SemesterFeedback.new }]
+    end.to_h
+  end
+
   def review_params
     params.require(:semester_process_volunteer).permit(
       volunteer_attributes: [:id, :waive, :iban, :bank],
-      semester_feedbacks_attributes: [[semester_feedback: [:mission, :goals, :achievements, :future, :comments, :conversation, :spv_mission_id]],
-                                     [hour: [:hours, :spv_mission_id, :activity]]]
+      semester_feedbacks_attributes: [
+        [
+          semester_feedback: [
+            :mission, :goals, :achievements, :future, :comments, :conversation, :spv_mission_id
+          ]
+        ],
+        [
+          hour: [
+            :hours, :spv_mission_id, :activity
+          ]
+        ]
+      ]
     )
   end
-end
\ No newline at end of file
+end
diff --git a/app/controllers/semester_process_volunteers_controller.rb b/app/controllers/semester_process_volunteers_controller.rb
index b700b3418..b3c076a91 100644
--- a/app/controllers/semester_process_volunteers_controller.rb
+++ b/app/controllers/semester_process_volunteers_controller.rb
@@ -2,8 +2,6 @@ class SemesterProcessVolunteersController < ApplicationController
   before_action :set_semester_process_volunteer, only: [:show, :edit, :update, :take_responsibility, :mark_as_done, :update_notes]
   before_action :set_semester, only: [:index]
 
-  include SemesterProcessVolunteerHelper
-
   def review_semester
     authorize SemesterProcessVolunteer.find(params[:id])
     redirect_to review_semester_review_semester_url params[:id]
@@ -80,6 +78,39 @@ class SemesterProcessVolunteersController < ApplicationController
     end
   end
 
+  def set_semester_process_volunteer
+    @spv = SemesterProcessVolunteer.find(params[:id])
+    authorize @spv
+    @semester_process = @spv.semester_process
+    @volunteer = @spv.volunteer
+  end
+
+  def set_responsibles
+    @responsibles = SemesterProcessVolunteer.joins(responsible: [profile: [:contact]])
+      .distinct
+      .select('users.id, contacts.full_name')
+      .map do |responsible|
+        {
+          q: :responsible_id_eq,
+          text: "Übernommen von #{responsible.full_name}",
+          value: responsible.id
+        }
+      end
+  end
+
+  def set_reviewers
+    @reviewers = SemesterProcessVolunteer.joins(reviewed_by: [profile: [:contact]])
+      .distinct
+      .select('users.id, contacts.full_name')
+      .map do |reviewed_by|
+        {
+          q: :reviewed_by_id_eq,
+          text: "Quittiert von #{reviewed_by.full_name}",
+          value: reviewed_by.id
+        }
+      end
+  end
+
   def semester_process_volunteer_params
     params.require(:semester_process_volunteer).permit(:semester, :notes)
   end
diff --git a/app/helpers/review_semester_helper.rb b/app/helpers/review_semester_helper.rb
deleted file mode 100644
index 563bbc5bc..000000000
--- a/app/helpers/review_semester_helper.rb
+++ /dev/null
@@ -1,78 +0,0 @@
-module ReviewSemesterHelper
-  def initialize_nested_objects
-    @nested_objects = {}
-    @semester_process_volunteer.semester_process_volunteer_missions.each do |spvm|
-      @nested_objects[spvm.id.to_s] = { feedback: @semester_process_volunteer.semester_feedback_with_mission(spvm.mission) || SemesterFeedback.new }
-    end
-    @nested_objects
-  end
-
-  def set_reviewed
-    @semester_process_volunteer.commited_by = current_user
-    @semester_process_volunteer.commited_at = Time.zone.now
-  end
-
-  def assign_volunteer_attributes
-    @volunteer.assign_attributes(review_params[:volunteer_attributes]
-      .slice(:waive, :bank, :iban))
-  end
-
-  def build_nested_objects
-    review_params[:semester_feedbacks_attributes].each do |_key, hash|
-      spv_mission = SemesterProcessVolunteerMission.find(hash[:semester_feedback][:spv_mission_id])
-      @nested_objects[spv_mission.id.to_s][:feedback] = SemesterFeedback.new(hash[:semester_feedback].merge({
-        author: current_user, semester_process_volunteer: @semester_process_volunteer
-      }))
-
-      if hash[:hour][:hours].to_i.positive?
-        @nested_objects[spv_mission.id.to_s][:hours] = Hour.new(hash[:hour].merge({
-          volunteer: spv_mission.volunteer,
-          meeting_date: spv_mission.semester_process_volunteer.semester.last.to_date,
-          hourable: spv_mission.mission.group_assignment? ? spv_mission.mission.group_offer : spv_mission.mission
-        }))
-      end
-    end
-  end
-
-  def null_reviewed
-    @semester_process_volunteer.commited_by = nil
-    @semester_process_volunteer.commited_at = nil
-  end
-
-  def save_feedback_data!
-    @semester_process_volunteer.volunteer.validate_waive_and_bank = true
-
-    ActiveRecord::Base.transaction do
-      @semester_process_volunteer.save!
-      @volunteer.save!
-
-      @nested_objects.each do |_key, hash|
-        hash.each { |_id, obj| obj.save! }
-      end
-    end
-    # reload the variable
-    @semester_process_volunteer.reload
-    true
-  rescue ActiveRecord::RecordInvalid => exception
-    null_reviewed
-    flash[:alert] = exception.message
-    false
-  end
-
-  def create_journals
-    spv = @semester_process_volunteer
-    return unless spv.commited_at?
-    volunteer = spv.volunteer
-    semester_feedbacks = spv.semester_feedbacks
-    Journal.create(user: volunteer.user, journalable: volunteer,
-      category: :feedback, title: "Semester Prozess Feedback vom #{I18n.l(Time.zone.today)}: ",
-      body: render_semester_feedbacks(semester_feedbacks))
-  end
-
-  def prepare_review
-    @semester_process_volunteer = SemesterProcessVolunteer.find(params[:id])
-    authorize @semester_process_volunteer
-    @volunteer = @semester_process_volunteer.volunteer
-    @missions = @semester_process_volunteer.missions
-  end
-end
\ No newline at end of file
diff --git a/app/helpers/semester_process_volunteer_helper.rb b/app/helpers/semester_process_volunteer_helper.rb
deleted file mode 100644
index 8bba7631f..000000000
--- a/app/helpers/semester_process_volunteer_helper.rb
+++ /dev/null
@@ -1,61 +0,0 @@
-module SemesterProcessVolunteerHelper
-  def render_semester_feedbacks(semester_feedbacks)
-    text = ''
-    semester_feedbacks.each do |semester_feedback|
-      text += semester_feedback.mission.to_label
-      text += "\n\n"
-      text += semester_feedback.slice(:goals, :achievements, :future, :comments).map do |key, sfb_quote|
-                "#{I18n.t("activerecord.attributes.feedback.#{key}")}:\n«#{sfb_quote}»" if sfb_quote.present?
-              end.compact.join("\n\n")
-      text += "\n\n"
-    end
-    text
-  end
-
-  def assign_volunteer_attributes
-    @volunteer.assign_attributes(review_params[:volunteer_attributes]
-      .slice(:waive, :bank, :iban))
-  end
-
-  def render_missions(spv)
-    html = ""
-    spv.missions.each do |m|
-      html += link_to m.to_label, "/#{m.class.name.underscore.pluralize}/#{m.id}/edit", target: '_blank'
-      html += "<br>"
-    end
-    html.html_safe
-  end
-
-  def set_responsibles
-    @responsibles = SemesterProcessVolunteer.joins(responsible: [profile: [:contact]])
-      .distinct
-      .select('users.id, contacts.full_name')
-      .map do |responsible|
-        {
-          q: :responsible_id_eq,
-          text: "Übernommen von #{responsible.full_name}",
-          value: responsible.id
-        }
-      end
-  end
-
-  def set_reviewers
-    @reviewers = SemesterProcessVolunteer.joins(reviewed_by: [profile: [:contact]])
-      .distinct
-      .select('users.id, contacts.full_name')
-      .map do |reviewed_by|
-        {
-          q: :reviewed_by_id_eq,
-          text: "Quittiert von #{reviewed_by.full_name}",
-          value: reviewed_by.id
-        }
-      end
-  end
-
-  def set_semester_process_volunteer
-    @spv = SemesterProcessVolunteer.find(params[:id])
-    authorize @spv
-    @semester_process = @spv.semester_process
-    @volunteer = @spv.volunteer
-  end
-end
diff --git a/app/models/semester_process_volunteer.rb b/app/models/semester_process_volunteer.rb
index f3fa6a583..9be59b2a6 100644
--- a/app/models/semester_process_volunteer.rb
+++ b/app/models/semester_process_volunteer.rb
@@ -70,7 +70,10 @@ class SemesterProcessVolunteer < ApplicationRecord
   end
 
   def semester_feedback_with_mission(mission)
-    semester_feedbacks.order(:created_at).select { |sf| sf.mission == mission }.last
+    scope = semester_feedbacks.order(:created_at)
+    scope.where(group_assignment: mission).or(
+      scope.where(assignment: mission)
+    ).last
   end
 
   # will only return an array, not a AD-result
-- 
GitLab


From 3845d77a34b1406a2493d657b8e084453cdac3cf Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 15:56:41 +0100
Subject: [PATCH 08/13] remove pre creation of jounral records for each
 volunteer in advance and improve rendering of feedback into body

- the journals where created, despite the fact that most of them where likeley never used and they only slowed down things even more
- the generation of the quoted feedback of the volunteers missions is now rendered in a template
---
 app/controllers/journals_controller.rb | 17 +++++------------
 app/views/journals/journal_body.erb    | 10 ++++++++++
 2 files changed, 15 insertions(+), 12 deletions(-)
 create mode 100644 app/views/journals/journal_body.erb

diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb
index d349644b8..2bc5ec3de 100644
--- a/app/controllers/journals_controller.rb
+++ b/app/controllers/journals_controller.rb
@@ -90,18 +90,11 @@ class JournalsController < ApplicationController
   def handle_semester_feedback_quote
     return unless @sp_volunteer
 
-    @journal.assign_attributes(category: :feedback, journalable: @journaled,
-      title: "Semester Prozess Feedback vom #{I18n.l(@semester_process_volunteer.created_at.to_date)}: ",
-      body: create_body(@semester_process_volunteer.semester_feedbacks&.first))
-  end
-
-  def create_body(semester_feedback = nil)
-    return unless semester_feedback
-
-    semester_feedback.slice(:goals, :achievements, :future, :comments)
-      .map do |key, sfb_quote|
-        "#{I18n.t("activerecord.attributes.feedback.#{key}")}:\n«#{sfb_quote}»" if sfb_quote.present?
-      end.compact.join("\n\n")
+    @journal.assign_attributes(
+      category: :feedback, journalable: @journaled,
+      title: "Semester Prozess Feedback vom #{I18n.l(@sp_volunteer.created_at.to_date)}: ",
+      body: render_to_string('journal_body', layout: false)
+    )
   end
 
   def journal_params
diff --git a/app/views/journals/journal_body.erb b/app/views/journals/journal_body.erb
new file mode 100644
index 000000000..add694182
--- /dev/null
+++ b/app/views/journals/journal_body.erb
@@ -0,0 +1,10 @@
+<% @sp_volunteer.semester_feedbacks.each do |feedback| %>
+<%= feedback.mission.to_label %>
+
+<% feedback.slice(:goals, :achievements, :future, :comments).compact.map do |key, sfb_quote| %>
+<%= t("activerecord.attributes.feedback.#{key}") %>:
+<%= "«#{sfb_quote}»" %>
+
+<% end %>
+
+<% end %>
-- 
GitLab


From 63634e29f9050f9beaab2421d1ac1d9b526c6fa0 Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 15:57:17 +0100
Subject: [PATCH 09/13] adding some indexes, in desperate hope to improve db
 performance a little

---
 ...or_some_created_at_and_updated_at_fields.rb | 18 ++++++++++++++++++
 db/schema.rb                                   | 16 +++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20190301134418_add_indexes_for_some_created_at_and_updated_at_fields.rb

diff --git a/db/migrate/20190301134418_add_indexes_for_some_created_at_and_updated_at_fields.rb b/db/migrate/20190301134418_add_indexes_for_some_created_at_and_updated_at_fields.rb
new file mode 100644
index 000000000..5ae6b628f
--- /dev/null
+++ b/db/migrate/20190301134418_add_indexes_for_some_created_at_and_updated_at_fields.rb
@@ -0,0 +1,18 @@
+class AddIndexesForSomeCreatedAtAndUpdatedAtFields < ActiveRecord::Migration[5.1]
+  def change
+    add_index :assignments, :created_at
+    add_index :group_assignments, :created_at
+    add_index :group_offers, :created_at
+    add_index :hours, :created_at
+    add_index :hours, :updated_at
+    add_index :semester_feedbacks, :created_at
+    add_index :semester_process_volunteer_missions, :created_at
+    add_index :semester_process_volunteers, :created_at
+    add_index :semester_process_volunteers, :reviewed_at
+    add_index :semester_process_volunteers, :responsibility_taken_at
+    add_index :semester_process_volunteers, :commited_at
+    add_index :semester_process_volunteers, :commit_visited_at
+    add_index :volunteers, :last_billing_expense_on
+    add_index :volunteers, :created_at
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f9d145704..5b1aec521 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181218094649) do
+ActiveRecord::Schema.define(version: 20190301134418) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -115,6 +115,7 @@ ActiveRecord::Schema.define(version: 20181218094649) do
     t.datetime "pdf_updated_at"
     t.bigint "submitted_by_id"
     t.index ["client_id"], name: "index_assignments_on_client_id"
+    t.index ["created_at"], name: "index_assignments_on_created_at"
     t.index ["creator_id"], name: "index_assignments_on_creator_id"
     t.index ["period_end"], name: "index_assignments_on_period_end"
     t.index ["period_end_set_by_id"], name: "index_assignments_on_period_end_set_by_id"
@@ -374,6 +375,7 @@ ActiveRecord::Schema.define(version: 20181218094649) do
     t.integer "pdf_file_size"
     t.datetime "pdf_updated_at"
     t.bigint "submitted_by_id"
+    t.index ["created_at"], name: "index_group_assignments_on_created_at"
     t.index ["deleted_at"], name: "index_group_assignments_on_deleted_at"
     t.index ["group_offer_id"], name: "index_group_assignments_on_group_offer_id"
     t.index ["period_end"], name: "index_group_assignments_on_period_end"
@@ -442,6 +444,7 @@ ActiveRecord::Schema.define(version: 20181218094649) do
     t.date "period_start"
     t.date "period_end"
     t.text "comments"
+    t.index ["created_at"], name: "index_group_offers_on_created_at"
     t.index ["creator_id"], name: "index_group_offers_on_creator_id"
     t.index ["deleted_at"], name: "index_group_offers_on_deleted_at"
     t.index ["department_id"], name: "index_group_offers_on_department_id"
@@ -466,10 +469,12 @@ ActiveRecord::Schema.define(version: 20181218094649) do
     t.bigint "hourable_id"
     t.bigint "reviewer_id"
     t.index ["billing_expense_id"], name: "index_hours_on_billing_expense_id"
+    t.index ["created_at"], name: "index_hours_on_created_at"
     t.index ["deleted_at"], name: "index_hours_on_deleted_at"
     t.index ["hourable_type", "hourable_id"], name: "index_hours_on_hourable_type_and_hourable_id"
     t.index ["meeting_date"], name: "index_hours_on_meeting_date"
     t.index ["reviewer_id"], name: "index_hours_on_reviewer_id"
+    t.index ["updated_at"], name: "index_hours_on_updated_at"
     t.index ["volunteer_id"], name: "index_hours_on_volunteer_id"
   end
 
@@ -615,6 +620,7 @@ ActiveRecord::Schema.define(version: 20181218094649) do
     t.datetime "updated_at", null: false
     t.index ["assignment_id"], name: "index_semester_feedbacks_on_assignment_id"
     t.index ["author_id"], name: "index_semester_feedbacks_on_author_id"
+    t.index ["created_at"], name: "index_semester_feedbacks_on_created_at"
     t.index ["deleted_at"], name: "index_semester_feedbacks_on_deleted_at"
     t.index ["group_assignment_id"], name: "index_semester_feedbacks_on_group_assignment_id"
   end
@@ -642,6 +648,7 @@ ActiveRecord::Schema.define(version: 20181218094649) do
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
     t.index ["assignment_id"], name: "semester_proc_volunteer_mission_assignment_index"
+    t.index ["created_at"], name: "index_semester_process_volunteer_missions_on_created_at"
     t.index ["deleted_at"], name: "index_semester_process_volunteer_missions_on_deleted_at"
     t.index ["group_assignment_id"], name: "semester_proc_volunteer_mission_group_assignment_index"
     t.index ["semester_process_volunteer_id"], name: "semester_proc_volunteer_mission_index"
@@ -661,9 +668,14 @@ ActiveRecord::Schema.define(version: 20181218094649) do
     t.datetime "deleted_at"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.index ["commit_visited_at"], name: "index_semester_process_volunteers_on_commit_visited_at"
+    t.index ["commited_at"], name: "index_semester_process_volunteers_on_commited_at"
     t.index ["commited_by_id"], name: "index_semester_process_volunteers_on_commited_by_id"
+    t.index ["created_at"], name: "index_semester_process_volunteers_on_created_at"
     t.index ["deleted_at"], name: "index_semester_process_volunteers_on_deleted_at"
+    t.index ["responsibility_taken_at"], name: "index_semester_process_volunteers_on_responsibility_taken_at"
     t.index ["responsible_id"], name: "index_semester_process_volunteers_on_responsible_id"
+    t.index ["reviewed_at"], name: "index_semester_process_volunteers_on_reviewed_at"
     t.index ["reviewed_by_id"], name: "index_semester_process_volunteers_on_reviewed_by_id"
     t.index ["semester_process_id"], name: "index_semester_process_volunteers_on_semester_process_id"
     t.index ["volunteer_id"], name: "index_semester_process_volunteers_on_volunteer_id"
@@ -802,10 +814,12 @@ ActiveRecord::Schema.define(version: 20181218094649) do
     t.index ["active"], name: "index_volunteers_on_active"
     t.index ["activeness_might_end"], name: "index_volunteers_on_activeness_might_end"
     t.index ["birth_year"], name: "index_volunteers_on_birth_year"
+    t.index ["created_at"], name: "index_volunteers_on_created_at"
     t.index ["deleted_at"], name: "index_volunteers_on_deleted_at"
     t.index ["department_id"], name: "index_volunteers_on_department_id"
     t.index ["external"], name: "index_volunteers_on_external"
     t.index ["invited_at"], name: "index_volunteers_on_invited_at"
+    t.index ["last_billing_expense_on"], name: "index_volunteers_on_last_billing_expense_on"
     t.index ["nationality"], name: "index_volunteers_on_nationality"
     t.index ["rejected_at"], name: "index_volunteers_on_rejected_at"
     t.index ["resigned_at"], name: "index_volunteers_on_resigned_at"
-- 
GitLab


From 8f48c10fd1e9ec276c48f1a708f9f167952cb67d Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 16:23:15 +0100
Subject: [PATCH 10/13] optimize hours and missions on semester process
 volunteer

- the iterables are now propper queries, ant they will be way faster
---
 app/controllers/review_semesters_controller.rb | 1 -
 app/models/semester_process_volunteer.rb       | 9 +++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/controllers/review_semesters_controller.rb b/app/controllers/review_semesters_controller.rb
index db2c69a23..27ce082e3 100644
--- a/app/controllers/review_semesters_controller.rb
+++ b/app/controllers/review_semesters_controller.rb
@@ -61,7 +61,6 @@ class ReviewSemestersController < ApplicationController
     @sp_volunteer = SemesterProcessVolunteer.find(params[:id])
     authorize @sp_volunteer
     @volunteer = @sp_volunteer.volunteer
-    @missions = @sp_volunteer.missions
   end
 
   def initialize_nested_objects
diff --git a/app/models/semester_process_volunteer.rb b/app/models/semester_process_volunteer.rb
index 9be59b2a6..f555354dc 100644
--- a/app/models/semester_process_volunteer.rb
+++ b/app/models/semester_process_volunteer.rb
@@ -64,9 +64,10 @@ class SemesterProcessVolunteer < ApplicationRecord
   attr_accessor :hours
 
   def hours
-    missions.map do |m|
-      m.hours.within_semester(semester)
-    end.flatten
+    scope = Hour.within_semester(semester)
+    scope.where(hourable_type: 'Assignment', hourable_id: assignments.ids).or(
+      scope.where(hourable_type: 'GroupOffer', hourable_id: group_assignments.pluck(:group_offer_id))
+    )
   end
 
   def semester_feedback_with_mission(mission)
@@ -78,7 +79,7 @@ class SemesterProcessVolunteer < ApplicationRecord
 
   # will only return an array, not a AD-result
   def missions
-    semester_process_volunteer_missions.map(&:mission)
+    assignments + group_assignments
   end
 
   def build_missions(semester)
-- 
GitLab


From 2ca9ec1a721ba2ec94f346a0107b4cbd878640fa Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 16:26:54 +0100
Subject: [PATCH 11/13] remove obsolete test

- creating a journal record for all volunteers in semester process makes no sense
---
 test/system/journals_test.rb          | 102 --------------------------
 test/system/semester_feedback_test.rb |  19 -----
 2 files changed, 121 deletions(-)
 delete mode 100644 test/system/journals_test.rb

diff --git a/test/system/journals_test.rb b/test/system/journals_test.rb
deleted file mode 100644
index dbc02a9e2..000000000
--- a/test/system/journals_test.rb
+++ /dev/null
@@ -1,102 +0,0 @@
-require 'application_system_test_case'
-
-class JournalsTest < ApplicationSystemTestCase
-  def setup
-    @superadmin = create :user
-    @department_manager = create :user, :department_manager
-    @volunteer = create :volunteer, registrar: @department_manager, acceptance: :accepted
-    @volunteer.journals = [FactoryBot.create(:journal, user: @superadmin)]
-    @journal_volunteer = @volunteer.journals.reload.first
-  end
-
-  def volunteer_has_link_to_their_journal_entry_as user
-    login_as user
-    visit volunteer_path(@volunteer)
-    first(:link, 'Journal').click
-
-    assert_link @journal_volunteer.user.full_name
-    assert_text @journal_volunteer.body
-  end
-
-  test 'volunteer has link to their journal entry as a superadmin' do
-    volunteer_has_link_to_their_journal_entry_as @superadmin
-  end
-
-  test 'volunteer has link to their journal entry as a department manager' do
-    volunteer_has_link_to_their_journal_entry_as @superadmin
-  end
-
-  def can_create_journal_as user
-    login_as user
-    visit client_journals_path(create(:client))
-    click_link 'Journal erfassen', match: :first
-
-    assert_text 'Journal erfassen'
-
-    fill_in 'Titel', with: 'Bogus Title'
-    fill_in 'Text', with: 'My bogus demo text body, just for this test.'
-    select 'Telefonat', from: 'Kategorie'
-    click_button 'Journaleintrag speichern'
-
-    assert_text 'Journal wurde erfolgreich erstellt.'
-    assert_text 'Bogus Title'
-    assert_text 'My bogus demo text body, just for this test.'
-  end
-
-  test 'can create journal as superadmin' do
-    can_create_journal_as @superadmin
-  end
-
-  test 'can create journal as department manager' do
-    can_create_journal_as @department_manager
-  end
-
-  def can_edit_journal_as user
-    login_as user
-    visit volunteer_journals_path(@volunteer)
-    click_link 'Bearbeiten'
-
-    assert_text 'Journal bearbeiten'
-
-    fill_in 'Text', with: 'New text'
-    click_button 'Journaleintrag aktualisieren'
-
-    assert_text 'Journal wurde erfolgreich geändert.'
-    assert_text 'New text'
-  end
-
-  test 'can edit journal as superadmin' do
-    can_edit_journal_as @superadmin
-  end
-
-  test 'can edit journal as department manager' do
-    can_edit_journal_as @department_manager
-  end
-
-  def can_delete_journal_as user
-    login_as user
-    visit volunteer_journals_path(@volunteer)
-
-    assert_text @journal_volunteer.body
-
-    click_link 'Bearbeiten'
-
-    assert_text 'Journal bearbeiten'
-
-    page.accept_confirm do
-      click_link 'Löschen'
-    end
-
-    assert_text 'Journal wurde erfolgreich gelöscht.'
-    assert_text 'Journal Liste'
-    refute_text @journal_volunteer.body
-  end
-
-  test 'can delete journal as superadmin' do
-    can_delete_journal_as @superadmin
-  end
-
-  test 'can delete journal as department manager' do
-    can_delete_journal_as @department_manager
-  end
-end
diff --git a/test/system/semester_feedback_test.rb b/test/system/semester_feedback_test.rb
index 0b05fbb1a..3ee378cff 100644
--- a/test/system/semester_feedback_test.rb
+++ b/test/system/semester_feedback_test.rb
@@ -144,24 +144,5 @@ class SemesterFeedbackTest < ApplicationSystemTestCase
     page.find('td', text: comments.truncate(300)).click
     assert page.has_text? comments
     click_button 'Schliessen'
-
-  end
-
-  test 'it should create a journal on submit' do
-    volunteer = create :volunteer_with_user
-    spv = create(:semester_process_volunteer, :with_missions, :with_mail, volunteer: volunteer,
-      semester_process: create(:semester_process))
-    login_as volunteer.user
-    visit review_semester_review_semester_url(spv)
-
-    assert_difference 'Journal.count', 1 do
-      fill_in_required_feedback_fields(0)
-      fill_in_required_feedback_fields(1)
-      check 'Ich verzichte auf die Auszahlung von Spesen.'
-      click_on 'Bestätigen', match: :first
-      spv.reload
-    end
-    assert Journal.last.body.include? volunteer.assignments.first.to_label
-    assert Journal.last.body.include? volunteer.assignments.second.to_label
   end
 end
-- 
GitLab


From 99e93c2ea2205b3b6808ba2b001f7ea12d12faf1 Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Fri, 1 Mar 2019 17:13:45 +0100
Subject: [PATCH 12/13] fix: hours scope method on new record

---
 app/models/semester_process_volunteer.rb | 38 +++++++++++++++++++++---
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/app/models/semester_process_volunteer.rb b/app/models/semester_process_volunteer.rb
index f555354dc..213ea5421 100644
--- a/app/models/semester_process_volunteer.rb
+++ b/app/models/semester_process_volunteer.rb
@@ -64,9 +64,9 @@ class SemesterProcessVolunteer < ApplicationRecord
   attr_accessor :hours
 
   def hours
-    scope = Hour.within_semester(semester)
-    scope.where(hourable_type: 'Assignment', hourable_id: assignments.ids).or(
-      scope.where(hourable_type: 'GroupOffer', hourable_id: group_assignments.pluck(:group_offer_id))
+    scope = Hour.within_semester(semester).where(volunteer: volunteer)
+    scope.where(hourable_type: 'Assignment', hourable_id: assignment_ids).or(
+      scope.where(hourable_type: 'GroupOffer', hourable_id: group_offer_ids)
     )
   end
 
@@ -79,7 +79,11 @@ class SemesterProcessVolunteer < ApplicationRecord
 
   # will only return an array, not a AD-result
   def missions
-    assignments + group_assignments
+    if new_record?
+      Assignment.find(assignment_ids) + GroupAssignment.find(group_assignment_ids)
+    else
+      assignments + group_assignments
+    end
   end
 
   def build_missions(semester)
@@ -126,4 +130,30 @@ class SemesterProcessVolunteer < ApplicationRecord
     self.commited_by = nil
     self.commited_at = nil
   end
+
+  private
+
+  def group_assignment_ids
+    if new_record?
+      semester_process_volunteer_missions.map(&:group_assignment_id).compact
+    else
+      group_assignments.ids
+    end
+  end
+
+  def group_offer_ids
+    if new_record?
+      GroupAssignment.find(group_assignment_ids).pluck(:group_offer_id)
+    else
+      group_assignments.pluck(:group_offer_id)
+    end
+  end
+
+  def assignment_ids
+    if new_record?
+      semester_process_volunteer_missions.map(&:assignment_id).compact
+    else
+      assignments.ids
+    end
+  end
 end
-- 
GitLab


From e199503370ef6c22ef423023904296cf0fc81521 Mon Sep 17 00:00:00 2001
From: Kaspar Vollenweider <vok@panter.ch>
Date: Mon, 4 Mar 2019 09:12:40 +0100
Subject: [PATCH 13/13] journal model validates not to have volunteer user as
 author

---
 app/models/journal.rb | 17 ++++++++++++-----
 config/locales/de.yml | 30 +++++++++++++++++-------------
 2 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/app/models/journal.rb b/app/models/journal.rb
index 2a62ba850..f36c72128 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -1,10 +1,6 @@
 class Journal < ApplicationRecord
   include ImportRelation
 
-  belongs_to :user, -> { with_deleted }, inverse_of: 'journals'
-  belongs_to :assignment, optional: true
-  belongs_to :journalable, polymorphic: true, required: false
-
   CATEGORIES = [
     :telephone,
     :conversation,
@@ -14,6 +10,13 @@ class Journal < ApplicationRecord
     :group_offer
   ].freeze
 
+  belongs_to :user, -> { with_deleted }, inverse_of: 'journals'
+  belongs_to :assignment, optional: true
+  belongs_to :journalable, polymorphic: true, required: false
+
+  validates :category, presence: true
+  validate :user_role_validation
+
   def self.categories_filters
     CATEGORIES.map do |category|
       {
@@ -24,5 +27,9 @@ class Journal < ApplicationRecord
     end
   end
 
-  validates :category, presence: true
+  private
+
+  def user_role_validation
+    errors.add(:user, :no_volunteer) if user.volunteer?
+  end
 end
diff --git a/config/locales/de.yml b/config/locales/de.yml
index 4155fe455..a03896582 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -547,6 +547,17 @@ de:
         working_percent: Stellenprozent
     errors:
       models:
+        event_volunteer:
+          attributes:
+            volunteer:
+              required: muss ausgewählt werden.
+        group_offer:
+          department:
+            blank: Freiwilligenverantwortliche können nur ihrem Standort zugeteilte Gruppenangebote erfassen
+        journal:
+          attributes:
+            user:
+              no_volunteer: Freiwillige können keine Journale verfassen.
         profile:
           attributes:
             avatar:
@@ -557,6 +568,12 @@ de:
               blank: darf nicht leer sein
             last_name:
               blank: darf nicht leer sein
+        semester_process_volunteer_mission:
+          attributes:
+            assignment: &id-mission-relation-insuficient
+              insuficient_relation: Es wird mindestens ein Assignment oder GroupAssignment benötigt
+              too_many_relations: Es darf nur ein Assignment oder ein GroupAssignment verbunden werden
+            group_assignment: *id-mission-relation-insuficient
         user:
           attributes:
             email:
@@ -576,19 +593,6 @@ de:
               spoofed_media_type: Die Inhalte entsprechen nicht ihrer Deklaration
             avatar_content_type:
               invalid: ist ungültig
-        group_offer:
-          department:
-            blank: Freiwilligenverantwortliche können nur ihrem Standort zugeteilte Gruppenangebote erfassen
-        event_volunteer:
-          attributes:
-            volunteer:
-              required: muss ausgewählt werden.
-        semester_process_volunteer_mission:
-          attributes:
-            assignment: &id-mission-relation-insuficient
-              insuficient_relation: Es wird mindestens ein Assignment oder GroupAssignment benötigt
-              too_many_relations: Es darf nur ein Assignment oder ein GroupAssignment verbunden werden
-            group_assignment: *id-mission-relation-insuficient
     models:
       assignment: Begleitung
       feedback: Halbjahres-Rapport
-- 
GitLab