Merge lp:~therp-nl/therp-addons/7.0_lp1215631 into lp:~therp-nl/therp-addons/7.0

Proposed by Ronald Portier (Therp)
Status: Merged
Merged at revision: 82
Proposed branch: lp:~therp-nl/therp-addons/7.0_lp1215631
Merge into: lp:~therp-nl/therp-addons/7.0
Diff against target: 20 lines (+6/-4)
1 file modified
fetchmail_invoice/model/fetchmail_invoice.py (+6/-4)
To merge this branch: bzr merge lp:~therp-nl/therp-addons/7.0_lp1215631
Reviewer Review Type Date Requested Status
Stefan Rijnhart (Opener) (community) Approve
Review via email: mp+181900@code.launchpad.net

Description of the change

Resubmit to merge with 7.0 branch.

Prevent crash of fetchmail invoice, when trying to retrieve default invoice partner.

To post a comment you must log in.
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Thanks!

One thing: line 15 will never be reached, because get_object_references raises ValueError if the data reference can not be found. You should catch this if you want to raise a more user friendly error than 'No such external ID defined'.

review: Needs Fixing
Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

Hello Stefan,

I am perfectly happy with the message 'No such external ID defined' (although it would help a lot if the exception included the actual ID).

Using an assert means that I EXPECT the code NOT to be reached, if all the rest of the code leading to the assert is correct. However, suppose get_object_reference no longer throws an exception, but ignores the error, or any other change in any other part of code means that my assumption that I will have a partner_id on line 15/16 is no longer true: at that moment the assert kicks in.

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

OK, thanks for the explanation

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'fetchmail_invoice/model/fetchmail_invoice.py'
--- fetchmail_invoice/model/fetchmail_invoice.py 2013-07-23 12:24:57 +0000
+++ fetchmail_invoice/model/fetchmail_invoice.py 2013-08-23 19:19:27 +0000
@@ -63,10 +63,12 @@
63 # Retrieve partner_id from the email address63 # Retrieve partner_id from the email address
64 # and add related field values64 # and add related field values
65 custom_values['partner_id'] = (65 custom_values['partner_id'] = (
66 msg_dict.get(66 ('author_id' in msg_dict and msg_dict['author_id'])
67 'author_id',67 or (data_pool.get_object_reference(
68 data_pool.get_object_reference(68 cr, uid, 'fetchmail_invoice', 'default_partner')[1])
69 cr, uid, 'fetchmail_invoice', 'default_partner')[1]))69 or False)
70 assert custom_values['partner_id'], _(
71 'No partner found to link invoice to')
70 custom_values.update(72 custom_values.update(
71 self.onchange_partner_id(73 self.onchange_partner_id(
72 cr, uid, [], 'in_invoice',74 cr, uid, [], 'in_invoice',

Subscribers

People subscribed via source and target branches

to all changes: