• Nicolas Wavrant's avatar
    Verify parameters passed to category setters · 3978dba1
    Nicolas Wavrant authored
    This merge request aims to prevent programming errors by raising instead of silently doing nothing (which is a source of bugs).
    
    Currently, if an object as category property for a category "category", 2 families of setters were created :
    
    1. The string setter, following the format ```_setCategory``` and taking a relative URL as the argument.
    2. The value setter, following the format ```_setCategoryValue``` and taking an object as the argument.
    
    The issue is that developers may pass the wrong argument to one of these functions, having for consequences :
    
    1. For case (1), if an object is passed, the code would silently do nothing : nothing is set as relation, but the code doesn't fail. This is the worst case. 
    2. For the second case, passing a relative URL to ```_setCategoryValue``` would "work" (in the meaning the relation is set to the correct object). This may sound like a feature, but in my opinion it is confusing given the way ERP5 developers apprehend these setters nowadays.
    
    For case 1, a test is existing that an exception is raised, but due to coding error the feature disappeared and no one noticed : https://lab.nexedi.com/Nicolas/erp5/blob/b3ed2210ed6b30390b901c8620de6eafcc27a574/product/CMFCategory/tests/testCMFCategory.py#L810-815
    
    For case 2, compatibility code exists in the underlying function ```_setValue``` : https://lab.nexedi.com/Nicolas/erp5/blob/b3ed2210ed6b30390b901c8620de6eafcc27a574/product/ERP5Type/Base.py#L1840-1842
    
    In this MR, the exception caused by case (1) has been restored (so now it fails loudly).
    
    Case (2) has been deprecated, in order to keep backward compatibility.
    
    I have run the tests with the ```DeprecationWarning``` raising an error instead of just a warning,  and fixed the code were the setters weren't used correctly.
    
    Ideally, tests should always run with this ```DeprecationWarning``` being a real error so both cases crash loudly. This won't be part of this Merge Request.
    
    
    /reviewed-on !938
    3978dba1
Name
Last commit
Last update
..
Constraint Loading commit data...
Document Loading commit data...
PropertySheet Loading commit data...
Tool Loading commit data...
dtml Loading commit data...
help Loading commit data...
tests Loading commit data...
CatalogTool.py Loading commit data...
MAINTAINERS.txt Loading commit data...
Permissions.py Loading commit data...
VERSION.txt Loading commit data...
__init__.py Loading commit data...
refresh.txt Loading commit data...
tool.png Loading commit data...