From 20ea5e8c070d213126da5d7192fdf88cf07796aa Mon Sep 17 00:00:00 2001 From: Jim Myers Date: Tue, 29 Mar 2022 20:31:07 -0400 Subject: [PATCH 1/4] Refactor/fix popup logic --- .../harvard/iq/dataverse/util/FileUtil.java | 76 ++++++++----------- 1 file changed, 32 insertions(+), 44 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java index 62d576193f6..6f6fa895d36 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java @@ -1471,73 +1471,61 @@ public static String getCiteDataFileFilename(String fileTitle, FileCitationExten * elaborate on the text "This file cannot be downloaded publicly." */ public static boolean isDownloadPopupRequired(DatasetVersion datasetVersion) { - // Each of these conditions is sufficient reason to have to - // present the user with the popup: - if (datasetVersion == null) { - logger.fine("Download popup required because datasetVersion is null."); - return false; - } - //0. if version is draft then Popup "not required" - if (!datasetVersion.isReleased()) { - logger.fine("Download popup required because datasetVersion has not been released."); - return false; - } - // 1. License and Terms of Use: - if (datasetVersion.getTermsOfUseAndAccess() != null) { - License license = datasetVersion.getTermsOfUseAndAccess().getLicense(); - if ((license == null && StringUtils.isNotBlank(datasetVersion.getTermsOfUseAndAccess().getTermsOfUse())) - || (license != null && !license.isDefault())) { - logger.fine("Download popup required because of license or terms of use."); - return true; - } - - // 2. Terms of Access: - if (!(datasetVersion.getTermsOfUseAndAccess().getTermsOfAccess() == null) && !datasetVersion.getTermsOfUseAndAccess().getTermsOfAccess().equals("")) { - logger.fine("Download popup required because of terms of access."); - return true; - } + logger.fine("Checking if download popup is required."); + Boolean answer = popupDueToStateOrTerms(datasetVersion); + if (answer != null) { + return answer; } - // 3. Guest Book: if (datasetVersion.getDataset() != null && datasetVersion.getDataset().getGuestbook() != null && datasetVersion.getDataset().getGuestbook().isEnabled() && datasetVersion.getDataset().getGuestbook().getDataverse() != null) { logger.fine("Download popup required because of guestbook."); return true; } - logger.fine("Download popup is not required."); return false; } - public static boolean isRequestAccessPopupRequired(DatasetVersion datasetVersion){ - // Each of these conditions is sufficient reason to have to - // present the user with the popup: + public static boolean isRequestAccessPopupRequired(DatasetVersion datasetVersion) { + + Boolean answer = popupDueToStateOrTerms(datasetVersion); + if (answer != null) { + return answer; + } + logger.fine("Request access popup is not required."); + return false; + } + + private static Boolean popupDueToStateOrTerms(DatasetVersion datasetVersion) { + Boolean answer = null; + // Each of these conditions is sufficient reason to have to + // present the user with the popup: if (datasetVersion == null) { - logger.fine("Download popup required because datasetVersion is null."); - return false; + logger.fine("Popup required because datasetVersion is null."); + answer = false; } - //0. if version is draft then Popup "not required" + // 0. if version is draft then Popup "not required" if (!datasetVersion.isReleased()) { - logger.fine("Download popup required because datasetVersion has not been released."); - return false; + logger.fine("Popup required because datasetVersion has not been released."); + answer = false; } // 1. License and Terms of Use: if (datasetVersion.getTermsOfUseAndAccess() != null) { - if (!datasetVersion.getTermsOfUseAndAccess().getLicense().isDefault() + License license = datasetVersion.getTermsOfUseAndAccess().getLicense(); + if ((license != null && !license.isDefault()) && !(datasetVersion.getTermsOfUseAndAccess().getTermsOfUse() == null - || datasetVersion.getTermsOfUseAndAccess().getTermsOfUse().equals(""))) { - logger.fine("Download popup required because of license or terms of use."); - return true; + || datasetVersion.getTermsOfUseAndAccess().getTermsOfUse().equals(""))) { + logger.fine("{opup required because of license or terms of use."); + answer = true; } // 2. Terms of Access: if (!(datasetVersion.getTermsOfUseAndAccess().getTermsOfAccess() == null) && !datasetVersion.getTermsOfUseAndAccess().getTermsOfAccess().equals("")) { - logger.fine("Download popup required because of terms of access."); - return true; + logger.fine("Popup required because of terms of access."); + answer = true; } - } + return answer; - logger.fine("Download popup is not required."); - return false; + } } /** From 6152b63890e3f1ed6824f092e6004d516f4a0cb8 Mon Sep 17 00:00:00 2001 From: Jim Myers Date: Tue, 29 Mar 2022 20:47:12 -0400 Subject: [PATCH 2/4] fix return - not clear why Eclipse didn't flag this --- src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java index 6f6fa895d36..b5ba114410d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java @@ -1523,9 +1523,8 @@ private static Boolean popupDueToStateOrTerms(DatasetVersion datasetVersion) { logger.fine("Popup required because of terms of access."); answer = true; } - return answer; - } + return answer; } /** From da5e7aa5e465a40cbdc3f1da9efa49ad502bb230 Mon Sep 17 00:00:00 2001 From: Jim Myers Date: Wed, 30 Mar 2022 09:22:02 -0400 Subject: [PATCH 3/4] Use download popup's license test code, cleanup --- .../harvard/iq/dataverse/util/FileUtil.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java index b5ba114410d..4b451d92a75 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java @@ -1496,35 +1496,34 @@ public static boolean isRequestAccessPopupRequired(DatasetVersion datasetVersion } private static Boolean popupDueToStateOrTerms(DatasetVersion datasetVersion) { - Boolean answer = null; + // Each of these conditions is sufficient reason to have to // present the user with the popup: if (datasetVersion == null) { logger.fine("Popup required because datasetVersion is null."); - answer = false; + return false; } // 0. if version is draft then Popup "not required" if (!datasetVersion.isReleased()) { logger.fine("Popup required because datasetVersion has not been released."); - answer = false; + return false; } // 1. License and Terms of Use: if (datasetVersion.getTermsOfUseAndAccess() != null) { License license = datasetVersion.getTermsOfUseAndAccess().getLicense(); - if ((license != null && !license.isDefault()) - && !(datasetVersion.getTermsOfUseAndAccess().getTermsOfUse() == null - || datasetVersion.getTermsOfUseAndAccess().getTermsOfUse().equals(""))) { - logger.fine("{opup required because of license or terms of use."); - answer = true; + if ((license == null && StringUtils.isNotBlank(datasetVersion.getTermsOfUseAndAccess().getTermsOfUse())) + || (license != null && !license.isDefault())) { + logger.fine("Download popup required because of license or terms of use."); + return true; } // 2. Terms of Access: if (!(datasetVersion.getTermsOfUseAndAccess().getTermsOfAccess() == null) && !datasetVersion.getTermsOfUseAndAccess().getTermsOfAccess().equals("")) { logger.fine("Popup required because of terms of access."); - answer = true; + return true; } } - return answer; + return null; } /** From 576a72395d09d00a9390195c1fd6d21fb563aa0f Mon Sep 17 00:00:00 2001 From: Jim Myers Date: Wed, 30 Mar 2022 13:02:18 -0400 Subject: [PATCH 4/4] logging/comment updates per review --- .../java/edu/harvard/iq/dataverse/util/FileUtil.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java index 4b451d92a75..8d3d63da99d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/FileUtil.java @@ -1495,17 +1495,22 @@ public static boolean isRequestAccessPopupRequired(DatasetVersion datasetVersion return false; } + /* Code shared by isDownloadPopupRequired and isRequestAccessPopupRequired. + * + * Returns Boolean to allow null = no decision. This allows the isDownloadPopupRequired method to then add another check w.r.t. guestbooks before returning its value. + * + */ private static Boolean popupDueToStateOrTerms(DatasetVersion datasetVersion) { // Each of these conditions is sufficient reason to have to // present the user with the popup: if (datasetVersion == null) { - logger.fine("Popup required because datasetVersion is null."); + logger.fine("Popup not required because datasetVersion is null."); return false; } // 0. if version is draft then Popup "not required" if (!datasetVersion.isReleased()) { - logger.fine("Popup required because datasetVersion has not been released."); + logger.fine("Popup not required because datasetVersion has not been released."); return false; } // 1. License and Terms of Use: @@ -1513,7 +1518,7 @@ private static Boolean popupDueToStateOrTerms(DatasetVersion datasetVersion) { License license = datasetVersion.getTermsOfUseAndAccess().getLicense(); if ((license == null && StringUtils.isNotBlank(datasetVersion.getTermsOfUseAndAccess().getTermsOfUse())) || (license != null && !license.isDefault())) { - logger.fine("Download popup required because of license or terms of use."); + logger.fine("Popup required because of license or terms of use."); return true; } @@ -1523,6 +1528,7 @@ private static Boolean popupDueToStateOrTerms(DatasetVersion datasetVersion) { return true; } } + //No decision based on the criteria above return null; }