Merge lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix into lp:~account-report-core-editor/account-financial-report/7.0

Proposed by Luc De Meyer (Noviat)
Status: Work in progress
Proposed branch: lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix
Merge into: lp:~account-report-core-editor/account-financial-report/7.0
Diff against target: 28 lines (+3/-15)
1 file modified
account_financial_report_webkit/report/common_partner_reports.py (+3/-15)
To merge this branch: bzr merge lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix
Reviewer Review Type Date Requested Status
Pedro Manuel Baeza Needs Resubmitting
Nicolas Bessi - Camptocamp (community) Needs Fixing
Yannick Vaucher @ Camptocamp Needs Information
Guewen Baconnier @ Camptocamp Needs Fixing
Review via email: mp+193717@code.launchpad.net

Description of the change

Add support for customers who start to use OpenERP accounting after one of more years of using OpenERP for other purposes by changing the '_get_first_special_period' logic.

New logic:

First special period is first opening period since you can have years without opening period, e.g. OpenERP systems used initially for non-accounting purposes.

To post a comment you must log in.
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hi,

Thanks for the proposal.

While I understand the logic you want to change I don't understand what problem you want to fix (I could maybe guess but I'm not sure).
The code you propose was the same and we had to replace it by the more convoluted version that you want to replace.

This method is used by the partner reports (computation of the initial balance) and we need to find the opening period of the "first year _with_ accounting entries". We want the balance of the very first opening period. If we just search by date, we may find an opening period on a year which has not been used for the accounting => balance would be mistakenly 0.

review: Needs Fixing
Revision history for this message
Luc De Meyer (Noviat) (luc-demeyer) wrote :

The problem is that you have many OpenERP systems out there which have been used for years for other purposes than accounting.
During those years, accounting entries have been generated but these entries were only used for e.g. one department or only sale/purchase of goods, ...
Real reconciliations in those systems are usually done outside OpenERP (in the accounting software).

Hence the partner data in those years are incorrect and the reports should start from the point where a first correct opening balance has been uploaded, not the first year with (incorrect) accounting entries.

The MP is a potential way to handle this : we assume that there is no opening period as long as the system is not used for full accounting purposes.
In most cases where we added accounting to legacy OpenERP systems, this was the case (but not always).
Another way to tackle this issue is to add a configuration parameter to the 'company' settings to indicate what the first special period is.

Regards,
Luc

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Guewen Baconnier @ Camptocamp
Sent: maandag 4 november 2013 10:01
To: <email address hidden>
Subject: Re: [Merge] lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix into lp:account-financial-report

Review: Needs Fixing

Hi,

Thanks for the proposal.

While I understand the logic you want to change I don't understand what problem you want to fix (I could maybe guess but I'm not sure).
The code you propose was the same and we had to replace it by the more convoluted version that you want to replace.

This method is used by the partner reports (computation of the initial balance) and we need to find the opening period of the "first year _with_ accounting entries". We want the balance of the very first opening period. If we just search by date, we may find an opening period on a year which has not been used for the accounting => balance would be mistakenly 0.

--
https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix/+merge/193717
You are the owner of lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix.

Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

> The problem is that you have many OpenERP systems out there which have been
> used for years for other purposes than accounting.
> During those years, accounting entries have been generated but these entries
> were only used for e.g. one department or only sale/purchase of goods, ...
> Real reconciliations in those systems are usually done outside OpenERP (in the
> accounting software).
>
> Hence the partner data in those years are incorrect and the reports should
> start from the point where a first correct opening balance has been uploaded,
> not the first year with (incorrect) accounting entries.

Thanks for the explanation, I understand better.

>
> The MP is a potential way to handle this : we assume that there is no opening
> period as long as the system is not used for full accounting purposes.
> In most cases where we added accounting to legacy OpenERP systems, this was
> the case (but not always).

Understand. Though, that's not working if you have a year without accounting entries at all and 1 (empty) opening period.

> Another way to tackle this issue is to add a configuration parameter to the
> 'company' settings to indicate what the first special period is.

Considering all the cases, that's probably the sole available option (eventually with a callback on the current method if not defined).

Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

2013/11/4 Luc De Meyer (Noviat) <email address hidden>

> The problem is that you have many OpenERP systems out there which have
> been used for years for other purposes than accounting.
> During those years, accounting entries have been generated but these
> entries were only used for e.g. one department or only sale/purchase of
> goods, ...
> Real reconciliations in those systems are usually done outside OpenERP (in
> the accounting software).
>

Concept review: No test yet.

Hello Luc.

We face this problem too, but we used to fix the not reconciled entries
(Importing historical paymens too globally and in some cases just to close
correctly all periods when we will start to use the accounting approach),
it means, if we report some periods in one way and other periods in other
way it can bring inconsistencies don't you think?. IMHO wire in some way
data to allow reports to show something different of what is loaded in the
system is not a good approach.

