Commit 6e18b0f6 authored by Nicolas Wavrant's avatar Nicolas Wavrant

erp5_pdm: add supply_cell_interaction_workflow

parent 22db9230
......@@ -7,6 +7,10 @@
<type>Internal Supply</type>
<workflow>edit_workflow, validation_workflow</workflow>
</chain>
<chain>
<type>Internal Supply Cell</type>
<workflow>supply_cell_interaction_workflow</workflow>
</chain>
<chain>
<type>Internal Supply Line</type>
<workflow>edit_workflow, reindex_object_interaction_workflow, supply_line_interaction_workflow</workflow>
......@@ -23,6 +27,10 @@
<type>Purchase Supply</type>
<workflow>edit_workflow, validation_workflow</workflow>
</chain>
<chain>
<type>Purchase Supply Cell</type>
<workflow>supply_cell_interaction_workflow</workflow>
</chain>
<chain>
<type>Purchase Supply Line</type>
<workflow>edit_workflow, reindex_object_interaction_workflow, supply_line_interaction_workflow</workflow>
......@@ -43,6 +51,10 @@
<type>Sale Supply</type>
<workflow>edit_workflow, validation_workflow</workflow>
</chain>
<chain>
<type>Sale Supply Cell</type>
<workflow>supply_cell_interaction_workflow</workflow>
</chain>
<chain>
<type>Sale Supply Line</type>
<workflow>edit_workflow, reindex_object_interaction_workflow, supply_line_interaction_workflow</workflow>
......@@ -51,6 +63,10 @@
<type>Service</type>
<workflow>edit_workflow, validation_workflow</workflow>
</chain>
<chain>
<type>Supply Cell</type>
<workflow>supply_cell_interaction_workflow</workflow>
</chain>
<chain>
<type>Supply Line</type>
<workflow>edit_workflow, reindex_object_interaction_workflow, supply_line_interaction_workflow</workflow>
......
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="InteractionWorkflowDefinition" module="Products.ERP5.InteractionWorkflow"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_objects</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>creation_guard</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>description</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>groups</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>supply_cell_interaction_workflow</string> </value>
</item>
<item>
<key> <string>manager_bypass</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value> <string>Supply Cell Interaction Workflow</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Interaction" module="Products.ERP5.Interaction"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_mapping</string> </key>
<value>
<dictionary/>
</value>
</item>
<item>
<key> <string>_objects</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>interactions</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="InteractionDefinition" module="Products.ERP5.Interaction"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>actbox_category</string> </key>
<value> <string>workflow</string> </value>
</item>
<item>
<key> <string>actbox_name</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>actbox_url</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>activate_script_name</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>after_script_name</string> </key>
<value>
<list>
<string>SupplyCell_updateSliceBasePrice</string>
</list>
</value>
</item>
<item>
<key> <string>before_commit_script_name</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>description</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>guard</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>SupplyCell_updateBasePrice</string> </value>
</item>
<item>
<key> <string>method_id</string> </key>
<value>
<list>
<string>_setBasePrice</string>
</list>
</value>
</item>
<item>
<key> <string>once_per_transaction</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>portal_type_filter</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>portal_type_group_filter</string> </key>
<value>
<none/>
</value>
</item>
<item>
<key> <string>script_name</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>temporary_document_disallowed</string> </key>
<value> <int>0</int> </value>
</item>
<item>
<key> <string>title</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>trigger_type</string> </key>
<value> <int>2</int> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Scripts" module="Products.DCWorkflow.Scripts"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_mapping</string> </key>
<value>
<dictionary/>
</value>
</item>
<item>
<key> <string>_objects</string> </key>
<value>
<tuple/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>scripts</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
supply_cell = state_change['object']
supply_line = supply_cell.getParentValue()
quantity_criterion_list = [
criterion for criterion in supply_cell.getCriterionList()
if criterion.property == 'quantity'
]
if len(quantity_criterion_list) == 1:
quantity_criterion = quantity_criterion_list[0]
else:
return
if supply_cell.getParentValue().isBasePricePerSlice():
  • This script depends on isBasePricePerSlice on the Line, but is not called when that value changes.

  • This is needed to calculate the price : https://lab.nexedi.com/nexedi/erp5/blob/master/product/ERP5/Document/Resource.py#L698-730

    Also, I prefer having these values as data instead of logic for different reasons, including backward compability and debugging.

    EDIT : deleted because it answers the wrong comment

    Edited by Nicolas Wavrant
  • No, when base_price_per_slice changes the cells are deleted (cf suplly line workflow interaction). This was done to circumvent UI limitation.

  • And they are deleted because this interaction is not triggered. This is circular reasoning, which is bad.

    Independent code must be independent. Not doing so leads to obscure the bugs against which you fought, spent time, and eventually gave up by falling back to a broken solution. We need a proper solution.

