Hi Kyra,
Actually it was an error to remove this comment (thanks Deepak!) at r1758006 https://s.apache.org/neiF. I wonder if other alike comments were not
wrongly removed...
But anyway I think it's clear those 2 methods (releaseInitialOrderHold and abortOrderProcessing) are no longer needed and a refactoring is needed. For
instance this block of code in OrderServices.setOrderStatus() is only dust and useless:
// release the inital hold if we are cancelled or approved
if ("ORDER_CANCELLED".equals(statusId) ||
"ORDER_APPROVED".equals(statusId)) {
OrderChangeHelper.releaseInitialOrderHold(ctx.getDispatcher(), orderId);
// cancel any order processing if we are cancelled
if ("ORDER_CANCELLED".equals(statusId)) {
OrderChangeHelper.abortOrderProcessing(ctx.getDispatcher(), orderId);
}
}
We no longer use a workflow engine, and will never use one now that we rely on
ECA (ie the transition is finished).
So please create a Jira to remove such old deprecated pieces of code, not only
those related with no use of a workflow engine.
A way to do that could be a review of OFBIZ-7536, and would be greatly
appreciated, ouch? ;)
Thanks for your effort on this.
Jacques
Le 04/08/2017 à 10:19, Kyra Pritzel-Hentley a écrit :
Hello Deepak,
thank you for this information. I understand now that there used to be an
implementation that is now deprecated. Would it not make sense to comment on
these methods so that who ever else may stumble upon them knows that nothing is
wrong? In the newest trunk, nothing indicates that there used to be code there
and the methods are used throughout the project as though they are still
implemented.
Thank you and best regards,
Kyra Pritzel-Hentley
ecomify GmbH
Bielefeld, Germany
Am 03.08.2017 um 13:39 schrieb Deepak Nigam <[email protected]>:
Hi Kyra,
It seems that these changes are intentional, you can also check the comment
inside the code itself:
"NOTE DEJ20080609 commenting out this code because the old OFBiz Workflow
Engine is being deprecated and this was only for that get the delegator
from the dispatcher"
For more information, please have a look at the following:
https://svn.apache.org/viewvc?view=revision&revision=665981
Thanks & Regards
--
Deepak Nigam
Technical Consultant/Team Lead
HotWax Systems
www.hotwaxsystems.com
On Thu, Aug 3, 2017 at 2:08 PM, Kyra Pritzel-Hentley <
[email protected]> wrote:
Hello OFBiz Enthusiasts,
I am working to rid OFBiz of Bugs using the help of a code analysis tool
called FindBugs. With this, I find all kinds of peculiar code in the OFBiz
project. This JIRA ticket describes further details about this effort:
https://issues.apache.org/jira/browse/OFBIZ-9450
FindBugs pointed out to me that inside PayPalEvents#payPalIPN the method
OrderChangeHelper.releaseInitialOrderHold is called. But looking at the
implementation of the method, it only returns true even though the
parameters LocalDispatcher dispatcher and String orderId are handed over to
the method. The same problem arises with OrderChangeHelper#
abortOrderProcessing.
There doesn’t seem to be a ticket working on this issue. I am fairly
certain, looking at the names of the methods, that something more should
happen other than just returning true. Does somebody have an idea what is
supposed to happen with these methods?
Thank you,
Kyra Pritzel-Hentley
ecomify GmbH
Bielefeld, Germany
