Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-710

EditInventoryItem.bsh has HtmlFormWrapper rendering code. This be replaced with screens.render in ftl.

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: product
    • Labels:
      None

      Description

      EditInventory.bsh file uses HtmlFormWrapper for rendering /inventory/InventoryForms.xml#CreatePhysicalInventoryAndVariance, ViewPhysicalInventoryAndVariance forms. Path to the forms file is relative to facility webapp. This causes limitation in using the EditInventory screen from other components.

      The code for rendering should be moved to EditInventory.ftl file and should use screens.render.

      1. EditInventoryItemRefactored.patch
        4 kB
        Anil K Patel
      2. EditInventoryItemRefactored.patch
        5 kB
        Anil K Patel
      3. EditInventoryItemRefactored.patch
        3 kB
        Anil K Patel

        Activity

        Hide
        anilpatel Anil K Patel added a comment -

        Implements better coding practice.

        Show
        anilpatel Anil K Patel added a comment - Implements better coding practice.
        Hide
        anilpatel Anil K Patel added a comment -

        One more improvement. Moved the screen rendering in ftl file to include form tag in screen defination.

        Show
        anilpatel Anil K Patel added a comment - One more improvement. Moved the screen rendering in ftl file to include form tag in screen defination.
        Hide
        cjhowe Chris Howe added a comment -

        Hey Anil,

        This is related to OFBIZ-278. This reminded me of it and I think I've found a solution for OFBIZ-278 that would take care of this as well. I'm going to test my guess and report back later today.

        Looking over your patch, why do the screen.render at all? Why not put
        <screen name="EditInventoryItem">
        <snip>
        <html-template location="component://product/webapp/facility/inventory/EditInventoryItem.ftl"/>
        <snip>
        <include-screen name="nonSerialFormScreens"/>
        <snip>
        </screen>

        <screen name="nonSerialFormScreens">
        <section>
        <conditional>
        <if-compare field="inventoryItem.inventoryItemTypeId" value="NON_SERIAL_INV_ITEM"/>
        </conditional>
        <widgets>
        <include-form name="ViewPhysicalInventoryAndVariance" location="component://product/webapp/facility/inventory/InventoryForms.xml"/>
        <include-form name="CreatePhysicalInventoryAndVariance" location="component://product/webapp/facility/inventory/InventoryForms.xml"/>
        </widgets>
        </section>
        </screen>

        and remove the final conditional test from the end of EditInventoryItem.ftl

        Show
        cjhowe Chris Howe added a comment - Hey Anil, This is related to OFBIZ-278 . This reminded me of it and I think I've found a solution for OFBIZ-278 that would take care of this as well. I'm going to test my guess and report back later today. Looking over your patch, why do the screen.render at all? Why not put <screen name="EditInventoryItem"> <snip> <html-template location="component://product/webapp/facility/inventory/EditInventoryItem.ftl"/> <snip> <include-screen name="nonSerialFormScreens"/> <snip> </screen> <screen name="nonSerialFormScreens"> <section> <conditional> <if-compare field="inventoryItem.inventoryItemTypeId" value="NON_SERIAL_INV_ITEM"/> </conditional> <widgets> <include-form name="ViewPhysicalInventoryAndVariance" location="component://product/webapp/facility/inventory/InventoryForms.xml"/> <include-form name="CreatePhysicalInventoryAndVariance" location="component://product/webapp/facility/inventory/InventoryForms.xml"/> </widgets> </section> </screen> and remove the final conditional test from the end of EditInventoryItem.ftl
        Hide
        cjhowe Chris Howe added a comment -

        you're sly, posting that up while i'm typing up a comment

        Show
        cjhowe Chris Howe added a comment - you're sly, posting that up while i'm typing up a comment
        Hide
        anilpatel Anil K Patel added a comment -

        Looks like a good Idea, I will modify code and post the patch.

        Show
        anilpatel Anil K Patel added a comment - Looks like a good Idea, I will modify code and post the patch.
        Hide
        anilpatel Anil K Patel added a comment -

        Code updated based on Chris comments.

        Show
        anilpatel Anil K Patel added a comment - Code updated based on Chris comments.
        Hide
        jacopoc Jacopo Cappellato added a comment -

        Anil, Chris,

        in rev. 505649 I've committed a slightly modified patch based on the #2 one submitted by Anil.
        Please let me know if you see something wrong.
        Thanks to both of you,

        Jacopo

        Show
        jacopoc Jacopo Cappellato added a comment - Anil, Chris, in rev. 505649 I've committed a slightly modified patch based on the #2 one submitted by Anil. Please let me know if you see something wrong. Thanks to both of you, Jacopo
        Hide
        cjhowe Chris Howe added a comment -

        In 505649, you appear to have skipped over the removal of t the last if conditional check in EditInventoryItem.ftl

        Show
        cjhowe Chris Howe added a comment - In 505649, you appear to have skipped over the removal of t the last if conditional check in EditInventoryItem.ftl
        Hide
        jacopoc Jacopo Cappellato added a comment -

        Thanks Chris,

        I had messed up the svn plugin of Netbeans...
        I've committed it in rev. 505715

        Show
        jacopoc Jacopo Cappellato added a comment - Thanks Chris, I had messed up the svn plugin of Netbeans... I've committed it in rev. 505715

          People

          • Assignee:
            jacopoc Jacopo Cappellato
            Reporter:
            anilpatel Anil K Patel
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development