Merge lp:~account-report-core-editor/account-financial-report/fix-opening-period-search into lp:~account-report-core-editor/account-financial-report/7.0

Proposed by Niels Huylebroeck on 2013-05-07
Status: Work in progress
Proposed branch: lp:~account-report-core-editor/account-financial-report/fix-opening-period-search
Merge into: lp:~account-report-core-editor/account-financial-report/7.0
Diff against target: 31 lines (+6/-15)
1 file modified
account_financial_report_webkit/report/common_partner_reports.py (+6/-15)
To merge this branch: bzr merge lp:~account-report-core-editor/account-financial-report/fix-opening-period-search
Reviewer Review Type Date Requested Status
Pedro Manuel Baeza Resubmit on 2014-08-06
Maxime Chambreuil (http://www.savoirfairelinux.com) Approve on 2013-09-22
Frederic Clementi - Camptocamp 2013-05-07 Disapprove on 2013-07-10
Review via email: mp+162772@code.launchpad.net

Description of the change

A fix for finding the first opening period.

We had a customer who had a backdated entry which confused the code, these adjustments should avoid such issues.

To post a comment you must log in.

hi,

I'm on my phone, in the train, so it's a bit difficult to be exhaustive. I
remember of a change in this method to fix a bug when a fiscal year had no
move lines at all. The goal was to exclude them.

I'll try to find the reason shortly.

The original code was meant to exclude fiscal years which would have been created in OpenERP but never used (no entries).

That's important for the partners reports:
We have to include the balances from the first opening period only.
If we only take the first opening period without considerate if it is used or not would be wrong.

What do you mean by backdated entry?

Niels, you maybe have good use case to give but did that way initialy for Canonical since their 1st fiscal year was not included any accounting entries... So we had to teach OpenERP how to find the 1st fy with move lines and look up at the special period.

So I suggest not to merge this at it is not generic I think.

Do not hesitate to give me your use cas.

Thanks

Frederic

review: Disapprove
Niels Huylebroeck (red15) wrote :

One can discover an old invoice at any time which you will have to book
into the current period if you have never booked this invoice before, thus
you can have a large discrepancy between the date and the period.

Say I found an invoice I had lost 2 years after date. I cannot book this
invoice in the period that date falls in because that fiscal year has
already passed (and possibly/probably has been closed).
The date of the created account.move still has to match the invoice date.

*account.invoice*
date: 2004-05-15

*account.move*
date: 2004-05-15
period_id: 07/2013

Now when this piece of code tries to find the "oldest" period it will find
this move and will assume that the period linked with it is the "oldest".
As you can see this is far from what has actually happend.
Also it now even considers that 2013 is the fiscal year to use for finding
the opening balance.

Perhaps we should add a check that sees if there are any move_lines which
use the period or not and find the next period which actually has some
entries.
But using the date field is a very bad idea, OpenERP SA has also made this
assumption many times and we had to report all of them.

I have been informed that Swiss localization does not allow entering
account.move lines which are not between the dates defined on the linked
period.
This is however legal and required in Belgium. *If anyone of the community
can also sound in on their local laws it would be great.*

> "So I suggest not to merg this as it is not generic I think."
Seems to me the current solution is also not very generic :)

2013/7/10 Frederic Clementi - Camptocamp.com <
<email address hidden>>

> Review: Disapprove
>
> Niels, you maybe have good use case to give but did that way initialy for
> Canonical since their 1st fiscal year was not included any accounting
> entries... So we had to teach OpenERP how to find the 1st fy with move
> lines and look up at the special period.
>
> So I suggest not to merge this at it is not generic I think.
>
> Do not hesitate to give me your use cas.
>
> Thanks
>
> Frederic
>
>
> --
>
> https://code.launchpad.net/~account-report-core-editor/account-financial-report/fix-opening-period-search/+merge/162772
> Your team Account Report Core Editors is subscribed to branch
> lp:account-financial-report.
>

--
Niels Huylebroeck
Lead Architect -- Agaplan
Tel. : +32 (0) 93 95 98 90
Web : http://www.agaplan.eu

Thanks Niels for your explanation. You are correct.

>
> Perhaps we should add a check that sees if there are any move_lines which
> use the period or not and find the next period which actually has some
> entries.
> But using the date field is a very bad idea, OpenERP SA has also made this
> assumption many times and we had to report all of them.
>

I think this is the solution which would make everyone happy.

+ 1 to Niels proposition.

Am I correct in concluding that this branch still waits for a refactoring that was worked out between Niels and Camp2camp in the comments above? In that case this proposal had rather go into 'Work in progress' state.

Pedro Manuel Baeza (pedro.baeza) wrote :
review: Resubmit

Unmerged revisions

29. By Niels Huylebroeck on 2013-05-07

[FIX] account_financial_report_webkit: fix issue where the search for the first opening period was confused if you had some account entry which was backdated
It now searches through the periods themselves while sorting on date_start ascending instead of looking at the "oldest" account entry.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'account_financial_report_webkit/report/common_partner_reports.py'
2--- account_financial_report_webkit/report/common_partner_reports.py 2012-12-11 16:32:26 +0000
3+++ account_financial_report_webkit/report/common_partner_reports.py 2013-05-07 12:55:30 +0000
4@@ -64,21 +64,12 @@
5
6 :return: browse record of the first special period.
7 """
8- move_line_obj = self.pool.get('account.move.line')
9- first_entry_id = move_line_obj.search(
10- self.cr, self.uid, [], order='date ASC', limit=1)
11- # it means there is no entry at all, that's unlikely to happen, but
12- # it may so
13- if not first_entry_id:
14- return
15- first_entry = move_line_obj.browse(self.cr, self.uid, first_entry_id[0])
16- fiscalyear = first_entry.period_id.fiscalyear_id
17- special_periods = [period for period in fiscalyear.period_ids if period.special]
18- # so, we have no opening period on the first year, nothing to return
19- if not special_periods:
20- return
21- return min(special_periods,
22- key=lambda p: datetime.strptime(p.date_start, DEFAULT_SERVER_DATE_FORMAT))
23+ period_obj = self.pool.get('account.period')
24+ period_id = period_obj.search(self.cr, self.uid,
25+ [('special', '=', True)], order='date_start asc', limit=1)
26+ if not period_id:
27+ return
28+ return period_obj.browse(self.cr, self.uid, period_id[0])
29
30 def _get_period_range_from_start_period(self, start_period, include_opening=False,
31 fiscalyear=False,