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

[Niels Huylebroeck]

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.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

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.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

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?

[Frederic Clementi - Camptocamp (frederic-clementi)]

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)]

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. : [phone]
Web : http://www.agaplan.eu

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

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.

[Frederic Clementi - Camptocamp (frederic-clementi)]

+ 1 to Niels proposition.

[Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903)]

review: Approve

[Stefan Rijnhart (Opener) (stefan-opener)]

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.

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

Related: https://code.launchpad.net/~luc-demeyer/account-financial-report/7.0-account_financial_report_webkit-first-special-fix/+merge/193717

[Pedro Manuel Baeza (pedro.baeza)]

This project is now hosted on https://github.com/OCA/account-financial-reporting. Please move your proposal there. This guide may help you https://github.com/OCA/maintainers-tools/wiki/How-to-move-a-Merge-Proposal-to-GitHub

review: Needs Resubmitting

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.

=== modified file 'account_financial_report_webkit/report/common_partner_reports.py'
--- account_financial_report_webkit/report/common_partner_reports.py	2012-12-11 16:32:26 +0000
+++ account_financial_report_webkit/report/common_partner_reports.py	2013-05-07 12:55:30 +0000
@@ -64,21 +64,12 @@
 
         :return: browse record of the first special period.
         """
-        move_line_obj = self.pool.get('account.move.line')
-        first_entry_id = move_line_obj.search(
-            self.cr, self.uid, [], order='date ASC', limit=1)
-        # it means there is no entry at all, that's unlikely to happen, but
-        # it may so
-        if not first_entry_id:
-            return
-        first_entry = move_line_obj.browse(self.cr, self.uid, first_entry_id[0])
-        fiscalyear = first_entry.period_id.fiscalyear_id
-        special_periods = [period for period in fiscalyear.period_ids if period.special]
-        # so, we have no opening period on the first year, nothing to return
-        if not special_periods:
-            return
-        return min(special_periods,
-                   key=lambda p: datetime.strptime(p.date_start, DEFAULT_SERVER_DATE_FORMAT))
+        period_obj = self.pool.get('account.period')
+        period_id = period_obj.search(self.cr, self.uid,
+            [('special', '=', True)], order='date_start asc', limit=1)
+        if not period_id:
+            return
+        return period_obj.browse(self.cr, self.uid, period_id[0])
 
     def _get_period_range_from_start_period(self, start_period, include_opening=False,
                                             fiscalyear=False,