Implement feedback

Author: njaeggi

app/controllers/event/tours/reports_controller.rb

@@ -35,7 +35,7 @@ def permitted_attrs
         :remarks,
         participations_attributes: [:state, :means_of_transport],
         revenues_attributes: [:id, :description, :count, :amount, :_destroy],
-        expenditures_attributes: [:id, :description, :count, :amount, :_destroy],
+        expenses_attributes: [:id, :description, :count, :amount, :_destroy],
         receipts_attributes: [:id, :description, :file, :_destroy]
       )
   end

app/models/event/cost.rb

@@ -12,6 +12,8 @@ class Event::Cost < ApplicationRecord
   validates :description, :count, :amount, presence: true
 
   def total
+    return 0 if amount.nil? || count.nil?
+
     amount * count
   end
 end

app/models/event/tour/report_form.rb

@@ -51,8 +51,8 @@ def revenues
     @revenues ||= report.costs.where(income: true).to_a
   end
 
-  def expenditures
-    @expenditures ||= report.costs.where(income: false).to_a
+  def expenses
+    @expenses ||= report.costs.where(income: false).to_a
   end
 
   def receipts
@@ -74,8 +74,8 @@ def revenues_attributes=(attrs)
     @revenues = build_records(attrs, collection: report.costs, fixed_attributes: {income: true})
   end
 
