Feature/tw automatic group order invoice generation #907
Conversation
…or invoice generation and sending
…itable in admin configs
|
This seems like a great feature. Thanks for taking the time to draft this! |
|
Hey. Excuse my late reply. |
add option vor vat_exempt inoices ~ refs automatic go invoices fix order.ordergroup nil pointer error ~ automatic go invoices fix nil pointer~ refs automatic go invoice rubocop styling ~ refs automatic go invoice
… github.com:Viehlieb/foodsoft into feature/tw_automatic_group_order_invoice_generation
…w_automatic_group_order_invoice_generation
… due to hover property
There was a problem hiding this comment.
When a link was provided for article details, it was not possible to follow the link because of the :hover feature.
Behavior before commit:
Hover over articles would render the article infos and if no article is hovered there's a green box showing
Behavior after commit:
Hover won't do anything.
Click article will render background of the article green and article details will be shown in green box in the footer
So it is possible to follow links to article details in the green box
|
Is there anything more needed to merge this feature into master branch? |
|
first: sorry for the late reply there are a bunch of style violations and i have no idea why they are not marked by the CI system 😞 |
|
Hey. Thanks for the hints.
|
| rescue => error | ||
| redirect_back fallback_location: root_path, notice: 'Something went wrong', :alert => I18n.t('errors.general_msg', :msg => error) | ||
| end | ||
| end |
There was a problem hiding this comment.
please add a newline at the end of the file
| end | ||
| end | ||
| else raise RecordInvalid | ||
| redirect_back fallback_location: root_path, notice: 'Something went wrong', :alert => I18n.t('errors.general_msg', :msg => "#{error} " + I18n.t('errors.check_tax_number')) |
There was a problem hiding this comment.
- what kind of error do you want to catch here?
- please use translations for user visible strings.
- please do not use
noticeandalertat the same time - there is usually no need to use
I18n.tand you could use justtin the controllers - please use newer syntax
name: valueinstead of:name => value
| if FoodsoftConfig[:contact][:tax_number] | ||
| respond_to do |format| | ||
| format.pdf do | ||
| send_group_order_invoice_pdf @group_order_invoice if FoodsoftConfig[:contact][:tax_number] |
There was a problem hiding this comment.
you check already for the tax_number in line 6. what's the purpose of the second check here?
|
|
||
| def show | ||
| @group_order_invoice = GroupOrderInvoice.find(params[:id]) | ||
| if FoodsoftConfig[:contact][:tax_number] |
There was a problem hiding this comment.
why can't we show an invoice without an tax number?
|
|
||
| def destroy | ||
| goi = GroupOrderInvoice.find(params[:id]) | ||
| @order = goi.group_order.order |
There was a problem hiding this comment.
what's the purpose of @order here?
| if FoodsoftConfig[:contact][:tax_number].blank? | ||
| errors.add(:group_order_invoice, "Keine Steuernummer in FoodsoftConfig :contact gesetzt") | ||
| end |
There was a problem hiding this comment.
this method does not access the database and therefore should be removed from the model and move to a better location (BTW: there are user visible string which are not translated)
| def init | ||
| self.invoice_number = generate_invoice_number(1) unless self.invoice_number | ||
| self.invoice_date = Time.now unless self.invoice_date | ||
| self.payment_method = FoodsoftConfig[:group_order_invoices]&.[](:payment_method) || I18n.t('activerecord.attributes.group_order_invoice.payment_method') unless self.payment_method |
There was a problem hiding this comment.
why is the payment method stored in FoodsoftConfig?
please do not use translated strings in the model. depending on the locale of the current user different data would be used and stored in the database
| end | ||
|
|
||
| def init | ||
| self.invoice_number = generate_invoice_number(1) unless self.invoice_number |
There was a problem hiding this comment.
please use a before_validation hook instead (see e.g. set_password in the User model)
| end | ||
|
|
||
| def name | ||
| I18n.t('activerecord.attributes.group_order_invoice.name') + "_#{invoice_number}" |
There was a problem hiding this comment.
the translation should be moved from the model to a controller, since the model should be language independent
| I18n.t('activerecord.attributes.group_order_invoice.name') + "_#{invoice_number}" | ||
| end | ||
|
|
||
| def load_data_for_invoice |
There was a problem hiding this comment.
why do you copy everything here? usually working with a GroupOrderInvoice instance should be the way to go.
Offers the possibility to generate Invoices for given Group Orders.
Generation is possible after an order is closed.
If order is 'settled' then there's additionally the possibility to let the software send e-mails automatically on button press. The corresponding invoices will be sent as attachment via mail to the order group members.
Notes:
To use automatic sending of invoices, select option in "Admin/Finance" config
You can enter a "payment method " in "Admin/Finance"
To make things work, it is mandatory to enter a tax_number in "Admin/Foodcoop" config
As for now "payment method" determines the "body" of the invoice.
An example of new options and the resulting invoice can be found here:
Additionally for older and already settled orders, group order invoices can be generated manually under "finance/balancing"
Invoices will be generated given the momentarily given information about invoicee and invoicer (tax_number, contact details...). SO take care when delting an invoice.