|
(0119638)
|
|
dmiguelez
|
2020-05-06 16:59
(edited on: 2020-05-06 17:11) |
|
This issue has been moved to Design Defect due to the complexity of finding the proper solution.
The proposed fix is a good starting point to find the solution, as the underlying problem happens because the Reservations are processed with the Triggers disabled.
However, as it is, this fix can be the cause of regressions.
It is important to find the proper place to enable the triggers.
Ideally, the BackEnd code should always consider that it is executed with the triggers enabled, as it was designed with this concept in mind.
However, this may not be true if it is called from a POS process that has disabled the triggers first.
In this case, the same process that has disabled the triggers should be responsible to enable them before calling a BackEnd process.
But this situation is quite complex and it is not easy to find where the triggers should be enabled.
First, the calls to this functionality both in the OrderLoader and in CancelLayawayLoader are executed intentionally with the triggers disabled:
- https://code.openbravo.com/erp/pmods/org.openbravo.retail.posterminal/rev/2d5a50ee702c [^] [^]
- https://code.openbravo.com/erp/pmods/org.openbravo.retail.posterminal/rev/70933f1a2d22 [^] [^]
Then, the triggers are enabled again, in BackEnd code for some other processes:
- https://code.openbravo.com/erp/devel/pi/rev/074035fe2953 [^] [^]
- https://code.openbravo.com/erp/devel/pi/rev/8344e88e2e18 [^] [^]
This approach of starting with the triggers disabled and then, in BackEnd code, enable the triggers in each specific process is not working, as it is raising plenty of issues.
The reason to move this issue to a design defect is because this functionality must be reviewed to understand what must be executed always with the triggers enabled and what should be executed sometimes with the triggers disabled.
With this research, the code should be refactored properly.
Take into account that the Cancel And Replace functionality peroforms plenty of actions such us:
- Update, Create and Process Orders
- Create and Process Shipments
- Create and Process Payments
- Update, Create and Process Reservations
Reviewing all this flows implies a significant effort.
Additionally, there is one extra concept to consider.
POS is not aware actually of Reservations.
This means that they are not taken into account by POS processes.
This issue can happen to anyone that uses POS, Reservations and Cancel And Replace, under some specific circumstances.
But it is true also that this issue is more obvius for clients that try to incorporate the concept of Reservations in the POS.
Until this functionality is properly included in the POS, issues related to try to incorporate Reservations in the POS, like this one, are bound to happen.
Finally, about the proposal.
There is a problem in which code that previously was executed with triggers disabled now is executed with triggers enabled.
If this patch is going to be used in a client, I propose to flush before enabling the triggers and then flush again before disabling them.
This will ensure that there are no side effects to the flows that are currently working with the triggers disabled.
On the other hand, this means that it is necessary to execute 2 more flushes, which might have an impact in performance.
|
|
|
|
Note:
With the proposed fix, it is necessary to apply the patch 0001-Reservations-related-to...
Since the Reservations are properly calculated by the triggers, the logic of the hook for CancelAndReplace should be removed, if not, the Stock will be calculated twice. |
|
0001-Reservations-Activate-triggers-to-manipulate-Reserva.patch (4,299) 2020-04-17 19:25