Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.0][IMP] contract. Enable automatic invoice generation before invoice date. #282

Merged
merged 2 commits into from
Jun 5, 2019

Conversation

NL66278
Copy link

@NL66278 NL66278 commented Mar 4, 2019

Implements the functional enhancement requested here: #137

Per company it will be possible to specify how many days before the next invoice date the invoices for recurring contracts will be generated.

Copy link

@ntsirintanis ntsirintanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, LGTM.

@sbidoul
Copy link
Member

sbidoul commented Mar 5, 2019

@sbejaoui can you review this? Is there something comparable in #207 ?

@NL66278 if you are interested in 12.0, I encourage you to have a look at #207. It's big change (moving recurrence to contract lines). Nevertheless it should migrate smoothly from 10.0. And it enables many additional use cases. There is also #208 which enables contract creation from sale orders in a much more robust way.

@sbejaoui
Copy link
Contributor

sbejaoui commented Mar 5, 2019

LGTM,

IMO, it is better if it is in a separate module.

@NL66278
Copy link
Author

NL66278 commented Mar 5, 2019

@sbejaoui Why a separate module? If the user does not need/want the functionality, nothing changes.

@sbejaoui
Copy link
Contributor

sbejaoui commented Mar 5, 2019

My idea is to keep cron_recurring_create_invoice as simple as possible and make separate module for each use case attending to influence the way it works. Otherwise, with all use cases we can imagine, we can quickly bring a big method hard to maintain, hard to debug.

This why we did some refactoring in #207 to make this logic overridable and easy to evolve.

@babatoko
Copy link

babatoko commented Mar 5, 2019

@NL66278,
It looks good to me. Which version of odoo is it built for. If not for 10 would it be possible to backport to it?

Cheers.

@NL66278
Copy link
Author

NL66278 commented Mar 5, 2019

@babatoko It is for 10.0

@babatoko
Copy link

babatoko commented Mar 5, 2019

That's a very good news thanks @NL66278. I'm going to test further.

Cheers

Copy link
Member

@flotho flotho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@mamojdick
Copy link

mamojdick commented Mar 14, 2019

@NL66278 nice. I tried and appreciated this feature, but the "date_invoice" field is filled with "recurring_next_date". would it be possible to leave it empty?

@NL66278
Copy link
Author

NL66278 commented Mar 15, 2019

@mamojdick Thanks for your appreciation! But with or without my PR, the date_invoice field is filled from recurring_next_date. My PR just enables creating the invoices some configurable time in advance, to allow checking, sending them out already, etc. What would the use-case be for not filling date_invoice? Actually that date would then be set to the current date, defeating the purpose of my PR. Maybe I did not understand your question well, can you elaborate?

@mamojdick
Copy link

But with or without my PR, the date_invoice field is filled from recurring_next_date.

Yes i know this.

Actually that date would then be set to the current date

Yes that is my idea: i want to create hundreds invoices for an ISP like this:
#START# 1° day of the month
#END# last dey of the month
X day before (with your PR)
check it and validate it in X+Y (where y is days spent to check)
so date_invoice is current day.

I am in Italy and we introduce the e-invoice, the date_invoice must be the same date we send it, not in the future.

I hope I was clear.
TNX

Copy link
Member

@angelmoya angelmoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small change, but LGTM

for company in company_model.search([]):
days_before = company.contract_pregenerate_days or 0
cutoffdate = (today + relativedelta(days=days_before)).strftime(
'%Y-%m-%d')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better use:

from odoo.tools import DEFAULT_SERVER_DATE_FORMAT

.strftime(DEFAULT_SERVER_DATE_FORMAT)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@angelmoya Thanks! Done!

@max3903 max3903 added this to the 10.0 milestone May 22, 2019
@rafaelbn
Copy link
Member

@NL66278 could you squash commits please before merging? Thanks!

@rafaelbn
Copy link
Member

rafaelbn commented Jun 5, 2019

Thanks a lot for this improvement @NL66278 . Just remember if you want to forward port it to 11.0 and 12.0 as you commented in #137 (comment)

@rafaelbn rafaelbn merged commit 14f3cc2 into OCA:10.0 Jun 5, 2019
sergiocorato pushed a commit to efatto/contract that referenced this pull request Jul 4, 2019
…ice date. (OCA#282)

* [IMP] contract. Enable automatic invoice generation before invoice date.

* [IMP] contract pregenerate - improvements after review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.