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

Logic to retrieve ad hoc recipient person incorrect causing issues adding people

    Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.4
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-11271Ad Hoc Recipients are shown/routed in the reverse order
      KULRICE-11753"Person not found" error adding Ad Hoc recipient without lookup
      KULRICE-12568Allows adding of Ad Hoc Recipient of person that doesn't exist
      KULRICE-7831Rice 2 docs - Ad Hoc Recipients tab collapses after selecting 'person' value from pop-up
      KULRICE-5495Rice Dev: Adding Ad Hoc recipient results in HTTP 500 error
      KULRICE-7746Rice 2 Error on adding an Ad Hoc Recipient most of the time
      KULRICE-12348Issue ad hoc routing a document if it has no nodes
      KULRICE-2809Remove workgroup reference on Ad Hoc Recipients tab
      KULRICE-6283Rice Dev: Ad Hoc routing bombs on add person in Ad Hoc Recipients section
      KULRICE-13996Problems sending ad hoc group requests on KRAD documents
    • Rice Module:
      KRAD
    • Sprint:
      2.4.0-rc1 Sprint 3
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      In Maintenance Screen, currently we can add the persons in Ad Hoc Recipients and submit, but it is working only for the preloaded person data from rice framework (like admin, dev2, eric, edna .. etc). We cant able to add the new person which are created from the person screen. To find out the issue i have debugged the code, i got the solution. I dont know whether this is exact solution..

      Take a look at line no: 75, in AdHocRoutePerson.java class person = KimApiServiceLocator.getPersonService().getPersonByPrincipalName(getId());

      Here we are getting the person from PrincipalName. But we are passing the person id.

      for example ( In rice users both the PrincipalName and PricipalId's are
      same) so we can add the person in Ad Hoc Recipients. But when we are creating the person from the person screen, the value of person id will start from 10000 and person name will based on our wish.

      so when we are adding the person in Ad Hoc Recipients tab, this line will get call, person =
      KimApiServiceLocator.getPersonService().getPersonByPrincipalName(getId()); // here id will be: 10000

      Here we are passing id and try to get the person using name. So person will be null. Here i have overridden the framework class locally and made the following changes and its working fine,

      /* Get the principal name based on id and then try to get the person based on principalName */ EntityNamePrincipalName entityNamePrincipalName= KimApiServiceLocator.getIdentityService().getDefaultNamesForPrincipalId(id);
      if(entityNamePrincipalName!=null)

      { principalName = entityNamePrincipalName.getPrincipalName(); }

      person =
      KimApiServiceLocator.getPersonService().getPersonByPrincipalName(principalName)

      In KRAD Transaction Screen we are not even able to add the newly created person to the Adhoc Recipients. I have modified the few class to fix the issue,

      Not only in the AdHocRoutePerson.java class, Few more classes are having the same code. These are the class names, 1. DocumentRuleBase.java 2. WorkflowDocumentServiceImpl.java 3. AdHocRoutePerson.java 4. PessimisticLockServiceImpl.java

        Issue Links

          Activity

          Hide
          Megan Nolan (Inactive) added a comment -

          Comment from Jeff Domeyer

          It's an old problem that all of the downstream projects have been fixing/dealing with in various ways.
          KULRICE-6686
          OLE-3029
          KULRICE-10572
          KULRICE-5495
          ...

          What happened is that at some point in time in the distant past someone crossed the id field with the principalName field. From then on, various projects started fixing it by mapping things in different ways. The confusion got exacerbated with the dwr calls that auto-populated the principals vs the lookups returning the correct ids. I remember at one point in time that various testing resulted in an id being passed to the same method as a principalName and the code was actually trying both service calls to the personService (by name and by id) to try to make sense of it!

          I've been straightening out rice 2.1.7, and the issues I encountered so far were exposed by the following tests:

          For both KRAD and KNS screens
          Typing in a principal name, hit add and save the document
          Lookup a principal name, hit add and save the document
          Typing in an incorrect principal name and hit add
          Lookup a principal, change the name to an incorrect name and hit add
          Typing in a principal name and hit add, changing the name to an incorrect name (after add) and hit delete
          Typing in a principal name and hit add, changing the name to an incorrect name (after add) and save the document

          The adhoc groups are just as susceptible to the problems encountered through those tests.

          The stuff I notice that isn't working are the delete actions (the rule event isn't fired for validating the information, sometimes dependent code then produces stack traces even though the screens still work).
          Sometimes you can get the lines to add the wrong people or groups depending on if you've typed over certain information (more applicable to groups since there's 2 fields that work in concert with each other).

          I can't say for certain which stuff has been fixed in the later versions of rice, but if the testing scenarios are made into junits or at least touched by hand when fixing this, then that will go a long way to solving people's problems.

          If you'd like me to attach the workarounds I've cobbled together to a Jira, just let me know which one.

          On a side note...
          Some stuff that bugs me is how the name field in adhoc groups is ignored and a new recipientName field is added. Also how the adhocroute person is a wrapper for a single person object whereas the group isn't, and how the personservice has a get implementationclass method for a person and the groupservice doesn't. Maybe there's reasons for these, but the dissimilarities between 2 closely related objects seems like something to review.

          Show
          Megan Nolan (Inactive) added a comment - Comment from Jeff Domeyer It's an old problem that all of the downstream projects have been fixing/dealing with in various ways. KULRICE-6686 OLE-3029 KULRICE-10572 KULRICE-5495 ... What happened is that at some point in time in the distant past someone crossed the id field with the principalName field. From then on, various projects started fixing it by mapping things in different ways. The confusion got exacerbated with the dwr calls that auto-populated the principals vs the lookups returning the correct ids. I remember at one point in time that various testing resulted in an id being passed to the same method as a principalName and the code was actually trying both service calls to the personService (by name and by id) to try to make sense of it! I've been straightening out rice 2.1.7, and the issues I encountered so far were exposed by the following tests: For both KRAD and KNS screens Typing in a principal name, hit add and save the document Lookup a principal name, hit add and save the document Typing in an incorrect principal name and hit add Lookup a principal, change the name to an incorrect name and hit add Typing in a principal name and hit add, changing the name to an incorrect name (after add) and hit delete Typing in a principal name and hit add, changing the name to an incorrect name (after add) and save the document The adhoc groups are just as susceptible to the problems encountered through those tests. The stuff I notice that isn't working are the delete actions (the rule event isn't fired for validating the information, sometimes dependent code then produces stack traces even though the screens still work). Sometimes you can get the lines to add the wrong people or groups depending on if you've typed over certain information (more applicable to groups since there's 2 fields that work in concert with each other). I can't say for certain which stuff has been fixed in the later versions of rice, but if the testing scenarios are made into junits or at least touched by hand when fixing this, then that will go a long way to solving people's problems. If you'd like me to attach the workarounds I've cobbled together to a Jira, just let me know which one. On a side note... Some stuff that bugs me is how the name field in adhoc groups is ignored and a new recipientName field is added. Also how the adhocroute person is a wrapper for a single person object whereas the group isn't, and how the personservice has a get implementationclass method for a person and the groupservice doesn't. Maybe there's reasons for these, but the dissimilarities between 2 closely related objects seems like something to review.
          Hide
          Kristina Taylor (Inactive) added a comment - - edited

          We are limited in how we fix the KNS at this point because of dependent applications testing the screens, so we really can't fix the bad mapping or the other strange KNS bugs. A later improvement might be to do a bulk database update to change principalName to principalId and also add a synthetic key to the table when we are only working on KRAD. I will attempt to address the KRAD errors though to see if I can make those screens behave without having to do too many changes to the data structure. I will also be reviving dead tests so we can detect problems with this screen in the future.

          Show
          Kristina Taylor (Inactive) added a comment - - edited We are limited in how we fix the KNS at this point because of dependent applications testing the screens, so we really can't fix the bad mapping or the other strange KNS bugs. A later improvement might be to do a bulk database update to change principalName to principalId and also add a synthetic key to the table when we are only working on KRAD. I will attempt to address the KRAD errors though to see if I can make those screens behave without having to do too many changes to the data structure. I will also be reviving dead tests so we can detect problems with this screen in the future.
          Hide
          Kristina Taylor (Inactive) added a comment -

          I was able to make this work better by correcting the Uif setup. We cannot change the code behind this because of KNS legacy calls. The screen may still have some weirdness to it, but we can't fix that until we fix the data structure. At the very least, now you can add a person where the principalId is different from the principalName (try using the system user kr).

          Show
          Kristina Taylor (Inactive) added a comment - I was able to make this work better by correcting the Uif setup. We cannot change the code behind this because of KNS legacy calls. The screen may still have some weirdness to it, but we can't fix that until we fix the data structure. At the very least, now you can add a person where the principalId is different from the principalName (try using the system user kr).

            People

            • Assignee:
              Kristina Taylor (Inactive)
              Reporter:
              Sheik Salahudeen
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 4 hours
                4h
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 4 hours
                4h

                  Agile

                    Structure Helper Panel