Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][ADD] stock_release_channel_process_end_time #550

Merged
merged 10 commits into from
Oct 6, 2023

Conversation

@rousseldenis rousseldenis added this to the 16.0 milestone Jan 30, 2023
@rousseldenis rousseldenis force-pushed the 16.0-release_channel_date-dro branch 3 times, most recently from a1f6668 to e6b24b9 Compare January 31, 2023 10:57

_inherit = "stock.release.channel"

process_end_date = fields.Datetime(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rousseldenis A release channel instance is not created every day. A release channel is a way to organize your pick to deliver process into your company. It's therefore useless to specify the date and time when the process of pickings should be completed. IMO, this process_end_date should be replaced by a process_end_time...
@jbaudoux What do you thing about?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rousseldenis A release channel instance is not created every day. A release channel is a way to organize your pick to deliver process into your company. It's therefore useless to specify the date and time when the process of pickings should be completed. IMO, this process_end_date should be replaced by a process_end_time...

@lmignon Ok, didn't catch that.

The only drawback I see is that is not flexible (but maybe it's wanted - I don't have the whole flow in mind) as one may want to have channel opened during the day, but pickings done during the night (so, overlap between two days)

@rousseldenis rousseldenis force-pushed the 16.0-release_channel_date-dro branch from e6b24b9 to 839f5ea Compare February 7, 2023 07:38
@rousseldenis rousseldenis changed the title [16.0][ADD] stock_release_channel_process_end_date [16.0][ADD] stock_release_channel_process_end_time Feb 7, 2023
@rousseldenis rousseldenis force-pushed the 16.0-release_channel_date-dro branch 2 times, most recently from 62efcc7 to 68d5262 Compare February 7, 2023 07:49
@@ -0,0 +1,2 @@
odoo-addon-stock-release-channel @ git+https://github.com/OCA/wms.git@refs/pull/506/head#subdirectory=setup/stock_release_channel
odoo-addon-stock-available-promise-release @ git+https://github.com/OCA/wms.git@refs/pull/478/head#subdirectory=setup/stock_available_promise_release
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
odoo-addon-stock-available-promise-release @ git+https://github.com/OCA/wms.git@refs/pull/478/head#subdirectory=setup/stock_available_promise_release
odoo-addon-stock-available-to-promise-release @ git+https://github.com/OCA/wms.git@refs/pull/478/head#subdirectory=setup/stock_available_to_promise_release

@rousseldenis rousseldenis force-pushed the 16.0-release_channel_date-dro branch from 68d5262 to ca9239c Compare February 8, 2023 10:26
@rousseldenis
Copy link
Contributor Author

@lmignon This one is ready

@api.model
def assign_release_channel(self, picking):
res = super().assign_release_channel(picking)
# Check if a channel has been assigned to the picking and write scheduled_date
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rousseldenis For my information, are you sure that a write is triggered if we assign the same value?

Copy link
Contributor Author

@rousseldenis rousseldenis Apr 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Checked.

We pass always in that line:

https://github.com/odoo/odoo/blob/16.0/odoo/fields.py#L1320

In fact, I think this is done at purpose to avoid maybe a unneeded read if field value is not in the cache (to be able to compare) and to be able to update the write_uid/write_date as in fact, people try to update the record even if value is the same.

Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little change required but work as expected

@rousseldenis rousseldenis force-pushed the 16.0-release_channel_date-dro branch from f17df21 to c29d5a6 Compare February 10, 2023 08:12
Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (Code review, functional tests)

Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rousseldenis The process_end_date field is not taken into account into the computation of domains used to retrieve pickings to release | late | done
As consequence, if the process_end_date is computed to tomorrow, the pickings assigned to the channel will not be taken visible since the scheduled_date will be set at tomorrow and the date criteria into the domains is set to today... (see https://github.com/OCA/wms/pull/493/files#diff-a8bb5a8dfb388e49242b1d9fe2274b33dccde4e032d0983647f70fa52399ecbcR174)

implemented in 8f20385

@lmignon
Copy link
Contributor

lmignon commented Feb 20, 2023

@rousseldenis Changes to the process_end_time is not applied to the process_end_date and pickings to process (maybe it's overkill???)

@rousseldenis rousseldenis force-pushed the 16.0-release_channel_date-dro branch from c29d5a6 to e7fdfdc Compare April 27, 2023 12:54
@lmignon lmignon force-pushed the 16.0-release_channel_date-dro branch from 8042afb to 8f20385 Compare May 16, 2023 09:46
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some typo in the helps

@sbejaoui sbejaoui force-pushed the 16.0-release_channel_date-dro branch 2 times, most recently from 2494ecf to 310a615 Compare July 27, 2023 16:11
Copy link
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason behind this new config? In which situation you don't want to plan the work for the given end date?
In my opinion, we should have by default the option active.
If the option is inactive, we should still never have a scheduled date > process end date. So it should still be min(release date + delay, process end date)

@jbaudoux
Copy link
Contributor

What's the reason behind this new config? In which situation you don't want to plan the work for the given end date? In my opinion, we should have by default the option active. If the option is inactive, we should still never have a scheduled date > process end date. So it should still be min(release date + delay, process end date)

So the option must remain inactive because it makes the test in stock_available_to_promise_release fail

@rousseldenis rousseldenis force-pushed the 16.0-release_channel_date-dro branch from 049175d to ac5d844 Compare September 28, 2023 07:35
@simahawk
Copy link
Contributor

simahawk commented Oct 5, 2023

@rousseldenis ping (for the build)

@simahawk
Copy link
Contributor

simahawk commented Oct 6, 2023

@rousseldenis do you have time to rebase and shorten/cleanup commits? I can do it otherwise

rousseldenis and others added 10 commits October 6, 2023 14:39
As time should be timezoned to correctly compute the real end date, consider
the channel warehouse timezone or company one. If not defined, consider the timezone
as UTC.
…utation

The compute of releasable picking must take into account the process end date of the stock release channel and not the current datetime.
The way the ORM is working into Odoo prevent to makes query on model with criteria based on values from linked model (comparing column from model A to column of model B). To work arround this limitation, we use a specialized field with a search method. Field's search methods are considered as 'internal' search methods and therefore allows you to use a special 'inselect' operator. This kind operator is usefull since it allows to express a criteria as a plain SQL quey. Thanks to this approach we've an easy way to implement our needs with the Odoo's ORM.
…e_tz

Co-authored-by: Jacques-Etienne Baudoux <je@bcim.be>
@rousseldenis rousseldenis force-pushed the 16.0-release_channel_date-dro branch from ac5d844 to 11372d0 Compare October 6, 2023 12:39
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@rousseldenis
Copy link
Contributor Author

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-550-by-rousseldenis-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f42ec6d into OCA:16.0 Oct 6, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at e6396f2. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants