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?
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 DavidThanks for the feedback, here are the main issues as I see it with theway 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, thedeveloper has to either remember how to write the query and what errormessage 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 currentlyno way to make sure when creating a new record that the initial statusis 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 onthe developer and this seems like a good opportunity for the frameworkto 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
smime.p7s
Description: S/MIME cryptographic signature