cleanup acquisition base categories in categories and properties definition
We no longer use order/delivery category to acquire categories and properties. Having needless acquisition base categories in category/property definition makes accessor slower. Yes, category accessor is slow in ERP5. So I propose to drop order and delivery from acquisition base categories from generic business templates.
One thing I'm still wondering is specialisethat is only used in variation's acquitision base categories. Is it really required ? I think that in getAggeregatedAmountList() usage, specialise is anyway taken into account by asComponent() result and we don't need explicit acquisition in variation base category. At least I find no reason to have specialise only in variation.
/cc @nexedi
-
767 original_acquisition_base_category_list = category.getAcquisitionBaseCategoryList() 768 category.setAcquisitionBaseCategoryList(['parent', 'order']) 769 770 line = order.newContent(portal_type='Sale Order Line') 771 self.assertEqual(line.getDestinationAdministrationPersonTitle(), 'toto') 768 772 769 packing_list.setOrderValue(order) 770 self.assertEqual(packing_list.getDestinationAdministrationPersonTitle(), 'toto') 773 packing_list.setDestinationAdministrationValue(None) 774 packing_list.setOrderValue(None) 775 self.assertEqual(packing_list.getDestinationAdministrationPersonTitle(), None) 776 777 packing_list.setOrderValue(order) 778 self.assertEqual(packing_list.getDestinationAdministrationPersonTitle(), 'toto') 779 finally: 780 category.setAcquisitionBaseCategoryList(original_acquisition_base_category_list) -
Edited
Shouldn't we just remove this part of the test ?
-
Looks ok to remove. arrow categories should be independent between order and delivery, so we should copy them.
-
I prefer keeping this test to check if acquisition base category in category definition well works including the special
parentkeyword. -
You are right, generally speaking, it's better to keep, but what made me worried is that over this test became so hard to read, it's not really clear what is being tested. Are you really sure it tests
parent?History of this test method is:
- ef033685 test added, testing acquisition of destination administration through causality
- 5385a1b3 we remove causality from the destination administration acquisition base category. Test failed, so in 7339f5a6 we update test to use order instead of causality.
The problem with this test method is that it relied too much on how
destination_administrationcategory is configured. I suggest we rewrite this in a test which creates a base category just for the purpose of this test. It would be easier to read and more future proof. I did that in ccb32b9a
-
-
One thing I'm still wondering is
specialisethat is only used invariation's acquitision base categories.I suspect something historically related with transformation, maybe so that a transformation acquire the variations from the specialised variations ? ( this is just a guess )
-
Would you mind making the commit message more explicit (eg explain the why as you have done in the merge request instead of what has been done which is quite obvious from the diff)?
-
Please register or sign in to post a comment