Merge lp:~camptocamp/openerp-connector-magento/7.0-magentoerpconnect-handle-shipping-tax-in-tax_included-context-rde into lp:~openerp-connector-core-editors/openerp-connector-magento/7.0

Proposed by Romain Deheele - Camptocamp
Status: Merged
Merged at revision: 942
Proposed branch: lp:~camptocamp/openerp-connector-magento/7.0-magentoerpconnect-handle-shipping-tax-in-tax_included-context-rde
Merge into: lp:~openerp-connector-core-editors/openerp-connector-magento/7.0
Diff against target: 447 lines (+413/-6)
3 files modified
magentoerpconnect/sale.py (+9/-5)
magentoerpconnect/tests/test_data.py (+373/-0)
magentoerpconnect/tests/test_synchronization.py (+31/-1)
To merge this branch: bzr merge lp:~camptocamp/openerp-connector-magento/7.0-magentoerpconnect-handle-shipping-tax-in-tax_included-context-rde
Reviewer Review Type Date Requested Status
Guewen Baconnier @ Camptocamp code review Approve
Review via email: mp+193927@code.launchpad.net

Description of the change

Hello,

This merge proposal concerns
https://bugs.launchpad.net/openerp-connector/+bug/1234117
and has been started here:
https://code.launchpad.net/~camptocamp/openerp-connector/7.0-magentoerpconnect-handle-shipping-tax-in-tax_included-context/+merge/189592
But I create a new cleaner merge proposal (the previous has some text conflicts)

It includes:
-better tax included management
-unit test

Romain

To post a comment you must log in.
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Thanks a lot!
LGTM

