Merge pull request #382 from folio-org/release_3.0.1

Release 3.0.1
folio-org · Nov 6, 2020 · 3059599 · 3059599
2 parents 013b21a + 18b030f
commit 3059599
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 60 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,3 +1,7 @@
+## 3.0.1 2020-11-06
+ * ERM-1213 Exporting Resources from Agreement when "All" filter selected does not export all resources
+ * ERM-1203 Agreement Resources Export contains duplicate resources and incorrect agreement line information
+
 ## 3.0.0 2020-10-14
  * ERM-1180 Add @ EnableScheduling annotation in mod-agreements
  * ERM-1163 Remove default values from publicationType

diff --git a/service/gradle.properties b/service/gradle.properties
@@ -11,6 +11,6 @@ gradleWrapperVersion=5.4
 
 # Application
 appName=mod-agreements
-appVersion=3.1.0
+appVersion=3.0.1
 dockerTagSuffix=
 dockerRepo=folioci
diff --git a/service/grails-app/controllers/org/olf/ExportController.groovy b/service/grails-app/controllers/org/olf/ExportController.groovy
@@ -44,7 +44,7 @@ class ExportController extends OkapiTenantAwareController<TitleInstance>  {
   }
 
   def current() {
-    log.debug("ExportController::index")
+    log.debug("ExportController::current")
     final String subscriptionAgreementId = params.get("subscriptionAgreementId")
     log.debug("Getting export for specific agreement: "+ subscriptionAgreementId)
     List<List> results = exportService.current(subscriptionAgreementId)

diff --git a/service/grails-app/controllers/org/olf/UrlMappings.groovy b/service/grails-app/controllers/org/olf/UrlMappings.groovy
@@ -25,7 +25,7 @@ class UrlMappings {
 
       "/resources/export/$format?"          (controller: 'export', method: 'GET')
       "/resources/export/current/$format?"  (controller: 'export', action: 'current', method: 'GET')
-      "/resources/export/all/$format?"  (controller: 'export', action: 'current', method: 'GET')
+      "/resources/export/all/$format?"  (controller: 'export', action: 'index', method: 'GET')
 //      "/resources/export/future/$format?"   (controller: 'export', action: 'future', method: 'GET')
 //      "/resources/export/dropped/$format?"  (controller: 'export', action: 'dropped', method: 'GET')
 

diff --git a/service/grails-app/services/org/olf/ExportService.groovy b/service/grails-app/services/org/olf/ExportService.groovy
@@ -28,6 +28,12 @@ public class ExportService {
 
     agreement
   }
+
+  /* Note - HQL query construction looks to have changed since v5.2.
+   * Before, we were using an OR in the WHERE block to "null out" certain rows on JOINs,
+   * but this is no longer happening. The solution here was to add the WHERE clauses to the JOIN,
+   * and replace the WHERE with a not-null constraint.  
+  */
 
   List<List> all(final String agreementId = null) {
     def results = null
@@ -36,40 +42,49 @@ public class ExportService {
         SELECT res, pkg_ent, direct_ent
         FROM ErmResource as res
           LEFT JOIN res.entitlements as direct_ent
+            ON (
+              (
+                direct_ent.owner.id = :id
+                  AND
+                res.class != Pkg
+              )
+            )
           LEFT JOIN res.pkg as pkg
 
             ON (res.class = PackageContentItem) 
             LEFT JOIN pkg.entitlements as pkg_ent
+              ON (
+                pkg_ent.owner.id = :id
+              )
         WHERE
-          (
-            direct_ent.owner.id = :id
-            AND
-            res.class != Pkg
-          )
-        OR
-          (
-            pkg_ent.owner.id = :id
-          )
+          (direct_ent IS NOT NULL)
+            OR
+          (pkg_ent IS NOT NULL)
+          
       """, [id: agreementId], [readOnly: true])
     } else {
       results = ErmResource.executeQuery("""
         SELECT res, pkg_ent, direct_ent
         FROM ErmResource as res
           LEFT JOIN res.entitlements as direct_ent
+            ON (
+              (
+                direct_ent.owner IS NOT NULL
+                AND
+                res.class != Pkg
+              )
+            )
           LEFT JOIN res.pkg as pkg
 
             ON res.class = PackageContentItem
             LEFT JOIN pkg.entitlements as pkg_ent
+              ON (
+                pkg_ent.owner IS NOT NULL
+              )
         WHERE
-          (
-            direct_ent.owner IS NOT NULL
-            AND
-            res.class != Pkg
-          )
-        OR
-          (
-            pkg_ent.owner IS NOT NULL
-          )
+          (direct_ent IS NOT NULL)
+            OR
+          (pkg_ent IS NOT NULL)
       """, [readOnly: true])
     }
 
@@ -102,6 +117,12 @@ public class ExportService {
               (direct_ent.activeTo IS NULL OR direct_ent.activeTo >= :today)
                 AND
                  (direct_ent.activeFrom IS NULL OR direct_ent.activeFrom  <= :today)
+                AND (
+                  direct_ent.owner.id = :agreementId
+                  AND
+                  res.class != Pkg
+                )
+
             )
           LEFT JOIN res.pkg as pkg
 
@@ -117,18 +138,14 @@ public class ExportService {
               (pkg_ent.activeTo IS NULL OR pkg_ent.activeTo >= :today)
                 AND
                  (pkg_ent.activeFrom IS NULL OR pkg_ent.activeFrom  <= :today)
+                 AND pkg_ent.owner.id = :agreementId
             )
-        WHERE
-          (
-            direct_ent.owner.id = :id
-            AND
-            res.class != Pkg
-          )
-        OR
-          (
-            pkg_ent.owner.id = :id
-          )
-      """, [id: agreementId, 'today': today], [readOnly: true])
+
+          WHERE
+            (direct_ent IS NOT NULL)
+            OR
+            (pkg_ent IS NOT NULL)
+      """, ['agreementId': agreementId, 'today': today], [readOnly: true])
     } else {
       results = ErmResource.executeQuery("""
         SELECT res, pkg_ent, direct_ent
@@ -138,6 +155,12 @@ public class ExportService {
               (direct_ent.activeTo IS NULL OR direct_ent.activeTo >= :today)
                 AND
                  (direct_ent.activeFrom IS NULL OR direct_ent.activeFrom  <= :today)
+                AND
+                (
+                  direct_ent.owner IS NOT NULL
+                  AND
+                  res.class != Pkg
+                )
             )
           LEFT JOIN res.pkg as pkg
 
@@ -153,17 +176,12 @@ public class ExportService {
                 (pkg_ent.activeTo IS NULL OR pkg_ent.activeTo >= :today)
                   AND
                    (pkg_ent.activeFrom IS NULL OR pkg_ent.activeFrom  <= :today)
+                   AND pkg_ent.owner IS NOT NULL
               )
         WHERE
-          (
-            direct_ent.owner IS NOT NULL
-            AND
-            res.class != Pkg
-          )
-        OR
-          (
-            pkg_ent.owner IS NOT NULL
-          )
+          (direct_ent IS NOT NULL)
+            OR
+          (pkg_ent IS NOT NULL)
       """, ['today': today], [readOnly: true])
     }
 
@@ -182,7 +200,8 @@ public class ExportService {
 
     return results
   }
-
+
+// TODO if this is uncommented and used, check the WHERE changes made above are actually working here
 //  List<ErmResource> future (final String agreementId = null) {
 //    final LocalDate today = LocalDate.now()
 //    
@@ -196,6 +215,12 @@ public class ExportService {
 //              (direct_ent.activeTo IS NULL OR direct_ent.activeTo >= :today)
 //                AND
 //                 (direct_ent.activeFrom IS NULL OR direct_ent.activeFrom  <= :today)
+//                AND
+//                  (
+//                    direct_ent.owner.id = :id
+//                      AND
+//                    res.class != Pkg
+//                  )
 //            )
 //          LEFT JOIN res.pkg as pkg
 //
@@ -211,17 +236,13 @@ public class ExportService {
 //              (pkg_ent.activeTo IS NULL OR pkg_ent.activeTo >= :today)
 //                AND
 //                 (pkg_ent.activeFrom IS NULL OR pkg_ent.activeFrom  <= :today)
+//                AND
+//                  (pkg_ent.owner.id = :id)
 //            )
 //        WHERE
-//          (
-//            direct_ent.owner.id = :id
-//            AND
-//            res.class != Pkg
-//          )
-//        OR
-//          (
-//            pkg_ent.owner.id = :id
-//          )
+//          (direct_ent IS NOT NULL)
+//            OR
+//          (pkg_ent IS NOT NULL)
 //      """, [id: agreementId, 'today': today], [readOnly: true])
 //    } else {
 //      results = ErmResource.executeQuery("""
@@ -232,6 +253,12 @@ public class ExportService {
 //              (direct_ent.activeTo IS NULL OR direct_ent.activeTo >= :today)
 //                AND
 //                 (direct_ent.activeFrom IS NULL OR direct_ent.activeFrom  <= :today)
+//                AND
+//                 (
+//                   direct_ent.owner IS NOT NULL
+//                     AND
+//                   res.class != Pkg
+//                 )
 //            )
 //          LEFT JOIN res.pkg as pkg
 //
@@ -247,17 +274,12 @@ public class ExportService {
 //                (pkg_ent.activeTo IS NULL OR pkg_ent.activeTo >= :today)
 //                  AND
 //                   (pkg_ent.activeFrom IS NULL OR pkg_ent.activeFrom  <= :today)
+//                  AND (pkg_ent.owner IS NOT NULL)
 //              )
 //        WHERE
-//          (
-//            direct_ent.owner IS NOT NULL
-//            AND
-//            res.class != Pkg
-//          )
-//        OR
-//          (
-//            pkg_ent.owner IS NOT NULL
-//          )
+//          (direct_ent IS NOT NULL)
+//            OR
+//          (pkg_ent IS NOT NULL)
 //      """, ['today': today], [readOnly: true])
 //    }
 //