Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-12157

RouteContext state corrupted with blanket approval

    Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.6
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
    • Similar issues:
      KULRICE-7568Figure out what to do with Blanket Approve Workgroup, Blanket Approve Group
      KULRICE-960Blanket Approve Valid actions is possibly incorrect
      KULRICE-2616Document Search by "Approver Network ID" does not include Blanket Approvers
      KULRICE-8481BlanketApproveTest.testBlanketApproveToMultipleNodes fails in CI with Should be at blanket approved node WD4B3.
      KULRICE-4083ALLOW_ENROUTE_BLANKET_APPROVE_WITHOUT_APPROVAL_REQUEST_IND doesn't appear to work the way it's being described.
      KULRICE-7172Should adhoc requests be persisted upon a blanket approve
      KULRICE-14271Blanket approve displays "Confirm Navigation" dialog
      KULRICE-6727Sub Fund Reviewer is not displaying Blanket Approved in Route log annotation
      KULRICE-8762CONTRIB: "Approve & Blanket Approve" button should be disabled for a person who is doing COMPLETE action and "CANCEL" button should be enabled
      KULRICE-6905Blanket Approve of document asks the user if they should Stay on Page or Leave page
    • Rice Module:
      KEW
    • Sprint:
      Core 2.5.0-m7 Sprint 1, Rice Sprint 2015-03-04
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes
    • Story Points:
      2

      Description

      We have found an issue with the blanket approve functionality at IU when one document issues a super user approval for a document from within the post processor of another document. It appears to be mishandling the state of the RouteContext since the BlanketApproveEngine is clearing the route context twice which causes the original document's post processor to forget what it is doing. While investigating this the implementation of the RouteContext class looks like it could use some work, or more preferably the workflow engine would be refactored so it doesn't need to store this state in a ThreadLocal and then we could do away with the RouteContext class entirely. The second option I mentioned above would probably cause some odd issues since so much seems to be dependent on the RouteContext state but it would be better overall if Rice did not use it.

      1. KULRICE-12157.1.patch
        7 kB
        Martin Taylor
      2. KULRICE-12157.patch
        1 kB
        Martin Taylor

        Activity

        Hide
        Martin Taylor (Inactive) added a comment -

        Includes new and release route context in BlanketApproveEngine to isolate effects of parent/child documents

        Show
        Martin Taylor (Inactive) added a comment - Includes new and release route context in BlanketApproveEngine to isolate effects of parent/child documents
        Hide
        Martin Taylor (Inactive) added a comment -

        I've attached a small patch which creates and releases a separate route context in the BlanketApproveEngine. This should prevent the child document's blanket approve from affecting the parent blanket approve. I've run it through the kew integration tests but I'm still working to setup a chained document with post processor for testing.

        Show
        Martin Taylor (Inactive) added a comment - I've attached a small patch which creates and releases a separate route context in the BlanketApproveEngine. This should prevent the child document's blanket approve from affecting the parent blanket approve. I've run it through the kew integration tests but I'm still working to setup a chained document with post processor for testing.
        Hide
        Martin Taylor (Inactive) added a comment -

        Spoke with James, finding edge cases like KULRICE-13209 the better solution looks to be isolating the RouteContext to the engine.process. This creation/removal should help with corrupted data. Will revert RouteContext changes and Ignore current test and hold a patch for 2.6 with current changes plus additional changes to create/release route contexts in StandardWorkflowEngine and RoutingReportAction.

        Show
        Martin Taylor (Inactive) added a comment - Spoke with James, finding edge cases like KULRICE-13209 the better solution looks to be isolating the RouteContext to the engine.process. This creation/removal should help with corrupted data. Will revert RouteContext changes and Ignore current test and hold a patch for 2.6 with current changes plus additional changes to create/release route contexts in StandardWorkflowEngine and RoutingReportAction.
        Hide
        Martin Taylor (Inactive) added a comment -

        Updated ticket with patch that includes change to OrchestrationConfig to hold requests nodes supress policies to avoid issue with KULRICE-13209.

        Show
        Martin Taylor (Inactive) added a comment - Updated ticket with patch that includes change to OrchestrationConfig to hold requests nodes supress policies to avoid issue with KULRICE-13209 .
        Hide
        Eric Westfall added a comment -

        While I agree this could probably use a refactoring, messing too much with the internals of the workflow engine is probably not worth the risk at this point, so let's scope this one to just the fix in the patches.

        For this one, work with James on getting a pull request submitted.

        Show
        Eric Westfall added a comment - While I agree this could probably use a refactoring, messing too much with the internals of the workflow engine is probably not worth the risk at this point, so let's scope this one to just the fix in the patches. For this one, work with James on getting a pull request submitted.
        Hide
        James Bennett added a comment -

        Just FYI there is no IU fix for this one. We worked around it locally and filed this to be fixed on the foundation side.

        Show
        James Bennett added a comment - Just FYI there is no IU fix for this one. We worked around it locally and filed this to be fixed on the foundation side.
        Hide
        Shannon Hess added a comment -

        Pull request for this issue - https://github.com/kuali/rice/pull/83

        I created a test to reproduce the problem and then verified that it was fixed after the modified patch was applied.

        Show
        Shannon Hess added a comment - Pull request for this issue - https://github.com/kuali/rice/pull/83 I created a test to reproduce the problem and then verified that it was fixed after the modified patch was applied.
        Hide
        Shannon Hess added a comment -

        pull request for master - https://github.com/kuali/rice/pull/86

        Show
        Shannon Hess added a comment - pull request for master - https://github.com/kuali/rice/pull/86
        Hide
        Claus Niesen added a comment -

        Commit was rolled back in the 2.5 branch.

        Show
        Claus Niesen added a comment - Commit was rolled back in the 2.5 branch.

          People

          • Assignee:
            Shannon Hess
            Reporter:
            James Bennett
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - Not Specified
              Not Specified
              Remaining:
              Remaining Estimate - 0 minutes
              0m
              Logged:
              Time Spent - 1 day
              1d

                Agile

                  Structure Helper Panel