Details

    • Flags:
      Patch

      Description

      With the purpose to deprecate mini-lang OFBIZ-9350, I tried to convert some mini-lang service to groovy script.

      I converted updateRateAmount, deleteRateAmount, updatePartyRate and deleteRateAmount.

      Delete isn't a good definition for the service who expire the element instead of remove if. So I propose with the patch to move the deleteRateAmount and deletePartyRate to expireRateAmount and expirePartyRate.

      1. OFBIZ-9381.patch
        23 kB
        Nicolas Malin
      2. OFBIZ-9381.patch
        22 kB
        Nicolas Malin
      3. OFBIZ-9381-1.patch
        18 kB
        Nicolas Malin

        Activity

        Hide
        soledad Nicolas Malin added a comment -

        I load a first patch version. I will continue to add some unit tests

        Show
        soledad Nicolas Malin added a comment - I load a first patch version. I will continue to add some unit tests
        Hide
        deepak.dixit Deepak Dixit added a comment - - edited

        Hi Nicoals,

        I reviewed it and it looks good to me, some minor improvement can be done:

        • I think we can use entity-auto for updateRateAmount/deleteRateAmount service, they are belongs to CRUD operation
        • Instead of Debug.log we can use Debug.logVerbose to print the newEntity.toMapString()
        • We can use different log methods to print log in groovy scripts define in GroovyBaseScript, if needed we can add logVerbose as well.
        • I think we can change delete to expire in separate commit
        • We can use default-value attribute for setting the default value for service in parameter.
        Show
        deepak.dixit Deepak Dixit added a comment - - edited Hi Nicoals, I reviewed it and it looks good to me, some minor improvement can be done: I think we can use entity-auto for updateRateAmount/deleteRateAmount service, they are belongs to CRUD operation Instead of Debug.log we can use Debug.logVerbose to print the newEntity.toMapString() We can use different log methods to print log in groovy scripts define in GroovyBaseScript, if needed we can add logVerbose as well. I think we can change delete to expire in separate commit We can use default-value attribute for setting the default value for service in parameter.
        Hide
        deepak.dixit Deepak Dixit added a comment -

        Converted this ticket as sub-ticket of OFBIZ-9350, It will help to track the whole effort.

        Show
        deepak.dixit Deepak Dixit added a comment - Converted this ticket as sub-ticket of OFBIZ-9350 , It will help to track the whole effort.
        Hide
        soledad Nicolas Malin added a comment -

        Hi Deepak, I appreciate your review,

        I think we can use entity-auto for updateRateAmount/deleteRateAmount service, they are belongs to CRUD operation

        I hesitated, to replace it by entity-auto because we have some default value instanciation manageable by defautl-value on the service definition and define seca to manage the pretreatment. Finally I took the choice that it's more readable on groovyDSL but it's also questionable

        Instead of Debug.log we can use Debug.logVerbose to print the newEntity.toMapString()

        Oups I forgot to remove it ^^

        I think we can change delete to expire in separate commit

        Completely agree, I will do it

        We can use default-value attribute for setting the default value for service in parameter

        I also hesitated, but I will follow your suggest

        Thanks Deepak

        Show
        soledad Nicolas Malin added a comment - Hi Deepak, I appreciate your review, I think we can use entity-auto for updateRateAmount/deleteRateAmount service, they are belongs to CRUD operation I hesitated, to replace it by entity-auto because we have some default value instanciation manageable by defautl-value on the service definition and define seca to manage the pretreatment. Finally I took the choice that it's more readable on groovyDSL but it's also questionable Instead of Debug.log we can use Debug.logVerbose to print the newEntity.toMapString() Oups I forgot to remove it ^^ I think we can change delete to expire in separate commit Completely agree, I will do it We can use default-value attribute for setting the default value for service in parameter I also hesitated, but I will follow your suggest Thanks Deepak
        Hide
        soledad Nicolas Malin added a comment - - edited

        Ok I separate the code to convert only the minilang to groovy with move default value on the service definition. If it's OK for you Deepak Dixit I will commit this and after I will move the delete service name to expire

        Show
        soledad Nicolas Malin added a comment - - edited Ok I separate the code to convert only the minilang to groovy with move default value on the service definition. If it's OK for you Deepak Dixit I will commit this and after I will move the delete service name to expire
        Hide
        deepak.dixit Deepak Dixit added a comment -

        Thanks Nicolas Malin, yes it looks good to me,

        You can remove unused Debug class from import.

        Show
        deepak.dixit Deepak Dixit added a comment - Thanks Nicolas Malin , yes it looks good to me, You can remove unused Debug class from import.
        Hide
        soledad Nicolas Malin added a comment -

        Ok done at revision 1796428.

        Show
        soledad Nicolas Malin added a comment - Ok done at revision 1796428.
        Hide
        soledad Nicolas Malin added a comment -

        I commit the rename delete to expire on trunk at 1799252

        Show
        soledad Nicolas Malin added a comment - I commit the rename delete to expire on trunk at 1799252

          People

          • Assignee:
            soledad Nicolas Malin
            Reporter:
            soledad Nicolas Malin
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development