diff --git a/NEWS.md b/NEWS.md index 670dac2fb..4f72d654a 100644 --- 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 index 570619fba..60b04f6d5 100644 --- 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 index 1229e80c4..0e47a889f 100644 --- 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 { } 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 results = exportService.current(subscriptionAgreementId) diff --git a/service/grails-app/controllers/org/olf/UrlMappings.groovy b/service/grails-app/controllers/org/olf/UrlMappings.groovy index 3c1e72fe6..ce225e0df 100644 --- 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 index def572ece..973b6bca9 100644 --- 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 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 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]) // } //