[18.0][MIG] stock_average_daily_sale#481
Conversation
jbaudoux
left a comment
There was a problem hiding this comment.
can you drop the migrations folder?
c660290 to
4a7a4e6
Compare
…evel + add demo The configurations are now stored on abc profiles. Some demo data are loaded in order to show this module features more easily.
…ame to recommended_qty
By default the priority in odoo is set to '0'. Before this change the daily sale was never computed for moves done in a normal process. There is no need to filter out moves based on the priority from the computation.
Co-authored-by: Jacques-Etienne Baudoux <je@bcim.be>
As tests create a Savepoint, there is a concurrent query when checking the view existence.
As for x reason, some products appear more than one time in the report, id generated by the concatenation of product_id and warehouse_id is irrelevant (as duplicate values possible). Use row_number() instead
Currently translated at 100.0% (58 of 58 strings) Translation: stock-logistics-reporting-16.0/stock-logistics-reporting-16.0-stock_average_daily_sale Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-16-0/stock-logistics-reporting-16-0-stock_average_daily_sale/it/
Remove product_abc_classification dependency. This module is not essential for the current module to function properly.
Only stock_user can see config and report; stock_manager can create and delete config entries
Set default average_daily_sale_root_location on WH create. Default stock location of the new WH is used for this purpose
Allow to decide if weekends should be included or excluded in average daily sale calculation.
Take also production location as dest location for daily usage calculation
…ny_id stock_average_daily_sale: - add form view for config - add company_id to config - allow editing ABC classification level and make sure the level is unique per warehouse
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: stock-logistics-reporting-16.0/stock-logistics-reporting-16.0-stock_average_daily_sale Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-16-0/stock-logistics-reporting-16-0-stock_average_daily_sale/
Currently translated at 100.0% (59 of 59 strings) Translation: stock-logistics-reporting-16.0/stock-logistics-reporting-16.0-stock_average_daily_sale Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-16-0/stock-logistics-reporting-16-0-stock_average_daily_sale/it/
- Allow to set a location from which consumption are computed - Properly compute the average daily quantity - Add max daily consumption metric - Remove stock level, this is not relevant for this report - Drop spike exclusion as it was counter-productive the way it was used - Moved safety and recommended quantity as computed field so that it can be extended - Added multi-company rules
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: stock-logistics-reporting-16.0/stock-logistics-reporting-16.0-stock_average_daily_sale Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-16-0/stock-logistics-reporting-16-0-stock_average_daily_sale/
Currently translated at 100.0% (57 of 57 strings) Translation: stock-logistics-reporting-16.0/stock-logistics-reporting-16.0-stock_average_daily_sale Translate-URL: https://translation.odoo-community.org/projects/stock-logistics-reporting-16-0/stock-logistics-reporting-16-0-stock_average_daily_sale/it/
4a7a4e6 to
934b126
Compare
c9fc84f to
27c782c
Compare
|
@jbaudoux ready |
rrebollo
left a comment
There was a problem hiding this comment.
I believe most of the findings are optional, but a few need to be addressed at this point. For all those cases, I've provided a suggestion that should make the change straightforward.
| from psycopg2.errors import ObjectNotInPrerequisiteState | ||
| from psycopg2.extensions import AsIs | ||
|
|
||
| from odoo import _, api, fields, models, registry |
There was a problem hiding this comment.
| from odoo import _, api, fields, models, registry | |
| from odoo import api, fields, models | |
| from odoo.modules.registry import Registry |
From OCA Migration Guidelines to v18:
Instead of doing from odoo import registry and then registry(db_name), now you have to do from odoo.modules.registry import Registry and Registry(db_name). More info at odoo/odoo#178784.
Also translations are environment related now.
|
|
||
| @api.model | ||
| def _check_view(self): | ||
| cr = registry(self._cr.dbname).cursor() |
There was a problem hiding this comment.
| cr = registry(self._cr.dbname).cursor() | |
| cr = Registry(self._cr.dbname).cursor() |
Idem
| def search(self, domain, offset=0, limit=None, order=None): | ||
| if not config["test_enable"] and not self._check_view(): | ||
| return self.browse() | ||
| return super().search(domain=domain, offset=offset, limit=limit, order=order) |
There was a problem hiding this comment.
Did you consider OCA Migration Guidelines? Given this model is really a materialized table I'm guessing this is not relevant. Am I wrong?
If overriding search method for modifying searches result, take into account that this method is not always called in this version. Override search_fetch instead.
| class StockAverageDailySaleConfig(models.Model): | ||
| _name = "stock.average.daily.sale.config" | ||
| _description = "Average daily sales computation parameters" | ||
| check_company_auto = True |
There was a problem hiding this comment.
| check_company_auto = True | |
| _check_company_auto = True |
Looks like a typo. Anyway I think it's better to address this now.
|
Also @rousseldenis, would you be so kind to review my #479 in return? |
| return self._check_materialize_view_populated(cr) | ||
| except ObjectNotInPrerequisiteState: | ||
| _logger.warning( | ||
| _("The materialized view has not been populated. Launch the cron.") |
There was a problem hiding this comment.
| _("The materialized view has not been populated. Launch the cron.") | |
| self.env._("The materialized view has not been populated. Launch the cron.") |
jbaudoux
left a comment
There was a problem hiding this comment.
I'm planning to improve and refactor this module in v18.0.
I noted as comment what I intend to change
| # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). | ||
|
|
||
| { | ||
| "name": "Stock Average Daily Sale", |
There was a problem hiding this comment.
Can we rename the module to
| "name": "Stock Average Daily Sale", | |
| "name": "Stock Average Daily Usage", |
| "maintainers": ["jbaudoux"], | ||
| "website": "https://github.com/OCA/stock-logistics-reporting", | ||
| "depends": [ | ||
| "sale", |
There was a problem hiding this comment.
It should only depend on stock
| "website": "https://github.com/OCA/stock-logistics-reporting", | ||
| "depends": [ | ||
| "sale", | ||
| "stock_storage_type_putaway_abc", |
There was a problem hiding this comment.
I would like to drop this strong dependency and move it to a glue module. Or even better, replace it with a products applicability domain. At the end, it is the only thing that it is doing.
Also it would be better to use the abc product classification level that is at product.product level (abc_storage is only at product.template level) and that supports multi-warehouses through the abc profiles.
| "depends": [ | ||
| "sale", | ||
| "stock_storage_type_putaway_abc", | ||
| "product_route_mto", |
There was a problem hiding this comment.
MTO should move to another glue module. It doesn't provide any value to this ADU computation.
New module: stock_average_daily_usage_mto
| nbr_sales = fields.Integer( | ||
| string="Number of Sales", |
There was a problem hiding this comment.
rename to reflect what it is
| abc_classification_level = fields.Selection( | ||
| selection=ABC_SELECTION, required=True, default="b" | ||
| ) |
No description provided.