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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Guewen Baconnier @ Camptocamp | code review | Approve | |
Review via email: mp+193927@code.launchpad.net |
Commit message
Description of the change
Hello,
This merge proposal concerns
https:/
and has been started here:
https:/
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 : | # |
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}) |
Thanks a lot!
LGTM