12.0 mig sale order line date #875

open-net-sarl · 2019-06-13T09:47:25Z

I took time to make the things as asked.
Hope this time it is okay.

Nb: I first wanted to add changes, that's why i did it again "properly". Finally, i did not do the changes wanted: it is still the module from 11.0 corrected, nothing was added or removed. The tests are not mine, i just took it from 11.0

Have a nice day

[FIX] Update readme to latest template

open-net-sarl · 2019-06-13T09:48:30Z

last pull request:
#868

rousseldenis

LGTM. Code review. Some comments

sale_order_line_date/__manifest__.py

rousseldenis · 2019-06-13T09:55:13Z

sale_order_line_date/models/sale_order_line.py

+class SaleOrderLine(models.Model):
+    _inherit = 'sale.order.line'
+
+    commitment_date = fields.Datetime()


Shouldn't have migration script as you changed the field ?

As we never used this module, we just migrated what we needed.
Maybe add the attributes "old_name='requested_date'" would be enough, but we won't do the retro-compatibility fix. Sorry

Yes, at least the old_name should be sufficent. Could you add it ?

It is done.
As told before, we tried to help the OCA, especially for the switzerland modules.
The process change <--> review does not apply only once but can last very long.
For that we are an enterprise, we can't spend a lot of time with reviews.

Thank you.

The process change <--> review does not apply only once but can last very long.
For that we are an enterprise, we can't spend a lot of time with reviews.

That's a point of view/ Most of people in OCA are tied to enterprise too. IMHO code review should be part of development as you can confont to other people, get ideas, stay in touch with changes, ... many advantages. But that depends on enterprise philosophy, that's true.

rousseldenis · 2019-06-13T09:57:50Z

sale_order_line_date/models/sale_order.py

+    @api.onchange('commitment_date')
+    def _onchange_commitment_date(self):
+        """Warn if the commitment dates is sooner than the commitment date"""
+        result = super(SaleOrder, self)._onchange_commitment_date()


All super could be simplified (not blocking) :

Suggested change

result = super(SaleOrder, self)._onchange_commitment_date()

result = super()._onchange_commitment_date()

I don't think it is a good way to do that.
I personally prefer being explicit, considering the result is the same.
Do as you want

open-net-sarl · 2019-06-13T10:24:18Z

I did a new commit adding the commitment_date after the description in the sale.order document.
That's the change i initially wanted to do.

rousseldenis · 2019-06-13T10:24:20Z

sale_order_line_date/README.rst

+* Aaron Henriquez <ahenriquez@eficent.com>
+* Serpent Consulting Services Pvt. Ltd. <support@serpentcs.com>
+* Francesco Apruzzese <f.apruzzese@apuliasoftware.it>
+


You could add yourself as contributor of course !

Changed version in __manifest__.py Co-Authored-By: Denis Roussel (ACSONE) <rousseldenis@users.noreply.github.com>

rousseldenis

Code review.

rousseldenis · 2019-06-13T12:11:15Z

Work as expected on runbot.

IMHO you should add a group on 'commitment_date' in views as for the field on 'sale.order'. Otherwise, field appear in lines but not on order level.

Group : 'sale.group_sale_order_dates'.

AaronHForgeFlow

The commitment_date is being not propagated to the stock moves.

rousseldenis · 2019-06-13T15:02:42Z

The commitment_date is being not propagated to the stock moves.

Oops, just tested the onchange...

open-net-sarl · 2019-06-17T09:22:25Z

The commitment_date is being not propagated to the stock moves.

Oops, just tested the onchange...

_prepare_procurement_values methods should work exactly as in V11.

…d onchange in sale.order

open-net-sarl · 2019-06-17T09:52:23Z

I just did some changes in the code.
In the write method of sale.order.line, if one line add no commitment_date, all lines would have taken its sale.order's commitment_date.
In onchange, using comprehension list instead of a loop + append

I will try to help as much as i can in my free time, i just hope that it won't become an infinite review loop..
I just fixed a bug, but the rest works like in V11

AaronHForgeFlow · 2019-06-17T14:19:23Z

@open-net-sarl Thank you for your improvements. There are still a couple of things missing:

Schedule date is still not passed to the stock moves when confirming the sales order
Test are calling to unexisting method:
AttributeError: 'sale.order' object has no attribute 'onchange_commitment_date

Regards.

AaronHForgeFlow · 2019-06-21T08:16:26Z

sorry @open-net-sarl there was an open PR for this module and it is much older than this one. SO better keep the other one: #703

rafaelbn · 2019-09-22T16:37:07Z

Dear @open-net-sarl , we are very happy to have a new contributor. And we all thank you for this contribution.

