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

7.0 sales analysis reporting in base currency #25

Conversation

llacroix
Copy link

No description provided.

sales_analysis_reporting_in_base_currency
- sales_analysis_reporting_in_base_currency_inherited
- sales_analysis_converting_sale_order_line_in_base_currency
The editor open a form view. We have to add an invisible field to
catch the on change events.

Also added a total converted to the form view.
It might not be a good idea to store the fields yet. Stored fields are
computed only once and have to be recalculated.
@joaoalf
Copy link

joaoalf commented Mar 24, 2015

@llacroix Can you please add some tests?

@llacroix
Copy link
Author

ok

@@ -0,0 +1,89 @@
from openerp import tools
Copy link

Choose a reason for hiding this comment

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

Copyright?

@llacroix llacroix force-pushed the 7.0-sales_analysis_reporting_in_base_currency branch from 8678153 to cf235d9 Compare March 25, 2015 16:55
_inherit = 'sale.order.line'

_columns = {
"order_line_currency": fields.many2one(
Copy link

Choose a reason for hiding this comment

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

@llacroix It's not mandatory but I would go with the name currency_id. We know that we are in the order line so the order-line_; prefix is not that relevant and since it's many2one it's more clear to have the _id suffix.

Copy link
Author

Choose a reason for hiding this comment

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

Alright. I'm good with currency_id too

@coveralls
Copy link

Coverage Status

Coverage increased (+11.76%) to 60.0% when pulling 5135cf2 on savoirfairelinux:7.0-sales_analysis_reporting_in_base_currency into 6596f52 on OCA:7.0.

@llacroix
Copy link
Author

llacroix commented Apr 3, 2015

😄

@coveralls
Copy link

Coverage Status

Coverage increased (+24.4%) to 72.63% when pulling a2ece1d on savoirfairelinux:7.0-sales_analysis_reporting_in_base_currency into 6596f52 on OCA:7.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+24.4%) to 72.63% when pulling a2ece1d on savoirfairelinux:7.0-sales_analysis_reporting_in_base_currency into 6596f52 on OCA:7.0.

@llacroix
Copy link
Author

llacroix commented Apr 3, 2015

Maxed the coverage.

@coveralls
Copy link

Coverage Status

Coverage increased (+28.61%) to 76.84% when pulling 090fc5b on savoirfairelinux:7.0-sales_analysis_reporting_in_base_currency into 6596f52 on OCA:7.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+28.61%) to 76.84% when pulling 090fc5b on savoirfairelinux:7.0-sales_analysis_reporting_in_base_currency into 6596f52 on OCA:7.0.

Even if the rounding for subtotal is set to two while price unit is 5,
OpenERP will keep 5 digits everywhere until it gets to the database or
the view. For this reason, we have to do roundings by ourselves in some
places.

In this case, OpenERP seems to be summing the rounded subtotals instead
of doing a sum on all lines and then rounding.
@llacroix llacroix force-pushed the 7.0-sales_analysis_reporting_in_base_currency branch from 40129a5 to fc12487 Compare April 17, 2015 16:43
@coveralls
Copy link

Coverage Status

Coverage increased (+28.73%) to 76.96% when pulling 716d97a on savoirfairelinux:7.0-sales_analysis_reporting_in_base_currency into 6596f52 on OCA:7.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+28.73%) to 76.96% when pulling 716d97a on savoirfairelinux:7.0-sales_analysis_reporting_in_base_currency into 6596f52 on OCA:7.0.

@eLBati
Copy link
Member

eLBati commented Jul 19, 2015

Hi @llacroix
as module description says

This total is wrong
if you have made sales in different currencies as
Odoo ignores currencies in this instance

Shouldn't this be fixed in the core module?

{
'name': 'sales Analysis converting sale order line in base currency',
'version': '0.1',
'author': 'Savoir-faire Linux',
Copy link
Member

Choose a reason for hiding this comment

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

add Odoo Community Association (OCA)

@llacroix
Copy link
Author

@eLBati It's been a long time... Obviously it should be fixed in the core modules but that would also mean that queries would be probably much more complicated than "fetch" and "show".

@eLBati
Copy link
Member

eLBati commented Jan 26, 2016

@llacroix I made something similar for invoice analysis: odoo/odoo#7550
Would that approach be followable/correct?

@max3903 max3903 added this to the 7.0 milestone Feb 11, 2016
@pedrobaeza
Copy link
Member

This is very old and without further activity, so I'm closing. If you are still interested in this, feel free to reopen it.

@pedrobaeza pedrobaeza closed this Apr 28, 2018
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.

7 participants