openerp-community-reviewer team mailing list archive

["Frederic Clementi - Camptocamp.com" <frederic.clementi@xxxxxxxxxxxxxx>]

Review: Needs Fixing functional

Thanks, With your resubmission I could do a full functional test. You did a good job. Congrats. However, I have several comments to do : 


1 - After module install, menus do not appears as epexted (under Accounting/Reporting/Legal Reports/Accounting Reports) even with user rights set to : financial report. I had to duplicate menu and give a new name to see them working (?)...

2 - In the wizard, in the section 'account to include', I noticed that if a parent account has a level lower that the level of the child account (yes, it happens :( especially on migrated databases), I get the following error :
File "/home/fclementi/openerp-instances/openerp_prod_camptocamp/parts/account-financial-report/account_financial_report_exportable/wizard/afr_report_wizard.py", line 53, in get_parser_method
res = getattr(acc_bal_obj, method)(args, param)
File "/home/fclementi/openerp-instances/openerp_prod_camptocamp/parts/account-financial-report/account_financial_report/report/parser.py", line 624, in lines
child_id.id).get('debit')
AttributeError: 'NoneType' object has no attribute 'get'


3 - About section 'account to include' : I see an ergonomy issue....but tell me be if you agree. If I want to select all P&L accounts, I have to select them one by one... it is a nightmare. Personnaly, I whould prefer to let the user select the a parent account and you would included recurcively all children accounts for the report (thats what we did on the webkit financial report we made)
I would also add that for a P&L or BS report it is pointless to select regular accounts, you do not want to print a such a report with one or few lines...

4 - The printout by quater and by month is barely readable...too small. Can you improve that ?

5 - typo : INICIAL instead of initial on analytic ledger.

6 - check box 'analytic ledger' seems to be rather a detailed of account move lines but nothing to de with analityc lines... Am I correct? if yes, simply change the semantic to 'general ledger'...However if this is used to print a general ledger then the report already exist and defenetly deserve to be a clearly separeted report... an accountants lwill never understand spontaneously.

7 - error if I check 'journal ledger :
Webkit render!



Traceback (most recent call last):
File "/home/fclementi/openerp-instances/openerp_prod_camptocamp/parts/addons/report_webkit/webkit_report.py", line 273, in create_single_pdf
**self.parser_instance.localcontext)
File "/home/fclementi/.buildout/eggs/Mako-0.6.2-py2.7.egg/mako/template.py", line 378, in render
return runtime._render(self, self.callable_, args, data)
File "/home/fclementi/.buildout/eggs/Mako-0.6.2-py2.7.egg/mako/runtime.py", line 646, in _render
**_kwargs_for_callable(callable_, data))
File "/home/fclementi/.buildout/eggs/Mako-0.6.2-py2.7.egg/mako/runtime.py", line 678, in _render_context
_exec_template(inherit, lclcontext, args=args, kwargs=kwargs)
File "/home/fclementi/.buildout/eggs/Mako-0.6.2-py2.7.egg/mako/runtime.py", line 704, in _exec_template
callable_(context, *args, **kwargs)
File "memory:0x4dc0a90", line 83, in render_body
%for j in line['journal']:
KeyError: 'journal'

8 - Checkbox 'summarized ? ' : most of the time I get an error (does not happen when I select in field colums 'End balance'

File "/home/fclementi/.buildout/eggs/Mako-0.6.2-py2.7.egg/mako/runtime.py", line 678, in _render_context
_exec_template(inherit, lclcontext, args=args, kwargs=kwargs)
File "/home/fclementi/.buildout/eggs/Mako-0.6.2-py2.7.egg/mako/runtime.py", line 704, in _exec_template
callable_(context, *args, **kwargs)
File "memory:0x7f11d55f7990", line 58, in render_body
<td class="celdaLineDataTotal" style="text-align:right;font-style:italic;">${line['code'] or ''}</td>
KeyError: 'code' 

9 - field 'up to level' : I see a conceptual/design issue here. I do not know for latin america, but in Europe a P&L or a BS report you cannot get a proper legal report with the level because you might need to display the level 2 in the equity but the level 4 in the assets...there are not such a logic. We need to be more flexible than that. So either you flag the accounts you need or the create such a structure (1 for the P&L and 1 for the B/S) with a virtual chart of account (I refer to the consolidated type of accounts)... Check out this idea if you have some time... so level for me is not the correct approach.



Frederic


-- 
https://code.launchpad.net/~vauxoo/account-financial-report/7.0-report_webkit_afr-_data_dev-jorge/+merge/192050
Your team Account Report Core Editors is subscribed to branch lp:account-financial-report.