[CORRECTION NEEDED] KULRICE-14124: Fixed an issue where returning a document to a previous n...

rice-middleware/impl/src/main/java/org/kuali/rice/kew/actionrequest/ActionRequestValue.java

@@ -23,6 +23,7 @@
 import org.kuali.rice.core.api.util.RiceConstants;
 import org.kuali.rice.kew.actionitem.ActionItem;
 import org.kuali.rice.kew.actiontaken.ActionTakenValue;
+import org.kuali.rice.kew.actiontaken.dao.impl.ActionTakenDaoJpa;
 import org.kuali.rice.kew.api.KewApiConstants;
 import org.kuali.rice.kew.api.action.ActionRequest;
 import org.kuali.rice.kew.api.action.ActionRequestPolicy;
@@ -82,7 +83,8 @@
   @NamedQuery(name="ActionRequestValue.FindPendingByResponsibilityIds", query = "SELECT DISTINCT(arv.documentId) FROM ActionRequestValue arv WHERE (arv.status = '"
           + KewApiConstants.ActionRequestStatusVals.INITIALIZED + "' OR arv.status = '" +
                   KewApiConstants.ActionRequestStatusVals.ACTIVATED
-          + "') AND arv.responsibilityId IN :respIds")
+          + "') AND arv.responsibilityId IN :respIds"),
+  @NamedQuery(name = ActionTakenDaoJpa.FIND_ACTIONS_TAKEN_AT_NODE_INSTANCE_NAME, query = ActionTakenDaoJpa.FIND_ACTIONS_TAKEN_AT_NODE_INSTANCE_QUERY)
 })
 public class ActionRequestValue implements Serializable {
 

rice-middleware/impl/src/main/java/org/kuali/rice/kew/actions/ReturnToPreviousNodeAction.java

@@ -382,10 +382,22 @@ private void assertValidProcess(RouteNodeInstance resultNodeInstance, Collection
     private void assertFinalApprovalNodeNotInPath(List path) throws InvalidActionTakenException {
     	for (Iterator iterator = path.iterator(); iterator.hasNext(); ) {
 			RouteNodeInstance  nodeInstance = (RouteNodeInstance ) iterator.next();
-			// if we have a complete final approval node in our path, we cannot return past it
-			if (nodeInstance.isComplete() && Boolean.TRUE.equals(nodeInstance.getRouteNode().getFinalApprovalInd())) {
-				throw new InvalidActionTakenException("Cannot return past or through the final approval node '"+nodeInstance.getName()+"'.");
-			}
+            List<ActionTakenValue> actionsTaken = KEWServiceLocator.getActionTakenService().getActionsTakenAtRouteNode(nodeInstance);
+            ActionTakenValue foundValue =  null;
+            String actionTakenCode = null;
+            if(!actionsTaken.isEmpty()) {
+                foundValue = actionsTaken.get(0);
+            }
+            if(foundValue != null){
+                actionTakenCode = foundValue.getActionTaken();
+            }
+            if(actionTakenCode != null) {
+                // if we have a complete final approval node in our path and if the action taken at that node is not return to previous we cannot return past it
+                if (nodeInstance.isComplete() && Boolean.TRUE.equals(nodeInstance.getRouteNode().getFinalApprovalInd()) &&
+                        actionsTaken.isEmpty() && !actionTakenCode.equals(KewApiConstants.ACTION_TAKEN_RETURNED_TO_PREVIOUS_CD)) {
+                    throw new InvalidActionTakenException("Cannot return past or through the final approval node '" + nodeInstance.getName() + "'.");
+                }
+            }
 		}
     }
 

rice-middleware/impl/src/main/java/org/kuali/rice/kew/actiontaken/dao/ActionTakenDao.java

@@ -17,9 +17,10 @@
 
 import org.kuali.rice.kew.actiontaken.ActionTakenValue;
 import org.kuali.rice.kew.api.action.ActionType;
+import org.kuali.rice.kew.engine.node.RouteNodeInstance;
 
 import java.sql.Timestamp;
-
+import java.util.List;
 
 /**
  * Data Access Object for {@link ActionTakenValue}s.
@@ -44,4 +45,11 @@ public interface ActionTakenDao {
      */
     Timestamp getLastActionTakenDate(String documentId, ActionType actionType);
 
+    /**
+     * Returns the actions taken at a route node instance
+     * @param nodeInstance the route node instance
+     * @return the list of actions taken at a route node instance
+     */
+    List<ActionTakenValue> findActionsTakenAtRouteNodeInstance(RouteNodeInstance nodeInstance);
+
 }

rice-middleware/impl/src/main/java/org/kuali/rice/kew/actiontaken/dao/impl/ActionTakenDaoJpa.java

@@ -16,14 +16,16 @@
 package org.kuali.rice.kew.actiontaken.dao.impl;
 
 import org.apache.commons.lang.StringUtils;
+import org.kuali.rice.kew.actiontaken.ActionTakenValue;
 import org.kuali.rice.kew.actiontaken.dao.ActionTakenDao;
 import org.kuali.rice.kew.api.action.ActionType;
+import org.kuali.rice.kew.engine.node.RouteNodeInstance;
 
 import javax.persistence.EntityManager;
 import javax.persistence.NoResultException;
 import javax.persistence.TypedQuery;
 import java.sql.Timestamp;
-
+import java.util.List;
 
 /**
  * JPA implementation of the {@link org.kuali.rice.kew.actiontaken.dao.ActionTakenDao}.
@@ -39,6 +41,9 @@ public class ActionTakenDaoJpa implements ActionTakenDao {
     public static final String GET_LAST_ACTION_TAKEN_DATE_NAME = "ActionTakenValue.getLastActionTakenDate";
     public static final String GET_LAST_ACTION_TAKEN_DATE_QUERY =
             "SELECT max(a.actionDate) from ActionTakenValue a where a.documentId = :documentId and a.actionTaken=:actionTaken";
+    public static final String FIND_ACTIONS_TAKEN_AT_NODE_INSTANCE_NAME = "ActionRequestValue.findActionsTakenAtRouteNodeInstance";
+    public static final String FIND_ACTIONS_TAKEN_AT_NODE_INSTANCE_QUERY =
+            "Select r.actionTaken from ActionRequestValue r where r.nodeInstance = :nodeInstance";
 
     public Timestamp getLastActionTakenDate(String documentId, ActionType actionType) {
         if (StringUtils.isBlank(documentId) || actionType == null) {
@@ -55,6 +60,14 @@ public Timestamp getLastActionTakenDate(String documentId, ActionType actionType
         }
     }
 
+    public List<ActionTakenValue> findActionsTakenAtRouteNodeInstance(RouteNodeInstance nodeInstance) {
+        TypedQuery<ActionTakenValue> query =
+                entityManager.createNamedQuery(FIND_ACTIONS_TAKEN_AT_NODE_INSTANCE_NAME, ActionTakenValue.class);
+        query.setParameter("nodeInstance", nodeInstance);
+
+        return query.getResultList();
+    }
+
     public EntityManager getEntityManager() {
         return this.entityManager;
     }

rice-middleware/impl/src/main/java/org/kuali/rice/kew/actiontaken/service/ActionTakenService.java

@@ -17,6 +17,7 @@
 
 import org.kuali.rice.kew.actionrequest.ActionRequestValue;
 import org.kuali.rice.kew.actiontaken.ActionTakenValue;
+import org.kuali.rice.kew.engine.node.RouteNodeInstance;
 
 import java.sql.Timestamp;
 import java.util.Collection;
@@ -50,4 +51,11 @@ public interface ActionTakenService {
 
     Timestamp getLastApprovedDate(String documentId);
 
+    /**
+     * Returns actions taken at a route node instance
+     * @param nodeInstance the route node instance
+     * @return actions taken at a route node instance
+     */
+    List<ActionTakenValue> getActionsTakenAtRouteNode(RouteNodeInstance nodeInstance);
+
 }

