OFBiz
  1. OFBiz
  2. OFBIZ-1434 General Ledger
  3. OFBIZ-1440

UI: Review/Improve/Cleanup the /accounting/control/EditAcctgTrans screen

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: accounting
    • Labels:
      None

      Description

      Review/Improve/Cleanup the /accounting/control/EditAcctgTrans screen: you can reach it from Accounting>General Ledger>Accounting>Accounting Transactions>then perform a search and click one of the accctgTransId

      • add more fields (from the AcctgTrans entity) to the form at the top of the screen
      • the form at the top of the screen, and the underlying updateAcctgTrans service, always allows to edit the fields of the AcctgTrans entity (even if the transactions are posted): should we limit this (both at the ui and service level)?
      1. CreateAcctgTrans.patch
        7 kB
        Vikas Mayur
      2. Review_EditAcctgTrans_Screens_issue.patch
        17 kB
        Brajesh Patel
      3. Review_EditAcctgTrans_Screens_issue.patch
        18 kB
        Brajesh Patel

        Activity

        Hide
        Vikas Mayur added a comment -

        Thanks Brajesh,
        Yours patch is in trunk rev 650883.

        Show
        Vikas Mayur added a comment - Thanks Brajesh, Yours patch is in trunk rev 650883.
        Hide
        Brajesh Patel added a comment -

        I have attached patch for this issue please collect this,
        if there is some improvements then let me know.

        Brajesh Patel

        Show
        Brajesh Patel added a comment - I have attached patch for this issue please collect this, if there is some improvements then let me know. Brajesh Patel
        Hide
        Jacopo Cappellato added a comment -

        David,

        thanks, this is an important point actually.
        I've checked the code and unfortunately the flag is not checked by the underlying services.
        For this reason, in order to complete the task, we have also to add the logic (and return an error if the transaction has been already posted: AcctgTrans.isPosted==Y) to the following services:

        • updateAcctgTrans
        • deleteAcctgTrans
        • createAcctgTransEntry
        • updateAcctgTransEntry
        • deleteAcctgTransEntry
        • completeAcctgTransEntries

        Jacopo

        Show
        Jacopo Cappellato added a comment - David, thanks, this is an important point actually. I've checked the code and unfortunately the flag is not checked by the underlying services. For this reason, in order to complete the task, we have also to add the logic (and return an error if the transaction has been already posted: AcctgTrans.isPosted==Y) to the following services: updateAcctgTrans deleteAcctgTrans createAcctgTransEntry updateAcctgTransEntry deleteAcctgTransEntry completeAcctgTransEntries Jacopo
        Hide
        David E. Jones added a comment -

        It's great to have something in the UI so they know they can't modify a posted transaction.

        Are the underlying services also checking for this to enforce it on the server side?

        Show
        David E. Jones added a comment - It's great to have something in the UI so they know they can't modify a posted transaction. Are the underlying services also checking for this to enforce it on the server side?
        Hide
        Jacopo Cappellato added a comment -

        Most, if not all, of the issues listed in my previous comments have been fixed, except for this one:

        "I think it is time to prevent the ability to modify an accounting transaction that is posted.
        Instead of adding a bunch of use-when in the form definition's fields, I'd suggest that we create two new forms and we add a <section><condition>... area to the screen definition to include the editable or not editable ones according to the isPosted flag. "

        It would be great to address this last issue and then close the task.

        Show
        Jacopo Cappellato added a comment - Most, if not all, of the issues listed in my previous comments have been fixed, except for this one: "I think it is time to prevent the ability to modify an accounting transaction that is posted. Instead of adding a bunch of use-when in the form definition's fields, I'd suggest that we create two new forms and we add a <section><condition>... area to the screen definition to include the editable or not editable ones according to the isPosted flag. " It would be great to address this last issue and then close the task.
        Hide
        Jacopo Cappellato added a comment -

        We should also include in the screens the following fields: glAccountTypeId, origAmount, origCurrencyUomId

        Show
        Jacopo Cappellato added a comment - We should also include in the screens the following fields: glAccountTypeId, origAmount, origCurrencyUomId
        Hide
        Jacopo Cappellato added a comment -

        The following fields, in the single form, should be set with allow-empty="true":

        Group Status Id
        Fixed Asset Id
        Role Type Id

        and in the list form below it, also the following field can have an empty value:

        Gl Account Id

        Show
        Jacopo Cappellato added a comment - The following fields, in the single form, should be set with allow-empty="true": Group Status Id Fixed Asset Id Role Type Id and in the list form below it, also the following field can have an empty value: Gl Account Id
        Hide
        Jacopo Cappellato added a comment -

        Another thing I've noticed: in the single form the field "Inventory Item Id" is rendered using a drop down box; since we usually have a huge number of inventory items in the system, we have to change this: a simple input box is fine (or a lookup link).

        Show
        Jacopo Cappellato added a comment - Another thing I've noticed: in the single form the field "Inventory Item Id" is rendered using a drop down box; since we usually have a huge number of inventory items in the system, we have to change this: a simple input box is fine (or a lookup link).
        Hide
        Jacopo Cappellato added a comment -

        I think it is time to prevent the ability to modify an accounting transaction that is posted.
        Instead of adding a bunch of use-when in the form definition's fields, I'd suggest that we create two new forms and we add a <section><condition>... area to the screen definition to include the editable or not editable ones according to the isPosted flag.

        Show
        Jacopo Cappellato added a comment - I think it is time to prevent the ability to modify an accounting transaction that is posted. Instead of adding a bunch of use-when in the form definition's fields, I'd suggest that we create two new forms and we add a <section><condition>... area to the screen definition to include the editable or not editable ones according to the isPosted flag.
        Hide
        Anil K Patel added a comment -

        Vikas your patch is in svn # 599753. We got a good start. I am keeping this issue open because I think there is lot more that can be done to improve this screen.

        Show
        Anil K Patel added a comment - Vikas your patch is in svn # 599753. We got a good start. I am keeping this issue open because I think there is lot more that can be done to improve this screen.

          People

          • Assignee:
            Vikas Mayur
            Reporter:
            Jacopo Cappellato
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development