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
[9.0] [ADD] partner_financial_risk #292
Conversation
570bd03
to
789e093
Compare
|
||
eval_risk_sale_order = fields.Boolean( | ||
string="Eval Sale Orders", help="Compute in total risk") | ||
max_risk_sale_order = fields.Monetary( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fields.Monetary
exists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't documented, but :https://github.com/odoo/odoo/blob/e37ba6183269b0d075833efbb9bf26e99404247e/openerp/fields.py#L1131-L1130
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thanks! Shouldn't you be using currency_field then?
16aad33
to
b0dc395
Compare
9fc6c7a
to
678711d
Compare
Travis fails with base_location_geonames_import module |
LINT_CHECK fails in base_partner_merge. Unrelated travis fail |
|
||
To configure this module, you need to: | ||
|
||
#. Go to *Invoicing > Configuration > Settings > Invoicing & Payments > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
split the line
@carlosdauden travis fails, could you check it please? |
6414425
to
d23eeec
Compare
@rafaelbn I explain travis errors a few comments before |
Better call the second module partner_sale_risk and the third one partner_stock_risk |
|
||
Adds a new page in partner to manage its *Financial Risk*. | ||
|
||
If any limit is exceed the partner gets forbidden to confirm sale orders. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/sale orders/the corresponding sales order or customer invoice (this is needed to be implemented if not already).
@makwin Now looks fine |
34066ef
to
665731a
Compare
580e2b0
to
3af944c
Compare
3af944c
to
a2754c2
Compare
Now, this PR is to review |
functional test 👍 |
|
||
@api.multi | ||
def invoice_open(self): | ||
if not self.env.context.get('bypass_risk', False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For avoiding this cyclomatic complexity, better to repeat the return:
if not self.env.context.get('bypass_risk', False):
return self.signal_workflow('invoice_open')
for invoice in self:
...
return self.signal_workflow('invoice_open')
|
||
@api.model | ||
def _get_depends_compute_risk_exception(self): | ||
# TODO: Improve code without lose performance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# TODO: Improve code without performance loss
'not exceeded, considering Due Margin set in account ' | ||
'settings') | ||
risk_invoice_unpaid_include = fields.Boolean( | ||
string='Include Due Invoices', help='Full risk computation') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change label to talk about 'unpaid', not 'due invoices', which is not the same.
class ResCompany(models.Model): | ||
_inherit = 'res.company' | ||
|
||
invoice_due_margin = fields.Integer( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be called invoice_unpaid_margin
for keeping homogeneity
return res | ||
|
||
@api.model | ||
def process_due_invoice(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Call this method process_unpaid_invoices
for consistency (in plural, as you're processing all the unpaid, not only one). Singular form is only used in model names and module names, not in methods.
|
||
Adds a new page in partner to manage its *Financial Risk*. | ||
|
||
If any limit is exceed the partner gets forbidden to confirm customer invoice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If any limit is exceeded, you won't be able to confirm any of its invoices unless you are authorized.
To configure this module, you need to: | ||
|
||
#. Go to *Invoicing > Configuration > Settings > Invoicing & Payments* | ||
#. Go to *Financial Risk* and set *Due Margin* (This field adds days of margin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the *Financial Risk* section, fill *Unpaid Margin* for setting the number of days to last after the due date to consider an invoice as unpaid.
To configure this module, you need to: | ||
|
||
#. Go to *Invoicing > Configuration > Settings > Invoicing & Payments* | ||
#. Go to *Financial Risk* and set *Due Margin* (This field adds days of margin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which are the authorized groups? Please explain here.
To use this module, you need to: | ||
|
||
#. Go to *Customers > Financial Risk* | ||
#. Set limits and choose options to compute in credit limit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explain which are the possible limits and their use.
|
||
To configure this module, you need to: | ||
|
||
#. Go to *Invoicing > Configuration > Settings > Invoicing & Payments* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Invoicing > Invoicing/Accounting (it can have both names, depending on the installed modules).
|
||
To use this module, you need to: | ||
|
||
#. Go to *Customers > Financial Risk* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#. Go to *Invoicing/Accounting > Sales > Customers*.
#. Select an existing customer or create a new one.
#. Open the *Financial Risk* tab.
|
||
#. Go to *Customers > Financial Risk* | ||
#. Set limits and choose options to compute in credit limit. | ||
#. Go to *Invoicing > Sales > Customer invoices* and create new customer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#. Test the restriction trying to create an invoice for the partner for an amount higher of the limit you have set.
Please include also Spanish translation. |
@carlosdauden @pedrobaeza Did you consider this? https://github.com/OCA/account-financial-tools/blob/8.0/account_credit_control. Why did you choose not to reuse? Naming a module 'financial risk' is too broad. What you are managing here is a customer credit risk. Customer credit control is the usual name given to this process. |
@jbeficent This is more than a credit control. See the possible options. I know that module, but it doesn't make the same and it's not the same approach. |
@pedrobaeza OK. Is interesting. I worked in something similar long ago here: OCA/sale-financial#9. |
Adds a new page in partner to manage its Financial Risk.
If any limit is exceed the partner gets forbidden to confirm sale orders.
@Tecnativa @pedrobaeza