rice-middleware/impl/src/main/java/org/kuali/rice/kew/actiontaken/service/impl/ActionTakenServiceImpl.java

@@ -24,6 +24,7 @@
 import org.kuali.rice.kew.actiontaken.dao.ActionTakenDao;
 import org.kuali.rice.kew.actiontaken.service.ActionTakenService;
 import org.kuali.rice.kew.api.action.ActionType;
+import org.kuali.rice.kew.engine.node.RouteNodeInstance;
 import org.kuali.rice.kim.api.group.GroupService;
 import org.kuali.rice.kim.api.services.KimApiServiceLocator;
 import org.kuali.rice.krad.data.DataObjectService;
@@ -170,6 +171,11 @@ private void checkNull(Object value, String valueName) throws RuntimeException {
         }
     }
 
+    @Override
+    public List<ActionTakenValue> getActionsTakenAtRouteNode(RouteNodeInstance nodeInstance) {
+
+        return getActionTakenDao().findActionsTakenAtRouteNodeInstance(nodeInstance);
+    }
 
     public ActionTakenDao getActionTakenDao() {
         return actionTakenDao;

rice-middleware/it/kew/src/test/java/org/kuali/rice/kew/actions/ReturnToPreviousNodeActionTest.java

@@ -355,6 +355,68 @@ private void assertAllBranchesSameParent(String documentId, String parentBranchN
     	document.blanketApprove("");
     	assertTrue("Document should be processed.", document.isProcessed());
     }
+
+    /**
+     * This test was implemented to address issue KULRICE-14124
+     * We want to ensure that a document can be returned to a previous node from a final approval node as long as approval/disapproval action has not be taken at the final approval node
+     * @throws Exception
+     */
+    @Test public void testFinalApprovalNodeReturnToPrevious() throws Exception {
+        //create a document
+        WorkflowDocument document = WorkflowDocumentFactory.createDocument(getPrincipalIdByName("ewestfal"), "BlanketApproveMandatoryNodeTest");
+        //route if for approval
+        document.route("");
+        assertTrue("Document should be enroute",document.isEnroute());
+        TestUtilities.assertAtNodeNew("Should be at the WorkflowDocument node.", document, "WorkflowDocument");
+
+        //approve the document as bmcgough and rkirkend
+        document = WorkflowDocumentFactory.loadDocument(getPrincipalIdByName("bmcgough"), document.getDocumentId());
+        assertTrue("Bmcgough should have an approve.", document.isApprovalRequested());
+        document.approve("");
+        document = WorkflowDocumentFactory.loadDocument(getPrincipalIdByName("rkirkend"), document.getDocumentId());
+        assertTrue("Rkirkend should have an approve.", document.isApprovalRequested());
+        document.approve("");
+
+        //verify the document is routed to final approval node i.e. WorkflowDocument2
+        TestUtilities.assertAtNodeNew("Document should be at WorkflowDocument2 node.", document,"WorkflowDocument2");
+        document = WorkflowDocumentFactory.loadDocument(getPrincipalIdByName("pmckown"), document.getDocumentId());
+        assertTrue("pmckown should have approve and return to previous actions", document.isApprovalRequested());
+
+        //return the document to its previous node, i.e. WorkflowDocument
+        document.returnToPreviousNode("", "WorkflowDocument");
+        assertTrue("Document should be enroute.", document.isEnroute());
+
+        //Verify that the document is now at WorkflowDocument node
+        TestUtilities.assertAtNodeNew("Should be at the old WorkflowDocument node.", document, "WorkflowDocument");
+        //approve the document by bmcgough and rkirkend
+        document = WorkflowDocumentFactory.loadDocument(getPrincipalIdByName("bmcgough"), document.getDocumentId());
+        assertTrue("Bmcgough should have an approve.", document.isApprovalRequested());
+        document.approve("");
+        document = WorkflowDocumentFactory.loadDocument(getPrincipalIdByName("rkirkend"), document.getDocumentId());
+        assertTrue("Rkirkend should have an approve.", document.isApprovalRequested());
+        document.approve("");
+
+        //Verify that the document is has now routed to WorkflowDocument2 node again
+        TestUtilities.assertAtNodeNew("Document should be at WorkflowDocument2 node.", document,"WorkflowDocument2");
+        document = WorkflowDocumentFactory.loadDocument(getPrincipalIdByName("pmckown"), document.getDocumentId());
+        assertTrue("pmckown should have approve and return to previous actions", document.isApprovalRequested());
+
+        //Return the document to the initiated(AdHoc) node. Before the bug fix this would fail with the error: Cannot return past or through the 'final' approval node
+        document.returnToPreviousNode("","AdHoc");
+        //Verify that return to previous node worked and the document is now at AdHoc node
+        TestUtilities.assertAtNodeNew("Document should be at the AdHoc node", document, "AdHoc");
+        document =  WorkflowDocumentFactory.loadDocument(getPrincipalIdByName("ewestfal"), document.getDocumentId());
+        //Blanket approve the document to WorkflowDocument2 node
+        document.blanketApprove("", "WorkflowDocument2");
+
+        // Approve the document at WorkflowDocument2 node to ensure the multiple 'return to previous' actions has not effected the routing of the document
+        document = WorkflowDocumentFactory.loadDocument(getPrincipalIdByName("pmckown"), document.getDocumentId());
+        assertTrue("pmckown should have approve.", document.isApprovalRequested());
+        document.approve("");
+        // The document should be processed
+        assertTrue("Document should be processed.", document.isProcessed());
+
+    }
 
     protected void loadTestData() throws Exception {
         loadXmlFile("ActionsConfig.xml");