BTW I understand the point, can you put some "Demo" data just to validate
this specific case for future usage? in this way we can have them as base
cases to show everybody in what cases it is necessary.

Best regards.

--
--------------------
Saludos Cordiales

Nhomar G. Hernandez M.
+58-414-4110269
Skype: nhomar00
Web-Blog: http://geronimo.com.ve
Servicios IT: http://vauxoo.com
Linux-Counter: 467724
Correos:
<email address hidden>
<email address hidden>
twitter @nhomar

Revision history for this message
Luc De Meyer (Noviat) (luc-demeyer) wrote :

> Understand. Though, that's not working if you have a year without accounting entries at all and 1 (empty) opening period.

We have seen this situation before and we just removed the opening period. This is of course a workaround that doesn't tackle the real issue.

> Considering all the cases, that's probably the sole available option (eventually with a callback on the current method if not defined).

I agree that this is probably the cleanest solution.

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Guewen Baconnier @ Camptocamp
Sent: maandag 4 november 2013 10:39
To: <email address hidden>
Subject: Re: RE: [Merge] lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix into lp:account-financial-report

> The problem is that you have many OpenERP systems out there which have
> been used for years for other purposes than accounting.
> During those years, accounting entries have been generated but these
> entries were only used for e.g. one department or only sale/purchase of goods, ...
> Real reconciliations in those systems are usually done outside OpenERP
> (in the accounting software).
>
> Hence the partner data in those years are incorrect and the reports
> should start from the point where a first correct opening balance has
> been uploaded, not the first year with (incorrect) accounting entries.

Thanks for the explanation, I understand better.

>
> The MP is a potential way to handle this : we assume that there is no
> opening period as long as the system is not used for full accounting purposes.
> In most cases where we added accounting to legacy OpenERP systems,
> this was the case (but not always).

Understand. Though, that's not working if you have a year without accounting entries at all and 1 (empty) opening period.

> Another way to tackle this issue is to add a configuration parameter
> to the 'company' settings to indicate what the first special period is.

Considering all the cases, that's probably the sole available option (eventually with a callback on the current method if not defined).

--
https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix/+merge/193717
You are the owner of lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix.

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

To have it as reference, there is another old MP that treats the same topic (and I think with the same solution):

https://code.launchpad.net/~account-report-core-editor/account-financial-report/fix-opening-period-search/+merge/162772

Regards.

Revision history for this message
Luc De Meyer (Noviat) (luc-demeyer) wrote :

>We face this problem too, but we used to fix the not reconciled entries (Importing historical paymens too globally and in some cases just to close correctly all periods when we will start to use the accounting approach), it means, if we >report some periods in one way and other periods in other way it can bring inconsistencies don't you think?. IMHO wire in some way data to allow reports to show something different of what is loaded in the system is not a good >approach.

Our experience: spending time to fix historical accounting records whereas the real historical data sits on another system is usually not the solution that the customer wants to pay for.
I have the impression that we move to a consensus to fix this issue by adding a parameter on the company settings with callback to current method if not defined.

Guewen,

Do you want Noviat to add this to the code and make a new merge proposal or do you prefer that C2C makes this fix ?

Regards,
Luc

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Nhomar - Vauxoo
Sent: maandag 4 november 2013 10:51
To: <email address hidden>
Subject: Re: [Merge] lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix into lp:account-financial-report

2013/11/4 Luc De Meyer (Noviat) <email address hidden>

> The problem is that you have many OpenERP systems out there which have
> been used for years for other purposes than accounting.
> During those years, accounting entries have been generated but these
> entries were only used for e.g. one department or only sale/purchase
> of goods, ...
> Real reconciliations in those systems are usually done outside OpenERP
> (in the accounting software).
>

Concept review: No test yet.

Hello Luc.

We face this problem too, but we used to fix the not reconciled entries (Importing historical paymens too globally and in some cases just to close correctly all periods when we will start to use the accounting approach), it means, if we report some periods in one way and other periods in other way it can bring inconsistencies don't you think?. IMHO wire in some way data to allow reports to show something different of what is loaded in the system is not a good approach.

BTW I understand the point, can you put some "Demo" data just to validate this specific case for future usage? in this way we can have them as base cases to show everybody in what cases it is necessary.

Best regards.

--
--------------------
Saludos Cordiales

Nhomar G. Hernandez M.
+58-414-4110269
Skype: nhomar00
Web-Blog: http://geronimo.com.ve
Servicios IT: http://vauxoo.com
Linux-Counter: 467724
Correos:
<email address hidden>
<email address hidden>
twitter @nhomar

