OFBiz
  1. OFBiz
  2. OFBIZ-499

Implement ShoppingCartEvents and CheckoutEvents as simple-mehtods

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: order
    • Labels:
      None

      Description

      Recreate the ShoppingCartEvents and CheckoutEvents as simple methods so they will be easier to override/play with.

      1. ShoppingCartItem.mm
        12 kB
        Chris Howe
      2. ShoppingCartItem.jpeg
        132 kB
        Chris Howe
      3. ShoppingCart.jpeg
        84 kB
        Chris Howe
      4. CompareJavaCodeToMinilang.patch
        8 kB
        Anil K Patel
      5. CompareJavaCodeToMinilang.patch
        8 kB
        Anil K Patel
      6. Shopping Cart.mm
        11 kB
        Chris Howe
      7. Shopping Cart.mm
        12 kB
        Chris Howe

        Activity

        Hide
        Si Chen added a comment -

        I do not like this idea.

        Show
        Si Chen added a comment - I do not like this idea.
        Hide
        Alex D. Fleming added a comment -

        Chris,

        I think we will keep the ShoppingCartEvents and CheckoutEvents code as it is in Java.
        There is no need to convert this code into simple-methods.

        +1 for Si's comment.

        Cheer
        Alex D. Fleming

        Show
        Alex D. Fleming added a comment - Chris, I think we will keep the ShoppingCartEvents and CheckoutEvents code as it is in Java. There is no need to convert this code into simple-methods. +1 for Si's comment. Cheer Alex D. Fleming
        Hide
        Chris Howe added a comment -

        Hey Si,

        Would you care to expand on your concerns? The implementation that I plan will be for a hot-deploy application and if present will simply override ofbiz's logic. If the implementation is preferred, then might replace ofbiz's implementation.

        Show
        Chris Howe added a comment - Hey Si, Would you care to expand on your concerns? The implementation that I plan will be for a hot-deploy application and if present will simply override ofbiz's logic. If the implementation is preferred, then might replace ofbiz's implementation.
        Hide
        Si Chen added a comment -

        Chris,

        Sorry, I think in my haste I may have misunderstood your intent.

        If you want to implement another version in minilang and show us how it could be done, that's great.

        I however would strongly prefer to keep the Java version of the cart methods rather than outright replace them with new ones written in minilang.

        Si

        Show
        Si Chen added a comment - Chris, Sorry, I think in my haste I may have misunderstood your intent. If you want to implement another version in minilang and show us how it could be done, that's great. I however would strongly prefer to keep the Java version of the cart methods rather than outright replace them with new ones written in minilang. Si
        Hide
        Jonathon Wong added a comment -

        +1 for Si's comment.

        I'm for maximum re-use. Unless there's a huge need for big overhaul, I don't see any point in discarding ShoppingCartEvents.java and CheckoutEvents.java.

        Besides, I just did a little (but critical!!) bugfix in CheckoutEvents.java in OFBIZ-627. So, if you drop CheckoutEvents.java now, my work will be wiped out. : (

        Show
        Jonathon Wong added a comment - +1 for Si's comment. I'm for maximum re-use. Unless there's a huge need for big overhaul, I don't see any point in discarding ShoppingCartEvents.java and CheckoutEvents.java. Besides, I just did a little (but critical!!) bugfix in CheckoutEvents.java in OFBIZ-627 . So, if you drop CheckoutEvents.java now, my work will be wiped out. : (
        Hide
        Andrew Sykes added a comment -

        -1 (I think)

        I've seen lots of noobs struggle with this code, I'd much prefer to see it in a form that they could more easily digest.

        The ecommerce stuff is a pretty common first point of contact with OfBiz, anything that makes it simpler to learn and modify should be encouraged in my opinion.

        Show
        Andrew Sykes added a comment - -1 (I think) I've seen lots of noobs struggle with this code, I'd much prefer to see it in a form that they could more easily digest. The ecommerce stuff is a pretty common first point of contact with OfBiz, anything that makes it simpler to learn and modify should be encouraged in my opinion.
        Hide
        Andrew Sykes added a comment -

        That last post was a bit ambiguous,
        +1 to minilang
        -1 to existing stuff

        Show
        Andrew Sykes added a comment - That last post was a bit ambiguous, +1 to minilang -1 to existing stuff
        Hide
        Jacques Le Roux added a comment -

        I think the best way here is to propose a patch and wil see

        Show
        Jacques Le Roux added a comment - I think the best way here is to propose a patch and wil see
        Hide
        Chris Howe added a comment -

        This has been a MUCH larger task to tackle than I had originally anticipated. First, because the checkout events are extremely hairy and long in the tooth. And second because of the issue brought up in
        http://issues.apache.org/jira/browse/OFBIZ-400#action_12452560 is making it difficult to call java methods that take non cached class types.
        For the time being, I've tabled this in my efforts. If someone wants to collaborate on this after the class types issue is resolved, I'd be more than happy to work on this in the sandbox project. But I keep running around in circles with many of the additions that have been added inline to the methods over the years to achieve specific feature sets.

        Show
        Chris Howe added a comment - This has been a MUCH larger task to tackle than I had originally anticipated. First, because the checkout events are extremely hairy and long in the tooth. And second because of the issue brought up in http://issues.apache.org/jira/browse/OFBIZ-400#action_12452560 is making it difficult to call java methods that take non cached class types. For the time being, I've tabled this in my efforts. If someone wants to collaborate on this after the class types issue is resolved, I'd be more than happy to work on this in the sandbox project. But I keep running around in circles with many of the additions that have been added inline to the methods over the years to achieve specific feature sets.
        Hide
        Jacques Le Roux added a comment -

        Yes timing is not easy on this. "hairy and long in the tooth" is new for me :o)

        Show
        Jacques Le Roux added a comment - Yes timing is not easy on this. "hairy and long in the tooth" is new for me :o)
        Hide
        Jonathon Wong added a comment -

        Chris,

        > This has been a MUCH larger task to tackle than I had originally anticipated.

        Which is why I suggested we continue to re-use the existing Java classes.

        First step is to clean up the codes in those classes. Then you can more easily translate them into minilang.

        Incremental progress. Always easier to climb stairs than leap over buildings.

        As we come into contact with the "front-line" ecommerce stuff, we clean it up more an more. Like I just did with CheckOutEvents.java. Make sure things work first, then we make the leap to minilang. No point coding a new set of buggy ecommerce codes in minilang.

        Show
        Jonathon Wong added a comment - Chris, > This has been a MUCH larger task to tackle than I had originally anticipated. Which is why I suggested we continue to re-use the existing Java classes. First step is to clean up the codes in those classes. Then you can more easily translate them into minilang. Incremental progress. Always easier to climb stairs than leap over buildings. As we come into contact with the "front-line" ecommerce stuff, we clean it up more an more. Like I just did with CheckOutEvents.java. Make sure things work first, then we make the leap to minilang. No point coding a new set of buggy ecommerce codes in minilang.
        Hide
        Chris Howe added a comment -

        While I would generally agree on the benefits of incremental progress, incremental progress within the community project on this issue may not really be a viable option.

        This is a code base that's rather complex and affects 99.9% of people using OFBiz. Because of this, every patch regarding this _needs the utmost scrutiny. IMO, any rewrite that would have a chance to make it back into the community project needs to be an alternative plug and play style approach that can be thoroughly tested outside of the community project before being accepted inside it. Otherwise the amount of time needed to test/accept or reject every incremental patch will be too much of a burden on the committers to warrant the perceived benefit without a pressing use case.

        I can give two valid use cases that would benefit from a rewrite (Return Orders and Transfer Orders utilizing the order entities) but both of those use cases have currently working solutions. It would likely be difficult to garner collaboration against the community project code for replacement Thus my desire to implement this as a hot-deploy solution.

        Show
        Chris Howe added a comment - While I would generally agree on the benefits of incremental progress, incremental progress within the community project on this issue may not really be a viable option. This is a code base that's rather complex and affects 99.9% of people using OFBiz. Because of this, every patch regarding this _needs the utmost scrutiny. IMO, any rewrite that would have a chance to make it back into the community project needs to be an alternative plug and play style approach that can be thoroughly tested outside of the community project before being accepted inside it. Otherwise the amount of time needed to test/accept or reject every incremental patch will be too much of a burden on the committers to warrant the perceived benefit without a pressing use case. I can give two valid use cases that would benefit from a rewrite (Return Orders and Transfer Orders utilizing the order entities) but both of those use cases have currently working solutions. It would likely be difficult to garner collaboration against the community project code for replacement Thus my desire to implement this as a hot-deploy solution.
        Hide
        Anil K Patel added a comment -

        I am for writing new implementation that will eventually replace ShoppingCartEvents.java and CheckoutEvents.java. The new Anonymous checkout process in E-Commerce kind of does that, may it be really very little.

        I see its need (Every week when I visit the users) and I am ready to contribute some work as well if we can get few committers to back up. The Purchase Order and Sales order Process in Order component is really difficult to customize. The default implementation may be good for some users and not for others.

        Show
        Anil K Patel added a comment - I am for writing new implementation that will eventually replace ShoppingCartEvents.java and CheckoutEvents.java. The new Anonymous checkout process in E-Commerce kind of does that, may it be really very little. I see its need (Every week when I visit the users) and I am ready to contribute some work as well if we can get few committers to back up. The Purchase Order and Sales order Process in Order component is really difficult to customize. The default implementation may be good for some users and not for others.
        Hide
        Chris Howe added a comment -

        This is a mindmap of the shopping cart using freemind http://freemind.sourceforge.net/wiki/index.php/Main_Page . I had tried going line by line recreating the shopping cart -> order in simple methods and it frankly, made my head hurt...a lot.

        I think I've decided to basically just ignore the current java and just deal with the shopping cart's fields. This mind map will help break up the shopping cart into obvious areas. If someone could review it and correct my groupings, I think we can move along this quite easily.

        Show
        Chris Howe added a comment - This is a mindmap of the shopping cart using freemind http://freemind.sourceforge.net/wiki/index.php/Main_Page . I had tried going line by line recreating the shopping cart -> order in simple methods and it frankly, made my head hurt...a lot. I think I've decided to basically just ignore the current java and just deal with the shopping cart's fields. This mind map will help break up the shopping cart into obvious areas. If someone could review it and correct my groupings, I think we can move along this quite easily.
        Hide
        Jacques Le Roux added a comment -

        Great work, thanks a bunch Chris, I will use it !

        I hope one day we will beneficiate from Giffy plugin in Confluence to be able to share such work (even if Giffy is not a mindmaps tool but only a shareable graphs drawing tool)...

        Show
        Jacques Le Roux added a comment - Great work, thanks a bunch Chris, I will use it ! I hope one day we will beneficiate from Giffy plugin in Confluence to be able to share such work (even if Giffy is not a mindmaps tool but only a shareable graphs drawing tool)...
        Hide
        Jacques Le Roux added a comment -

        Afterthoughs : thoug in the meantime we may share and update mindmaps also.

        Show
        Jacques Le Roux added a comment - Afterthoughs : thoug in the meantime we may share and update mindmaps also.
        Hide
        Chris Howe added a comment -

        Shopping Cart.mm

        Replaces previous. setup by entities

        Show
        Chris Howe added a comment - Shopping Cart.mm Replaces previous. setup by entities
        Hide
        Anil K Patel added a comment -

        This is part of my effort to contribute back to community that has given such a wonderful ERP. I am trying to figure out what is best to do in this situation.

        CompareJavaCodeToMinilang.patch has implementation of logic in Java as well has Mini lang, I'll like to get feedback on which one to keep and which to discard.

        I like the Mini lang version.

        Show
        Anil K Patel added a comment - This is part of my effort to contribute back to community that has given such a wonderful ERP. I am trying to figure out what is best to do in this situation. CompareJavaCodeToMinilang.patch has implementation of logic in Java as well has Mini lang, I'll like to get feedback on which one to keep and which to discard. I like the Mini lang version.
        Hide
        Si Chen added a comment -

        Not suprisingly, I would definitely prefer the Java version.

        Show
        Si Chen added a comment - Not suprisingly, I would definitely prefer the Java version.
        Hide
        Chris Howe added a comment -

        Hey Si,

        Can you expand on that?

        Show
        Chris Howe added a comment - Hey Si, Can you expand on that?
        Hide
        Chris Howe added a comment -

        jpeg of the mind map

        Show
        Chris Howe added a comment - jpeg of the mind map
        Hide
        Si Chen added a comment -

        Can we close this issue and consolidate it with OFBIZ-659? Is there some reason to have two threads?

        Show
        Si Chen added a comment - Can we close this issue and consolidate it with OFBIZ-659 ? Is there some reason to have two threads?
        Hide
        Anil K Patel added a comment -

        To the best of my understanding these two issues will are dealing with different issues. The Checkout Refactoring issue is dealing with making the checkout process modular by moving the controller code embedded in these files into controller.xml file. In the refactoring process I don't intend rewrite any code if its not required.
        While this issue is about redoing these files in minilang.

        Show
        Anil K Patel added a comment - To the best of my understanding these two issues will are dealing with different issues. The Checkout Refactoring issue is dealing with making the checkout process modular by moving the controller code embedded in these files into controller.xml file. In the refactoring process I don't intend rewrite any code if its not required. While this issue is about redoing these files in minilang.
        Hide
        Chris Howe added a comment -

        That is my understanding as well.

        OFBIZ-499 is dealing with the business logic process of moving a ShoppingCart -> order using simple-methods

        OFBIZ-659 is dealing with the UI process

        Show
        Chris Howe added a comment - That is my understanding as well. OFBIZ-499 is dealing with the business logic process of moving a ShoppingCart -> order using simple-methods OFBIZ-659 is dealing with the UI process
        Hide
        Chris Howe added a comment -

        ShoppingCartItem.jpeg
        Image modeling the ShoppingCartItem

        Show
        Chris Howe added a comment - ShoppingCartItem.jpeg Image modeling the ShoppingCartItem
        Hide
        Chris Howe added a comment -

        ShoppingCartItem.mm

        Mind Map of the Shoping Cart Item broken up into their ultimate data storage place.

        This is a first pass, guessing at their location by the name.
        If anyone can offer assistance/ modification, I would appreciate it. I will do a second pass while going through the actual code.

        Show
        Chris Howe added a comment - ShoppingCartItem.mm Mind Map of the Shoping Cart Item broken up into their ultimate data storage place. This is a first pass, guessing at their location by the name. If anyone can offer assistance/ modification, I would appreciate it. I will do a second pass while going through the actual code.
        Hide
        Chris Howe added a comment -

        I've done a little bit of work on this to show the direction, nowhere near complete and certainly has not compared against the current java to catch use cases. I think it shows the direction and the benefit of how someone with less programming skills can rather easily change up the logic to suit their deployment.

        I've had to work on this on two different boxes so I used the sandbox for version control, rather than posting a patch (and subsequent patches as the multitude of google checkout patches didn't really help anyone), I'll just provide the link and if/when someone wants to help out, I'll provide a patch here.

        http://ofbiz-sandbox.svn.sourceforge.net/viewvc/ofbiz-sandbox/ofbiz-sandbox/trunk/hot-deploy/shoppingcart/

        Show
        Chris Howe added a comment - I've done a little bit of work on this to show the direction, nowhere near complete and certainly has not compared against the current java to catch use cases. I think it shows the direction and the benefit of how someone with less programming skills can rather easily change up the logic to suit their deployment. I've had to work on this on two different boxes so I used the sandbox for version control, rather than posting a patch (and subsequent patches as the multitude of google checkout patches didn't really help anyone), I'll just provide the link and if/when someone wants to help out, I'll provide a patch here. http://ofbiz-sandbox.svn.sourceforge.net/viewvc/ofbiz-sandbox/ofbiz-sandbox/trunk/hot-deploy/shoppingcart/
        Hide
        Jacques Le Roux added a comment -

        Thanks for the JPGs Chris. I tried to print from mind map and had no success (on Win 2000).

        Show
        Jacques Le Roux added a comment - Thanks for the JPGs Chris. I tried to print from mind map and had no success (on Win 2000).

          People

          • Assignee:
            Unassigned
            Reporter:
            Chris Howe
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:

              Development