But It was a PR before yours, so the correct way to work is continuing the PR before: #703

As you can see here you can make PR to the original author #703 (comment)

I just want you to understand that you could be in the same situation of @mpanarin , that nobody review your PR and after some time a new contributor like you, make the same PR despite of checking before if it was done.

You question could be: and how could I know that? Well:

The easy way is the migration issue: Migration to version 12.0 #700
The other way is searching before in the existing PR

cc @alexey-pelykh FYI

rafaelbn · 2019-09-22T16:41:20Z

IMHO the interested people in having this module in v12 should contribute in #703 Thanks and feel free to comment.

The most delicate thing here ( cc @jgrandguillaume ) is that I don't want to discourage @open-net-sarl but also I don't want to discourage @mpanarin .

rafaelbn · 2019-09-22T16:58:45Z

I'm going to test this one as #703 (review) is not working and author @mpanarin is not going to work on it anymore.

rafaelbn

Tested in runbot functionally: doesn't work

I used commitment date 1st October and this date doesn't propagate any where

alexeirivera87 · 2019-09-25T21:03:14Z

I know why the SO line's commitment dates are not being propagated to stock.moves.
Must I get some permission to contribute?

Regards

CasVissers-360ERP · 2019-10-04T11:10:54Z

@alexeirivera87

I know why the SO line's commitment dates are not being propagated to stock.moves.
Must I get some permission to contribute?

Regards

What fix do you want to propose?

RLeeOSI

The picking is not inheriting a scheduled date because it expects there to be a commitment date on the sale order. A possible solution would be to add an onchange to the line commitment date that sets the order commitment date if it doesn't exist. That way, there will be a date for the picking to inherit.

I would also suggest a warning message if the commitment date of a line is later than the commitment date of the order. In the case that the user wants one product to be promised later than the order itself, it will be possible, but since this is abnormal, there should be a message. In a normal situation, the order commitment date should be the same as the latest line commitment date, and the picking should be scheduled for the latest date.

The moves are not inheriting the commitment date because the module does not depend on sale_stock. Right now, sale_stock is calling super on sale.order.line._prepare_procurement_values and overwriting the changes from this module. Add the dependency and the moves will reflect the line commitment date.

RLeeOSI · 2019-10-23T18:07:48Z

sale_order_line_date/models/sale_order.py

