Code review comment for lp:~akretion-team/account-invoicing/70-add-invoice_fiscal_position_update

Revision history for this message
Graeme Gellatly (gdgellatly) wrote :

on_change

pros:
can add warning for non product lines
manages every use case except where maybe a fiscal position needs to be different on different lines (only one I can think is items shipped to 2 different locations with different tax regimes).
cons:
wastes user time if changes in error.

button:
cons:
can't add warning although can add confirmation dialogue but not in as much detail.
pros:
gives user full control on whether to update existing (also a con as Ronald points out)
faster - as there is no onscreen change, then write, then screen refresh

on balance I would slightly favour the onchange, simply because the button gives more room for error and unexpected behaviour for the user, one less thing for user to think about ond one less thing on screen. Besides I imagine after waiting a few minutes for an erroneous change that user behaviour would be reinforced.

But I am also fine with how it is.

« Back to merge proposal