Merge lp:~camptocamp/new-report-intrastat/7.0-post-all-errors-rather-than-one-by-one into lp:new-report-intrastat

Proposed by Guewen Baconnier @ Camptocamp
Status: Needs review
Proposed branch: lp:~camptocamp/new-report-intrastat/7.0-post-all-errors-rather-than-one-by-one
Merge into: lp:new-report-intrastat
Diff against target: 463 lines (+235/-69)
1 file modified
l10n_fr_intrastat_product/intrastat_product.py (+235/-69)
To merge this branch: bzr merge lp:~camptocamp/new-report-intrastat/7.0-post-all-errors-rather-than-one-by-one
Reviewer Review Type Date Requested Status
Alexis de Lattre Needs Fixing
Review via email: mp+222924@code.launchpad.net

Commit message

Post a message with all errors rather than raising them one by one.

Description of the change

In l10n_fr_intrastat_product:

When:
When we generate DEB lines and several lines have errors (misconfiguration of products, partners, ...).

Current behavior:
The first error is raised so the user must correct it and launch again the generation, and repeat this cycle until all errors are fixed.

Proposal:
Accumulate all the errors and show them in a mail message at the bottom of the view.

To post a comment you must log in.
Revision history for this message
Alexis de Lattre (alexis-via) wrote :

I merged this earlier, but I found a regression tonight and I reverted the commit.

There is a regression in this MP :
the modifications l 235 and 269 are wrong : putting the keys in the parent_values dict to False should only be made when parent_values['is_fiscal_only'] == True, that's why it was inside the "else"... and you removed the "else" !

It may not be the only regression ; I think there is the same kind of thing on line 199.

You made some modifications to the "functionnal code" (it seems to me that you thought it was to clean-it-up) that are not linked to the error messaging system, but it causes some issues.

Revision history for this message
Alexis de Lattre (alexis-via) :
review: Needs Fixing
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Sorry for the regression.
I did not changed that for code cleaning. The problem when initializing the keys in the 'else' was that we could have KeyError when they are accessed later, that's why I initialized them before the if.
I will try to find another way to do that.

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

I reverted the change for the keys that were already initialized earlier in the process. I left the initialization of the keys that were never set before.

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

I completely checked when the keys are initialized for the first time and I'm sure that keys are no longer squashed.

94. By Guewen Baconnier @ Camptocamp on 2014-08-12

skip the source uom checks when a line has no uom

Unmerged revisions

94. By Guewen Baconnier @ Camptocamp on 2014-08-12

skip the source uom checks when a line has no uom

93. By Guewen Baconnier @ Camptocamp on 2014-07-04

'partner_country_id_to_write' could have been initialized before

92. By Guewen Baconnier @ Camptocamp on 2014-07-04

do not reset 'transaction_code_to_write', it should be False only when is_fiscal_only is true

91. By Guewen Baconnier @ Camptocamp on 2014-06-12

