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

Look into suggestions for document instantiation improvements

    Details

    • Type: Task Task
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: Backlog
    • Component/s: Development
    • Labels:
    • Similar issues:
      KULRICE-5257Suggest Box - Improvements
      KULRICE-12551Improvements on handling of template options
      KULRICE-10531a6-sub not returning suggestion.
      KULRICE-2560Improve the look of the Action List Filter
      KULRICE-6735Document the Performance Improvements
      KULRICE-2380Remove instantiations of PojoPropertyUtilsBean
      KULRICE-6745Document the Data Dictionary Validation improvements
      KULRICE-6736Document the UI Framework code improvements
      KULRICE-5101Improve messaging for simultaneous workflow actions
      KULRICE-3550Rice portal improvements
    • Rice Module:
      KEW

      Description

      From an email thread on the subject....

      ________________________________________
      From: Warren Liang
      Sent: Friday, September 18, 2009 3:37 PM
      To: Neal, Jerry K
      Cc: Byrne, Ailish M; 'Jonathan Keller'; 'James Smith'
      Subject: Re: FW: Changes regarding document instantiation - please read if you are getting NPE's on documents

      Seems like there are no dissenting opinions, so I will go ahead with
      #2. I was initially hesitant to go with #2 (because it would require
      duplicate configuration), but I guess we can't take the risk of it not
      working.

      Thanks,

      Warren
      ________________________________________
      From: Byrne, Ailish M
      Sent: Tuesday, September 22, 2009 1:18 AM
      To: Warren Liang
      Cc: Neal, Jerry K; 'Jonathan Keller'; 'James Smith'
      Subject: RE: FW: Changes regarding document instantiation - please read if you are getting NPE's on documents

      thank you for the thorough work on this issue (as usual) warren

      i will share with eric to see if he wants any tweaks in future versions of rice.

      ________________________________________
      From: Warren Liang
      Sent: Monday, September 21, 2009 1:16 PM
      To: Byrne, Ailish M
      Cc: Neal, Jerry K; 'Jonathan Keller'; 'James Smith'
      Subject: Re: FW: Changes regarding document instantiation - please read if you are getting NPE's on documents

      If the method is not implemented, then the default implementation will
      return an empty string. Therefore, the framework will not attempt to
      instantiate the document, and it will require the developer to either 1)
      call setDocument(...) in the form constructor, or 2) rely on the doc
      handler to instantiate the document (and write their
      populate/execution/action code to not access the document before
      docHandler). In other words, it works the same way as it did before the
      work was done.

      Byrne, Ailish M wrote:
      > > Cool. Thanks for working this out guys. I noticed Warren reassigned the bug reports in question to himself, so I assume we're going with #2. one quick question about 2. if that method isn't implemented what happens? does the default impl return null - should the method be abstract on the parent (if not now, then at least at some point when we aren't afraid of making impacting api changes?)
      > >
      > > thanks again!
      > >
      > > ----Original Message----
      > > From: Neal, Jerry K
      > > Sent: Friday, September 18, 2009 10:36 AM
      > > To: 'Warren Liang'; Byrne, Ailish M
      > > Cc: 'Jonathan Keller'; 'James Smith'
      > > Subject: RE: FW: Changes regarding document instantiation - please read if you are getting NPE's on documents
      > >
      > > Thanks Warren.
      > >
      > > I personally like the second option. That is basically what we were doing
      > > before when you had to instantiate the document
      > > instance in the form. Now we just give the document type name and call
      > > service to get the instance. With the other two solutions
      > > again I would worry about things like the doc type not being passed in some
      > > situations like you mentioned. We need to go
      > > with the solution that assures us the most all functionality will continue
      > > to work as expected.
      > >
      > > Jerry
      > >
      > > ----Original Message----
      > > From: Warren Liang
      > > Sent: Thursday, September 17, 2009 4:39 PM
      > > To: Byrne, Ailish M
      > > Cc: Neal, Jerry K; 'Jonathan Keller'; 'James Smith'
      > > Subject: Re: FW: Changes regarding document instantiation - please read if
      > > you are getting NPE's on documents
      > >
      > > I agree those Jiras are probably related to this issue.
      > >
      > > The reason it wasn't done in the form constructor was because the struts
      > > RequestProcessor processing paradigm says that I'm not supposed to have
      > > access to the request parameters when it constructs the form instance
      > > (hence the noarg constructors). That being said, with some
      > > GlobalVariables hackery, I could of course make a way of constructing a
      > > document instance based on the docTypeName request parameter (assuming
      > > all of our requests do have a docTypeParam – need to check how links
      > > work from the doc search). However, the form reset() method (which is
      > > called before populate, execute, action) does have access to the
      > > request, and we could override it to instantiate the document based on
      > > the request params.
      > >
      > > What I tried to do was to rely on the docHandler to instantiate the
      > > document, but I guess too many people are accessing the document before
      > > it is created.
      > >
      > > What I can do is to do one of the following:
      > > 1) revert everything back to the way it was
      > > 2) modify each of the form classes to implement
      > > org.kuali.rice.kns.web.struts.form.KualiDocumentFormBase.getDefaultDocumentT
      > > ypeName(),
      > > which basically returns the doc type name associated with the form, and
      > > then instantiate the appropriate document class. Doing so will ensure
      > > that the documents are all instantiated by the time the
      > > execute/populate/action handler methods are called.
      > > 3) implement the GlobalVariables solution above
      > > 4) override
      > > org.kuali.rice.kns.web.struts.form.KualiForm.reset(ActionMapping,
      > > ServletRequest) to populate it in reset().
      > >
      > > Any preferences?
      > >
      > > Thanks,
      > >
      > > Warren
      > >
      > > Byrne, Ailish M wrote:
      > >
      >> >> Hey Warren,
      >> >>
      >> >> I was a little surprised when this came through. I hadn't realized we
      >> >>
      > > still had a Jira like this out there. Can you remind me what Jira this was
      > > part of?... Actually I think I just answered my own question: KFSMI-4217 -
      > > unfortunately because I asked you to focus on our parallel efforts, this has
      > > been out there since late July - my fault for losing track of impact and
      > > timing.
      > >
      >> >> I've been trying to figure out how it relates to:
      >> >>
      > > https://test.kuali.org/jira/browse/KFSMI-3898
      > >
      >> >> Jerry expressed some concerns to me about making this change at this stage
      >> >>
      > > of the game - afraid not everything would get retested. I share his
      > > concern. He made this suggestion...
      > >
      >> >> "Could we not in the form constructor do whatever Warren put in to doc
      >> >>
      > > handler to get a new document instance? That should fix the issue of not
      > > hard-coding a document class and give us reasonable assurance that things
      > > will continue to work as expected. It might be a better design to put it in
      > > doc handler but I just don't think we should be taking the risk of breaking
      > > something at this point and it going unnoticed."
      > >
      >> >> I think the following new Jiras (at least) may be related to this change:
      >> >>
      > > KFSMI-4906, KFSMI-4890, KFSMI-4889, KFSMI-4888
      > >
      >> >> I'm sorry for the delay. Been meaning to touch base with you since you
      >> >>
      > > sent this, but lost track.
      > >
      >> >> Please LMK what you think about Jerry's suggestion.
      >> >>
      >> >> Thanks,
      >> >> A
      >> >>
      >> >> ----Original Message----
      >> >> From: owner-krnet-l@LISTSERV.INDIANA.EDU
      >> >>
      > > owner-krnet-l@LISTSERV.INDIANA.EDU On Behalf Of Warren Liang
      > >
      >> >> Sent: Thursday, September 10, 2009 11:54 AM
      >> >> To: 'kftdiscuss-l@indiana.edu'
      >> >> Subject: Changes regarding document instantiation - please read if you are
      >> >>
      > > getting NPE's on documents
      > >
      >> >> Hi all,
      >> >>
      >> >> I made a change a few days ago that changes how document forms are
      >> >> constructed. In the past, we would always construct our forms by also
      >> >> instantiating an hard-coded document class. For example:
      >> >>
      >> >> public DisbursementVoucherForm()

      { >> >> super(); >> >> ..... >> >> setDocument(new DisbursementVoucherDocument()); <---- I have >> >> removed such lines >> >> }

      >> >>
      >> >> This was bad, because this made documents harder to customize. Now,
      >> >> session based documents (which most, if not all, documents in KFS are),
      >> >> will create their document instance in the docHandler() method. The
      >> >> docHandler uses the DD to determine the class, and is not hard coded.
      >> >>
      >> >> However, the problem is that some documents were relying on the old
      >> >> "create in the constructor" behavior and are accessing the documents
      >> >> before the docHandler is able to create it (typically in the form's
      >> >> populate method and the action's execute) method. We will need to
      >> >> rearrange the code in those methods, or do "if" tests to ensure the
      >> >> document is not null before referencing those documents. Please contact
      >> >> me if you are experiencing any problems.
      >> >>
      >> >> Thanks,
      >> >>
      >> >> Warren
      >> >>
      >> >>
      >> >>
      > >
      > >
      >

        Activity

        There are no comments yet on this issue.

          People

          • Assignee:
            Unassigned
            Reporter:
            Eric Westfall
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Structure Helper Panel