Code review comment for lp:~noviat/openobject-addons/6.1

Thanks Xavier,

I included your suggestions into https://code.launchpad.net/~noviat/openobject-addons/6.1/+merge/104342.
I have also attached the two files that have been changed to this email since I didn't update my local 6.1 branch before submitting the merge (with as a consequence a huge and meaningless diff file).

Changes:

>> VAT Declaration
>> ---------------
>> * using table "account_period_rel" for period_ids is wrong because it's already used by "vat_intra" wizard (will raise IntegrityErrors from PostgreSQL).

I have renamed the 'account_period_rel' tables into 'vat_declaration_period_rel' and ' vat_intra_period_rel'

>> * removing "period_id" field is acceptable here, because if user doesn't upgrade his DB it won't be able to run wizard normally (NOT NULL constraints will prevent user to save the wizard data).

>> Is the multiple periods a legal requirement here ? or is it acceptable to live without it on v6.1 ?

Yes and we have (at least) two customers running that way.

>> VAT Intra
>> ---------
>> * You could remove the "hack" for exists(), this is not needed anymore on v6.1.

I have removed the 'exists()' hack.

Regards,
Luc

-----Original Message-----
From: <email address hidden> [mailto:<email address hidden>] On Behalf Of Xavier ALT (OpenERP)
Sent: maandag 30 april 2012 13:24
To: <email address hidden>
Subject: Re: [Merge] lp:~noviat/openobject-addons/6.1 into lp:openobject-addons/6.1

Review: Needs Fixing

Hi,

Some problems with your MP:

VAT Declaration
---------------
As for v6.1 "osv_memory" are also stored in the DB,

* using table "account_period_rel" for period_ids is wrong because it's already used by "vat_intra" wizard (will raise IntegrityErrors from PostgreSQL).
* removing "period_id" field is acceptable here, because if user doesn't upgrade his DB it won't be able to run wizard normally (NOT NULL constraints will prevent user to save the wizard data).

Is the multiple periods a legal requirement here ? or is it acceptable to live without it on v6.1 ?

VAT Intra
---------
* You could remove the "hack" for exists(), this is not needed anymore on v6.1.

Regards,
Xavier
--
https://code.launchpad.net/~noviat/openobject-addons/6.1/+merge/102753
Your team Noviat is subscribed to branch lp:~noviat/openobject-addons/6.1.