From: "Scott Gray" <scott.g...@hotwaxmedia.com>
No need to apologize Hans, comments were exactly what I was after so
thank you for taking the time.
Based on your input and David's, I'm not going to worry about
automating the checks as the current approach doesn't really seem to
bother anyone.
I am still a little concerned about us not checking the validity of
statuses for newly created records, does it bother anyone else?
Hi Scott,
Yes, this is annoying and your examples prove that not only some exist but
almost surely some new examples will be commited :/
Having something keeping this clean automatically would be not only cool for
developpers but also for code quality.
Jacques
Thanks
Scott
On 7/05/2009, at 6:36 PM, Hans Bakker wrote:
Hi Scott,
sorry to jump in.
In general i tried to concentrate the setting of the statusId in a
single service which is used for EVERY status change. examples are in
invoice and customerRequest and other places.
is that a possible solution?
Regards,
Hans
On Thu, 2009-05-07 at 18:24 +1200, Scott Gray wrote:
Hi David
Thanks for the feedback, here are the main issues as I see it with
the
way things are now:
1. The developer has to know/remember to check for a valid status
change, I'm willing to bet that there are numerous places where it
isn't checked at the moment but should be.
2. While generally the check is only around 8-12 lines of code, the
developer has to either remember how to write the query and what
error
message should be displayed or find some existing code to copy/paste
(is the status set, has the status changed, do the lookup, report an
error).
I don't think options #2 and #3 below cover problem #1 above but they
may help with #2.
Also another issue with StatusValidChange is that there is currently
no way to make sure when creating a new record that the initial
status
is valid, I stumbled across this while fixing the test cases where a
Shipment was being created in the SHIPMENT_PACKED status and it was
subsequent ECA code that was failing. To solve that I would like to
remove the not empty constraint on StatusValidChange.statusId so that
we can use the entity to indicate valid status entry points and start
checking for them during record creation.
My only goal with these suggestions is to try and lighten the load on
the developer and this seems like a good opportunity for the
framework
to step in because the checks are very generic anyway.
Regards
Scott
On 7/05/2009, at 4:56 PM, David E Jones wrote:
A couple of notes:
1. if we add something to the entity engine to check this we would
need to move the Status* entities from the common component to the
entity component
2. I played with doing a separate method that genericized checking
the StatusValidChange, but it made the code bigger and (IMO) more
difficult to follow and customize, especially since the
StatusValidChange checking is pretty simple in the first place.
However, the code would be more generic and perhaps slightly more
simply (ie maybe not fewer lines, but some of the lines would be
less complex), so there might be some merit in it...
3. related to #1, but a variation on that approach: we could perhaps
have a service that checks this generically that is called through
an EECA or SECA rule, and then all we have to do is setup that ECA
rule for each entity we want checked
-David
On May 6, 2009, at 9:29 PM, Scott Gray wrote:
Hi All
Currently there is a ton of code in various places that checks to
make sure a status change is valid using the StatusValidChange
entity, and there are also places where there is no check even
though records exist (createShipment for example). I'm wondering
we couldn't add support for this to the entity engine itself so
that it doesn't have to be done manually, we could add some sort of
flag to the entity field def and then GenericEntity could perform
the check when setting the field. Thoughts?
The only thing I'm not too sure on is the purpose of the condition
field on StatusValidChange and how it is intended to be used, there
are no examples of its use in the existing data.
Failing that I'd at least like to add a utility method and a simple
method operation to simplify the task.
Thanks
Scott
HotWax Media
http://www.hotwaxmedia.com
--
Antwebsystems.com: Quality OFBiz services for competitive rates
