[RFC] Order package entities

[cjhowe]

While looking into reorganizing the shopping cart -> order business
logic I thought I'd take a look at the data model for the order
entities to get better acquainted with some of the specialized features
that get tucked away over the months.  I came across a few things that
are appear on their surface to be inconsistent or that might be
reintroduced a bit more generically. I want to make clear that I'm not
suggesting we break out the demolition crew just reorganize our thought
processes and identify where we may want begin some incremental
changes.  I have the org.ofbiz.order.* packages in separate pages of a
visio file and will export them to images and attach to a JIRA issue if
it's determined that we want to act on any of these peculiarities.
Otherwise I will attach them to docs.ofbiz.org for the benefit of those
that learn better by pictures.

The following appear as if they could be reintroduced as orderTypes
with additional tables for their unique fields
order.request
order.quote
order.return

The remainder will be broken up by package then entity with a short
note

order.order

OrderDeliverySchedule - should this be related to a package or
something else in shipping or is this just a strangely denormalized
entity?

OrderHeaderWorkEffort - redundant with OrderItemWorkEffort with
OrderItemSeqId = null

OrderSummaryEntry - denormalized information, very app specific?

OrderItemAssociation - There are probably other order associations that
would be beneficial aside from PO <-> SO.  Was this done to accommodate
the drop shipping improvements?

ProductOrderItem - Looks to be identical to OrderItemAssociation except
not specific to PO <-> SO with a redundant productId field

OrderAdjustment - Could be broken up with subcategory tables

OrderItemShipGroupAssoc-
OrderItemShipGrpInvRes-
OrderShipment-
OrderItemShipGroup- all of these have a field shipGroupSeqId. Should
this be shipGroupId?

OrderItemContactMech - replaced by OrderContactMech?

OrderRole - redundant with OrderItemRole with orderItemSeqId = null

OrderNotification - more consistent as CommunicationEventOrder or
redundant with it?

OrderHeaderNote - needs orderItemSeqId field

order.quote

QuoteCoefficient - Looks an awful lot like QuoteAttribute

order.request

CustRequestWorkEffort - redundant with CustRequestItemWorkEffort where
custRequestItemSeqId = null

CustRequestNote - needs custRequestItemSeqId field

CustRequestRole - needs custRequestItemSeqId field

CustRequestCommEvent - needs custRequestItemSeqId field

CustRequestStatus - needs custRequestItemSeqId field

order.requirement

RequirementAttribute - statusId present at all much less as PK?

RequirementBudgetAllocation -  statusId present at all much less as PK?

OrderRequirementCommitment - statusId present at all much less as PK?

RequirementRole - statusId present at all much less as PK?

RequirementCustRequest - statusId present at all much less as PK?

WorkEffortFulfillment - statusId present at all much less as PK?

RespondingParty - should be in order.request, patch already in JIRA

DesiredFeature - statusId present at all much less as PK?,
desiredFeatureId needs to be lone PK field

Thanks for any feedback!

Chris

[Jacopo Cappellato]

Chris,

Chris Howe wrote:
>
> OrderItemAssociation - There are probably other order associations that
> would be beneficial aside from PO <-> SO.  Was this done to accommodate
> the drop shipping improvements?
>

No. That is an old entity. When I contributed my work on drop shipments,
as a subtask I also refactored that part of the system and:
- deprecated the OrderItemAssociation entity (that is now called
OldOrder...)
- created two new entities OrderItemAssoc and OrderItemAssocType
- created seed data for them
- improve the system here and there to make use of the new features of
associating orders by type of association
- created a service to migrate old associations to the new model

Jacopo

[Jacopo Cappellato]

Chris Howe wrote:
>
> The following appear as if they could be reintroduced as orderTypes
> with additional tables for their unique fields
> order.request
> order.quote
> order.return

