Merge lp:~akretion-team/openobject-addons/addons_purchase into lp:openobject-addons

[Benoit Guillot - http://www.akretion.com]

This is a refactoring of date_planned in the class purchase_order_line in order to overload it easily. I define a new fonction: _get_date_planned. I need it for a module calculating the delivery orders considering the working time.

Moreover, it seems to me that having int(seller_delay) or 0.0 is useless. Indeed, if seller_delay == False, int() will return 0.
I can add a commit to delete "or 0.0" if it's necessary.

[Florent (florent.x)]

The patch looks OK.
However, the returned value can be simplified as:

  return str(datetime.now() + relativedelta(days=int(seller_delay)))

Because str() returns the correct value:
 - on a 'date' it returns '%Y-%m-%d'
 - on a 'datetime' it returns '%Y-%m-%d %H:%M:%S'

In this case the date_planned is a 'fields.date' object, so it's ok to store '%Y-%m-%d' on it.
The docstring should be fixed accordingly.

[Florent (florent.x)]

On second reading, the patch is not correct.

The method _get_date_planned() should return a date (or datetime) object (as described in its docstring), not a string representing the date.

This is consistent with the sibling methods in this module: _get_purchase_schedule_date and _get_purchase_order_date

The date formatting should remain in the caller method (product_id_change):

    dt = self._get_date_planned(...).strftime(DEFAULT_SERVER_DATETIME_FORMAT)

review: Needs Fixing

[lp:~akretion-team/openobject-addons/addons_purchase]

6337. By Benoit Guillot - http://www.akretion.com on 2012-01-17

[FIX] Change the returned type to fit with the other methods: _get_purchase_schedule_date and _get_purchase_order_date

[Benoit Guillot - http://www.akretion.com (benoit-guillot-z)]

Hello Florent,

Thanks for your reply.

Indeed the return type should be a datetime, it's better to fit with the similar methods : _get_purchase_schedule_date and _get_purchase_order_date as you said.
I pushed the changes according to your advice.

[Florent (florent.x)]

LGTM.

review: Approve

[lp:~akretion-team/openobject-addons/addons_purchase]

6338. By Benoit Guillot - http://www.akretion.com on 2012-01-23

[REF] Refactor the function purchase/_get_date_planned to have the seller informations in the function

[Benoit Guillot - http://www.akretion.com (benoit-guillot-z)]

Hello,

I made a refactor of my merge proposal. Instead of giving the seller delay as argument of the function _get_date_planned(), I propose to give the seller. It's better for me because in the module calculating the delivery delays I'm creating, I have an other information in the seller useful to get the date planned. Moreover, in general it's better to have a global argument.

Best regards,

[Florent (florent.x)]

small typo in docstring `param int seller`.

[Florent (florent.x)]

> small typo in docstring `param int seller`.

And I would do the convert "int(seller.delay)" in the "if".

[lp:~akretion-team/openobject-addons/addons_purchase]

6339. By Benoit Guillot - http://www.akretion.com on 2012-01-23

[FIX] fix typo and change the "int" conversion

[Benoit Guillot - http://www.akretion.com (benoit-guillot-z)]

Thank you Florent for your wise advices !

[Olivier Dony (Odoo) (odo-openerp)]

Looks good, but some minor things could still be fixed:

- improved docstring: browse -> browse_record, second param is not a supplier, it's a "supplier info"
- better names: seller parameter would better be named "supplier_info"
- l.16 - simpler test:
   seller_delay = int(seller.delay) if seller else 0
- minor refactoring needing in the original code: l.39 - the `for` loop is rather weird and unintuitive. The search() is not supposed to return multiple results, but if it does, the loop effect may be inconsistent.

We'll fix the above when merging.

Benoit and Florent, thanks a lot for your great collaborative work!

review: Approve

[Benoit Guillot - http://www.akretion.com (benoit-guillot-z)]

Thank you Olivier for the fix and the merge.

[Benoit Guillot - http://www.akretion.com (benoit-guillot-z)]

Hello Olivier,

After a quick look on the updated addons, I can't find the changes I proposed.
Is the merge still planned in the 6.1 ?

Best regards,

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello, I did a fresh rebase of this MP here https://code.launchpad.net/~akretion-team/openobject-addons/purchase-date-planned2/+register-merge

Can you merge the new one so we close this one too?

[Antony Lesuisse (OpenERP) (al-openerp)]

Closing this one superseded by:
https://code.launchpad.net/~akretion-team/openobject-addons/purchase-date-planned2/+merge/93316

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Not sure this link work for you, here is a better one for the new MP:
https://code.launchpad.net/~akretion-team/openobject-addons/purchase-date-planned2/+merge/93316

Unmerged revisions

6339. By Benoit Guillot - http://www.akretion.com on 2012-01-23

[FIX] fix typo and change the "int" conversion

6338. By Benoit Guillot - http://www.akretion.com on 2012-01-23

[REF] Refactor the function purchase/_get_date_planned to have the seller informations in the function

6337. By Benoit Guillot - http://www.akretion.com on 2012-01-17

[FIX] Change the returned type to fit with the other methods: _get_purchase_schedule_date and _get_purchase_order_date

6336. By Benoit Guillot - http://www.akretion.com on 2012-01-12

[REF]refactor of the date_planned in the purchase order line

=== modified file 'purchase/purchase.py'
--- purchase/purchase.py	2012-01-11 08:53:48 +0000
+++ purchase/purchase.py	2012-01-23 16:00:32 +0000
@@ -697,6 +697,20 @@
         default.update({'state':'draft', 'move_ids':[],'invoiced':0,'invoice_lines':[]})
         return super(purchase_order_line, self).copy_data(cr, uid, id, default, context)
 
+    def _get_date_planned(self, cr, uid, seller, context=None):
+        """Return the datetime value to use as Schedule Date (``date_planned``) for the
+           Purchase Order Lines created in the purchase order.
+
+           :param browse seller: the supplier of the product.
+           :rtype: datetime
+           :return: the desired Schedule Date for the PO lines
+        """
+        if seller: 
+            seller_delay = int(seller.delay)
+        else:
+            seller_delay = 0
+        return datetime.now() + relativedelta(days=seller_delay)
+
     #TOFIX:
     # - name of method should "onchange_product_id"
     # - docstring
@@ -728,7 +742,7 @@
         if not date_order:
             date_order = time.strftime('%Y-%m-%d')
         qty = qty or 1.0
-        seller_delay = 0
+        seller = False
         if uom:
             uom1_cat = prod.uom_id.category_id.id
             uom2_cat = product_uom_pool.browse(cr, uid, uom).category_id.id
@@ -739,7 +753,7 @@
         res = {}
         for s in prod.seller_ids:
             if s.name.id == partner_id:
-                seller_delay = s.delay
+                seller = s
                 if s.product_uom:
                     temp_qty = product_uom_pool._compute_qty(cr, uid, s.product_uom.id, s.min_qty, to_uom_id=prod.uom_id.id)
                     uom = s.product_uom.id #prod_uom_po
@@ -753,7 +767,7 @@
                         'uom': uom,
                         'date': date_order,
                         })[pricelist]
-        dt = (datetime.now() + relativedelta(days=int(seller_delay) or 0.0)).strftime('%Y-%m-%d %H:%M:%S')
+        dt = self._get_date_planned(cr, uid, seller, context=context).strftime(DEFAULT_SERVER_DATETIME_FORMAT)
 
 
         res.update({'value': {'price_unit': price, 'name': prod_name,