From cd3342354cad89263f6a1f67b5b0f28be8dd38fc Mon Sep 17 00:00:00 2001 From: Niklas <68543592+njaeggi@users.noreply.github.com> Date: Tue, 20 Aug 2024 16:00:07 +0200 Subject: [PATCH] Feature/744/membership invoice form (#799) * create form and adjsut controller to create external invoice with passed params * imrpove validation of passed params * fix rubocop and cleanup code * Adjust form validation to use active model * remove trailing whitespaces * Add acitve model to validate membership invoice * fix rubocop and adjust translations * change to standard form * implement review * adjust specs and fix rubocop * Some hopefully safe refactorings * Some more refactorings * further cleanup and add some additional specs * Render instead of redirect * further refactoring * cleanup currently_paying method filter * adjust methods in invoice form --------- Co-authored-by: Andreas Maierhofer --- .../people/membership_invoices_controller.rb | 72 ++++----- app/domain/people/sac_membership.rb | 28 +++- .../sheet/people/membership_invoice.rb | 14 ++ app/models/people/membership/invoice_form.rb | 53 ++++++ .../people/external_invoices/_list.html.haml | 5 +- .../people/membership_invoices/new.html.haml | 37 +++++ config/locales/wagon.de.yml | 20 ++- config/routes.rb | 2 +- .../membership_invoices_controller_spec.rb | 152 ++++++++++++++---- 9 files changed, 298 insertions(+), 85 deletions(-) create mode 100644 app/helpers/sheet/people/membership_invoice.rb create mode 100644 app/models/people/membership/invoice_form.rb create mode 100644 app/views/people/membership_invoices/new.html.haml diff --git a/app/controllers/people/membership_invoices_controller.rb b/app/controllers/people/membership_invoices_controller.rb index 4650fcb09..4e44c26ef 100644 --- a/app/controllers/people/membership_invoices_controller.rb +++ b/app/controllers/people/membership_invoices_controller.rb @@ -7,53 +7,53 @@ class People::MembershipInvoicesController < ApplicationController def create - authorize!(:update, person) + authorize!(:create, external_invoice) - generate_invoice - redirect_to group_person_path(params[:group_id], person.id) - end - - private + invoice_form.attributes = invoice_form_params - def generate_invoice - handle_exceptions do - if invoicer.generate - set_flash(:success, abacus_key: invoicer.invoice.abacus_sales_order_key) - else - set_flash(:alert, message: invoicer.error_messages.join(", ")) - end + if invoice_form.valid? && create_invoice + redirect_to external_invoices_group_person_path(group, person), notice: t("people.membership_invoices.success_notice") + else + @group = group + render :new, status: :unprocessable_entity end end - def set_flash(type, **args) - kind = (type == :success) ? :notice : :alert - flash[kind] = t("people.membership_invoices.#{type}_notice", **args) # rubocop:disable Rails/ActionControllerFlashBeforeRender - end + def new + authorize!(:update, external_invoice) - 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")} - end - Raven.capture_exception(e, options) + @invoice_form = invoice_form + @group = group end - def invoicer - @invoicer ||= Invoices::Abacus::MembershipInvoiceGenerator.new(person, date: date) - end + private - def person - @person ||= context.people_with_membership_years.find(params[:person_id]) + def invoice_form_params + params.require(:people_membership_invoice_form).permit(:reference_date, :invoice_date, :send_date, :section_id, :new_entry, :discount) end - def context - @context ||= Invoices::SacMemberships::Context.new(date) + def create_invoice + 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 date - @date ||= params[:date].present? ? Date.parse(params[:date]) : Time.zone.today - end + def external_invoice = @external_invoice ||= ExternalInvoice.new(person: person) + + def invoice_form = @invoice_form ||= People::Membership::InvoiceForm.new({}, person) + + def person = @person ||= context.people_with_membership_years.find(params[:person_id]) + + def context = @context ||= Invoices::SacMemberships::Context.new(date) + + def group = @group ||= Group.find(params[:group_id]) + + def date = @date ||= params[:date].present? ? Date.parse(params[:date]) : today + + def today = @today ||= Time.zone.today end diff --git a/app/domain/people/sac_membership.rb b/app/domain/people/sac_membership.rb index 15ed8f392..30870a28d 100644 --- a/app/domain/people/sac_membership.rb +++ b/app/domain/people/sac_membership.rb @@ -36,20 +36,24 @@ def anytime? stammsektion_role.present? || any_future_role? || any_past_role? end - def stammsektion_role - active_roles_of_type(mitglied_stammsektion_types).first - end - def stammsektion stammsektion_role&.layer_group end - def future_stammsektion_roles - @person.roles.future.where(convert_to: mitglied_stammsektion_types) + def stammsektion_role(currently_paying: false) + active_roles_of_type(mitglied_stammsektion_types).then do |roles| + currently_paying ? select_currently_paying(roles) : roles + end.first end - def zusatzsektion_roles - active_roles_of_type(mitglied_zusatzsektion_types) + def zusatzsektion_roles(currently_paying: false) + active_roles_of_type(mitglied_zusatzsektion_types).then do |roles| + currently_paying ? select_currently_paying(roles) : roles + end + end + + def future_stammsektion_roles + @person.roles.future.where(convert_to: mitglied_stammsektion_types) end def neuanmeldung_nv_stammsektion_roles @@ -151,6 +155,14 @@ def any_past_role? @person.roles.deleted.where(type: mitglied_stammsektion_types).exists? end + def select_currently_paying(roles) + roles.select { |role| paying_person?(role.beitragskategorie) } + end + + def paying_person?(beitragskategorie) + !beitragskategorie.family? || @person.sac_family_main_person? + end + def mitglied_types = SacCas::MITGLIED_ROLES.map(&:sti_name) def mitglied_stammsektion_types = SacCas::MITGLIED_STAMMSEKTION_ROLES.map(&:sti_name) diff --git a/app/helpers/sheet/people/membership_invoice.rb b/app/helpers/sheet/people/membership_invoice.rb new file mode 100644 index 000000000..f92a74bf4 --- /dev/null +++ b/app/helpers/sheet/people/membership_invoice.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Copyright (c) 2024, 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_sac_cas + +module Sheet + module People + class MembershipInvoice < Base + self.parent_sheet = Sheet::Person + end + 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..1d28f57f4 --- /dev/null +++ b/app/models/people/membership/invoice_form.rb @@ -0,0 +1,53 @@ +# 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: [:min_date, :max_date], allow_blank: true + validates_date :send_date, between: [:min_date, :max_send_date], allow_blank: true + + validates :discount, inclusion: {in: DISCOUNTS} + + def initialize(attributes = {}, person = nil) + super(attributes) + @person = person + end + + def date_range(attr = nil) + max_date = (attr == :send_date && !already_member_next_year?) ? date_today.end_of_year : date_today.next_year.end_of_year + + {minDate: date_today.beginning_of_year, maxDate: max_date} + end + + private + + def already_member_next_year? + next_year = date_today.next_year.year + @person.sac_membership.stammsektion_role.delete_on&.year&.>= next_year + end + + def min_date = date_range[:minDate] + + def max_date = date_range[:maxDate] + + def max_send_date = date_range(:send_date)[:maxDate] + + def date_today = Time.zone.today +end diff --git a/app/views/people/external_invoices/_list.html.haml b/app/views/people/external_invoices/_list.html.haml index 99186c498..143d5950d 100644 --- a/app/views/people/external_invoices/_list.html.haml +++ b/app/views/people/external_invoices/_list.html.haml @@ -5,8 +5,7 @@ - if @person.sac_membership_invoice? && can?(:create_membership_invoice, @person) = action_button(t('.create_membership_invoice'), - group_person_membership_invoices_path(@group, @person), + new_group_person_membership_invoice_path(@group, @person), :envelope, - method: :post, - data: { confirm: t('.create_membership_invoice_confirmation') }) + method: :get) = render "table" diff --git a/app/views/people/membership_invoices/new.html.haml b/app/views/people/membership_invoices/new.html.haml new file mode 100644 index 000000000..9514274fb --- /dev/null +++ b/app/views/people/membership_invoices/new.html.haml @@ -0,0 +1,37 @@ +- title t(".form_title") +- if @person.sac_membership_invoice? + .alert.alert-info=t(".alert_info") + + = standard_form(@invoice_form, url: group_person_membership_invoices_path(@group, @person), method: :post) do |f| + = f.error_messages + = f.labeled(:reference_date) do + = f.date_field(:reference_date, @invoice_form.date_range) + = f.labeled(:invoice_date) do + = f.date_field(:invoice_date, @invoice_form.date_range) + = f.labeled(:send_date) do + = f.date_field(:send_date, @invoice_form.date_range(:send_date)) + + = f.labeled(:section_id) do + - main_section = @person.sac_membership.stammsektion_role.layer_group + = f.inline_radio_button :section_id, main_section.id, t(".mv_yearly_invoice"), true, checked: true + + .nested-radio-group + = f.labeled(:new_entry, class: "ms-3") do + = f.inline_radio_button :new_entry, true, t("global.yes"), true + = f.inline_radio_button :new_entry, false, t("global.no"), true, checked: true + + - @person.sac_membership.zusatzsektion_roles(currently_paying: true).map(&:layer_group).each do |section| + %div + = f.inline_radio_button :section_id, section.id, "#{t(".zusatzsektion_eintrittsrechnung")} #{section.name}", false, checked: false + + = f.labeled(:discount) do + = f.inline_radio_button :discount, 0, t("global.no"), true, checked: true + = f.inline_radio_button :discount, 50, "50%", true + = f.inline_radio_button :discount, 100, "100%", true + + = f.indented do + = submit_button(f, t(".create_invoice")) + = cancel_link(external_invoices_group_person_path(@group, @person)) +- else + .alert.alert-warning= t(".alert_warning") + = cancel_link(external_invoices_group_person_path(@group, @person)) diff --git a/config/locales/wagon.de.yml b/config/locales/wagon.de.yml index 7edfaae69..19db234c3 100644 --- a/config/locales/wagon.de.yml +++ b/config/locales/wagon.de.yml @@ -24,6 +24,13 @@ de: activemodel: attributes: + people/membership/invoice_form: + reference_date: "Stichtag" + invoice_date: "Rechnungsdatum" + send_date: "Versanddatum" + discount: "Rabatt" + new_entry: "Eintrittsgebühr" + section_id: "Mitgliedschaft" wizards/steps/signup/main_email_field: email: E-Mail wizards/steps/signup/person_fields: @@ -97,6 +104,7 @@ de: youth_not_allowed_in_family: | Jugendliche im Alter von 18 bis 21 Jahre können nicht in einer Familienmitgliedschaft aufgenommen werden. + invalid_date_range: "muss zwischen %{start_on} und %{end_on} liegen" models: household: @@ -128,7 +136,7 @@ de: must_be_family_main_person: muss Hauptperson der Familie sein person: data_quality_issue: - message: '%{attr} %{key}' + message: "%{attr} %{key}" wizards/steps/choose_sektion: requires_admin: Wir bitten dich den gewünschten Sektionswechsel @@ -1129,8 +1137,14 @@ de: membership_years: "Anzahl Mitgliedsjahre: %{membership_years}" membership_invoices: - success_notice: "Die Rechnung wurde erfolgreich an Abacus übermittelt. Auftrag-Nr. %{abacus_key}" - alert_notice: "Die Rechnung konnte nicht an Abacus übermittelt werden. Fehlermeldung: %{message}" + success_notice: "Die gewünschte Rechnung wird erzeugt und an Abacus übermittelt" + 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" terminate_sac_memberships: create: diff --git a/config/routes.rb b/config/routes.rb index a677f22c0..6d8736fe5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -36,7 +36,7 @@ resources :people, only: [] do resources :external_trainings, except: [:edit, :show, :index] - resources :membership_invoices, only: [:create], module: :people + resources :membership_invoices, only: [:create, :new], module: :people resources :sac_remarks, only: %i[index edit update], module: :person resource :join_zusatzsektion, module: :memberships, only: [:show, :create] resource :switch_stammsektion, module: :memberships, only: [:show, :create] diff --git a/spec/controllers/people/membership_invoices_controller_spec.rb b/spec/controllers/people/membership_invoices_controller_spec.rb index 86bcf045c..04cfd1b7e 100644 --- a/spec/controllers/people/membership_invoices_controller_spec.rb +++ b/spec/controllers/people/membership_invoices_controller_spec.rb @@ -9,60 +9,144 @@ describe People::MembershipInvoicesController, type: :controller do let(:person) { people(:mitglied) } - let(:client) { instance_double(Invoices::Abacus::Client) } + let(:today) { Time.zone.today } before { sign_in(people(:admin)) } before do + Role.update_all(delete_on: today.end_of_year) SacMembershipConfig.update_all(valid_from: 2015) SacSectionMembershipConfig.update_all(valid_from: 2015) - expect(Invoices::Abacus::Client).to receive(:new).and_return(client) end describe "POST create" do - it "sends invoice to abacus" do - person.update!(zip_code: 3600, town: "Thun") - - expect(client).to receive(:create).with(:subject, Hash).and_return({id: 7}) - expect(client).to receive(:create).with(:address, Hash) - expect(client).to receive(:create).with(:communication, Hash) - expect(client).to receive(:create).with(:customer, Hash) - expect(client).to receive(:create).with(:sales_order, Hash).and_return({sales_order_id: 19}) - expect(client).to receive(:endpoint).with(:sales_order, Hash) - expect(client).to receive(:request).with(:post, String, Hash) - + it "creates external invoice" do expect do - post :create, params: {group_id: groups(:bluemlisalp_mitglieder).id, person_id: person.id, date: "2015-03-01"} + post :create, params: { + group_id: groups(:bluemlisalp_mitglieder).id, + person_id: person.id, + people_membership_invoice_form: { + reference_date: today, + invoice_date: today, + send_date: today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } + } end.to change { ExternalInvoice.count }.by(1) - expect(response).to redirect_to(group_person_path(groups(:bluemlisalp_mitglieder).id, person.id)) + expect(response).to redirect_to(external_invoices_group_person_path(groups(:bluemlisalp_mitglieder).id, person.id)) expect(flash[:alert]).to be_nil - expect(flash[:notice]).to eq("Die Rechnung wurde erfolgreich an Abacus übermittelt. Auftrag-Nr. 19") + expect(flash[:notice]).to eq("Die gewünschte Rechnung wird erzeugt und an Abacus übermittelt") end - it "handles failure in abacus request" do - person.update!(zip_code: 3600, town: "Thun") + 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_form: { + reference_date: today, + invoice_date: today, + send_date: "", + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } + } + end.not_to change { ExternalInvoice.count } - expect(client).to receive(:create).with(:subject, Hash).and_return({id: 7}) - expect(client).to receive(:create).with(:address, Hash) - expect(client).to receive(:create).with(:communication, Hash) - expect(client).to receive(:create).with(:customer, Hash) - expect(client).to receive(:create).with(:sales_order, Hash).and_raise("Something went wrong") + expect(response).to have_http_status(:unprocessable_entity) + end - expect do - post :create, params: {group_id: groups(:bluemlisalp_mitglieder).id, person_id: person.id, date: "2015-03-01"} - end.to change { ExternalInvoice.count }.by(1) + 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_form: { + reference_date: "", + invoice_date: "", + send_date: today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } + } + end.not_to change { ExternalInvoice.count } + expect(response).to have_http_status(:unprocessable_entity) + end - expect(response).to redirect_to(group_person_path(groups(:bluemlisalp_mitglieder).id, person.id)) - expect(flash[:alert]).to eq("Die Rechnung konnte nicht an Abacus übermittelt werden. Fehlermeldung: Something went wrong") - 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_form: { + reference_date: today.next_year(5), + invoice_date: today, + send_date: today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } + } + end.not_to change { ExternalInvoice.count } + + expect(response).to have_http_status(:unprocessable_entity) + 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_form: { + reference_date: today.last_year, + invoice_date: today, + send_date: today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } + } + end.not_to change { ExternalInvoice.count } + + expect(response).to have_http_status(:unprocessable_entity) + end + + it "doesnt create external invoice send date is next year" do + expect do + post :create, params: { + group_id: groups(:bluemlisalp_mitglieder).id, + person_id: person.id, + people_membership_invoice_form: { + reference_date: today, + invoice_date: today, + send_date: today.next_year, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 0 + } + } + end.not_to change { ExternalInvoice.count } + + expect(response).to have_http_status(:unprocessable_entity) + end - it "cannot send abacus if address is incomplete" do - people(:mitglied).update!(zip_code: nil, town: nil) - post :create, params: {group_id: groups(:bluemlisalp_mitglieder).id, person_id: person.id, date: "2015-03-01"} + 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_form: { + reference_date: today, + invoice_date: today, + send_date: today, + section_id: groups(:bluemlisalp_mitglieder).id, + discount: 16 + } + } + end.not_to change { ExternalInvoice.count } - expect(response).to redirect_to(group_person_path(groups(:bluemlisalp_mitglieder).id, person.id)) - expect(flash[:alert]).to eq("Die Rechnung konnte nicht an Abacus übermittelt werden. Fehlermeldung: Ort muss ausgefüllt werden, PLZ muss ausgefüllt werden") + expect(response).to have_http_status(:unprocessable_entity) + end end end end