In general, I'm against this kind of refactorings: there are definitely
pros and cons for the two approaches and we could definitely discuss
about them for days and days (probably without any agreement on the best
solution); so I am pragmatic on this: there is a lot of existing code
built on top of these entities, no specific problems caused by this
design -> thus no reason to change; especially if the change is
suggested by the bare analysis of the data model and not also on the
existing business logic and existing problems that the data model is
causing.
So, no I'm against this change.

Jacopo

[cjhowe]

Thanks Jacopo,  That's my mistake.  I had referenced a deployment that
was last updated in mid September.  I'll review and update my comments.

--- Jacopo Cappellato <[hidden email]> wrote:

> Chris,
>
> Chris Howe wrote:
> >
> > OrderItemAssociation - There are probably other order associations
> that
> > would be beneficial aside from PO <-> SO.  Was this done to
> accommodate
> > the drop shipping improvements?
> >
>
> No. That is an old entity. When I contributed my work on drop
> shipments,
> as a subtask I also refactored that part of the system and:
> - deprecated the OrderItemAssociation entity (that is now called
> OldOrder...)
> - created two new entities OrderItemAssoc and OrderItemAssocType
> - created seed data for them
> - improve the system here and there to make use of the new features
> of
> associating orders by type of association
> - created a service to migrate old associations to the new model
>
> Jacopo
>
>
>

[Jacopo Cappellato]

Chris Howe wrote:
>
> ProductOrderItem - Looks to be identical to OrderItemAssociation except
> not specific to PO <-> SO with a redundant productId field
>

This should be probably deprecated and the OrderItemAssoc entity used
instead (not the OrderItemAssociation that is the old entity, see my
previous mail on this); however I'm not sure about the requirements
behind the productId field... so maybe I could be wrong.

Jacopo

>
> Chris

[cjhowe]

--- Jacopo Cappellato <[hidden email]> wrote:

> Chris Howe wrote:
> >
> > The following appear as if they could be reintroduced as orderTypes
> > with additional tables for their unique fields
> > order.request
> > order.quote
> > order.return
>
> In general, I'm against this kind of refactorings: there are
> definitely
> pros and cons for the two approaches and we could definitely discuss
> about them for days and days (probably without any agreement on the
> best
> solution); so I am pragmatic on this: there is a lot of existing code
>
> built on top of these entities, no specific problems caused by this
> design -> thus no reason to change; especially if the change is
> suggested by the bare analysis of the data model and not also on the
> existing business logic and existing problems that the data model is
> causing.
> So, no I'm against this change.
>
> Jacopo
>
>

There's just seems to be a lot of redundancy with this.  I'm not so
concerned with request, quote or shoppinglist (i meant to include that
as well) as there isn't much business logic overlap.  However, with
return there is quite a lot (inventory management, possibly shipping,
OrderItemAssoc, Adjustments etc).  While it currently works, it makes
it quite a bit more difficult to learn as someone trying to pick it up
is likely to erroneously assume it has less in common with OrderHeader,
etc than it actually does.  I think there's some interest in making
these simpler to use/ more generic.  Are you against because of
resources (time actually spent rewriting/reviewing it) or is there more
of a technical aspect (backwards compatibility, fragile code, etc)?

Chris

[Jacopo Cappellato]

Chris Howe wrote:

>
> There's just seems to be a lot of redundancy with this.  I'm not so
> concerned with request, quote or shoppinglist (i meant to include that
> as well) as there isn't much business logic overlap.  However, with
> return there is quite a lot (inventory management, possibly shipping,
> OrderItemAssoc, Adjustments etc).  While it currently works, it makes
> it quite a bit more difficult to learn as someone trying to pick it up
> is likely to erroneously assume it has less in common with OrderHeader,
> etc than it actually does.  I think there's some interest in making
> these simpler to use/ more generic.  Are you against because of
> resources (time actually spent rewriting/reviewing it) or is there more
> of a technical aspect (backwards compatibility, fragile code, etc)?
>
> Chris

I don't think that the learning curve needed to use returns could be
much more different if the returns are stored in the order entities
instead of their own. For example, my clients are usually surprised (and
sometimes concerned) when I explain them that the purchase and the sales
orders are actually stored in the same entity (that is a good thing!)...
 From the developers point of view, in my opinion there are pros and