Post a message with all errors rather than raising them one by one.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'l10n_fr_intrastat_product/intrastat_product.py'
2--- l10n_fr_intrastat_product/intrastat_product.py 2014-07-03 22:18:44 +0000
3+++ l10n_fr_intrastat_product/intrastat_product.py 2014-08-12 13:38:07 +0000
4@@ -20,8 +20,10 @@
5 #
6 ##############################################################################
7
8+from collections import defaultdict
9 from openerp.osv import orm, fields
10 from openerp.tools.translate import _
11+from openerp import pooler
12 import openerp.addons.decimal_precision as dp
13 from datetime import datetime
14 from dateutil.relativedelta import relativedelta
15@@ -213,6 +215,7 @@
16 lines_to_create = []
17 total_invoice_cur_accessory_cost = 0.0
18 total_invoice_cur_product_value = 0.0
19+ errors = defaultdict(list)
20 for line in invoice.invoice_line:
21 line_qty = line.quantity
22 source_uom = line.uos_id
23@@ -248,47 +251,77 @@
24 total_invoice_cur_product_value += line.price_subtotal
25 invoice_currency_id_to_write = invoice.currency_id.id
26
27+ weight_to_write = False
28+ source_uom_id_to_write = False
29+ intrastat_code_id_to_write = False
30+ intrastat_code_to_write = False
31+ quantity_to_write = False
32+ intrastat_uom_id_to_write = False
33+ product_country_origin_id_to_write = False
34+
35 if not parent_values['is_fiscal_only']:
36
37 if not source_uom:
38- raise orm.except_orm(_('Error :'), _("Missing unit of measure on the line with %d product(s) '%s' on invoice '%s'.") % (line_qty, line.product_id.name, invoice.number))
39+ errors[line.id].append(
40+ _("Missing unit of measure on the line with %d "
41+ "product(s) '%s' on invoice '%s'.") %
42+ (line_qty, line.product_id.name, invoice.number)
43+ )
44 else:
45 source_uom_id_to_write = source_uom.id
46
47- if source_uom.id == kg_uom_id:
48- weight_to_write = line_qty
49- elif source_uom.category_id.id == weight_uom_categ_id:
50- dest_uom_kg = self.pool.get('product.uom').browse(cr, uid,
51- kg_uom_id, context=context)
52- weight_to_write = self.pool.get('product.uom')._compute_qty_obj(cr, uid,
53- source_uom, line_qty, dest_uom_kg, context=context)
54- elif source_uom.category_id.id == pce_uom_categ_id:
55- if not line.product_id.weight_net:
56- raise orm.except_orm(_('Error :'), _("Missing net weight on product '%s'.") % (line.product_id.name))
57- if source_uom.id == pce_uom_id:
58- weight_to_write = line.product_id.weight_net * line_qty
59+ if source_uom.id == kg_uom_id:
60+ weight_to_write = line_qty
61+ elif source_uom.category_id.id == weight_uom_categ_id:
62+ dest_uom_kg = self.pool.get('product.uom').browse(cr, uid,
63+ kg_uom_id, context=context)
64+ weight_to_write = self.pool.get('product.uom')._compute_qty_obj(cr, uid,
65+ source_uom, line_qty, dest_uom_kg, context=context)
66+ elif source_uom.category_id.id == pce_uom_categ_id:
67+ if not line.product_id.weight_net:
68+ errors[line.id].append(
69+ _("Missing net weight on product '%s'.") %
70+ (line.product_id.name)
71+ )
72+ elif source_uom.id == pce_uom_id:
73+ weight_to_write = line.product_id.weight_net * line_qty
74+ else:
75+ dest_uom_pce = self.pool.get('product.uom').browse(cr, uid,
76+ pce_uom_id, context=context)
77+ # Here, I suppose that, on the product, the weight is per PCE and not per uom_id
78+ weight_to_write = line.product_id.weight_net * self.pool.get('product.uom')._compute_qty_obj(cr, uid, source_uom, line_qty, dest_uom_pce, context=context)
79+
80 else:
81- dest_uom_pce = self.pool.get('product.uom').browse(cr, uid,
82- pce_uom_id, context=context)
83- # Here, I suppose that, on the product, the weight is per PCE and not per uom_id
84- weight_to_write = line.product_id.weight_net * self.pool.get('product.uom')._compute_qty_obj(cr, uid, source_uom, line_qty, dest_uom_pce, context=context)
85-
86- else:
87- raise orm.except_orm(_('Error :'), _("Conversion from unit of measure '%s' to 'Kg' is not implemented yet.") % (source_uom.name))
88+ errors[line.id].append(
89+ _("Conversion from unit of measure '%s' to "
90+ "'Kg' is not implemented yet.") % (source_uom.name)
91+ )
92
93 product_intrastat_code = line.product_id.intrastat_id
94 if not product_intrastat_code:
95 # If the H.S. code is not set on the product, we check if it's set
96 # on it's related category
97 product_intrastat_code = line.product_id.categ_id.intrastat_id
98- if not product_intrastat_code:
99- raise orm.except_orm(_('Error :'), _("Missing H.S. code on product '%s' or on it's related category '%s'.") % (line.product_id.name, line.product_id.categ_id.complete_name))
100- intrastat_code_id_to_write = product_intrastat_code.id
101
102- if not product_intrastat_code.intrastat_code:
103- raise orm.except_orm(_('Error :'), _("Missing intrastat code on H.S. code '%s' (%s).") % (product_intrastat_code.name, product_intrastat_code.description))
104+ if not product_intrastat_code:
105+ errors[line.id].append(
106+ _("Missing H.S. code on product '%s' "
107+ "or on it's related category '%s'.") %
108+ (line.product_id.name,
109+ line.product_id.categ_id.complete_name)
110+ )
111 else:
112- intrastat_code_to_write = product_intrastat_code.intrastat_code
113+ intrastat_code_id_to_write = product_intrastat_code.id
114+
115+ if not product_intrastat_code.intrastat_code:
116+ errors[line.id].append(
117+ _("Missing intrastat code on H.S. code "
118+ "'%s' (%s).") %
119+ (product_intrastat_code.name,
120+ product_intrastat_code.description)
121+ )
122+ else:
123+ intrastat_code_to_write = product_intrastat_code.intrastat_code
124
125 if not product_intrastat_code.intrastat_uom_id:
126 intrastat_uom_id_to_write = False
127@@ -302,7 +335,15 @@
128 uid, source_uom, line_qty,
129 product_intrastat_code.intrastat_uom_id, context=context)
130 else:
131- raise orm.except_orm(_('Error :'), _("On invoice '%s', the line with product '%s' has a unit of measure (%s) which can't be converted to UoM of it's intrastat code (%s).") % (invoice.number, line.product_id.name, source_uom_id_to_write, intrastat_uom_id_to_write))
132+ errors[line.id].append(
133+ _("On invoice '%s', the line with product "
134+ "'%s' has a unit of measure (%s) which can't "
135+ "be converted to UoM of it's intrastat "
136+ "code (%s).") % (invoice.number,
137+ line.product_id.name,
138+ source_uom_id_to_write,
139+ intrastat_uom_id_to_write)
140+ )
141
142 # The origin country should only be declated on Import
143 if intrastat.type == 'export':
144@@ -321,26 +362,27 @@
145 ('origin_country_id', '!=', False)
146 ], context=context)
147 if not supplier_ids:
148- raise orm.except_orm(_('Error :'),
149- _("Missing country of origin on product '%s' or on it's supplier information for partner '%s'.")
150- % (line.product_id.name, parent_values.get('origin_partner_name', 'none')))
151+ errors[line.id].append(
152+ _("Missing country of origin on product "
153+ "'%s' or on it's supplier information for "
154+ "partner '%s'.") %
155+ (line.product_id.name,
156+ parent_values.get('origin_partner_name', 'none'))
157+ )
158 else:
159 product_country_origin_id_to_write = supplieri_obj.read(cr, uid,
160 supplier_ids[0], ['origin_country_id'],
161 context=context)['origin_country_id'][0]
162 else:
163- raise orm.except_orm(_('Error :'),
164- _("Missing country of origin on product '%s' (it's not possible to get the country of origin from the 'supplier information' in this case because we don't know the supplier of this product for the invoice '%s').")
165- % (line.product_id.name, invoice.number))
166-
167- else:
168- weight_to_write = False
169- source_uom_id_to_write = False
170- intrastat_code_id_to_write = False
171- intrastat_code_to_write = False
172- quantity_to_write = False
173- intrastat_uom_id_to_write = False
174- product_country_origin_id_to_write = False
175+ errors[line.id].append(
176+ _("Missing country of origin on product "
177+ "'%s' (it's not possible to get the country "
178+ "of origin from the 'supplier information' "
179+ "in this case because we don't know the "
180+ "supplier of this product for the "
181+ "invoice '%s').") %
182+ (line.product_id.name, invoice.number)
183+ )
184
185
186 create_new_line = True
187@@ -388,24 +430,34 @@
188 # is always True
189 # So we should not block with a raise before the end of the loop on the
190 # invoice lines
191+ invoice_errors = []
192 if lines_to_create:
193+ parent_values['partner_vat_to_write'] = False
194 if parent_values['is_vat_required']:
195 # If I have invoice.intrastat_country_id and the invoice partner
196 # is outside the EU, then I look for the fiscal rep of the partner
197 if invoice.intrastat_country_id and not invoice.partner_id.country_id.intrastat:
198 if not invoice.partner_id.intrastat_fiscal_representative:
199- raise orm.except_orm(_('Error :'), _("Missing fiscal representative for partner '%s'. It is required for invoice '%s' which has an invoice partner outside the EU but the goods were delivered to or received from inside the EU.") % (invoice.partner_id.name, invoice.number))
200+ invoice_errors.append(
201+ _("Missing fiscal representative for partner "
202+ "'%s'. It is required for invoice '%s' which "
203+ "has an invoice partner outside the EU but "
204+ "the goods were delivered to or received from "
205+ "inside the EU.") %
206+ (invoice.partner_id.name, invoice.number)
207+ )
208 else:
209 parent_values['partner_vat_to_write'] = invoice.partner_id.intrastat_fiscal_representative.vat
210 # Otherwise, I just read the vat number on the partner of the invoice
211 else:
212
213 if not invoice.partner_id.vat:
214- raise orm.except_orm(_('Error :'), _("Missing VAT number on partner '%s'.") % invoice.partner_id.name)
215+ invoice_errors.append(
216+ _("Missing VAT number on partner '%s'.") %
217+ invoice.partner_id.name
218+ )
219 else:
220 parent_values['partner_vat_to_write'] = invoice.partner_id.vat
221- else:
222- parent_values['partner_vat_to_write'] = False
223
224 for line_to_create in lines_to_create:
225 line_to_create['partner_vat'] = parent_values['partner_vat_to_write']
226@@ -422,7 +474,6 @@
227
228 line_to_create['amount_invoice_currency'] = line_to_create['amount_product_value_inv_cur'] + line_to_create['amount_accessory_cost_inv_cur']
229
230-
231 # We do currency conversion NOW
232 if invoice.currency_id.name != 'EUR':
233 # for currency conversion
234@@ -439,13 +490,13 @@
235 for value in ['quantity', 'weight']: # These 2 fields are char
236 if line_to_create[value]:
237 line_to_create[value] = str(int(round(line_to_create[value], 0)))
238- line_obj.create(cr, uid, line_to_create, context=context)
239-
240- return True
241-
242+ if not errors and not invoice_errors:
243+ line_obj.create(cr, uid, line_to_create, context=context)
244+
245+ return invoice_errors, errors
246
247 def compute_invoice_values(self, cr, uid, intrastat, invoice, parent_values, context=None):
248-
249+ errors = []
250 intrastat_type = self.pool.get('report.intrastat.type').read(cr, uid, parent_values['intrastat_type_id_to_write'], context=context)
251 parent_values['procedure_code_to_write'] = intrastat_type['procedure_code']
252 parent_values['transaction_code_to_write'] = intrastat_type['transaction_code']
253@@ -456,10 +507,20 @@
254 # force to is_fiscal_only
255 parent_values['is_fiscal_only'] = True
256
257+ parent_values.update({
258+ 'department_to_write': False,
259+ 'transport_to_write': False,
260+ })
261+ parent_values.setdefault('partner_country_id_to_write', False)
262 if not parent_values['is_fiscal_only']:
263 if not invoice.intrastat_transport:
264 if not intrastat.company_id.default_intrastat_transport:
265- raise orm.except_orm(_('Error :'), _("The mode of transport is not set on invoice '%s' nor the default mode of transport on the company '%s'.") % (invoice.number, intrastat.company_id.name))
266+ errors.append(
267+ _("The mode of transport is not set on invoice '%s' "
268+ "nor the default mode of transport on "
269+ "the company '%s'.") %
270+ (invoice.number, intrastat.company_id.name)
271+ )
272 else:
273 parent_values['transport_to_write'] = intrastat.company_id.default_intrastat_transport
274 else:
275@@ -467,18 +528,22 @@
276
277 if not invoice.intrastat_department:
278 if not intrastat.company_id.default_intrastat_department:
279- raise orm.except_orm(_('Error :'), _("The intrastat department hasn't been set on invoice '%s' and the default intrastat department is missing on the company '%s'.") % (invoice.number, intrastat.company_id.name))
280+ errors.append(
281+ _("The intrastat department hasn't been set on "
282+ "invoice '%s' and the default intrastat department "
283+ "is missing on the company '%s'.") %
284+ (invoice.number, intrastat.company_id.name)
285+ )
286 else:
287 parent_values['department_to_write'] = intrastat.company_id.default_intrastat_department
288 else:
289 parent_values['department_to_write'] = invoice.intrastat_department
290 else:
291- parent_values['department_to_write'] = False
292- parent_values['transport_to_write'] = False
293- parent_values['transaction_code_to_write'] = False
294- parent_values['partner_country_id_to_write'] = False
295- #print "parent_values =", parent_values
296- return parent_values
297+ parent_values.update({
298+ 'transaction_code_to_write': False,
299+ 'partner_country_id_to_write': False,
300+ })
301+ return parent_values, errors
302
303
304 def generate_product_lines_from_invoice(self, cr, uid, ids, context=None):
305@@ -497,7 +562,7 @@
306 if intrastat.type == 'import':
307 # Les régularisations commerciales à l'HA ne sont PAS
308 # déclarées dans la DEB, cf page 50 du BOD 6883 du 06 janvier 2011
309- invoice_type = ('in_invoice', 'POUET') # I need 'POUET' to make it a tuple
310+ invoice_type = 'in_invoice', # must be a tuple
311 if intrastat.type == 'export':
312 invoice_type = ('out_invoice', 'out_refund')
313 invoice_ids = invoice_obj.search(cr, uid, [
314@@ -508,13 +573,15 @@
315 ('company_id', '=', intrastat.company_id.id)
316 ], order='date_invoice', context=context)
317 #print "invoice_ids=", invoice_ids
318+ errors = defaultdict(list)
319+ line_errors = {}
320 for invoice in invoice_obj.browse(cr, uid, invoice_ids, context=context):
321 #print "INVOICE num =", invoice.number
322 parent_values = {}
323
324 # We should always have a country on partner_id
325 if not invoice.partner_id.country_id:
326- raise orm.except_orm(_('Error :'), _("Missing country on partner '%s'.") % invoice.partner_id.name)
327+ errors[invoice.id].append(_("Missing country on partner '%s'.") % invoice.partner_id.name)
328
329 # If I have no invoice.intrastat_country_id, which is the case the first month
330 # of the deployment of the module, then I use the country on invoice partner
331@@ -540,17 +607,38 @@
332 if intrastat.company_id.default_intrastat_type_out_invoice:
333 parent_values['intrastat_type_id_to_write'] = intrastat.company_id.default_intrastat_type_out_invoice.id
334 else:
335- raise orm.except_orm(_('Error :'), _("The intrastat type hasn't been set on invoice '%s' and the 'default intrastat type for customer invoice' is missing for the company '%s'.") % (invoice.number, intrastat.company_id.name))
336+ raise orm.except_orm(
337+ _('Error'),
338+ _("The intrastat type hasn't been set on "
339+ "invoice '%s' and the 'default intrastat type "
340+ "for customer invoice' is missing for "
341+ "the company '%s'.") % (invoice.number,
342+ intrastat.company_id.name)
343+ )
344 elif invoice.type == 'out_refund':
345 if intrastat.company_id.default_intrastat_type_out_refund:
346 parent_values['intrastat_type_id_to_write'] = intrastat.company_id.default_intrastat_type_out_refund.id
347 else:
348- raise orm.except_orm(_('Error :'), _("The intrastat type hasn't been set on refund '%s' and the 'default intrastat type for customer refund' is missing for the company '%s'.") % (invoice.number, intrastat.company_id.name))
349+ raise orm.except_orm(
350+ _('Error'),
351+ _("The intrastat type hasn't been set on refund "
352+ "'%s' and the 'default intrastat type for "
353+ "customer refund' is missing for the company"
354+ "'%s'.") % (invoice.number,
355+ intrastat.company_id.name)
356+ )
357 elif invoice.type == 'in_invoice':
358 if intrastat.company_id.default_intrastat_type_in_invoice:
359 parent_values['intrastat_type_id_to_write'] = intrastat.company_id.default_intrastat_type_in_invoice.id
360 else:
361- raise orm.except_orm(_('Error :'), _("The intrastat type hasn't been set on invoice '%s' and the 'Default intrastat type for supplier invoice' is missing for the company '%s'.") % (invoice.number, intrastat.company_id.name))
362+ raise orm.except_orm(
363+ _('Error'),
364+ _("The intrastat type hasn't been set on invoice "
365+ "'%s' and the 'Default intrastat type for "
366+ "supplier invoice' is missing for the company "
367+ "'%s'.") % (invoice.number,
368+ intrastat.company_id.name)
369+ )
370 else:
371 raise orm.except_orm(_('Error :'), "Hara kiri... we can't have a supplier refund")
372
373@@ -567,10 +655,88 @@
374 parent_values['origin_partner_id'] = invoice.partner_id.id
375 parent_values['origin_partner_name'] = invoice.partner_id.name
376
377- parent_values = self.compute_invoice_values(cr, uid, intrastat, invoice, parent_values, context=context)
378-
379- self.create_intrastat_product_lines(cr, uid, ids, intrastat, invoice, parent_values, context=context)
380-
381+ parent_values, vals_errors = self.compute_invoice_values(
382+ cr, uid, intrastat, invoice, parent_values, context=context)
383+ if vals_errors:
384+ errors[invoice.id] += vals_errors
385+
386+ # message_post
387+ create_errors, create_line_errors = self.create_intrastat_product_lines(
388+ cr, uid, ids, intrastat, invoice,
389+ parent_values, context=context)
390+
391+ if create_errors:
392+ errors[invoice.id] += create_errors
393+ if create_line_errors:
394+ line_errors[invoice.id] = create_line_errors
395+
396+ if errors or line_errors:
397+ message_template = _(
398+ "<b>The generation of lines could not be completed "
399+ "due to the following issues:</b>"
400+ "<p>"
401+ "%s"
402+ "</p>"
403+ )
404+ invoice_template = _(
405+ "<b>Invoice %(name)s:</b>"
406+ "<ul>"
407+ "%(errors)s"
408+ "</ul>"
409+ "<ul>"
410+ "%(lines)s"
411+ "</ul>"
412+ )
413+ lines_template = _(
414+ "<b>Line %(name)s:</b>"
415+ "<ul>"
416+ "%(errors)s"
417+ "</ul>"
418+ )
419+ invoice_line_obj = self.pool['account.invoice.line']
420+ invoice_messages = []
421+ for invoice_id in invoice_ids:
422+ line_message = []
423+ for line_id, line_errs in line_errors.get(invoice_id, {}).iteritems():
424+ line = invoice_line_obj.browse(cr, uid, line_id,
425+ context=context)
426+ line_msg = lines_template % {
427+ 'name': line.name,
428+ 'errors': ''.join("<li>%s</li>" % err for err
429+ in line_errs)
430+ }
431+ line_message.append(line_msg)
432+
433+ invoice = invoice_obj.browse(cr, uid, invoice_id,
434+ context=context)
435+ inv_errs = errors.get(invoice_id, [])
436+ if inv_errs or line_message:
437+ invoice_message = invoice_template % {
438+ 'name': invoice.number,
439+ 'errors': ''.join("<li>%s</li>" % err for err
440+ in inv_errs),
441+ 'lines': ''.join(line_message),
442+ }
443+ invoice_messages.append(invoice_message)
444+ message = message_template % ''.join(invoice_messages)
445+
446+ db = pooler.get_db(cr.dbname)
447+ local_cr = db.cursor()
448+ try:
449+ self.message_post(local_cr, uid, ids, body=message,
450+ subtype='mail.mt_comment', context=context)
451+ except:
452+ local_cr.rollback()
453+ raise
454+ else:
455+ local_cr.commit()
456+ finally:
457+ local_cr.close()
458+ raise orm.except_orm(
459+ _('Error'),
460+ _('The lines could not be generated, please '
461+ 'refresh the form and see the messages.')
462+ )
463 return True
464
465

Subscribers

People subscribed via source and target branches