review: Approve (code review)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'magentoerpconnect/sale.py'
2--- magentoerpconnect/sale.py 2013-10-30 13:15:07 +0000
3+++ magentoerpconnect/sale.py 2013-11-05 13:59:19 +0000
4@@ -675,11 +675,15 @@
5 sess = self.session
6 # TODO: refactor: do no longer store the transient fields in the
7 # result, use a ConnectorUnit to create the lines
8- result = sess.pool['sale.order']._convert_special_fields(sess.cr,
9- sess.uid,
10- result,
11- result['magento_order_line_ids'],
12- sess.context)
13+ backend = self.backend_record
14+ # in tax_included context, need to pass it in context
15+ tax_included = backend.catalog_price_tax_included
16+ with self.session.change_context({'is_tax_included': tax_included}):
17+ result = sess.pool['sale.order']._convert_special_fields(sess.cr,
18+ sess.uid,
19+ result,
20+ result['magento_order_line_ids'],
21+ sess.context)
22 # remove transient fields otherwise OpenERP will raise a warning
23 # or even fail to create the record because the fields do not
24 # exist
25
26=== modified file 'magentoerpconnect/tests/test_data.py'
27--- magentoerpconnect/tests/test_data.py 2013-10-30 13:16:37 +0000
28+++ magentoerpconnect/tests/test_data.py 2013-11-05 13:59:19 +0000
29@@ -25398,4 +25398,377 @@
30 'updated_at': '2013-10-30 09:43:20',
31 'weight': '2.0000',
32 'x_forwarded_for': None},
33+('sales_order.info', (900000695, None)): {'adjustment_negative': None,
34+ 'adjustment_positive': None,
35+ 'applied_rule_ids': None,
36+ 'base_adjustment_negative': None,
37+ 'base_adjustment_positive': None,
38+ 'base_currency_code': 'EUR',
39+ 'base_discount_amount': '0.0000',
40+ 'base_discount_canceled': None,
41+ 'base_discount_invoiced': '0.0000',
42+ 'base_discount_refunded': None,
43+ 'base_grand_total': '97.5000',
44+ 'base_hidden_tax_amount': '0.0000',
45+ 'base_hidden_tax_invoiced': '0.0000',
46+ 'base_hidden_tax_refunded': None,
47+ 'base_shipping_amount': '7.8700',
48+ 'base_shipping_canceled': None,
49+ 'base_shipping_discount_amount': '0.0000',
50+ 'base_shipping_hidden_tax_amnt': '0.0000',
51+ 'base_shipping_hidden_tax_amount': '0.0000',
52+ 'base_shipping_incl_tax': '8.5000',
53+ 'base_shipping_invoiced': '7.8700',
54+ 'base_shipping_refunded': None,
55+ 'base_shipping_tax_amount': '0.6300',
56+ 'base_shipping_tax_refunded': None,
57+ 'base_subtotal': '82.4100',
58+ 'base_subtotal_canceled': None,
59+ 'base_subtotal_incl_tax': '89.0000',
60+ 'base_subtotal_invoiced': '82.4100',
61+ 'base_subtotal_refunded': None,
62+ 'base_tax_amount': '7.2200',
63+ 'base_tax_canceled': None,
64+ 'base_tax_invoiced': '7.2200',
65+ 'base_tax_refunded': None,
66+ 'base_to_global_rate': '1.0000',
67+ 'base_to_order_rate': '1.0000',
68+ 'base_total_canceled': None,
69+ 'base_total_due': '0.0000',
70+ 'base_total_invoiced': '97.5000',
71+ 'base_total_invoiced_cost': '0.0000',
72+ 'base_total_offline_refunded': None,
73+ 'base_total_online_refunded': None,
74+ 'base_total_paid': '97.5000',
75+ 'base_total_qty_ordered': None,
76+ 'base_total_refunded': None,
77+ 'billing_address': {'address_id': '38468',
78+ 'address_type': 'billing',
79+ 'city': 'Lyon',
80+ 'company': None,
81+ 'country_id': 'FR',
82+ 'customer_address_id': '4999',
83+ 'customer_id': None,
84+ 'email': 'john@doe.fr',
85+ 'fax': None,
86+ 'firstname': 'John',
87+ 'lastname': 'Doe',
88+ 'middlename': None,
89+ 'parent_id': '7360',
90+ 'postcode': '1006',
91+ 'prefix': None,
92+ 'quote_address_id': None,
93+ 'region': None,
94+ 'region_id': None,
95+ 'street': 'Victor Hugo',
96+ 'suffix': None,
97+ 'telephone': '31763163390',
98+ 'vat_id': None,
99+ 'vat_is_valid': None,
100+ 'vat_request_date': None,
101+ 'vat_request_id': None,
102+ 'vat_request_success': None},
103+ 'billing_address_id': '38468',
104+ 'can_ship_partially': None,
105+ 'can_ship_partially_item': None,
106+ 'coupon_code': None,
107+ 'coupon_rule_name': None,
108+ 'created_at': '2013-10-14 13:20:56',
109+ 'customer_dob': None,
110+ 'customer_email': 'john@doe.fr',
111+ 'customer_firstname': 'John',
112+ 'customer_gender': None,
113+ 'customer_group_id': '1',
114+ 'customer_id': '1',
115+ 'customer_is_guest': '0',
116+ 'customer_lastname': 'Doe',
117+ 'customer_middlename': None,
118+ 'customer_note': None,
119+ 'customer_note_notify': '1',
120+ 'customer_prefix': None,
121+ 'customer_suffix': None,
122+ 'customer_taxvat': None,
123+ 'discount_amount': '0.0000',
124+ 'discount_canceled': None,
125+ 'discount_description': None,
126+ 'discount_invoiced': '0.0000',
127+ 'discount_refunded': None,
128+ 'edit_increment': None,
129+ 'email_sent': '1',
130+ 'employee_email': None,
131+ 'employee_name': None,
132+ 'employee_phone': None,
133+ 'ext_customer_id': None,
134+ 'ext_order_id': None,
135+ 'forced_do_shipment_with_invoice': None,
136+ 'forced_shipment_with_invoice': None,
137+ 'gift_message_id': None,
138+ 'global_currency_code': 'EUR',
139+ 'grand_total': '97.5000',
140+ 'hidden_tax_amount': '0.0000',
141+ 'hidden_tax_invoiced': '0.0000',
142+ 'hidden_tax_refunded': None,
143+ 'hold_before_state': None,
144+ 'hold_before_status': None,
145+ 'imported': '0',
146+ 'increment_id': '100005281',
147+ 'is_virtual': '0',
148+ 'items': [{'additional_data': None,
149+ 'amount_refunded': '0.0000',
150+ 'applied_rule_ids': None,
151+ 'base_amount_refunded': '0.0000',
152+ 'base_cost': None,
153+ 'base_discount_amount': '0.0000',
154+ 'base_discount_invoiced': '0.0000',
155+ 'base_discount_refunded': None,
156+ 'base_hidden_tax_amount': None,
157+ 'base_hidden_tax_invoiced': '0.0000',
158+ 'base_hidden_tax_refunded': None,
159+ 'base_original_price': '89.0000',
160+ 'base_price': '82.4100',
161+ 'base_price_incl_tax': '89.0000',
162+ 'base_row_invoiced': '82.4100',
163+ 'base_row_total': '82.4100',
164+ 'base_row_total_incl_tax': '89.0000',
165+ 'base_tax_amount': '6.5900',
166+ 'base_tax_before_discount': None,
167+ 'base_tax_invoiced': '6.5900',
168+ 'base_tax_refunded': None,
169+ 'base_weee_tax_applied_amount': '0.0000',
170+ 'base_weee_tax_applied_row_amnt': '0.0000',
171+ 'base_weee_tax_applied_row_amount': '0.0000',
172+ 'base_weee_tax_disposition': '0.0000',
173+ 'base_weee_tax_row_disposition': '0.0000',
174+ 'created_at': '2013-10-14 13:20:56',
175+ 'description': None,
176+ 'discount_amount': '0.0000',
177+ 'discount_invoiced': '0.0000',
178+ 'discount_percent': '0.0000',
179+ 'discount_refunded': None,
180+ 'earned_points_hash': None,
181+ 'ext_order_item_id': None,
182+ 'free_shipping': '0',
183+ 'gift_message_available': None,
184+ 'gift_message_id': None,
185+ 'hidden_tax_amount': None,
186+ 'hidden_tax_canceled': None,
187+ 'hidden_tax_invoiced': '0.0000',
188+ 'hidden_tax_refunded': None,
189+ 'is_nominal': '0',
190+ 'is_qty_decimal': '0',
191+ 'is_virtual': '0',
192+ 'item_id': '12751',
193+ 'locked_do_invoice': None,
194+ 'locked_do_ship': None,
195+ 'name': 'Dior Homme',
196+ 'no_discount': '0',
197+ 'order_id': '7360',
198+ 'original_price': '89.0000',
199+ 'parent_item_id': None,
200+ 'price': '82.4100',
201+ 'price_incl_tax': '89.0000',
202+ 'product_id': '157',
203+ 'product_options': 'a:1:{s:15:"info_buyRequest";a:6:{s:4:"uenc";s:176:"aHR0cDovL3d3dy4xMDAwcGFyZnVtcy5jaC9ib3V0aXF1ZTEuNy4wLjIvZnIvcGFyZnVtLWhvbW1lcy1ldC1mZW1tZXMtcGFzLWNoZXIvcGFyZnVtLWRpb3ItcGFzLWNoZXIvcGFyZnVtLWRpb3ItaG9tbWUtcGFzLWNoZXIuaHRtbA,,";s:7:"product";s:4:"4773";s:15:"related_product";s:0:"";s:3:"qty";s:1:"1";s:1:"x";s:2:"70";s:1:"y";s:2:"23";}}',
204+ 'product_type': 'simple',
205+ 'qty_backordered': None,
206+ 'qty_canceled': '0.0000',
207+ 'qty_invoiced': '1.0000',
208+ 'qty_ordered': '1.0000',
209+ 'qty_refunded': '0.0000',
210+ 'qty_shipped': '0.0000',
211+ 'quote_item_id': '46485',
212+ 'redeemed_points_hash': None,
213+ 'row_invoiced': '82.4100',
214+ 'row_total': '82.4100',
215+ 'row_total_after_redemptions': None,
216+ 'row_total_after_redemptions_incl_tax': None,
217+ 'row_total_before_redemptions': None,
218+ 'row_total_before_redemptions_incl_tax': None,
219+ 'row_total_incl_tax': '89.0000',
220+ 'row_weight': '0.0000',
221+ 'sku': '1625',
222+ 'store_id': '1',
223+ 'tax_amount': '6.5900',
224+ 'tax_before_discount': None,
225+ 'tax_canceled': None,
226+ 'tax_invoiced': '6.5900',
227+ 'tax_percent': '8.0000',
228+ 'tax_refunded': None,
229+ 'updated_at': '2013-10-14 13:22:00',
230+ 'weee_tax_applied': 'a:0:{}',
231+ 'weee_tax_applied_amount': '0.0000',
232+ 'weee_tax_applied_row_amount': '0.0000',
233+ 'weee_tax_disposition': '0.0000',
234+ 'weee_tax_row_disposition': '0.0000',
235+ 'weight': None}],
236+ 'order_currency_code': 'EUR',
237+ 'order_id': '7360',
238+ 'original_increment_id': None,
239+ 'payment': {'account_status': None,
240+ 'additional_data': None,
241+ 'address_status': None,
242+ 'amount_authorized': None,
243+ 'amount_canceled': None,
244+ 'amount_ordered': '97.5000',
245+ 'amount_paid': '97.5000',
246+ 'amount_refunded': None,
247+ 'anet_trans_method': None,
248+ 'base_amount_authorized': None,
249+ 'base_amount_canceled': None,
250+ 'base_amount_ordered': '97.5000',
251+ 'base_amount_paid': '97.5000',
252+ 'base_amount_paid_online': None,
253+ 'base_amount_refunded': None,
254+ 'base_amount_refunded_online': None,
255+ 'base_shipping_amount': '7.8700',
256+ 'base_shipping_captured': '7.8700',
257+ 'base_shipping_refunded': None,
258+ 'cc_approval': None,
259+ 'cc_avs_status': None,
260+ 'cc_cid_status': None,
261+ 'cc_debug_request_body': None,
262+ 'cc_debug_response_body': None,
263+ 'cc_debug_response_serialized': None,
264+ 'cc_exp_month': '0',
265+ 'cc_exp_year': '0',
266+ 'cc_last4': None,
267+ 'cc_number_enc': None,
268+ 'cc_owner': None,
269+ 'cc_secure_verify': None,
270+ 'cc_ss_issue': None,
271+ 'cc_ss_start_month': '0',
272+ 'cc_ss_start_year': '0',
273+ 'cc_status': None,
274+ 'cc_status_description': None,
275+ 'cc_trans_id': None,
276+ 'cc_type': None,
277+ 'cybersource_token': None,
278+ 'echeck_account_name': None,
279+ 'echeck_account_type': None,
280+ 'echeck_bank_name': None,
281+ 'echeck_routing_number': None,
282+ 'echeck_type': None,
283+ 'flo2cash_account_id': None,
284+ 'ideal_issuer_id': None,
285+ 'ideal_issuer_title': None,
286+ 'ideal_transaction_checked': None,
287+ 'last_trans_id': '1P06829259760661R',
288+ 'method': 'checkmo',
289+ 'parent_id': '7360',
290+ 'paybox_question_number': None,
291+ 'paybox_request_number': None,
292+ 'payment_id': '32844',
293+ 'po_number': None,
294+ 'protection_eligibility': None,
295+ 'quote_payment_id': None,
296+ 'shipping_amount': '7.8700',
297+ 'shipping_captured': '7.8700',
298+ 'shipping_refunded': None},
299+ 'payment_auth_expiration': None,
300+ 'payment_authorization_amount': None,
301+ 'payment_authorization_expiration': None,
302+ 'paypal_ipn_customer_notified': None,
303+ 'protect_code': '49accf',
304+ 'quote_address_id': None,
305+ 'quote_id': '33794',
306+ 'relation_child_id': None,
307+ 'relation_child_real_id': None,
308+ 'relation_parent_id': None,
309+ 'relation_parent_real_id': None,
310+ 'remote_ip': None,
311+ 'rewards_base_discount_amount': None,
312+ 'rewards_base_discount_tax_amount': None,
313+ 'rewards_discount_amount': None,
314+ 'rewards_discount_tax_amount': None,
315+ 'shipping_address': {'address_id': '38469',
316+ 'address_type': 'shipping',
317+ 'city': 'Lyon',
318+ 'company': None,
319+ 'country_id': 'FR',
320+ 'customer_address_id': None,
321+ 'customer_id': None,
322+ 'email': 'john@doe.fr',
323+ 'fax': None,
324+ 'firstname': 'John',
325+ 'lastname': 'Doe',
326+ 'middlename': None,
327+ 'parent_id': '7360',
328+ 'postcode': '1006',
329+ 'prefix': None,
330+ 'quote_address_id': None,
331+ 'region': None,
332+ 'region_id': None,
333+ 'street': 'Victor Hugo',
334+ 'suffix': None,
335+ 'telephone': '31763163390',
336+ 'vat_id': None,
337+ 'vat_is_valid': None,
338+ 'vat_request_date': None,
339+ 'vat_request_id': None,
340+ 'vat_request_success': None},
341+ 'shipping_address_id': '38469',
342+ 'shipping_amount': '7.8700',
343+ 'shipping_canceled': None,
344+ 'shipping_description': 'La Poste',
345+ 'shipping_discount_amount': '0.0000',
346+ 'shipping_hidden_tax_amount': '0.0000',
347+ 'shipping_incl_tax': '8.5000',
348+ 'shipping_invoiced': '7.8700',
349+ 'shipping_method': 'flatrate_flatrate',
350+ 'shipping_refunded': None,
351+ 'shipping_tax_amount': '0.6300',
352+ 'shipping_tax_refunded': None,
353+ 'state': 'processing',
354+ 'status': 'processing',
355+ 'status_history': [{'comment': u'Notifier le client pour la facture n\xb0100003489.',
356+ 'created_at': '2013-10-14 13:22:02',
357+ 'entity_name': 'invoice',
358+ 'is_customer_notified': '1',
359+ 'is_visible_on_front': '0',
360+ 'parent_id': '7360',
361+ 'status': 'processing',
362+ 'store_id': '1'},
363+ {'comment': u'IPN "Completed". Registered notification about captured amount of 97,50\xa0EUR. Transaction ID: "1P06829259760661R".',
364+ 'created_at': '2013-10-14 13:22:00',
365+ 'entity_name': 'invoice',
366+ 'is_customer_notified': '2',
367+ 'is_visible_on_front': '0',
368+ 'parent_id': '7360',
369+ 'status': 'processing',
370+ 'store_id': '1'},
371+ {'comment': None,
372+ 'created_at': '2013-10-14 13:20:58',
373+ 'entity_name': 'order',
374+ 'is_customer_notified': '0',
375+ 'is_visible_on_front': '0',
376+ 'parent_id': '7360',
377+ 'status': 'pending_payment',
378+ 'store_id': '1'}],
379+ 'store_currency_code': 'EUR',
380+ 'store_id': '1',
381+ 'store_name': u'France',
382+ 'store_to_base_rate': '1.0000',
383+ 'store_to_order_rate': '1.0000',
384+ 'subtotal': '82.4100',
385+ 'subtotal_canceled': None,
386+ 'subtotal_incl_tax': '89.0000',
387+ 'subtotal_invoiced': '82.4100',
388+ 'subtotal_refunded': None,
389+ 'tax_amount': '7.2200',
390+ 'tax_canceled': None,
391+ 'tax_invoiced': '7.2200',
392+ 'tax_refunded': None,
393+ 'total_canceled': None,
394+ 'total_due': '0.0000',
395+ 'total_invoiced': '97.5000',
396+ 'total_item_count': '1',
397+ 'total_offline_refunded': None,
398+ 'total_online_refunded': None,
399+ 'total_paid': '97.5000',
400+ 'total_qty_ordered': '1.0000',
401+ 'total_refunded': None,
402+ 'updated_at': '2013-10-14 13:22:00',
403+ 'website_id': u'1',
404+ 'weight': '0.0000',
405+ 'x_forwarded_for': None},
406 }
407
408=== modified file 'magentoerpconnect/tests/test_synchronization.py'
409--- magentoerpconnect/tests/test_synchronization.py 2013-10-31 08:10:45 +0000
410+++ magentoerpconnect/tests/test_synchronization.py 2013-11-05 13:59:19 +0000
411@@ -286,4 +286,34 @@
412 self.uid,
413 order_line_id[0],
414 ['price_unit'])['price_unit']
415- self.assertEqual(price_unit, 41.0500)
416\ No newline at end of file
417+ self.assertEqual(price_unit, 41.0500)
418+
419+ def test_34_import_sale_order_with_taxes_included(self):
420+ """ Import a sale order with taxes included """
421+ backend_id = self.backend_id
422+ self.backend_model.write(self.cr, self.uid, self.backend_id,
423+ {'catalog_price_tax_included': True})
424+ with mock_api(magento_base_responses):
425+ with mock_urlopen_image():
426+ import_record(self.session,
427+ 'magento.sale.order',
428+ backend_id, 900000695)
429+ mag_order_model = self.registry('magento.sale.order')
430+ mag_order_ids = mag_order_model.search(self.cr,
431+ self.uid,
432+ [('backend_id', '=', backend_id),
433+ ('magento_id', '=', '900000695')])
434+ self.assertEqual(len(mag_order_ids), 1)
435+ order_id = mag_order_model.read(self.cr,
436+ self.uid,
437+ mag_order_ids[0],
438+ ['openerp_id'])['openerp_id']
439+ order_model = self.registry('sale.order')
440+ amount_total = order_model.read(self.cr,
441+ self.uid,
442+ order_id[0],
443+ ['amount_total'])['amount_total']
444+ #97.5 is the amount_total if connector takes correctly included tax prices.
445+ self.assertEqual(amount_total, 97.5000)
446+ self.backend_model.write(self.cr, self.uid, self.backend_id,
447+ {'catalog_price_tax_included': False})