cons (as I said before) for storing returns in the order entities:
less separate services that (probably, but I did not do a serious
analysis on this) could do similar tasks; on the other hand, the order
services will be more complex (and also data extraction and preparation).
For this, since I don't see real problems in the current implementation
that will be addressed by this refactoring, I don't see it really
something worth of the effort.

Jacopo

[Tim Ruppert]

On Feb 15, 2007, at 12:52 AM, Jacopo Cappellato wrote:

> Chris Howe wrote:
>> There's just seems to be a lot of redundancy with this.  I'm not so
>> concerned with request, quote or shoppinglist (i meant to include  
>> that
>> as well) as there isn't much business logic overlap.  However, with
>> return there is quite a lot (inventory management, possibly shipping,
>> OrderItemAssoc, Adjustments etc).  While it currently works, it makes
>> it quite a bit more difficult to learn as someone trying to pick  
>> it up
>> is likely to erroneously assume it has less in common with  
>> OrderHeader,
>> etc than it actually does.  I think there's some interest in making
>> these simpler to use/ more generic.  Are you against because of
>> resources (time actually spent rewriting/reviewing it) or is there  
>> more
>> of a technical aspect (backwards compatibility, fragile code, etc)?
>> Chris
>
> I don't think that the learning curve needed to use returns could  
> be much more different if the returns are stored in the order  
> entities instead of their own. For example, my clients are usually  
> surprised (and sometimes concerned) when I explain them that the  
> purchase and the sales orders are actually stored in the same  
> entity (that is a good thing!)...
> From the developers point of view, in my opinion there are pros and  
> cons (as I said before) for storing returns in the order entities:
> less separate services that (probably, but I did not do a serious  
> analysis on this) could do similar tasks; on the other hand, the  
> order services will be more complex (and also data extraction and  
> preparation).
> For this, since I don't see real problems in the current  
> implementation that will be addressed by this refactoring, I don't  
> see it really something worth of the effort.
>
> Jacopo
>

+1

[David E Jones]

On Feb 15, 2007, at 12:52 AM, Jacopo Cappellato wrote:

> Chris Howe wrote:
>> There's just seems to be a lot of redundancy with this.  I'm not so
>> concerned with request, quote or shoppinglist (i meant to include  
>> that
>> as well) as there isn't much business logic overlap.  However, with
>> return there is quite a lot (inventory management, possibly shipping,
>> OrderItemAssoc, Adjustments etc).  While it currently works, it makes
>> it quite a bit more difficult to learn as someone trying to pick  
>> it up
>> is likely to erroneously assume it has less in common with  
>> OrderHeader,
>> etc than it actually does.  I think there's some interest in making
>> these simpler to use/ more generic.  Are you against because of
>> resources (time actually spent rewriting/reviewing it) or is there  
>> more
>> of a technical aspect (backwards compatibility, fragile code, etc)?
>> Chris
>
> I don't think that the learning curve needed to use returns could  
> be much more different if the returns are stored in the order  
> entities instead of their own. For example, my clients are usually  
> surprised (and sometimes concerned) when I explain them that the  
> purchase and the sales orders are actually stored in the same  
> entity (that is a good thing!)...
> From the developers point of view, in my opinion there are pros and  
> cons (as I said before) for storing returns in the order entities:
> less separate services that (probably, but I did not do a serious  
> analysis on this) could do similar tasks; on the other hand, the  
> order services will be more complex (and also data extraction and  
> preparation).
> For this, since I don't see real problems in the current  
> implementation that will be addressed by this refactoring, I don't  
> see it really something worth of the effort.

I agree with Jacopo there. Using something generic for all of these  
would be a huge mess. It may seem that there is redundancy in them,  
but there really isn't. They are very different things. They are  
created using similar patterns and because they are often associated  
with one another there are also some similar patterns. But no, they  
really aren't the same thing at all and have VERY different  
requirements for process and data to store.

-David