From a7820a86a1d6a9b3bde86421f5a878f648d655d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niklas=20J=C3=A4ggi?= Date: Mon, 19 Aug 2024 16:28:27 +0200 Subject: [PATCH] implement review --- .../people/membership_invoices_controller.rb | 74 ++++--- app/helpers/membership_invoices_helper.rb | 25 --- app/models/people/membership/invoice.rb | 62 ------ app/models/people/membership/invoice_form.rb | 42 ++++ .../people/membership_invoices/new.html.haml | 11 +- config/locales/wagon.de.yml | 4 +- .../membership_invoices_controller_spec.rb | 202 ++++++++++-------- 7 files changed, 202 insertions(+), 218 deletions(-) delete mode 100644 app/helpers/membership_invoices_helper.rb delete mode 100644 app/models/people/membership/invoice.rb create mode 100644 app/models/people/membership/invoice_form.rb diff --git a/app/controllers/people/membership_invoices_controller.rb b/app/controllers/people/membership_invoices_controller.rb index 45375c46b..841641883 100644 --- a/app/controllers/people/membership_invoices_controller.rb +++ b/app/controllers/people/membership_invoices_controller.rb @@ -6,69 +6,75 @@ # https://github.com/hitobito/hitobito_sac_cas. class People::MembershipInvoicesController < ApplicationController + + helper_method :invoice_possible?, :date_range, :currently_paying_zusatzsektionen + def create - authorize!(:update, person) + authorize!(:create, external_invoice) invoice_form.attributes = invoice_form_params - if invoice_form.valid? - generate_invoice - redirect_to external_invoices_group_person_path(group, person) + if invoice_form.valid? && generate_invoice + redirect_to external_invoices_group_person_path(group, person), notice: t("people.membership_invoices.success_notice") else - set_flash(:alert, message: invoice_form.errors.full_messages.join(", ")) - redirect_to new_group_person_membership_invoice_path(group, person) + redirect_to new_group_person_membership_invoice_path(group, person), alert: I18n.t("people.membership_invoices.alert_notice", message: invoice_form.errors.full_messages.join(", ")) end end def new - authorize!(:update, person) + authorize!(:update, external_invoice) @invoice_form = invoice_form @group = group - @date = date - @person = person - @context = context @member = member end private + def external_invoice = @external_invoice ||= ExternalInvoice.new(person: person) + def invoice_form - @invoice_form ||= People::Membership::Invoice.new({}, person) + @invoice_form ||= People::Membership::InvoiceForm.new({}, person) end def invoice_form_params - params.require(:people_membership_invoice).permit(:reference_date, :invoice_date, :send_date, :section_id, :new_entry, :discount) + params.require(:people_membership_invoice_form).permit(:reference_date, :invoice_date, :send_date, :section_id, :new_entry, :discount) end def generate_invoice - handle_exceptions do - ExternalInvoice::SacMembership.create!( - state: :draft, - year: Date.parse(@invoice_form.reference_date).year, - issued_at: @invoice_form.invoice_date, - sent_at: @invoice_form.send_date, - person: person, - link: Group.find(@invoice_form.section_id) - ) - set_flash(:success) - end + ExternalInvoice::SacMembership.create( + state: :draft, + year: @invoice_form.reference_date.year, + issued_at: @invoice_form.invoice_date, + sent_at: @invoice_form.send_date, + person: person, + link: Group.find(@invoice_form.section_id) + ) end - def set_flash(type, **args) - kind = (type == :success) ? :notice : :alert - flash[kind] = t("people.membership_invoices.#{type}_notice", **args) # rubocop:disable Rails/ActionControllerFlashBeforeRender + def invoice_possible?(member, date) + memberships = member.active_memberships + memberships.present? && Invoices::Abacus::MembershipInvoice.new(member, memberships).invoice? end - def handle_exceptions - yield - rescue => e - set_flash(:alert, message: e.message) - options = {} - if e.respond_to?(:response) - options[:extra] = {response: e.response.body.force_encoding("UTF-8")} + def date_range(attr) + if attr == :send_date + Time.zone.today.beginning_of_year..(already_member_next_year?(@person) ? Time.zone.today.next_year.end_of_year : Time.zone.today.end_of_year) + else + Time.zone.today.beginning_of_year..Time.zone.today.next_year.end_of_year end - Raven.capture_exception(e, options) + end + + def already_member_next_year?(person) + next_year = Time.zone.today.year + 1 + delete_on_date = person.sac_membership.stammsektion_role.delete_on + delete_on_date >= Date.new(next_year, 1, 1) && delete_on_date <= Date.new(next_year, 12, 31) + end + + def currently_paying_zusatzsektionen(member) + memberships = member.additional_membership_roles + member.new_additional_section_membership_roles + paying_memberships = memberships.select { |membership| member.paying_person?(membership.beitragskategorie) } + paying_memberships.map(&:layer_group) end def member diff --git a/app/helpers/membership_invoices_helper.rb b/app/helpers/membership_invoices_helper.rb deleted file mode 100644 index 556f06005..000000000 --- a/app/helpers/membership_invoices_helper.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) 2024, Schweizer Alpen-Club. This file is part of -# hitobito and licensed under the Affero General Public License version 3 -# or later. See the COPYING file at the top-level directory or at -# https://github.com/hitobito/hitobito_sac_cas. - -module MembershipInvoicesHelper - def invoice_possible?(member, date) - memberships = member.active_memberships - memberships.present? && Invoices::Abacus::MembershipInvoice.new(member, memberships).invoice? - end - - def already_member_next_year?(person) - next_year = Time.zone.today.year + 1 - delete_on_date = person.sac_membership.stammsektion_role.delete_on - delete_on_date >= Date.new(next_year, 1, 1) && delete_on_date <= Date.new(next_year, 12, 31) - end - - def currently_paying_zusatzsektionen(member) - memberships = member.additional_membership_roles + member.new_additional_section_membership_roles - paying_memberships = memberships.select { |membership| member.paying_person?(membership.beitragskategorie) } - paying_memberships.map(&:layer_group) - end -end diff --git a/app/models/people/membership/invoice.rb b/app/models/people/membership/invoice.rb deleted file mode 100644 index cd8553a90..000000000 --- a/app/models/people/membership/invoice.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) 2023, Schweizer Alpen-Club. This file is part of -# hitobito_sac_cas and licensed under the Affero General Public License version 3 -# or later. See the COPYING file at the top-level directory or at -# https://github.com/hitobito/hitobito - -class People::Membership::Invoice - include ActiveModel::Model - include ActiveModel::Attributes - - attr_accessor :reference_date, :invoice_date, :send_date, :discount, :new_entry, :section_id - - validates :reference_date, :invoice_date, :send_date, :discount, presence: true - - validate :reference_date_within_range - validate :invoice_date_within_range - validate :send_date_within_range - - validates :discount, inclusion: {in: ["0", "50", "100"]} - - def initialize(attributes = {}, person = nil) - super(attributes) - @person = person - end - - private - - def reference_date_within_range - date_within_range(reference_date, :reference_date) - end - - def invoice_date_within_range - date_within_range(invoice_date, :invoice_date) - end - - def date_within_range(date, field) - if date.present? && !(date_today.beginning_of_year..date_today.next_year.end_of_year).cover?(Date.parse(date)) - errors.add(field, :invalid_date_range, start_on: date_today.beginning_of_year, end_on: date_today.next_year.end_of_year) - end - end - - def send_date_within_range - if @person && send_date.present? - delete_on = @person.sac_membership.stammsektion_role.delete_on - - valid_range = if delete_on.year > date_today.year - date_today.all_year - else - date_today.beginning_of_year..date_today.next_year.end_of_year - end - - unless valid_range.cover?(Date.parse(send_date)) - errors.add(:send_date, :invalid_date_range, start_on: valid_range.begin, end_on: valid_range.end) - end - end - end - - def date_today - Time.zone.today - end -end diff --git a/app/models/people/membership/invoice_form.rb b/app/models/people/membership/invoice_form.rb new file mode 100644 index 000000000..a94ba79e5 --- /dev/null +++ b/app/models/people/membership/invoice_form.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +# Copyright (c) 2023, Schweizer Alpen-Club. This file is part of +# hitobito_sac_cas and licensed under the Affero General Public License version 3 +# or later. See the COPYING file at the top-level directory or at +# https://github.com/hitobito/hitobito + +class People::Membership::InvoiceForm + include ActiveModel::Model + include ActiveModel::Attributes + + DISCOUNTS = ["0", "50", "100"].freeze + + attribute :reference_date, :date + attribute :invoice_date, :date + attribute :send_date, :date + attribute :discount, :string + attribute :new_entry, :boolean + attribute :section_id, :string + + validates :reference_date, :invoice_date, :send_date, :discount, presence: true + + validates_date :reference_date, :invoice_date, between: Time.zone.today.beginning_of_year..Time.zone.today.next_year.end_of_year + validates_date :send_date, between: [Time.zone.today.beginning_of_year, :send_date_end_date] + + validates :discount, inclusion: {in: DISCOUNTS} + + def initialize(attributes = {}, person = nil) + super(attributes) + @person = person + end + + private + + def send_date_end_date + @person.sac_membership.stammsektion_role.delete_on.year > date_today.year ? date_today.end_of_year : date_today.next_year.end_of_year + end + + def date_today + Time.zone.today + end +end diff --git a/app/views/people/membership_invoices/new.html.haml b/app/views/people/membership_invoices/new.html.haml index a6fd23497..ede1a5f81 100644 --- a/app/views/people/membership_invoices/new.html.haml +++ b/app/views/people/membership_invoices/new.html.haml @@ -2,13 +2,14 @@ .alert.alert-info=t(".alert_info") = standard_form(@invoice_form, url: group_person_membership_invoices_path(@group, @person), method: :post) do |f| - %h1 Mitgliedschaftsrechnung erstellen + %h1 + = t(".form_title") = f.labeled(:reference_date) do - = f.date_field(:reference_date, class: 'date col-6', minDate: Date.today.beginning_of_year, maxDate: (Date.today.end_of_year + 1.year)) + = f.date_field(:reference_date, class: 'date col-6', minDate: date_range(:reference_date).min, maxDate: date_range(:reference_date_date).max) = f.labeled(:invoice_date) do - = f.date_field(:invoice_date, class: 'date col-6', minDate: Date.today.beginning_of_year, maxDate: (Date.today.end_of_year + 1.year)) + = f.date_field(:invoice_date, class: 'date col-6', minDate: date_range(:invoice_date).min, maxDate: date_range(:invoice_date).max) = f.labeled(:send_date) do - = f.date_field(:send_date, class: 'date col-6', minDate: Date.today.beginning_of_year, maxDate: (already_member_next_year?(@person) ? Date.today.end_of_year + 1.year : Date.today.end_of_year)) + = f.date_field(:send_date, class: 'date col-6', minDate: date_range(:send_date).min, maxDate: date_range(:send_date).max) = f.labeled(:section_id) do - main_section = @person.sac_membership.stammsektion_role.layer_group @@ -28,7 +29,7 @@ = f.inline_radio_button :discount, 100, "100%", true = f.indented do - = submit_button(f, "Rechnung erstellen") + = submit_button(f, t(".create_invoice")) = cancel_link(external_invoices_group_person_path(@group, @person)) - else .alert.alert-warning= t(".alert_warning") diff --git a/config/locales/wagon.de.yml b/config/locales/wagon.de.yml index ab3073cee..ece515e69 100644 --- a/config/locales/wagon.de.yml +++ b/config/locales/wagon.de.yml @@ -24,7 +24,7 @@ de: activemodel: attributes: - people/membership/invoice: + people/membership/invoice_form: reference_date: "Stichtag" invoice_date: "Rechnungsdatum" send_date: "Versanddatum" @@ -1086,8 +1086,10 @@ de: success_notice: "Die gewünschte Rechnung wird erzeugt und an Abacus übermittelt" alert_notice: "Die Rechnung konnte nicht erstellt werden: %{message}" new: + form_title: "Mitgliedschaftsrechnung erstellen" alert_info: "Nach Bestätigung dieses Dialogs wird eine Mitgliedschaftsrechnung erstellt und per E-Mail oder Post an das Mitglied versandt. Es erfolgt keine Überprüfung, ob bereits Mitgliedschaftsrechnungen existieren oder in welchem Status sich diese befinden. Es wird in jedem Fall eine neue Rechnung ausgestellt. Es werden keine Rollenmutationen oder -verlängerungen vorgenommen." alert_warning: "Diese Person verfügt über keine eigene Mitgliedschaftsrechnung. Die Gebühren werden allenfalls mit der Rechnung einer anderen Person verrechnet." + create_invoice: "Rechnung erstellen" mv_yearly_invoice: "MV-Jahresrechnung" zusatzsektion_eintrittsrechnung: "Zusatzsektions-Eintrittsrechnung Sektion" diff --git a/spec/controllers/people/membership_invoices_controller_spec.rb b/spec/controllers/people/membership_invoices_controller_spec.rb index 8babe6f49..57e1e1f3a 100644 --- a/spec/controllers/people/membership_invoices_controller_spec.rb +++ b/spec/controllers/people/membership_invoices_controller_spec.rb @@ -21,8 +21,6 @@ describe "POST create" do it "creates external invoice" do - person.update!(zip_code: 3600, town: "Thun") - expect do post :create, params: { group_id: groups(:bluemlisalp_mitglieder).id, @@ -32,7 +30,7 @@ invoice_date: Time.zone.today, send_date: Time.zone.today, section_id: groups(:bluemlisalp_mitglieder).id, - discount: "0" + discount: 0 } } end.to change { ExternalInvoice.count }.by(1) @@ -42,100 +40,122 @@ expect(flash[:notice]).to eq("Die gewünschte Rechnung wird erzeugt und an Abacus übermittelt") end - it "doesnt create external invoice when params invalid" do - person.update!(zip_code: 3600, town: "Thun") - - # no send date - expect do - post :create, params: { - group_id: groups(:bluemlisalp_mitglieder).id, - person_id: person.id, - people_membership_invoice: { - reference_date: Time.zone.today, - invoice_date: Time.zone.today, - send_date: "", - section_id: groups(:bluemlisalp_mitglieder).id, - discount: "0" + context "invalid params" do + it "doesnt create external invoice without send date" do + expect do + post :create, params: { + group_id: groups(:bluemlisalp_mitglieder).id, + person_id: person.id, + people_membership_invoice: { + reference_date: Time.zone.today, + invoice_date: Time.zone.today, + send_date: "", + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } } - } - end.to change { ExternalInvoice.count }.by(0) - - expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) - expect(flash[:alert]).to include("Versanddatum muss ausgefüllt werden") - - # no reference date and no invoice date - expect do - post :create, params: { - group_id: groups(:bluemlisalp_mitglieder).id, - person_id: person.id, - people_membership_invoice: { - reference_date: "", - invoice_date: "", - send_date: Time.zone.today, - section_id: groups(:bluemlisalp_mitglieder).id, - discount: "0" + end.not_to change { ExternalInvoice.count } + + expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) + expect(flash[:alert]).to include("Versanddatum muss ausgefüllt werden") + end + + it "doesnt create external invoice without referenc and invoice date" do + expect do + post :create, params: { + group_id: groups(:bluemlisalp_mitglieder).id, + person_id: person.id, + people_membership_invoice: { + reference_date: "", + invoice_date: "", + send_date: Time.zone.today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } } - } - end.to change { ExternalInvoice.count }.by(0) - expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) - expect(flash[:alert]).to include("Stichtag muss ausgefüllt werden, Rechnungsdatum muss ausgefüllt werden") - - # reference date in invalid year - expect do - post :create, params: { - group_id: groups(:bluemlisalp_mitglieder).id, - person_id: person.id, - people_membership_invoice: { - reference_date: Time.zone.today.next_year(5), - invoice_date: Time.zone.today, - send_date: Time.zone.today, - section_id: groups(:bluemlisalp_mitglieder).id, - discount: "0" + end.not_to change { ExternalInvoice.count } + expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) + expect(flash[:alert]).to include("Stichtag muss ausgefüllt werden, Rechnungsdatum muss ausgefüllt werden") + end + + it "doesnt create external invoice if reference date is in invalid year" do + expect do + post :create, params: { + group_id: groups(:bluemlisalp_mitglieder).id, + person_id: person.id, + people_membership_invoice: { + reference_date: Time.zone.today.next_year(5), + invoice_date: Time.zone.today, + send_date: Time.zone.today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } } - } - end.to change { ExternalInvoice.count }.by(0) - - expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) - expect(flash[:alert]).to include("Stichtag muss zwischen 2024-01-01 und 2025-12-31 liegen") - - # set person stammsektion to be continued in next year - person.sac_membership.stammsektion_role.update!(delete_on: Time.zone.today.next_year.end_of_year) - - # send date cant be next year - expect do - post :create, params: { - group_id: groups(:bluemlisalp_mitglieder).id, - person_id: person.id, - people_membership_invoice: { - reference_date: Time.zone.today, - invoice_date: Time.zone.today, - send_date: Time.zone.today.next_year, - section_id: groups(:bluemlisalp_mitglieder).id, - discount: "0" + end.not_to change { ExternalInvoice.count } + + expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) + expect(flash[:alert]).to include("Stichtag muss 31.12.2025 oder davor sein") + end + + it "doesnt create external invoice if reference date is in past year" do + expect do + post :create, params: { + group_id: groups(:bluemlisalp_mitglieder).id, + person_id: person.id, + people_membership_invoice: { + reference_date: Time.zone.today.last_year, + invoice_date: Time.zone.today, + send_date: Time.zone.today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } } - } - end.to change { ExternalInvoice.count }.by(0) - - expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) - expect(flash[:alert]).to include("Versanddatum muss zwischen 2024-01-01 und 2024-12-31 liegen") - - # invalid discount - expect do - post :create, params: { - group_id: groups(:bluemlisalp_mitglieder).id, - person_id: person.id, - people_membership_invoice: { - reference_date: Time.zone.today, - invoice_date: Time.zone.today, - send_date: Time.zone.today, - section_id: groups(:bluemlisalp_mitglieder).id, - discount: "16" + end.not_to change { ExternalInvoice.count } + + expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) + expect(flash[:alert]).to include("Stichtag muss 01.01.2024 oder danach sein") + end + + it "doesnt create external invoice send date is next year" do + # set person stammsektion to be continued in next year + person.sac_membership.stammsektion_role.update!(delete_on: Time.zone.today.next_year.end_of_year) + + expect do + post :create, params: { + group_id: groups(:bluemlisalp_mitglieder).id, + person_id: person.id, + people_membership_invoice: { + reference_date: Time.zone.today, + invoice_date: Time.zone.today, + send_date: Time.zone.today.next_year, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } } - } - end.to change { ExternalInvoice.count }.by(0) + end.not_to change { ExternalInvoice.count } + + expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) + expect(flash[:alert]).to include("Versanddatum muss 31.12.2024 oder davor sein") + end + + it "doesnt create external invoice if discount is invalid" do + expect do + post :create, params: { + group_id: groups(:bluemlisalp_mitglieder).id, + person_id: person.id, + people_membership_invoice: { + reference_date: Time.zone.today, + invoice_date: Time.zone.today, + send_date: Time.zone.today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 16 + } + } + end.not_to change { ExternalInvoice.count } - expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) - expect(flash[:alert]).to include("Rabatt ist kein gültiger Wert") + expect(response).to redirect_to(new_group_person_membership_invoice_path(groups(:bluemlisalp_mitglieder).id, person.id)) + expect(flash[:alert]).to include("Rabatt ist kein gültiger Wert") + end end end end