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

[MIG][13.0] account_financial_risk: Migration to v13.0 #52

Closed

Conversation

sergio-teruel
Copy link
Contributor

@sergio-teruel sergio-teruel commented Feb 8, 2020

cc @Tecnativa TT21602
@carlosdauden @rafaelbn review please

@sergio-teruel sergio-teruel force-pushed the 13.0-mig-account_financial_risk branch 2 times, most recently from 231a7f8 to 7642a95 Compare February 9, 2020 09:49
@sergio-teruel sergio-teruel changed the title [WIP][MIG][13.0] account_financial_risk: Migration to v13.0 [MIG][13.0] account_financial_risk: Migration to v13.0 Feb 9, 2020
if not customers:
return
groups = self._risk_account_groups()
for key, group in groups.items():
for _key, group in groups.items():
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for _key, group in groups.items():
for group in groups.values():

@sergio-teruel
Copy link
Contributor Author

@carlosdauden Changes done

Copy link
Contributor

@alan196 alan196 left a comment

Choose a reason for hiding this comment

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

👍 Functional and Technical Review

carlosdauden and others added 21 commits February 27, 2020 17:46
* [9.0][IMP] partner_financial_risk: Improve performance

* [9.0][IMP] partner_financial_risk: Improve tests and partner hierarchy

* [9.0][IMP] partner_stock_risk: Test
- Refunds adds partner risk instead of reduce.
- If date_maturity changes to previous today date "never" compute partner risk.
* [9.0][IMP] partner_financial_risk: Improve multicompany cron

* [9.0][IMP] partner_financial_risk: Exec with sudo

(cherry picked from commit 6590bbf)
oca-travis and others added 13 commits February 27, 2020 17:46
Currently translated at 71.2% (52 of 73 strings)

Translation: credit-control-12.0/credit-control-12.0-account_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-account_financial_risk/fr/
Currently translated at 100.0% (73 of 73 strings)

Translation: credit-control-12.0/credit-control-12.0-account_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-account_financial_risk/fr/
Currently translated at 79.5% (58 of 73 strings)

Translation: credit-control-12.0/credit-control-12.0-account_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-account_financial_risk/it/
Currently translated at 80.8% (59 of 73 strings)

Translation: credit-control-12.0/credit-control-12.0-account_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-account_financial_risk/it/
Currently translated at 100.0% (73 of 73 strings)

Translation: credit-control-12.0/credit-control-12.0-account_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-account_financial_risk/es/
Currently translated at 13.7% (10 of 73 strings)

Translation: credit-control-12.0/credit-control-12.0-account_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-account_financial_risk/es_CL/
Currently translated at 100.0% (73 of 73 strings)

Translation: credit-control-12.0/credit-control-12.0-account_financial_risk
Translate-URL: https://translation.odoo-community.org/projects/credit-control-12-0/credit-control-12-0-account_financial_risk/es_VE/
@alan196
Copy link
Contributor

alan196 commented Feb 28, 2020

I have an issue when I try to open a partner that is not a customer.

res.partner(23),risk_exception

I have created the following PR to fix this Tecnativa#4

Copy link
Contributor

@alan196 alan196 left a comment

Choose a reason for hiding this comment

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

Add changes of PR Tecnativa#4

[FIX] account_financial_risk: allow to define non stored computed fields in all partners
@pedrobaeza
Copy link
Member

I have added the PR but Travis is red now. I agree on not classifying by customer_rank (show for all), but if you think it shouldn't be shown only for partners with rank, please modify it. @carlosdauden what is your opinion on this?

Copy link
Contributor

@gdgellatly gdgellatly left a comment

Choose a reason for hiding this comment

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

Static Review, 1 very minor nitpick. I don't even care about it and I could be wrong for all I know. The only other comment is the invoice_unpaid_margin title in settings is not great English, but seeing it was ported and I imagine translations etc rely on it, it wasn't bad enough to make a fuss over.

new_margin = vals.get('invoice_unpaid_margin')
if (new_margin is not None and
all(new_margin == x.invoice_unpaid_margin for x in self)):
vals = vals.copy()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick, is copy really needed here?

@pedrobaeza
Copy link
Member

Closing in favor of #67

@pedrobaeza pedrobaeza closed this Jun 9, 2020
@pedrobaeza pedrobaeza added this to the 13.0 milestone Jun 9, 2020
@yajo yajo deleted the 13.0-mig-account_financial_risk branch November 16, 2020 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet