Taher, Thanks for the feedback and sharing your insights.
Please see my comments inline. Best regards, Pierre Smits ORRTIZ.COM <http://www.orrtiz.com> OFBiz based solutions & services OFBiz Extensions Marketplace http://oem.ofbizci.net/oci-2/ On Wed, Mar 23, 2016 at 5:17 PM, Taher Alkhateeb <[email protected] > wrote: > Hi Pierre, > > I would say that SystemProperty is definitely not the best solution. For > one thing, a system property is sort of a framework configuration, like the > theme, mail settings, some paths, etc ... > > The SystemProperty entity is intended to configure not only how an OFBiz implementation is setup, but also how each individual component is configured to operate. This is especially important in a multi-tenant environment where (component) configurations can differ per tenant. For exactly that reason I have created the https://issues.apache.org/jira/browse/OFBIZ-6164 issue and associated sub tasks. > If you want an association between a calendar and a workeffort, then just > create an assoc entity. I would recommend to steer away from "not so > obvious" places as this is exactly why we started the refactoring project > in > the first place. > Spawning new entities where others can be reused - and SystemProperty can be used more extensively for configuration of component operations - should be avoided as much as possible. Here at ORRTIZ.COM this (the SystemProperty entity) is used extensively in each (hot-deploy) component, some of which mimic functionalities in base components. In fact configuration is an essential element to enhance the UX in each component. See also So, for us it not just a 'framework only' aspect, as it enables us to avoid spawning new entities for every new component. As an example as to how we applied this, checkout the MultiSafepay solution we made available to the community (http://oem.ofbizci.net/oci-2/products/p_omultisafepay , code repository: https://github.com/ORRTIZ/omultisafepay ). Unlike the OOTB 3rd party payment solutions, this solution doesn't implement any new entities to make it work. And the component has its own configuration functionality. Moreover, we have enhanced the ant create-component function to ensure that configuration functionality is available in every hot-deploy component we build, with specifics per component type (generic back-end, theme, webshop, cmssite). > > Another thing to notice is that the service getWorkEffortEventsByPeriod > actually checks first in the incoming parameters whether a list of > entityExprList is passed to it, if not, it will call > getDefaultWorkEffortExprList passing in the WorkEffortType and > CalendarType. So maybe you can avoid the whole thing by making the filtration logic on the > screen level (or data preparation level) by passing the correct > entityExprList. > That is exactly the reason why the getDefaultWorkEffortExprList function exist, to provide a way out when no parameters are provided. The current functionality validates only based ontwo options CAL_PERSONAL and CAL_MANUFACTURING, with a fall-back to CAL_PERSONAL. And it is used wherever a calendar function is provided. But having it limited, or replacing it with requirement specifics per component-function combination isn't what we - as a community - should strive for. Let's leave that at the individual implementers/developers. > > My personal preference would be something like the following: > - actually delete getDefaultWorkEffortExprList and fix the calling code > - do not add any mapping entities > - either hardcode the filtration on screen level OR > Hard coding is not an option. And we should not advocate it. > - create a new service that applies the filters for each component > separately (manufacturing, project, etc ...) > > In other words, change the world around the edges, not at the heart of the > thing! This makes the code more resilient and not too specialised and > component dependent. > > Enhancing the calendar functions to be utilised in various component doesn't make the functions (more) specialised. In fact, applying SystemProperty records make it more component independent. And configurable. > My 2 cents. > > Regards, > > Taher Alkhateeb > >
