You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Pierre Smits <pi...@gmail.com> on 2016/03/23 13:00:13 UTC
[DISCUSSION] Refactor WorkEffort - Calendar(s)
HI all,
[pre-amble]
Currently, the workeffort component is also housing functionalities for
calendars. For this we have the following datasets:
- <EnumerationType description="Calendar Type"
enumTypeId="CALENDAR_TYPE" hasTable="N" parentTypeId=""/>
- <Enumeration description="Personal Calendar" enumCode="PERSONAL"
enumId="CAL_PERSONAL" sequenceId="01" enumTypeId="CALENDAR_TYPE"/>
- <Enumeration description="Manufacturing Calendar"
enumCode="MANUFACTURING" enumId="CAL_MANUFACTURING" sequenceId="02"
enumTypeId="CALENDAR_TYPE"/>
- <Enumeration description="None" enumId="CAL_NONE" sequenceId="03"
enumTypeId="CALENDAR_TYPE"/>
The Enumerations mentioned above are used in functions to help filtering
the overview of calendar items in the calendar screen(s), as per function
getDefaultWorkEffortExprList in WorkEffortServices.java.
[case]
Calendars are applied in various business domains to serve specific
purposes. We can consider the following:
- personal calendar
- manufacturing calendar
- maintenance calendar
- project calendar
- etc.
We have only the first two defined and we have hardcoded the filtering
conditions (based on the workEffortTypeId).
[suggestion]
1. We should remove the hardcoded filtering based on the workEffortType
2. We should remove the hardcoded definitions
3. Association of CALENDAR_TYPE to the WorkEffort records should be
based on a mapping.
[discussion]
What are your thoughts on this? Should we have the mapping (item #3) in an
existing entity, and which one should that be? What are the aspect to
consider when refactoring this.
I love to hear from you.
Best regards,
Pierre Smits
ORRTIZ.COM <http://www.orrtiz.com>
OFBiz based solutions & services
OFBiz Extensions Marketplace
http://oem.ofbizci.net/oci-2/
Re: [DISCUSSION] Refactor WorkEffort - Calendar(s)
Posted by Pierre Smits <pi...@gmail.com>.
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 <slidingfilaments@gmail.com
> 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
>
>
RE: [DISCUSSION] Refactor WorkEffort - Calendar(s)
Posted by Taher Alkhateeb <sl...@gmail.com>.
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 ...
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.
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.
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
- 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 specialized and
component dependent.
My 2 cents.
Regards,
Taher Alkhateeb
-----Original Message-----
From: Pierre Smits [mailto:pierre.smits@gmail.com]
Sent: 23 March 2016 15:40
To: dev@ofbiz.apache.org
Subject: Re: [DISCUSSION] Refactor WorkEffort - Calendar(s)
One solution we could apply is storing the mapping in the SystemProperty
entity, like:
{code}
<SystemProperty systemResourceId="workeffort" systemPropertyId=
"calendarType.CAL_MAINTENANCE" systemPropertyValue="WEPT_MAINTENANCE"
description="Calendar Type to Work Effort Purpose Type association"/>
{code}
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 1:00 PM, Pierre Smits <pi...@gmail.com>
wrote:
> HI all,
>
> [pre-amble]
> Currently, the workeffort component is also housing functionalities
> for calendars. For this we have the following datasets:
>
> - <EnumerationType description="Calendar Type"
> enumTypeId="CALENDAR_TYPE" hasTable="N" parentTypeId=""/>
> - <Enumeration description="Personal Calendar" enumCode="PERSONAL"
> enumId="CAL_PERSONAL" sequenceId="01" enumTypeId="CALENDAR_TYPE"/>
> - <Enumeration description="Manufacturing Calendar"
> enumCode="MANUFACTURING" enumId="CAL_MANUFACTURING" sequenceId="02"
> enumTypeId="CALENDAR_TYPE"/>
> - <Enumeration description="None" enumId="CAL_NONE" sequenceId="03"
> enumTypeId="CALENDAR_TYPE"/>
>
> The Enumerations mentioned above are used in functions to help
> filtering the overview of calendar items in the calendar screen(s), as
> per function getDefaultWorkEffortExprList in WorkEffortServices.java.
>
> [case]
> Calendars are applied in various business domains to serve specific
> purposes. We can consider the following:
>
> - personal calendar
> - manufacturing calendar
> - maintenance calendar
> - project calendar
> - etc.
>
> We have only the first two defined and we have hardcoded the filtering
> conditions (based on the workEffortTypeId).
>
> [suggestion]
>
> 1. We should remove the hardcoded filtering based on the
workEffortType
> 2. We should remove the hardcoded definitions
> 3. Association of CALENDAR_TYPE to the WorkEffort records should be
> based on a mapping.
>
> [discussion]
> What are your thoughts on this? Should we have the mapping (item #3)
> in an existing entity, and which one should that be? What are the
> aspect to consider when refactoring this.
>
> I love to hear from you.
>
> Best regards,
>
> Pierre Smits
>
> ORRTIZ.COM <http://www.orrtiz.com>
> OFBiz based solutions & services
>
> OFBiz Extensions Marketplace
> http://oem.ofbizci.net/oci-2/
>
Re: [DISCUSSION] Refactor WorkEffort - Calendar(s)
Posted by Pierre Smits <pi...@gmail.com>.
One solution we could apply is storing the mapping in the SystemProperty
entity, like:
{code}
<SystemProperty systemResourceId="workeffort" systemPropertyId=
"calendarType.CAL_MAINTENANCE" systemPropertyValue="WEPT_MAINTENANCE"
description="Calendar Type to Work Effort Purpose Type association"/>
{code}
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 1:00 PM, Pierre Smits <pi...@gmail.com>
wrote:
> HI all,
>
> [pre-amble]
> Currently, the workeffort component is also housing functionalities for
> calendars. For this we have the following datasets:
>
> - <EnumerationType description="Calendar Type"
> enumTypeId="CALENDAR_TYPE" hasTable="N" parentTypeId=""/>
> - <Enumeration description="Personal Calendar" enumCode="PERSONAL"
> enumId="CAL_PERSONAL" sequenceId="01" enumTypeId="CALENDAR_TYPE"/>
> - <Enumeration description="Manufacturing Calendar"
> enumCode="MANUFACTURING" enumId="CAL_MANUFACTURING" sequenceId="02"
> enumTypeId="CALENDAR_TYPE"/>
> - <Enumeration description="None" enumId="CAL_NONE" sequenceId="03"
> enumTypeId="CALENDAR_TYPE"/>
>
> The Enumerations mentioned above are used in functions to help filtering
> the overview of calendar items in the calendar screen(s), as per function
> getDefaultWorkEffortExprList in WorkEffortServices.java.
>
> [case]
> Calendars are applied in various business domains to serve specific
> purposes. We can consider the following:
>
> - personal calendar
> - manufacturing calendar
> - maintenance calendar
> - project calendar
> - etc.
>
> We have only the first two defined and we have hardcoded the filtering
> conditions (based on the workEffortTypeId).
>
> [suggestion]
>
> 1. We should remove the hardcoded filtering based on the workEffortType
> 2. We should remove the hardcoded definitions
> 3. Association of CALENDAR_TYPE to the WorkEffort records should be
> based on a mapping.
>
> [discussion]
> What are your thoughts on this? Should we have the mapping (item #3) in an
> existing entity, and which one should that be? What are the aspect to
> consider when refactoring this.
>
> I love to hear from you.
>
> Best regards,
>
> Pierre Smits
>
> ORRTIZ.COM <http://www.orrtiz.com>
> OFBiz based solutions & services
>
> OFBiz Extensions Marketplace
> http://oem.ofbizci.net/oci-2/
>