Please register or sign in to reply
quantity_step_list = [None] + supply_line.getQuantityStepList(base_id='path') + [None]
try:
index = quantity_step_list.index(quantity_criterion.min)
except KeyError:
# _range_criterion is set to {} if criterion is None
index = 0
min_quantity = quantity_step_list[index]
max_quantity = quantity_step_list[index+1]
supply_cell.setSliceQuantityRange((min_quantity, max_quantity))
supply_cell.setSliceBasePrice(supply_cell.getBasePrice())
  • Why do we even need these properties ? They are just duplicating other already-available values, and they could just as well be computed when needed without requiring the use of interactions to try to keep them consistent.

  • This is needed to calculate the price : https://lab.nexedi.com/nexedi/erp5/blob/master/product/ERP5/Document/Resource.py#L698-730

    Also, I prefer having these values as data instead of logic for different reasons, including backward compability and debugging.

  • One of ERP5 design principles is that we must never store the result of computations. This design goes against this principle. If it must stay, it will need stronger motivations.

    Also, note how backwards it is that this duplication, intended for easier debugging, is causing bugs.

  • After putting thoughts in it...

    Main reason we have these properties is to get the price_parameter_dict , as computed here : https://lab.nexedi.com/nexedi/erp5/blob/a3c40d0b7e717daa9ffe7e1ef7ac97859b2bc32e/product/ERP5/Document/Resource.py#L698-730

    If we decide to not use the slice_base_price and ````slice_quantity_range``` properties in Cells, we may escape this hell of mess of workflow interactions.

    We could go with a patch like this (update of the snippet of Resource.py linked above) :

    [...]
          # Get price parameters
          price_parameter_dict = {
            [...]
            'getSliceBasePrice': [],
            'getSliceQuantityRange': [],
          }
         [...]
          for mapped_value_property in mapped_value.getMappedValuePropertyList():
            value = getattr(mapped_value, mapped_value_property)
            try:
              value = value()
            except TypeError:
              continue

    So the properties are replaced by instance methods getSliceBasePrice and getSliceQuantityRange which will have for side effect of loading the parent (Supply Line) in memory.

    Then the supply_cell_interaction_workflow cloud be dropped.

    The issue remaining is that we need to update the _range_criterion property of Cells with a workflow interaction on SupplyLine._setBasePricePerSlice(). I think this is not done by updateCellRange, but by Base_edit (correct me if I'm wrong). Then I don't know how to solve this problem (iow : if I just change the value of base_price_per_slice of the line, cells won't be edited, thus their criterion won't change, thus they won't be found by portal_domains.generateMultivaluedMappedValue)

  • Main reason we have these properties is to get the price_parameter_dict, as computed here :

    Interesting. It seems quite un-ERP5 to access values with mere getattr and not accessors. It would indeed seem much better to use individual getters, which we can then customise so they dynamically compute their result. I'm curious of the reasons which led to such implementation.

    which will have for side effect of loading the parent (Supply Line) in memory.

    Which which is basically free, because parents are virtually always loaded when accessing any document as they are either part of the acquisition chain or were used to even get the object to begin with.

    For the predicate part of the question, I sadly do not have my examples at hand now, so I cannot comment in great details. All I remember is that there is a weird duplication of predicates: the cell is a predicate but we also create a separate predicate-only document.

Please register or sign in to reply
else:
supply_cell.setSliceQuantityRange(None)
supply_cell.setSliceBasePrice(None)
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>state_change</string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>SupplyCell_updateSliceBasePrice</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Variables" module="Products.DCWorkflow.Variables"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_mapping</string> </key>
<value>
<dictionary/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>variables</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="Worklists" module="Products.DCWorkflow.Worklists"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>_mapping</string> </key>
<value>
<dictionary/>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>worklists</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
Component | edit_workflow
Component | validation_workflow
Internal Supply Cell | supply_cell_interaction_workflow
Internal Supply Line | edit_workflow
Internal Supply Line | reindex_object_interaction_workflow
Internal Supply Line | supply_line_interaction_workflow
......@@ -9,6 +10,7 @@ Measure | conversion_interaction_workflow
Measure | edit_workflow
Product | edit_workflow
Product | validation_workflow
Purchase Supply Cell | supply_cell_interaction_workflow
Purchase Supply Line | edit_workflow
Purchase Supply Line | reindex_object_interaction_workflow
Purchase Supply Line | supply_line_interaction_workflow
......@@ -21,6 +23,7 @@ Quantity Unit Conversion Group | conversion_interaction_workflow
Quantity Unit Conversion Group | edit_workflow
Quantity Unit Conversion Group | validation_workflow
Resource Measures Consistency Constraint | dynamic_class_generation_interaction_workflow
Sale Supply Cell | supply_cell_interaction_workflow
Sale Supply Line | edit_workflow
Sale Supply Line | reindex_object_interaction_workflow
Sale Supply Line | supply_line_interaction_workflow
......@@ -28,6 +31,7 @@ Sale Supply | edit_workflow
Sale Supply | validation_workflow
Service | edit_workflow
Service | validation_workflow
Supply Cell | supply_cell_interaction_workflow
Supply Line | edit_workflow
Supply Line | reindex_object_interaction_workflow
Supply Line | supply_line_interaction_workflow
......
conversion_interaction_workflow
supply_cell_interaction_workflow
supply_line_interaction_workflow
transformation_interaction_workflow
\ No newline at end of file
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment