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

ordermgr/control/deleteCustRequestParty should delete the record rather than updating the through date

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Implemented
    • Affects Version/s: Trunk
    • Fix Version/s: 16.11.01
    • Component/s: order
    • Labels:
      None

      Description

      Users expect to be able to remove accidentally added faulty CustRequestParty but the current implementation only updates the through date when the delete button is clicked.

      1. OFBIZ-6463.patch
        5 kB
        Christian Carlow

        Issue Links

          Activity

          Hide
          ofbizzer Christian Carlow added a comment -

          This patch replaces order/request/CustRequestServices.xml#deleteCustRequestParty <store-value> with <remove-value> so CustRequestParty will be removed rather than just the through date updated.

          Show
          ofbizzer Christian Carlow added a comment - This patch replaces order/request/CustRequestServices.xml#deleteCustRequestParty <store-value> with <remove-value> so CustRequestParty will be removed rather than just the through date updated.
          Hide
          adrianc@hlmksw.com Adrian Crum added a comment - - edited

          The attached patch breaks the design pattern that exists throughout the project. Intersection entities that include a date range are expired, not deleted.

          Instead of replacing expiration with deletion, maybe deletion could be ADDED to expiration.

          Show
          adrianc@hlmksw.com Adrian Crum added a comment - - edited The attached patch breaks the design pattern that exists throughout the project. Intersection entities that include a date range are expired, not deleted. Instead of replacing expiration with deletion, maybe deletion could be ADDED to expiration.
          Hide
          ofbizzer Christian Carlow added a comment -

          Thanks for the review Adrian.

          Expiration is still supported with the update button but I suppose that does require more clicks. Would an expire button to replace current delete button and having the delete actually delete the record be acceptable? This seems like it would be a little easier for users to understand. At the very least it seems the delete button text should be changed to "Expire".

          Show
          ofbizzer Christian Carlow added a comment - Thanks for the review Adrian. Expiration is still supported with the update button but I suppose that does require more clicks. Would an expire button to replace current delete button and having the delete actually delete the record be acceptable? This seems like it would be a little easier for users to understand. At the very least it seems the delete button text should be changed to "Expire".
          Hide
          adrianc@hlmksw.com Adrian Crum added a comment -

          That would make more sense.

          Show
          adrianc@hlmksw.com Adrian Crum added a comment - That would make more sense.
          Hide
          ofbizzer Christian Carlow added a comment -

          The patch keeps <remove-value> of deleteCustRequestPartyId provided in the previous patch and replaces old deleteCustRequestPartyId with a new expireCustRequestPartyId service that is executed with the new Expire button.

          Show
          ofbizzer Christian Carlow added a comment - The patch keeps <remove-value> of deleteCustRequestPartyId provided in the previous patch and replaces old deleteCustRequestPartyId with a new expireCustRequestPartyId service that is executed with the new Expire button.
          Hide
          paul_foxworthy Paul Foxworthy added a comment -

          I agree with Adrian. Please don't remove "expire" behaviour. It's standard practice in OFBiz.

          Many of the "delete" services in fact expire an entity. Where both are really needed, there are two services, e.g.:

          deletePaymentGroupMember
          expirePaymentGroupMember

          I've linked a similar issue OFBIZ-5205

          Show
          paul_foxworthy Paul Foxworthy added a comment - I agree with Adrian. Please don't remove "expire" behaviour. It's standard practice in OFBiz. Many of the "delete" services in fact expire an entity. Where both are really needed, there are two services, e.g.: deletePaymentGroupMember expirePaymentGroupMember I've linked a similar issue OFBIZ-5205
          Hide
          ofbizzer Christian Carlow added a comment -

          Thanks for the feedback Paul, the new patch follows yours and Adrian's suggested design. Quote roles will be handled similarly for OFBIZ-6462.

          Show
          ofbizzer Christian Carlow added a comment - Thanks for the feedback Paul, the new patch follows yours and Adrian's suggested design. Quote roles will be handled similarly for OFBIZ-6462 .
          Hide
          ofbizzer Christian Carlow added a comment -

          Fixed in:
          trunk r1683953

          Show
          ofbizzer Christian Carlow added a comment - Fixed in: trunk r1683953
          Hide
          pfm.smits Pierre Smits added a comment -

          Setting the through date on party expiration is default behaviour and part of GRC protocols. Having functions available for common order management officers to delete such records (even if added accidentally)undermines such protocols. Deletion of accidental records can always be done through functions in Webtools/entitymanagement.

          Show
          pfm.smits Pierre Smits added a comment - Setting the through date on party expiration is default behaviour and part of GRC protocols. Having functions available for common order management officers to delete such records (even if added accidentally)undermines such protocols. Deletion of accidental records can always be done through functions in Webtools/entitymanagement.
          Hide
          ofbizzer Christian Carlow added a comment -

          Hey Pierre,

          So the delete button and service should be removed? Should that be created as a separate issue?

          Show
          ofbizzer Christian Carlow added a comment - Hey Pierre, So the delete button and service should be removed? Should that be created as a separate issue?
          Hide
          ofbizzer Christian Carlow added a comment -

          Also Pierre,

          QuoteRoles allows for delete and hasn't been replaced by an entity such as QuoteParty to allow for expiry so it also breaks the GRC protocols you mentioned.

          Show
          ofbizzer Christian Carlow added a comment - Also Pierre, QuoteRoles allows for delete and hasn't been replaced by an entity such as QuoteParty to allow for expiry so it also breaks the GRC protocols you mentioned.
          Hide
          pfm.smits Pierre Smits added a comment -

          This can indeed be done in a separate and new issue.

          Show
          pfm.smits Pierre Smits added a comment - This can indeed be done in a separate and new issue.
          Hide
          pfm.smits Pierre Smits added a comment -

          I have created a placeholder issue regarding the improvement of the various roles and their associated functions in relation to their lifespan aspects. See OFBIZ-5959

          Show
          pfm.smits Pierre Smits added a comment - I have created a placeholder issue regarding the improvement of the various roles and their associated functions in relation to their lifespan aspects. See OFBIZ-5959
          Hide
          ofbizzer Christian Carlow added a comment -

          Awesome. Thanks Pierre I'll follow OFBIZ-5959 for future role development.

          Show
          ofbizzer Christian Carlow added a comment - Awesome. Thanks Pierre I'll follow OFBIZ-5959 for future role development.
          Hide
          jacques.le.roux Jacques Le Roux added a comment -

          A well organised organisation should also always have at least a daily backup of at least essential data (the all DB for OFBiz). Without (at least) a daily backup you are sure to fail one day...

          Show
          jacques.le.roux Jacques Le Roux added a comment - A well organised organisation should also always have at least a daily backup of at least essential data (the all DB for OFBiz). Without (at least) a daily backup you are sure to fail one day...
          Hide
          pfm.smits Pierre Smits added a comment -

          A well organised organisation has such in its GRC.

          Show
          pfm.smits Pierre Smits added a comment - A well organised organisation has such in its GRC.

            People

            • Assignee:
              Unassigned
              Reporter:
              ofbizzer Christian Carlow
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development