OFBiz
  1. OFBiz
  2. OFBIZ-710

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

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major 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
        Anil K Patel added a comment -

        Implements better coding practice.

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

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

        Show
        Anil K Patel added a comment - One more improvement. Moved the screen rendering in ftl file to include form tag in screen defination.
        Hide
        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
        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
        Chris Howe added a comment -

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

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

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

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

        Code updated based on Chris comments.

        Show
        Anil K Patel added a comment - Code updated based on Chris comments.
        Hide
        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
        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
        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
        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
        Jacopo Cappellato added a comment -

        Thanks Chris,

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

        Show
        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:
            Jacopo Cappellato
            Reporter:
            Anil K Patel
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development