Merge lp:~openerp-dev/openobject-addons/trunk-bug-744789-qdp into lp:openobject-addons

[qdp (OpenERP)]

in the query_get() of account.move.line, if we don't give any other criteria in the context than the fiscalyear when computing an initial balance then it simply means that the entries returned will be exactly the same as without initial balance, which lead to count the entries twice...

this hack is a little nasty (not so say "a lot nasty"), but i can't think to something else.

[xrg (xrg)]

On Tuesday 19 April 2011, you wrote:
> qdp (OpenERP) has proposed merging
> lp:~openerp-dev/openobject-addons/trunk-bug-744789-qdp into
> lp:openobject-addons.
>
> Requested reviews:
> Olivier Dony (OpenERP) (odo)
> Related bugs:
> Bug #744789 in OpenERP Addons: "[V6] Third party ledger : Initial
> Balance" https://bugs.launchpad.net/openobject-addons/+bug/744789
>
> For more details, see:
> https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-bug-744789-
> qdp/+merge/58334
>
> in the query_get() of account.move.line, if we don't give any other
> criteria in the context than the fiscalyear when computing an initial
> balance then it simply means that the entries returned will be exactly the
> same as without initial balance, which lead to count the entries twice...
>
> this hack is a little nasty (not so say "a lot nasty"), but i can't think
> to something else.

you could use "AND false"

But I'm trying to think if a direct exception would also make sense in that
case.

--
Say NO to spam and viruses. Stop using Microsoft Windows!

[qdp (OpenERP) (qdp)]

actually, i slept on that and now i think it's not a good idea: the cleaner way to "fix" this "bug" would be to
* raise an error if someone is trying to compute an initial balance without enough argument,
* and in the same time we should improve the usability of the wizards in order to make readonly the boolean to include initial balance if the filter = 'no filter'...

that seems a better idea, no?

[xrg (xrg)]

On Wednesday 20 April 2011, you wrote:
> actually, i slept on that and now i think it's not a good idea: the cleaner
> way to "fix" this "bug" would be to * raise an error if someone is trying
> to compute an initial balance without enough argument, * and in the same
> time we should improve the usability of the wizards in order to make
> readonly the boolean to include initial balance if the filter = 'no
> filter'...
>
> that seems a better idea, no?

Yes, much better like that.

--
Say NO to spam and viruses. Stop using Microsoft Windows!

[Purnendu Singh (OpenERP) (purnendu-singh)]

Hello,

patch propose fixes for these bugs:
1) https://bugs.launchpad.net/openobject-addons/+bug/744789

    -- when No Filter is select then we are not allowing the user to tick the Initial Balance Option
    -- Currently the report was not proper if we filter based on periods. Say for e.g:
       1) Create a invoice for the 1'st period on 2011,
       2) Now try to generate the Partner ledger report with Filter By: Period and Initial Balance check box ticked
       3) If we select the Start period 2 feb and End Period 4 March report is ok.
       4) If we select the Start period 1 Jan and End Period 4 March report is not ok.
       To fix this problem i made change in _get_query() method of the account.move.line object @ line no 8, 14,15 of the proposed patch

2) https://bugs.launchpad.net/openobject-addons/+bug/734823
     -- We are not allowing the user to We are not allowing the user to select both 'Include Initial Balances' and 'Include Reconcile Entries' options at the same time.

Thanks,
Purnendu Singh

review: Needs Resubmitting

[qdp (OpenERP) (qdp)]

i checked and for me it's solving all the known problems of the partner ledger. The only problem with this merge proposal is that it is creating error in yaml test because of the new constraint we've added.

[2011-05-04 14:46:25,524][trunk_mai_03] INFO:init:module account: loading test/account_report.yml
....
  File "/home/qdp/tinydev/addons/trunk/account/account_move_line.py", line 102, in _query_get
    raise osv.except_osv(_('Warning !'),_("You haven't supplied enough argument to compute the initial balance"))

Please check it,

thanks,
Quentin

review: Needs Fixing

[qdp (OpenERP) (qdp)]

while testing i found out that there was another bug: if i select the periods from 01/2011 -> 05/2011 it was displaying also the opening entries though it should have been added in the initial balance.

So i used this branch to fix that too. ;-)

It will be merged into the trunk as soon as the errors in yaml will be fixed.

Thanks

[qdp (OpenERP) (qdp)]

a last thought: we should replace the boolean 'include reconciled entries' by a fourth option in the filter selection box "Unreconciled Entries".

So, the 4 options would be:
filter by: No filter
filter by: Date
filter by: Periods
filter by: Unreconciled Entries

so that we will be sure that the initial balance is always complete.

