Details

    • Type: Bug Fix Bug Fix
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.5
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-9719Add "in development" note to KRAD conversion script
      KULRICE-9589Add documentation notes for KRAD Phase 3 items to Release Notes
      KULRICE-1425can't add notes to documents - error message about attachment, although there is no attachment
      KULRICE-10361Adding document notes broken
      KULRICE-9210Add note in IG on how to configure Rice without KRAD (KNS only)
      KULRICE-1881can't associate attachments with notes
      KULRICE-4870Refactor future krad-note in to krad-app-framework
      KULRICE-12809In KRAD, the Note Text after adding a note is shown in a light grey box
      KULRICE-11785Unable to attach note to Lab - Maintenance - BO Attachment Sample
      KULRICE-7735Exception when trying to add a note
    • Rice Module:
      KRAD
    • Application Requirement:
      Rice
    • Sprint:
      Core 2.5.0-m3 Sprint 2, Core 2.5.0-m4 Sprint 1
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      When routing to someone that acknowledges he/she can't add notes and attachments in KRAD. This is allowed in KNS and should be allowed in KRAD.

        Issue Links

          Activity

          Hide
          Claus Niesen added a comment -

          Relevant code:

          ./rice-framework/krad-web-framework/target/classes/org/kuali/rice/krad/uif/UifDocumentDefinitions.xml:              value="Notes @{#DocumentEntry.allowsNoteAttachments ? 'and Attachments' : ''} (@{document.notes.size()})"/>
          ./rice-framework/krad-web-framework/target/classes/org/kuali/rice/krad/uif/UifDocumentDefinitions.xml:              value="@{!document.notes.empty or #component.defaultOpen}"/>
          
          Show
          Claus Niesen added a comment - Relevant code: ./rice-framework/krad-web-framework/target/classes/org/kuali/rice/krad/uif/UifDocumentDefinitions.xml: value= "Notes @{#DocumentEntry.allowsNoteAttachments ? 'and Attachments' : ''} (@{document.notes.size()})" /> ./rice-framework/krad-web-framework/target/classes/org/kuali/rice/krad/uif/UifDocumentDefinitions.xml: value= "@{!document.notes.empty or #component.defaultOpen}" />
          Hide
          Shannon Hess added a comment -

          Notes:

          – allowsNoteAttachments indicates if the document view allows note attachments (So nothing to do with Notes only - it is just to say if Notes can have attachments or not)
          – The method canAnnotate(Document document, Person user) should be used to indicate if notes can be added to a document by a particular user (This is MY understanding)

          The reason Acknowleders cannot add notes is due to how addLineActions work. In GroupBase.afterEvaluateExpression() the following code is setting the notes collection to readOnly:

          if (getReadOnly() == null) {
             Component parent = ViewLifecycle.getPhase().getParent();
             setReadOnly(parent == null ? null : parent.getReadOnly());
          }
          

          The parent is the page, which is read only for an acknowledgment. Then in CollectionGroupBuilder.build(), the add line is not built since the collectionGroup was set to readonly.

          if (collectionGroup.isRenderAddLine() && !Boolean.TRUE.equals(collectionGroup.getReadOnly()) &&
                 !collectionGroup.isRenderAddBlankLineButton()) {
                      buildAddLine(view, model, collectionGroup);
          }
          

          I corrected the issue by fixing the canAnnotate methods – It now returns true if the person canAddNoteAttachment. It used to return true if canEdit was also true. I think it's important to note that canAnnotate is not used anywhere in the code so I don't think it was ever correct (Assuming canAnnotate means what I think it means). The old Notes.tag called a function called kfunc:canAddNoteAttachment which did what I am now doing in canAnnotate.

          I added the following line in Uif-DocumentNotesSection to indicate that the notes are read only if the user CANNOT add notes. If the note collection is not read only, the add note line will be built.

              <property name="readOnly" value="@{!#actionFlags[#Constants.KUALI_ACTION_CAN_ANNOTATE]}"/>
          

          Before committing this code I would like someone to verify that my analysis of canAnnotate is correct. It would also be good to know if there is a better solution to this issue.

          Show
          Shannon Hess added a comment - Notes: – allowsNoteAttachments indicates if the document view allows note attachments (So nothing to do with Notes only - it is just to say if Notes can have attachments or not) – The method canAnnotate(Document document, Person user) should be used to indicate if notes can be added to a document by a particular user (This is MY understanding) The reason Acknowleders cannot add notes is due to how addLineActions work. In GroupBase.afterEvaluateExpression() the following code is setting the notes collection to readOnly: if (getReadOnly() == null ) { Component parent = ViewLifecycle.getPhase().getParent(); setReadOnly(parent == null ? null : parent.getReadOnly()); } The parent is the page, which is read only for an acknowledgment. Then in CollectionGroupBuilder.build(), the add line is not built since the collectionGroup was set to readonly. if (collectionGroup.isRenderAddLine() && ! Boolean .TRUE.equals(collectionGroup.getReadOnly()) && !collectionGroup.isRenderAddBlankLineButton()) { buildAddLine(view, model, collectionGroup); } I corrected the issue by fixing the canAnnotate methods – It now returns true if the person canAddNoteAttachment. It used to return true if canEdit was also true. I think it's important to note that canAnnotate is not used anywhere in the code so I don't think it was ever correct (Assuming canAnnotate means what I think it means). The old Notes.tag called a function called kfunc:canAddNoteAttachment which did what I am now doing in canAnnotate. I added the following line in Uif-DocumentNotesSection to indicate that the notes are read only if the user CANNOT add notes. If the note collection is not read only, the add note line will be built. <property name= "readOnly" value= "@{!#actionFlags[#Constants.KUALI_ACTION_CAN_ANNOTATE]}" /> Before committing this code I would like someone to verify that my analysis of canAnnotate is correct. It would also be good to know if there is a better solution to this issue.
          Hide
          Kristina Taylor (Inactive) added a comment -

          Which canAnnotate did you change? I'm not thinking those should be changed, because they are basically the same as they are in the KNS, that is, they merely check permissions. I think however you have the right approach when using the action flags, because eventually, DocumentViewAuthorizerBase will decide whether that action flag will be there or not. I have a feeling there's something I'm missing, as you pointed out with the canAddNoteAttachment which does exactly the same thing as the authorizers do. If it is possible to simplify that logic down to just the KUALI_ACTION_CAN_ANNOTATE, that would be ideal. Are there impediments to that approach?

          Show
          Kristina Taylor (Inactive) added a comment - Which canAnnotate did you change? I'm not thinking those should be changed, because they are basically the same as they are in the KNS, that is, they merely check permissions. I think however you have the right approach when using the action flags, because eventually, DocumentViewAuthorizerBase will decide whether that action flag will be there or not. I have a feeling there's something I'm missing, as you pointed out with the canAddNoteAttachment which does exactly the same thing as the authorizers do. If it is possible to simplify that logic down to just the KUALI_ACTION_CAN_ANNOTATE , that would be ideal. Are there impediments to that approach?
          Hide
          Shannon Hess added a comment -

          I actually changed multiple canAnnotate methods (kns and krad) -

          • org.kuali.rice.kns.document.authorization.DocumentAuthorizerBase.canAnnotate()
          • org.kuali.rice.krad.document.DocumentAuthorizerBase.canAnnotate()
          • org.kuali.rice.krad.document.DocumentPresentationControllerBase.DocumentPresentationControllerBase.canAnnotate()

          The reason for changing the KNS and KRAD methods was for two reasons. 1 – the original way felt like a bug to me since canAnnotate currently just returns the canEdit() result and this is not correct. By default, you should be able to annotate if you are the initiator or a reviewer (basically on the route log). Since the canAnnotate method was not used to display the add button and add row for notes in the KNS (or for anything else for that matter), it was never caught as a bug. 2 – I've been testing using contexts, which actually use the KNS documentAuthorizerClass so I needed to change both the KNS and KRAD methods. It may be a bug that the ContextMaintenanceDocument is using the KNS class though.

          In ContextMaintenanceDocument.xml:

              <property name="documentTypeName" value="ContextMaintenanceDocument"/>
              <property name="documentAuthorizerClass" value="org.kuali.rice.kns.document.authorization.MaintenanceDocumentAuthorizerBase"/>
          

          I believe I did what you suggested doing, but I need to discuss it with you a bit more to be sure. I'll contact you via skype tomorrow and perhaps we can chat to verify we are on the same page.

          Show
          Shannon Hess added a comment - I actually changed multiple canAnnotate methods (kns and krad) - org.kuali.rice.kns.document.authorization.DocumentAuthorizerBase.canAnnotate() org.kuali.rice.krad.document.DocumentAuthorizerBase.canAnnotate() org.kuali.rice.krad.document.DocumentPresentationControllerBase.DocumentPresentationControllerBase.canAnnotate() The reason for changing the KNS and KRAD methods was for two reasons. 1 – the original way felt like a bug to me since canAnnotate currently just returns the canEdit() result and this is not correct. By default, you should be able to annotate if you are the initiator or a reviewer (basically on the route log). Since the canAnnotate method was not used to display the add button and add row for notes in the KNS (or for anything else for that matter), it was never caught as a bug. 2 – I've been testing using contexts, which actually use the KNS documentAuthorizerClass so I needed to change both the KNS and KRAD methods. It may be a bug that the ContextMaintenanceDocument is using the KNS class though. In ContextMaintenanceDocument.xml: <property name= "documentTypeName" value= "ContextMaintenanceDocument" /> <property name= "documentAuthorizerClass" value= "org.kuali.rice.kns.document.authorization.MaintenanceDocumentAuthorizerBase" /> I believe I did what you suggested doing, but I need to discuss it with you a bit more to be sure. I'll contact you via skype tomorrow and perhaps we can chat to verify we are on the same page.
          Hide
          Shannon Hess added a comment -

          Talked to Kristina and she and Peter think we should talk to the KRAD group about this issue. I created a review of the changes I made so they can be evaluated.

          Patch -https://fisheye.kuali.org/cru/rice-421#CFR-52172

          Show
          Shannon Hess added a comment - Talked to Kristina and she and Peter think we should talk to the KRAD group about this issue. I created a review of the changes I made so they can be evaluated. Patch - https://fisheye.kuali.org/cru/rice-421#CFR-52172

            People

            • Assignee:
              Shannon Hess
              Reporter:
              Claus Niesen
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 2 days, 2 hours
                2d 2h
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 7 hours Time Not Required
                7h

                  Agile

                    Structure Helper Panel