[16.0][FIX] product_abc_classification_sale_stock: fix deprecated methods and tests #1807
Conversation
|
Hi @rousseldenis, @lmignon, @lmarion-source, |
kobros-tech
force-pushed
the
16.0-fix-product_abc_classification_sale_stock
branch
from
8b7f11e to
0b0f277
Compare
kobros-tech
changed the title
| FROM | ||
| abc_classification_profile_product_rel abc_rel | ||
| abc_classification_profile_product_rel |
There was a problem hiding this comment.
This should not be changed.... These changes fixes an issue. see #1723
| @@ -9,7 +9,7 @@ | |||
| "version": "16.0.1.0.2", | |||
| "license": "AGPL-3", | |||
| "website": "https://github.com/OCA/product-attribute", | |||
| "maintainers": ["rousseldenis", "lmignon", "lmarion-source"], | |||
| "maintainers": ["rousseldenis", "lmignon", "lmarion-source", "kobros-tech"], | |||
There was a problem hiding this comment.
You could add your name into the contributors but not into the authors since your changes are related to some little issue, not to a large change, improvement, new functionality.
| self.assertEqual(len(levels.sale_stock_level_history_ids), 1) | ||
| logs = self.env["abc.sale_stock.level.history"].search( | ||
| [("product_level_id", "in", levels.ids)] | ||
| ) | ||
| self.assertEqual(len(logs), 1) |
There was a problem hiding this comment.
This should not be changed. The sale_stock_level_history_ids should have been invalidated by the call to _compute_abc_classification This is the purpose of the initial assert. That means that the invalidation done into the _log_history method is not complete and we should add
self.env["product.template"].invalidate_cache(
["abc_classification_product_level_ids"]
)
self.env["product.product"].invalidate_cache(
["abc_classification_product_level_ids"]
)
There was a problem hiding this comment.
@lmignon
I tried changes, because module testing failed in the last test, the reason why I tested version 16.0 is that refrsh() method is deprecated in version 16.0 but I didn't see the warning in the log for version 17.0.
I preferred to use one of the recommended three methods that I saw in the log of version 16.0.
Even this method:
self.env["product.template"].invalidate_cache(
["abc_classification_product_level_ids"]
)
is deprecated in version 16.0 and raising errors in version 17.0 without giving warnings in the log, so shall we override deprecated methods and see how can it work with the new not deprecated ones?
Also the database column is changed by the pre-commit command, this one if I return to the original name can the levels be computed automatically?
For now, let me remove my name from maintainers and return the old column name in the DB, but will need your help for the remaining ones as I inquired above.
There was a problem hiding this comment.
To be clear the history records were always equal to 0, and assertions should be 1==1, and 2==2.
The original 16.0 version had two issues:
1 deprecated methods for migration to 17.0
2 the last test for logging history
| @@ -255,15 +255,15 @@ def test_01(self): | |||
| # test computed classification and check that inactive products are | |||
| # not taken into account | |||
| self.product1.active = False | |||
| self.product1.refresh() | |||
| self.product1.invalidate_model() | |||
There was a problem hiding this comment.
IMO this should be self.product1.flush_recordset()
kobros-tech
force-pushed
the
16.0-fix-product_abc_classification_sale_stock
branch
2 times, most recently
from
7425538 to
a71d0bb
Compare
|
I just wonder how can a test on my local machine requires these values: instead on testing the module in OCA, Odoo the values should be: self._assertLevelIs(self.product1, "a") I don't know should I test on a new DB or what? |
As I guessed, new clean testing DB is following OCA tests and Odoo tests. |
kobros-tech
force-pushed
the
16.0-fix-product_abc_classification_sale_stock
branch
from
a71d0bb to
665aaa1
Compare
| self.env["abc.classification.product.level"].invalidate_cache( | ||
| ["sale_stock_level_history_ids"] | ||
| ) | ||
| self.env.invalidate_all() |
There was a problem hiding this comment.
you invalidate too much fields....
| self.env.invalidate_all() | |
| self.env["abc.classification.product.level"].invalidate_model( | |
| ["sale_stock_level_history_ids"] | |
| ) | |
| self.env["product.template"].invalidate_model( | |
| ["abc_classification_product_level_ids"] | |
| ) | |
| self.env["product.product"].invalidate_model( | |
| ["abc_classification_product_level_ids"] | |
| ) |
| @@ -315,6 +315,7 @@ def _compute_abc_classification(self): | |||
| sale_stock_data.total_so_lines = total | |||
| sale_stock_data.product_level = product_abc_classification | |||
| previous_data = sale_stock_data | |||
|
|
|||
There was a problem hiding this comment.
This change is useless. Can you remove this blank line plz?
kobros-tech
force-pushed
the
16.0-fix-product_abc_classification_sale_stock
branch
from
665aaa1 to
91029a2
Compare
@lmignon What the module that you asks me to add instead of invalidate all, it causes that your tests fail. Only invalidata all method makes tests succeed, not any other one even the one you have mentioned. That is why I ask for your help as in version 17.0 challenges are more, we shall discuss once we finish this one. |
@rvalyi
@zeroheure
@ovnicraft