+        if 'warning' not in result:
+            lines = []
+            for line in self.order_line:
+                lines.append((1, line.id, {'commitment_date':


Suggested change

lines.append((1, line.id, {'commitment_date':

if not line.commitment_date:

lines.append((1, line.id, {'commitment_date':

Is there a reason why the order commitment date should overwrite an existing line commitment date?

If not, there is a solution to the issue with the date not propagating to picking. The reason the picking scheduled date doesn't inherit properly is because it's looking for an order commitment date. If you add an onchange to the line commitment date that overwrites order commitment date if it doesn't exist, then the picking will have a date to inherit.

RLeeOSI · 2019-10-23T18:10:26Z

sale_order_line_date/models/sale_order.py

+    @api.multi
+    @api.onchange('commitment_date')
+    def _onchange_commitment_date(self):
+        """Warn if the commitment dates is sooner than the commitment date"""


Suggested change

"""Warn if the commitment dates is sooner than the commitment date"""

"""Warn if the commitment date is sooner than the expected date"""

Comment from base sale module

NuriaXifre · 2019-10-30T15:04:20Z

@open-net-sarl I have done a functional review and it does not work. The commitment date does not propagate. Do you have in mind to continue working on this module?

MiquelRForgeFlow · 2019-11-04T20:31:02Z

Superseded by #984.

AaronHForgeFlow and others added 9 commits June 13, 2019 11:24

Update code

bfc1f61

[FIX] Update readme to latest template

sale_order_line_date v9

6ab451c

OCA Transbot updated translations from Transifex

6f2604d

[MIG] sale_order_line_date v10

57268a4

OCA Transbot updated translations from Transifex

dc3089a

[MIG] sale_order_line_date: Migration to 11.0

ab0bacf

[FIX] Function name to prepare procurement values

db8810f

[UPD] Update sale_order_line_date.pot

63f458d

[ADD] icon.png

6609680

rousseldenis requested changes Jun 13, 2019

View reviewed changes

[MIG] sale_order_line_date: Migration to 12.0

af86d4a

open-net-sarl force-pushed the 12.0-mig-sale_order_line_date branch from 00cb65d to af86d4a Compare June 13, 2019 10:14

add commitment_date to document

a220ffa

rousseldenis reviewed Jun 13, 2019

View reviewed changes

open-net-sarl and others added 2 commits June 13, 2019 13:13

Update sale_order_line_date/__manifest__.py

fa07399

Changed version in __manifest__.py Co-Authored-By: Denis Roussel (ACSONE) <rousseldenis@users.noreply.github.com>

add old_name attribute in commitment_date

44902d4

rousseldenis approved these changes Jun 13, 2019

View reviewed changes

AaronHForgeFlow suggested changes Jun 13, 2019

View reviewed changes

corrected default commitment_date in sale.order.line write and cleane…

0f42bc6

…d onchange in sale.order

rafaelbn added this to the 12.0 milestone Sep 22, 2019

rafaelbn mentioned this pull request Sep 22, 2019

add module sale_order_line_date #868

Closed

rafaelbn added the duplicate label Sep 22, 2019

rafaelbn closed this Sep 22, 2019

rafaelbn reopened this Sep 22, 2019

rafaelbn added the needs review label Sep 22, 2019

OCA-git-bot mentioned this pull request Sep 22, 2019

Migration to version 12.0 #700

Closed

54 tasks

rafaelbn requested changes Sep 22, 2019

View reviewed changes

RLeeOSI reviewed Oct 23, 2019

View reviewed changes

LoisRForgeFlow mentioned this pull request Oct 28, 2019

12.0 mig sale order line date #703

Closed

MiquelRForgeFlow mentioned this pull request Nov 4, 2019

[12.0][MIG] sale_order_line_date #984

Merged

pedrobaeza closed this Nov 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

12.0 mig sale order line date #875

12.0 mig sale order line date #875

open-net-sarl commented Jun 13, 2019

open-net-sarl commented Jun 13, 2019

rousseldenis left a comment

rousseldenis Jun 13, 2019

open-net-sarl Jun 13, 2019

rousseldenis Jun 13, 2019

open-net-sarl Jun 13, 2019

rousseldenis Jun 13, 2019

rousseldenis Jun 13, 2019

open-net-sarl Jun 13, 2019 •

edited

open-net-sarl commented Jun 13, 2019

rousseldenis Jun 13, 2019

rousseldenis left a comment

rousseldenis commented Jun 13, 2019

AaronHForgeFlow left a comment

rousseldenis commented Jun 13, 2019

open-net-sarl commented Jun 17, 2019

open-net-sarl commented Jun 17, 2019

AaronHForgeFlow commented Jun 17, 2019

AaronHForgeFlow commented Jun 21, 2019

rafaelbn commented Sep 22, 2019

rafaelbn commented Sep 22, 2019

rafaelbn commented Sep 22, 2019

rafaelbn left a comment

alexeirivera87 commented Sep 25, 2019

CasVissers-360ERP commented Oct 4, 2019

RLeeOSI left a comment

RLeeOSI Oct 23, 2019

RLeeOSI Oct 23, 2019

NuriaXifre commented Oct 30, 2019

MiquelRForgeFlow commented Nov 4, 2019

	result = super(SaleOrder, self)._onchange_commitment_date()
	result = super()._onchange_commitment_date()

	lines.append((1, line.id, {'commitment_date':
	if not line.commitment_date:
	lines.append((1, line.id, {'commitment_date':

	"""Warn if the commitment dates is sooner than the commitment date"""
	"""Warn if the commitment date is sooner than the expected date"""

12.0 mig sale order line date #875

12.0 mig sale order line date #875

Conversation

open-net-sarl commented Jun 13, 2019

open-net-sarl commented Jun 13, 2019

rousseldenis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

open-net-sarl Jun 13, 2019 • edited

Choose a reason for hiding this comment

open-net-sarl commented Jun 13, 2019

Choose a reason for hiding this comment

rousseldenis left a comment

Choose a reason for hiding this comment

rousseldenis commented Jun 13, 2019

AaronHForgeFlow left a comment

Choose a reason for hiding this comment

rousseldenis commented Jun 13, 2019

open-net-sarl commented Jun 17, 2019

open-net-sarl commented Jun 17, 2019

AaronHForgeFlow commented Jun 17, 2019

AaronHForgeFlow commented Jun 21, 2019

rafaelbn commented Sep 22, 2019

rafaelbn commented Sep 22, 2019

rafaelbn commented Sep 22, 2019

rafaelbn left a comment

Choose a reason for hiding this comment

alexeirivera87 commented Sep 25, 2019

CasVissers-360ERP commented Oct 4, 2019

RLeeOSI left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NuriaXifre commented Oct 30, 2019

MiquelRForgeFlow commented Nov 4, 2019

open-net-sarl Jun 13, 2019 •

edited