https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix/+merge/193717
You are the owner of lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix.

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Good improvement!

review: Approve (code review)
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

@Luc anything was done about this proposed change ?

> I have the impression that we move to a consensus to fix this issue by adding a parameter on the company settings with callback to current method if not defined.

review: Needs Information
Revision history for this message
Luc De Meyer (Noviat) (luc-demeyer) wrote :

No,

If you agree with the proposed approach, than I an make an MP.

Luc

www.noviat.com
Rusatiralaan 1, 1083 Brussel
+32 2 808 86 38

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Yannick Vaucher @ Camptocamp
Sent: vrijdag 14 maart 2014 9:48
To: <email address hidden>
Subject: Re: [Merge] lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix into lp:account-financial-report

Review: Needs Information

@Luc anything was done about this proposed change ?

> I have the impression that we move to a consensus to fix this issue by adding a parameter on the company settings with callback to current method if not defined.
--
https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix/+merge/193717
You are the owner of lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix.

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Hello,

The parameter/fallback solution seems reasonable assuming that period parameter is not mandatory.
For me you can update the MP with that solution.

Also as the change will alter data structure the revision number of the module must be increased in MP.

I'm putting the MP in work in progress while waiting new patch.

Thanks for the work.

Regards

Nicolas

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) :
review: Needs Fixing
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

hello,

Any news on this one

Revision history for this message
Luc De Meyer (Noviat) (luc-demeyer) wrote :

I am just back from a week of holiday.
I have reserved a bit of time in my agenda to make the MP by the end of this week.

Luc

www.noviat.com
Rusatiralaan 1, 1083 Brussel
+32 2 808 86 38

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Nicolas Bessi - Camptocamp
Sent: donderdag 10 april 2014 14:12
To: <email address hidden>
Subject: Re: [Merge] lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix into lp:account-financial-report

hello,

Any news on this one
--
https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix/+merge/193717
You are the owner of lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix.

Revision history for this message
Luc De Meyer (Noviat) (luc-demeyer) wrote :

MP: cf. https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix2/+merge/216380

www.noviat.com
Rusatiralaan 1, 1083 Brussel
+32 2 808 86 38

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Luc De Meyer (Noviat)
Sent: maandag 14 april 2014 11:36
To: <email address hidden>
Subject: RE: [Merge] lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix into lp:account-financial-report

I am just back from a week of holiday.
I have reserved a bit of time in my agenda to make the MP by the end of this week.

Luc

www.noviat.com
Rusatiralaan 1, 1083 Brussel
+32 2 808 86 38

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Nicolas Bessi - Camptocamp
Sent: donderdag 10 april 2014 14:12
To: <email address hidden>
Subject: Re: [Merge] lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix into lp:account-financial-report

hello,

Any news on this one
--
https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix/+merge/193717
You are the owner of lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix.

https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix/+merge/193717
You are the owner of lp:~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix.

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :
review: Needs Resubmitting

Unmerged revisions

57. By Luc De Meyer

fix first special period

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'account_financial_report_webkit/report/common_partner_reports.py'
--- account_financial_report_webkit/report/common_partner_reports.py 2013-05-07 06:56:51 +0000
+++ account_financial_report_webkit/report/common_partner_reports.py 2013-11-03 20:56:22 +0000
@@ -64,21 +64,9 @@
6464
65 :return: browse record of the first special period.65 :return: browse record of the first special period.
66 """66 """
67 move_line_obj = self.pool.get('account.move.line')67 period_obj = self.pool.get('account.period')
68 first_entry_id = move_line_obj.search(68 first_special_period_id = period_obj.search(self.cr, self.uid, [('special', '=', True)], order='date_start ASC', limit=1)
69 self.cr, self.uid, [], order='date ASC', limit=1)69 return first_special_period_id and period_obj.browse(self.cr, self.uid, first_special_period_id[0]) or None
70 # it means there is no entry at all, that's unlikely to happen, but
71 # it may so
72 if not first_entry_id:
73 return
74 first_entry = move_line_obj.browse(self.cr, self.uid, first_entry_id[0])
75 fiscalyear = first_entry.period_id.fiscalyear_id
76 special_periods = [period for period in fiscalyear.period_ids if period.special]
77 # so, we have no opening period on the first year, nothing to return
78 if not special_periods:
79 return
80 return min(special_periods,
81 key=lambda p: datetime.strptime(p.date_start, DEFAULT_SERVER_DATE_FORMAT))
8270
83 def _get_period_range_from_start_period(self, start_period, include_opening=False,71 def _get_period_range_from_start_period(self, start_period, include_opening=False,
84 fiscalyear=False,72 fiscalyear=False,