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

UIF-related annotations should not be in krad data module

    Details

    • Type: Improvement Improvement
    • Status: Open Open
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 2.6
    • Component/s: Development, JPA
    • Security Level: Public (Public: Anyone can view)
    • Labels:
    • Similar issues:
      KULRICE-9580Update krad-data to support more standard types of validations via annotations
      KULRICE-11290Support Custom Uif Annotations for KRAD-Data
      KULRICE-8919Create krad-data module
      KULRICE-10905Refactor quickfinder (and other lookup code) to use new KRAD data module
      KULRICE-9347Implement Extension framework in new KRAD data module
      KULRICE-9524Ensure that the krad-data module is fully javadoc'd
      KULRICE-9089Design and analysis for how new metadata and Data Dictionary will be loaded and split from current data dictionary
      KULRICE-10126Create section in documentation for krad-data module
      KULRICE-9530Create a tutorial for the krad-data module using JPA
      KULRICE-9537Document the approach for conversion to krad-data of the various internal Rice modules
    • Sprint:
      2.4.0-rc1 Sprint 3, 2.4.0-rc1 Sprint 4
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      It looks like these are currently under the krad data annotation provider package.

      For example:

      import org.kuali.rice.krad.data.provider.annotation.UifDisplayHint;

      Anything related to UIF at all should be up in the web framework module, not down in the data module.

        Activity

        Hide
        Claus Niesen added a comment -

        Move following classes to /rice-framework/krad-web-framework/src/main/java/org/kuali/rice/krad/provider/annotation:

        • UifAutoCreateViews
        • UifAutoCreateViewType
        • UifDisplayHint
        • UifDisplayHints
        • UifDisplayHintType
        • UifValidCharactersConstraintBeanName
        Show
        Claus Niesen added a comment - Move following classes to /rice-framework/krad-web-framework/src/main/java/org/kuali/rice/krad/provider/annotation: UifAutoCreateViews UifAutoCreateViewType UifDisplayHint UifDisplayHints UifDisplayHintType UifValidCharactersConstraintBeanName
        Hide
        Kristina Taylor (Inactive) added a comment - - edited

        Moving any of these causes the krad-data framework not to be able to find them since it does not depend on krad-web-framework, and for good reason. IntelliJ reports a circular dependency between

        "rice-krad-data"
        "rice-krad-web-framework"
        "rice-krad-app-framework"
        "rice-kim-framework"

        The kim-framework doesn't even have to be included here. There is a cycle between the first three. It seems to be a bit of an iffy design to have krad-data depend on krad-web-framework. Shouldn't it be the other way around? If so, what we might do is have some extensions for the Uif to include these annotations. I'll keep digging deeper.

        Show
        Kristina Taylor (Inactive) added a comment - - edited Moving any of these causes the krad-data framework not to be able to find them since it does not depend on krad-web-framework, and for good reason. IntelliJ reports a circular dependency between "rice-krad-data" "rice-krad-web-framework" "rice-krad-app-framework" "rice-kim-framework" The kim-framework doesn't even have to be included here. There is a cycle between the first three. It seems to be a bit of an iffy design to have krad-data depend on krad-web-framework. Shouldn't it be the other way around? If so, what we might do is have some extensions for the Uif to include these annotations. I'll keep digging deeper.
        Hide
        Kristina Taylor (Inactive) added a comment -

        These are almost too tied together that doing this refactoring during this sprint would probably be counterproductive. I suggest that we move this to 2.5.

        One major problem that could hinder this entire process is the @InheritProperty which is a krad-data annotation but uses a Uif annotation inside of it. Those would probably have to be separated first and tested. I did a bit of refactoring work on this but it doesn't compile because of that and it won't work because the new Uif classes are not accessed. Maybe a better way to handle what is in the patch is via composition instead of inheritance, but I wanted to do a quick and dirty test to see what the barriers would be.

        Show
        Kristina Taylor (Inactive) added a comment - These are almost too tied together that doing this refactoring during this sprint would probably be counterproductive. I suggest that we move this to 2.5. One major problem that could hinder this entire process is the @InheritProperty which is a krad-data annotation but uses a Uif annotation inside of it. Those would probably have to be separated first and tested. I did a bit of refactoring work on this but it doesn't compile because of that and it won't work because the new Uif classes are not accessed. Maybe a better way to handle what is in the patch is via composition instead of inheritance, but I wanted to do a quick and dirty test to see what the barriers would be.
        Hide
        Eric Westfall added a comment -

        Yes, these are definitely tied together. The problem is that they made their way into things like DataObjectAttribute (which they shouldn't have). Instead, the uif-related metadata should have been kept at a higher level in KRAD but that was a problem that wasn't really solved during 2.4 development. We should probably chat about the best way to design this when this issue gets reapproached.

        For now, in the 2.4 code base I've annotated these classes and methods related to them in krad-data with the @Beta annotation from google guava which essentially indicates they should be used at your own risk and may change or be removed entirely in the future.

        Show
        Eric Westfall added a comment - Yes, these are definitely tied together. The problem is that they made their way into things like DataObjectAttribute (which they shouldn't have). Instead, the uif-related metadata should have been kept at a higher level in KRAD but that was a problem that wasn't really solved during 2.4 development. We should probably chat about the best way to design this when this issue gets reapproached. For now, in the 2.4 code base I've annotated these classes and methods related to them in krad-data with the @Beta annotation from google guava which essentially indicates they should be used at your own risk and may change or be removed entirely in the future.

          People

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

            Dates

            • Created:
              Updated:

              Time Tracking

              Estimated:
              Original Estimate - 2 days
              2d
              Remaining:
              Time Spent - 2 hours Remaining Estimate - 1 day, 6 hours
              1d 6h
              Logged:
              Time Spent - 2 hours Remaining Estimate - 1 day, 6 hours
              2h

                Agile

                  Structure Helper Panel