-  def expenditures_attributes=(attrs)
-    @expenditures = build_records(attrs, collection: report.costs,
+  def expenses_attributes=(attrs)
+    @expenses = build_records(attrs, collection: report.costs,
       fixed_attributes: {income: false})
   end
 
@@ -98,7 +98,7 @@ def assert_participation_states_editable
   end
 
   def assert_cost_records_valid
-    cost_records.flat_map { _1.reject(&:valid?) }.each do |model|
+    cost_records.flat_map { _1.reject(&:marked_for_destruction?).reject(&:valid?) }.each do |model|
       model.errors.full_messages.each { errors.add(:base, _1) }
     end
   end
@@ -123,7 +123,7 @@ def participation_by_id
   end
 
   def cost_records
-    [@revenues, @expenditures, @receipts].compact
+    [@revenues, @expenses, @receipts].compact
   end
 
   def build_records(attrs, collection:, fixed_attributes: {})

app/views/event/tours/reports/_cost_summary_fields.html.haml

@@ -8,8 +8,8 @@
     = f.button t(".add_tour_fees"), type: :button, data: {action: "click->sac--prefill-revenues#prefill"}, class: "btn btn-sm btn-outline-primary mb-3"
     = render "event/tours/reports/cost_table_fields", f: f, entries: entry.revenues, cost_type: :revenues
 
-= field_set_tag(t(".expenditures")) do
-  = render "event/tours/reports/cost_table_fields", f: f, entries: entry.expenditures, cost_type: :expenditures
+= field_set_tag(t(".expenses")) do
+  = render "event/tours/reports/cost_table_fields", f: f, entries: entry.expenses, cost_type: :expenses
 
 = field_set_tag(t(".receipts")) do
   = render "event/tours/reports/receipt_table_fields", f: f, entries: entry.receipts

app/views/event/tours/reports/_cost_table_fields.html.haml

@@ -20,7 +20,7 @@
               = ff.input_field :description
             .col-xl-3
               .d-xl-none.muted= Event::Cost.human_attribute_name(:count)
-              = ff.input_field :count, step: 1, data: {field_multiplication_target: "source", action: "input->field-multiplication#recalculate"}
+              = ff.input_field :count, step: 1, value: format("%g", ff.object.count), data: {field_multiplication_target: "source", action: "input->field-multiplication#recalculate"}
             .col-xl-3
               .d-xl-none.muted= Event::Cost.human_attribute_name(:amount)
               = ff.input_field :amount, step: 0.05, data: {field_multiplication_target: "source", action: "input->field-multiplication#recalculate"}

config/locales/wagon.de.yml

@@ -1729,7 +1729,7 @@ de:
         cost_summary_fields:
           revenues: Einnahmen
           add_tour_fees: Tourengebühren hinzufügen
-          expenditures: Ausgaben
+          expenses: Ausgaben
           receipts: Belege
         form_tabs:
           status: Status

spec/controllers/event/tours/reports_controller_spec.rb

@@ -106,10 +106,10 @@
         expect(event_costs(:participation_fee).reload.description).to eq "We made moneyyyyyyy"
       end
 
-      it "updates expenditures" do
+      it "updates expenses" do
         put :update, params: params.merge(
           event_tour_report_form: {
-            expenditures_attributes: {
+            expenses_attributes: {
               "0" => {
                 id: event_costs(:transport).id,
                 description: "Spent it all again sadly...",

spec/features/event/tours/reports_spec.rb

@@ -12,7 +12,7 @@
   let(:event) { events(:section_tour) }
   let(:report) { event_reports(:section_tour_report) }
   let(:revenue) { event_costs(:participation_fee) }
-  let(:expenditure) { event_costs(:transport) }
+  let(:expense) { event_costs(:transport) }
   let(:receipt) { event_cost_receipts(:tankstelle) }
 
   def create_participation(price_category:, price:)
@@ -89,7 +89,7 @@ def set_field(name, value)
       end
 
       it "updates the section total when a row is removed" do
-        table_total = find("#expenditures_total")
+        table_total = find("#expenses_total")
 
         within("fieldset", text: "Einnahmen") do
           click_link "Entfernen"
@@ -155,10 +155,32 @@ def set_field(name, value)
 
         expect(page).not_to have_field(name: /\[description\]/, with: "Kosten SAC-Mitglied (extern)")
       end
+
+      it "does not add duplicate dom rows when clicking add tour prices twice" do
+        create_participation(price_category: "price_member", price: 10)
+
+        click_button "Tourengebühren hinzufügen"
+
+        expect(all(".fields", visible: :all).size).to eq 3
+
+        click_button "Tourengebühren hinzufügen"
+
+        expect(all(".fields", visible: :all).size).to eq 3
+      end
+
+      it "assigns unique field names to rows added in the same click" do
+        create_participation(price_category: "price_member", price: 10)
+        create_participation(price_category: "price_special", price: 40)
+
+        click_button "Tourengebühren hinzufügen"
+
+        field_names = all(".fields[data-new-record] input[name*='[description]']").pluck(:name)
+        expect(field_names).to match_array(field_names.uniq)
+      end
     end
 
-    describe "expenditures" do
-      it "can add an expenditure" do
+    describe "expenses" do
+      it "can add an expense" do
         within("fieldset", text: "Ausgaben") do
           click_link "Eintrag hinzufügen"
           within(".fields[data-new-record]") do
@@ -175,7 +197,7 @@ def set_field(name, value)
       end
 
       it "updates the row total and section total when count and amount change" do
-        table_total = find("#expenditures_total")
+        table_total = find("#expenses_total")
 
         within("fieldset", text: "Ausgaben") do
           click_link "Eintrag hinzufügen"
@@ -209,7 +231,7 @@ def set_field(name, value)
       end
 
       it "updates the section total when a row is removed" do
-        table_total = find("#expenditures_total")
+        table_total = find("#expenses_total")
 
         within("fieldset", text: "Ausgaben") do
           click_link "Entfernen"

spec/models/event/cost_spec.rb

@@ -38,5 +38,17 @@
     it "calculates total of amount and count" do
       expect(event_cost.total).to eq 300
     end
+
+    it "is zero when amount is nil" do
+      event_cost.amount = nil
+
+      expect(event_cost.total).to be_zero
+    end
+
+    it "is zero when count is nil" do
+      event_cost.count = nil
+
+      expect(event_cost.total).to be_zero
+    end
   end
 end

spec/models/event/tour/report_form_spec.rb

@@ -15,7 +15,7 @@
       participation: Fabricate(:event_participation, participant: people(:mitglied), event: event)).participation
   end
   let(:revenue) { event_costs(:participation_fee) }
-  let(:expenditure) { event_costs(:transport) }
+  let(:expense) { event_costs(:transport) }
   let(:receipt) { event_cost_receipts(:tankstelle) }
   let(:form) { described_class.new(report) }
 
@@ -81,21 +81,21 @@
       expect { form.save }.to change { report.costs.count }.by(-1)
     end
 
-    it "persists changes to existing expenditure" do
-      form.expenditures_attributes = {
+    it "persists changes to existing expense" do
+      form.expenses_attributes = {
         "0" => {
-          id: expenditure.id,
+          id: expense.id,
           description: "something else"
         }
       }
 
-      expect { form.save }.to change { expenditure.reload.description }.to("something else")
+      expect { form.save }.to change { expense.reload.description }.to("something else")
     end
 
-    it "persists new expenditure" do
-      form.expenditures_attributes = {
+    it "persists new expense" do
+      form.expenses_attributes = {
         "0" => {
-          description: "New expenditure",
+          description: "New expense",
           count: "2",
           amount: "75"
         }
@@ -104,10 +104,10 @@
       expect { form.save }.to change { report.costs.where(income: false).count }.by(1)
     end
 
-    it "destroys expenditure marked for destruction" do
-      form.expenditures_attributes = {
+    it "destroys expense marked for destruction" do
+      form.expenses_attributes = {
         "0" => {
-          id: expenditure.id,
+          id: expense.id,
           _destroy: "1"
         }
       }
@@ -162,14 +162,14 @@
   describe "#revenues" do
     it "returns costs with income true" do
       expect(form.revenues).to include(revenue)
-      expect(form.revenues).not_to include(expenditure)
+      expect(form.revenues).not_to include(expense)
     end
   end
 
-  describe "#expenditures" do
+  describe "#expenses" do
     it "returns costs with income false" do
-      expect(form.expenditures).to include(expenditure)
-      expect(form.expenditures).not_to include(revenue)
+      expect(form.expenses).to include(expense)
+      expect(form.expenses).not_to include(revenue)
     end
   end
 
@@ -227,17 +227,17 @@
       expect(form.revenues.first.errors.full_messages).to match_array ["Bezeichnung muss ausgefüllt werden"]
     end
 
-    it "is not valid if expenditure is invalid" do
-      form.expenditures_attributes = {
+    it "is not valid if expense is invalid" do
+      form.expenses_attributes = {
         "0" => {
-          id: expenditure.id,
+          id: expense.id,
           description: ""
         }
       }
 
       expect(form).not_to be_valid
       expect(form.errors.full_messages).to match_array ["Bezeichnung muss ausgefüllt werden"]
-      expect(form.expenditures.first.errors.full_messages).to match_array ["Bezeichnung muss ausgefüllt werden"]
+      expect(form.expenses.first.errors.full_messages).to match_array ["Bezeichnung muss ausgefüllt werden"]
     end
 
     it "is not valid if receipt is invalid" do
@@ -252,6 +252,18 @@
       expect(form.errors.full_messages).to match_array ["Beschreibung muss ausgefüllt werden"]
       expect(form.receipts.first.errors.full_messages).to match_array ["Beschreibung muss ausgefüllt werden"]
     end
+
+    it "is valid if invalid revenue is marked for destruction" do
+      form.revenues_attributes = {
+        "0" => {
+          id: revenue.id,
+          description: "",
+          _destroy: "1"
+        }
+      }
+
+      expect(form).to be_valid
+    end
   end
 
   describe "#tour_completed?" do