Can you do that do before i merge in the trunk?
Thanks

[Purnendu Singh (OpenERP) (purnendu-singh)]

Hello sir,

I have added Unreconciled Entries as fourth filter option and
set Initial balance check box readonly while filter = Unreconciled Entries,

And For the YML traceback as we are getting this due to the general ledger report(Not b'coz of Partner ledger) i will soon place a merge proposal for that into a separate branch.

Thanks and regards,
Purnendu Singh

review: Needs Resubmitting

[Mustufa Rangwala (Open ERP) (mra-tinyerp)]

conflicts in the branch.
please resolve it.

review: Needs Fixing

[Purnendu Singh (OpenERP) (purnendu-singh)]

> conflicts in the branch.
> please resolve it.

Hello sir,

Conflicts are due to the changes in main addons branch(lp:openobject-addons).

Can you please merge main branch to this branch to remove these conflicts.

Thanks,
Purnendu Singh

[Purnendu Singh (OpenERP) (purnendu-singh)]

Hello,

I have merged the main addons branch with this branch to resolve conflicts.

Thanks
Purnendu Singh

review: Needs Resubmitting

[Bogdan Stanciu (bstanciu)]

On 04. 05. 11 17:09, qdp (OpenERP) wrote:
> while testing i found out that there was another bug: if i select the periods from 01/2011 -> 05/2011 it was displaying also the opening entries though it should have been added in the initial balance.
>
> So i used this branch to fix that too. ;-)
>
> It will be merged into the trunk as soon as the errors in yaml will be fixed.
>
> Thanks
having a look at this code:

    def build_ctx_periods(self, cr, uid, period_from_id, period_to_id):
        period_from = self.browse(cr, uid, period_from_id)
        period_date_start = period_from.date_start
        company1_id = period_from.company_id.id
        period_to = self.browse(cr, uid, period_to_id)
        period_date_stop = period_to.date_stop
        company2_id = period_to.company_id.id
        if company1_id != company2_id:
            raise osv.except_osv(_('Error'), _('You should have chosen
periods that belongs to the same company'))
        if period_date_start > period_date_stop:
            raise osv.except_osv(_('Error'), _('Start period should be
smaller then End period'))
        return self.search(cr, uid, [('date_start', '>=',
period_date_start), ('date_stop', '<=', period_date_stop),
('company_id', '=', company1_id)])

i would say that it's a pitty that we start with precisely identified
periods (period_fom_id, period_to_id) to get lost in this "date" check.
but maybe this is the only way to get the result...

however, I do not understand how this check works, as there is no loop,
therefore, how the period_from.special gets [re]initialised.

 - return self.search(cr, uid, [('date_start', '>=', period_date_start),
('date_stop', '<=', period_date_stop), ('company_id', '=', company1_id)])
18 + #for period from = january, we want to exclude the opening period
(but it has same date_from, so we have to check if period_from is
special or not to include that clause or not in the search).
19 + if period_from.special:
20 + return self.search(cr, uid, [('date_start', '>=',
period_date_start), ('date_stop', '<=', period_date_stop),
('company_id', '=', company1_id)])
21 + return self.search(cr, uid, [('date_start', '>=',
period_date_start), ('date_stop', '<=', period_date_stop),
('company_id', '=', company1_id), ('special', '=', False)])

I think that the solution is NOT the best one, as you [want to] check
whether or not the period is "special". A proper identification

 of the period would be maybe better, as I might want to print a report
with or without the closing period, and the easiest way is to select the
proper periods.

additionally, I am not sure it works for a special period and the end of
the year, or in the middle (e.g. quarterly closings).

thank you for giving a thought at my remarks.

regards,
Bogdan

=== modified file 'account/account.py'
--- account/account.py	2011-05-05 14:43:40 +0000
+++ account/account.py	2011-05-06 08:39:17 +0000
@@ -879,7 +879,7 @@
     _defaults = {
         'state': 'draft',
     }
-    _order = "date_start"
+    _order = "date_start, special desc"
 
     def _check_duration(self,cr,uid,ids,context=None):
         obj_period = self.browse(cr, uid, ids[0], context=context)
@@ -960,7 +960,10 @@
             raise osv.except_osv(_('Error'), _('You should have chosen periods that belongs to the same company'))
         if period_date_start > period_date_stop:
             raise osv.except_osv(_('Error'), _('Start period should be smaller then End period'))
-        return self.search(cr, uid, [('date_start', '>=', period_date_start), ('date_stop', '<=', period_date_stop), ('company_id', '=', company1_id)])
+        #for period from = january, we want to exclude the opening period (but it has same date_from, so we have to check if period_from is special or not to include that clause or not in the search).
+        if period_from.special:
+            return self.search(cr, uid, [('date_start', '>=', period_date_start), ('date_stop', '<=', period_date_stop), ('company_id', '=', company1_id)])
+        return self.search(cr, uid, [('date_start', '>=', period_date_start), ('date_stop', '<=', period_date_stop), ('company_id', '=', company1_id), ('special', '=', False)])
 
 account_period()
 

=== modified file 'account/account_move_line.py'
--- account/account_move_line.py	2011-04-25 06:11:14 +0000
+++ account/account_move_line.py	2011-05-06 08:39:17 +0000
@@ -68,7 +68,6 @@
         if state:
             if state.lower() not in ['all']:
                 where_move_state= " AND "+obj+".move_id IN (SELECT id FROM account_move WHERE account_move.state = '"+state+"')"
-
         if context.get('period_from', False) and context.get('period_to', False) and not context.get('periods', False):
             if initial_bal:
                 period_company_id = fiscalperiod_obj.browse(cr, uid, context['period_from'], context=context).company_id.id
@@ -82,17 +81,20 @@
                 period_ids = fiscalperiod_obj.search(cr, uid, [('id', 'in', context['periods'])], order='date_start', limit=1)
                 if period_ids and period_ids[0]:
                     first_period = fiscalperiod_obj.browse(cr, uid, period_ids[0], context=context)
-                    # Find the old periods where date start of those periods less then Start period
-                    periods = fiscalperiod_obj.search(cr, uid, [('date_start', '<', first_period.date_start)])
-                    periods = ','.join([str(x) for x in periods])
-                    if periods:
-                        query = obj+".state <> 'draft' AND "+obj+".period_id IN (SELECT id FROM account_period WHERE fiscalyear_id IN (%s) AND id IN (%s)) %s %s" % (fiscalyear_clause, periods, where_move_state, where_move_lines_by_date)
+                    ids = ','.join([str(x) for x in context['periods']])
+                    query = obj+".state <> 'draft' AND "+obj+".period_id IN (SELECT id FROM account_period WHERE fiscalyear_id IN (%s) AND date_start <= '%s' AND id NOT IN (%s)) %s %s" % (fiscalyear_clause, first_period.date_start, ids, where_move_state, where_move_lines_by_date)
             else:
                 ids = ','.join([str(x) for x in context['periods']])
                 query = obj+".state <> 'draft' AND "+obj+".period_id IN (SELECT id FROM account_period WHERE fiscalyear_id IN (%s) AND id IN (%s)) %s %s" % (fiscalyear_clause, ids, where_move_state, where_move_lines_by_date)
         else:
             query = obj+".state <> 'draft' AND "+obj+".period_id IN (SELECT id FROM account_period WHERE fiscalyear_id IN (%s)) %s %s" % (fiscalyear_clause, where_move_state, where_move_lines_by_date)
 
+        if initial_bal and not context.get('periods', False) and not where_move_lines_by_date:
+            #we didn't pass any filter in the context, and the initial balance can't be computed using only the fiscalyear otherwise entries will be summed twice
+            #so we have to invalidate this query
+            raise osv.except_osv(_('Warning !'),_("You haven't supplied enough argument to compute the initial balance"))
+
+
         if context.get('journal_ids', False):
             query += ' AND '+obj+'.journal_id IN (%s)' % ','.join(map(str, context['journal_ids']))
 

=== modified file 'account/report/account_partner_ledger.py'
--- account/report/account_partner_ledger.py	2011-04-29 08:49:48 +0000
+++ account/report/account_partner_ledger.py	2011-05-06 08:39:17 +0000
@@ -58,10 +58,13 @@
         obj_partner = self.pool.get('res.partner')
         self.query = obj_move._query_get(self.cr, self.uid, obj='l', context=data['form'].get('used_context', {}))
         ctx2 = data['form'].get('used_context',{}).copy()
-        ctx2.update({'initial_bal': True})
+        self.initial_balance = data['form'].get('initial_balance', True)
+        if self.initial_balance:
+            ctx2.update({'initial_bal': True})
         self.init_query = obj_move._query_get(self.cr, self.uid, obj='l', context=ctx2)
-        self.reconcil = data['form'].get('reconcil', True)
-        self.initial_balance = data['form'].get('initial_balance', True)
+        self.reconcil = True
+        if data['form']['filter'] == 'unreconciled':
+            self.reconcil = False
         self.result_selection = data['form'].get('result_selection', 'customer')
         self.amount_currency = data['form'].get('amount_currency', False)
         self.target_move = data['form'].get('target_move', 'all')
@@ -169,7 +172,6 @@
             RECONCILE_TAG = " "
         else:
             RECONCILE_TAG = "AND l.reconcile_id IS NULL"
-
         self.cr.execute(
             "SELECT COALESCE(SUM(l.debit),0.0), COALESCE(SUM(l.credit),0.0), COALESCE(sum(debit-credit), 0.0) " \
             "FROM account_move_line AS l,  " \

=== modified file 'account/wizard/account_report_common.py'
--- account/wizard/account_report_common.py	2011-04-15 10:09:01 +0000
+++ account/wizard/account_report_common.py	2011-05-06 08:39:17 +0000
@@ -56,7 +56,7 @@
         return res
 
     def onchange_filter(self, cr, uid, ids, filter='filter_no', fiscalyear_id=False, context=None):
-        res = {}
+        res = {'value': {}}
         if filter == 'filter_no':
             res['value'] = {'period_from': False, 'period_to': False, 'date_from': False ,'date_to': False}
         if filter == 'filter_date':
@@ -68,7 +68,7 @@
                                FROM account_period p
                                LEFT JOIN account_fiscalyear f ON (p.fiscalyear_id = f.id)
                                WHERE f.id = %s
-                               ORDER BY p.date_start ASC
+                               ORDER BY p.date_start ASC, p.special ASC
                                LIMIT 1) AS period_start
                 UNION
                 SELECT * FROM (SELECT p.id

=== modified file 'account/wizard/account_report_partner_ledger.py'
--- account/wizard/account_report_partner_ledger.py	2011-04-15 10:09:01 +0000
+++ account/wizard/account_report_partner_ledger.py	2011-05-06 08:39:17 +0000
@@ -32,22 +32,26 @@
     _columns = {
         'initial_balance': fields.boolean('Include Initial Balances',
                                     help='It adds initial balance row on report which display previous sum amount of debit/credit/balance'),
-        'reconcil': fields.boolean('Include Reconciled Entries', help='Consider reconciled entries'),
-        'page_split': fields.boolean('One Partner per Page', help='Display Ledger Report with One partner per page'),
+        'filter': fields.selection([('filter_no', 'No Filters'), ('filter_date', 'Date'), ('filter_period', 'Periods'), ('unreconciled', 'Unreconciled Entries')], "Filter by", required=True),
+        'page_split': fields.boolean('One Partner Per Page', help='Display Ledger Report with One partner per page'),
         'amount_currency': fields.boolean("With Currency", help="It adds the currency column if the currency is different then the company currency"),
-
     }
     _defaults = {
-       'reconcil': True,
-       'initial_balance': True,
+       'initial_balance': False,
        'page_split': False,
     }
 
+    def onchange_filter(self, cr, uid, ids, filter='filter_no', fiscalyear_id=False, context=None):
+        res = super(account_partner_ledger, self).onchange_filter(cr, uid, ids, filter=filter, fiscalyear_id=fiscalyear_id, context=context)
+        if filter in ['filter_no', 'unreconciled']:
+            res['value'].update({'initial_balance': False, 'period_from': False, 'period_to': False, 'date_from': False ,'date_to': False})
+        return res
+
     def _print_report(self, cr, uid, ids, data, context=None):
         if context is None:
             context = {}
         data = self.pre_print_report(cr, uid, ids, data, context=context)
-        data['form'].update(self.read(cr, uid, ids, ['initial_balance', 'reconcil', 'page_split', 'amount_currency'])[0])
+        data['form'].update(self.read(cr, uid, ids, ['initial_balance', 'filter', 'page_split', 'amount_currency'])[0])
         if data['form']['page_split']:
             return {
                 'type': 'ir.actions.report.xml',

=== modified file 'account/wizard/account_report_partner_ledger_view.xml'
--- account/wizard/account_report_partner_ledger_view.xml	2011-01-14 00:11:01 +0000
+++ account/wizard/account_report_partner_ledger_view.xml	2011-05-06 08:39:17 +0000
@@ -15,12 +15,14 @@
             </xpath>
             <xpath expr="//field[@name='target_move']" position="after">
                 <field name="result_selection"/>
-                <field name="initial_balance"/>
-                <field name="reconcil"/>
+                <field name="initial_balance" attrs="{'readonly':[('filter', 'in', ('filter_no', 'unreconciled'))]}" />
                 <field name="amount_currency"/>
                 <field name="page_split"/>
                 <newline/>
             </xpath>
+            <xpath expr="//field[@name='filter']" position="replace">
+                <field name="filter" strint="replaced" on_change="onchange_filter(filter, fiscalyear_id)" colspan="4"/>
+            </xpath>
             </data>
             </field>
         </record>