Merge lp://staging/~camptocamp/ocb-addons/lp1191036 into lp://staging/ocb-addons

Proposed by Frederic Clementi - Camptocamp
Status: Merged
Merged at revision: 9729
Proposed branch: lp://staging/~camptocamp/ocb-addons/lp1191036
Merge into: lp://staging/ocb-addons
Diff against target: 34 lines (+4/-4)
2 files modified
account_analytic_analysis/account_analytic_analysis_view.xml (+2/-2)
analytic_contract_hr_expense/analytic_contract_hr_expense_view.xml (+2/-2)
To merge this branch: bzr merge lp://staging/~camptocamp/ocb-addons/lp1191036
Reviewer Review Type Date Requested Status
Holger Brunn (Therp) code review Approve
Maxime Chambreuil (http://www.savoirfairelinux.com) code review Approve
Stefan Rijnhart (Opener) Needs Information
Alexandre Fayolle - camptocamp code review, no test Approve
Review via email: mp+171050@code.staging.launchpad.net

Description of the change

Prerequisite : You need to install project management + timesheet + activate invoicing on timesheet

From analytic account form view ->
Make the fields pricelist & invoicing type non-requiered when invisible. Theses fields are invisible for instance if the account type is normal (=analytic account)

My test :
I have create a analytical account with type contrat/project + ticked the checkbox 'on timesheet' (from invoicing table) -> then you can see 2 extra fields : pricelist & invoicing type. Then I select 100% invoicing for ex. and I save

I reopen the account and set to blank the invoicing rate ; change the analytic account type to 'analytic account' -> with this patch I can save it.

To post a comment you must log in.
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

LGTM

review: Approve (code review, no test)
Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

#27, #31 make the fields required for all type='contract' and type='template'.
Shouldn't it rather be
['|',('type','=','template'),('type','=','contract'), '|', ('invoice_on_timesheets','=',True),('charge_expenses','=',True)]
?

review: Needs Fixing
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

If I understand correctly (but I'm having a slow day to be honest), it will still be possible, even with this fix that OpenERP generates invoice lines due to hidden settings.

Would it therefore not be better to clear the invoicing fields when they become invisible, using an onchange method on the analytic account type?

review: Needs Information
Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

Thanks for the reviews

This is the copy of the following bug fix MP:

https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-opw-593811-dhr/+merge/169993

You may report your comments there.

Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

There's been activity on the original MP, should you maybe update your MP and resubmit?

review: Needs Information
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

I replicated the changes of the original MP.

Revision history for this message
Maxime Chambreuil (http://www.savoirfairelinux.com) (max3903) :
review: Approve (code review)
Revision history for this message
Holger Brunn (Therp) (hbrunn) :
review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.