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_location_fill_state #2188

Open
wants to merge 5 commits into
base: 16.0
Choose a base branch
from

Conversation

rousseldenis
Copy link
Contributor

@rousseldenis rousseldenis commented Nov 7, 2024

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.

According to me, the right implementation is here
https://github.com/OCA/wms/blob/16.0/stock_storage_type/models/stock_location.py#L319

I would have migrated the stock_location_empty and extracted the computation from the above code to compute stock_amount and location_is_empty. It's not the exact same computation as in 14.0 but it's the most correct one in my opinion and improving previous implementation. Then we don't end-up with 3 modules computing the same concept in 3 different ways.

I prefer empty than void. And occupied state is like there is an hostile army ;)

Occupancy is also misleading as I thought initially it was about volume...

Comment on lines 35 to 37
quants_result = self.env["stock.quant"].read_group(
[("location_id", "in", self.ids)], ["location_id"], ["location_id"]
)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can still have quants with 0 quantity. So that's not inline with the help no product quantity

@jbaudoux
Copy link
Contributor

cc @p-tombez @TDu @sebalix

@rousseldenis
Copy link
Contributor Author

According to me, the right implementation is here
https://github.com/OCA/wms/blob/16.0/stock_storage_type/models/stock_location.py#L319

Yes, in fact, I had this in mind but wanted simpler implementation.

I would say, we should maybe transform the boolean state to a Selection one, with an 'Operation in Progress' (yellow color) there:

image

@jbaudoux
Copy link
Contributor

I would say, we should maybe transform the boolean state to a Selection one, with an 'Operation in Progress' (yellow color)

  • empty = green
  • being filled = yellow
  • filled = red
  • being emptied = orange

@rousseldenis rousseldenis marked this pull request as draft November 15, 2024 13:27
@rousseldenis
Copy link
Contributor Author

I would say, we should maybe transform the boolean state to a Selection one, with an 'Operation in Progress' (yellow color)

  • empty = green
  • being filled = yellow
  • filled = red
  • being emptied = orange

@jbaudoux

being emptied and being filled : which one has priority on the other ?

That's why I proposed operation in progress as we can have both at the same time.

@jbaudoux
Copy link
Contributor

being emptied and being filled : which one has priority on the other ?

Being emptied is when there is stock but entirely reserved for move lines with qty done. Equals to current implementation of is empty in storage type module.

Being filled is when it is empty but there are move line with location as destination.

As one is when there is stock and the other when there is no stock, you cannot have both, it is exclusive

@rousseldenis rousseldenis force-pushed the 16.0-add-stock-location-void-dro branch from 8cb5171 to 964a55b Compare November 19, 2024 09:05
@rousseldenis rousseldenis changed the title [16.0][ADD] stock_location_occupancy [16.0][ADD] stock_location_fill_state Nov 19, 2024
@rousseldenis rousseldenis marked this pull request as ready for review November 19, 2024 09:06
stock_location_fill_state/__manifest__.py Outdated Show resolved Hide resolved
stock_location_fill_state/__manifest__.py Outdated Show resolved Hide resolved
stock_location_fill_state/i18n/fr_BE.po Outdated Show resolved Hide resolved
stock_location_fill_state/readme/CONTEXT.md Outdated Show resolved Hide resolved
stock_location_fill_state/readme/USAGE.md Outdated Show resolved Hide resolved
stock_location_fill_state/i18n/fr_BE.po Outdated Show resolved Hide resolved
stock_location_fill_state/models/stock_location.py Outdated Show resolved Hide resolved
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.

2 participants