From 5f4dc6c2ae8d02ed444466886378d55c6af34747 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Tue, 5 Nov 2024 13:13:48 -0500 Subject: [PATCH 01/10] fix: Merge participant ids independently of task ids --- .../service/ListingMergeService.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/service/ListingMergeService.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/service/ListingMergeService.java index a978c8ef0f..0dcc21ab59 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/service/ListingMergeService.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/service/ListingMergeService.java @@ -3,6 +3,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; import org.apache.commons.beanutils.BeanUtils; import org.apache.commons.collections.CollectionUtils; @@ -252,8 +253,15 @@ private void setIdsForSed(CertifiedProductSed updatedListingSed, CertifiedProduc if (!CollectionUtils.isEmpty(updatedListingSed.getTestTasks())) { updatedListingSed.getTestTasks().stream() .forEach(updatedTestTask -> setIdForTestTask(updatedTestTask, currListingSed.getTestTasks())); - } + Set currentTestParticipants = currListingSed.getTestTasks().stream() + .flatMap(currTestTask -> currTestTask.getTestParticipants().stream()) + .collect(Collectors.toSet()); + + updatedListingSed.getTestTasks().stream() + .flatMap(updatedTestTask -> updatedTestTask.getTestParticipants().stream()) + .forEach(updatedTestParticipant -> setIdForTestParticipant(updatedTestParticipant, currentTestParticipants)); + } } private void setIdForTestTask(TestTask updatedTestTask, List currTestTasks) { @@ -267,10 +275,6 @@ private void setIdForTestTask(TestTask updatedTestTask, List currTestT .orElse(null); if (matchedCurrTestTask != null) { updatedTestTask.setId(matchedCurrTestTask.getId()); - if (!CollectionUtils.isEmpty(updatedTestTask.getTestParticipants())) { - updatedTestTask.getTestParticipants().stream() - .forEach(updatedTestParticipant -> setIdForTestParticipant(updatedTestParticipant, matchedCurrTestTask.getTestParticipants())); - } } } From ef362e2bc1ee178a06d2c47190e7ba5eba9ae25a Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Wed, 13 Nov 2024 10:27:24 -0500 Subject: [PATCH 02/10] fix: Allow ACB ownership change when logged in with Cognito [#OCD-4705] --- .../CertifiedProductController.java | 3 +-- .../chpl/manager/CertifiedProductManager.java | 3 ++- .../CertifiedProductDomainPermissions.java | 6 +++++- .../ChangeAcbOwnerActionPermissions.java | 21 +++++++++++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/certifiedproduct/ChangeAcbOwnerActionPermissions.java diff --git a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertifiedProductController.java b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertifiedProductController.java index 7782e0601b..d3029ad649 100644 --- a/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertifiedProductController.java +++ b/chpl/chpl-api/src/main/java/gov/healthit/chpl/web/controller/CertifiedProductController.java @@ -705,8 +705,7 @@ public ResponseEntity updateCertifiedProduct( CertifiedProductSearchDetails existingListing = cpdManager.getCertifiedProductDetails(updatedListing.getId()); Long acbId = Long.parseLong(existingListing.getCertifyingBody().get(CertifiedProductSearchDetails.ACB_ID_KEY).toString()); - // if the ACB owner is changed this is a separate action with different - // security + // if the ACB owner is changed this is a separate action with different security Long newAcbId = Long .valueOf(updatedListing.getCertifyingBody().get(CertifiedProductSearchDetails.ACB_ID_KEY).toString()); if (acbId.longValue() != newAcbId.longValue()) { diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertifiedProductManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertifiedProductManager.java index cc4a323ecd..8e3cd9e956 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertifiedProductManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertifiedProductManager.java @@ -239,7 +239,8 @@ public List getByProduct(Long productId) throws Enti return cpDao.getDetailsByProductId(productId); } - @PreAuthorize("hasAnyRole('ROLE_ADMIN', 'ROLE_ONC')") + @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).CERTIFIED_PRODUCT, " + + "T(gov.healthit.chpl.permissions.domains.CertifiedProductDomainPermissions).CHANGE_ACB_OWNER)") @Transactional(readOnly = false) @CacheEvict(value = { CacheNames.GET_DECERTIFIED_DEVELOPERS, CacheNames.COLLECTIONS_DEVELOPERS diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/CertifiedProductDomainPermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/CertifiedProductDomainPermissions.java index f5b4627668..ca98e35b0b 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/CertifiedProductDomainPermissions.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/CertifiedProductDomainPermissions.java @@ -4,6 +4,7 @@ import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Component; +import gov.healthit.chpl.permissions.domains.certifiedproduct.ChangeAcbOwnerActionPermissions; import gov.healthit.chpl.permissions.domains.certifiedproduct.ConvertToCsvActionPermissions; import gov.healthit.chpl.permissions.domains.certifiedproduct.CreateActionPermissions; import gov.healthit.chpl.permissions.domains.certifiedproduct.UpdateActionPermissions; @@ -17,6 +18,7 @@ public class CertifiedProductDomainPermissions extends DomainPermissions { public static final String CREATE = "CREATE"; public static final String UPDATE = "UPDATE"; public static final String CONVERT_TO_CSV = "CONVERT_TO_CSV"; + public static final String CHANGE_ACB_OWNER = "CHANGE_ACB_OWNER"; @Autowired public CertifiedProductDomainPermissions( @@ -24,11 +26,13 @@ public CertifiedProductDomainPermissions( @Qualifier("certifiedProductUploadPiuActionPermissions") UploadPiuActionPermissions uploadPiuActionPermissions, @Qualifier("certifiedProductCreateActionPermissions") CreateActionPermissions createActionPermissions, @Qualifier("certifiedProductUpdateActionPermissions") UpdateActionPermissions updateActionPermissions, - @Qualifier("certifiedProductConvertToCsvActionPermissions") ConvertToCsvActionPermissions convertToCsvActionPermissions) { + @Qualifier("certifiedProductConvertToCsvActionPermissions") ConvertToCsvActionPermissions convertToCsvActionPermissions, + @Qualifier("certifiedProductChangeAcbOwnerActionPermissions") ChangeAcbOwnerActionPermissions changeAcbOwnerActionPermissions) { getActionPermissions().put(UPLOAD, uploadActionPermissions); getActionPermissions().put(UPLOAD_PIU, uploadPiuActionPermissions); getActionPermissions().put(CREATE, createActionPermissions); getActionPermissions().put(UPDATE, updateActionPermissions); getActionPermissions().put(CONVERT_TO_CSV, convertToCsvActionPermissions); + getActionPermissions().put(CHANGE_ACB_OWNER, changeAcbOwnerActionPermissions); } } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/certifiedproduct/ChangeAcbOwnerActionPermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/certifiedproduct/ChangeAcbOwnerActionPermissions.java new file mode 100644 index 0000000000..14ba1f9968 --- /dev/null +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/certifiedproduct/ChangeAcbOwnerActionPermissions.java @@ -0,0 +1,21 @@ +package gov.healthit.chpl.permissions.domains.certifiedproduct; + +import org.springframework.stereotype.Component; + +import gov.healthit.chpl.permissions.domains.ActionPermissions; + +@Component("certifiedProductChangeAcbOwnerActionPermissions") +public class ChangeAcbOwnerActionPermissions extends ActionPermissions { + + @Override + public boolean hasAccess() { + return getResourcePermissions().isUserRoleAdmin() + || getResourcePermissions().isUserRoleOnc(); + } + + @Override + public boolean hasAccess(Object obj) { + return false; + } + +} From 5819961c698a6b3b18ca7d9c8c74cf639069e453 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Wed, 13 Nov 2024 10:33:22 -0500 Subject: [PATCH 03/10] fix: API Documentation File Upload, ACB Retire with Cognito user [#OCD-4705] --- .../chpl/manager/SchedulerManager.java | 3 ++- .../manager/impl/CHPLFilesManagerImpl.java | 3 ++- .../chpl/permissions/Permissions.java | 1 + .../ApiDocumentationDomainPermissions.java | 19 +++++++++++++++++ .../CreateActionPermissions.java | 21 +++++++++++++++++++ 5 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/ApiDocumentationDomainPermissions.java create mode 100644 chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/apiDocumentation/CreateActionPermissions.java diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/SchedulerManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/SchedulerManager.java index d87a991e7c..a426fd6a10 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/SchedulerManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/SchedulerManager.java @@ -329,7 +329,8 @@ public ChplJob updateJob(ChplJob job) throws SchedulerException { } } - @PreAuthorize("hasAnyRole('ROLE_ADMIN', 'ROLE_ONC')") + @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).CERTIFICATION_BODY, " + + "T(gov.healthit.chpl.permissions.domains.CertificationBodyDomainPermissions).RETIRE)") public void retireAcb(String acb) throws SchedulerException, ValidationException, EmailNotSentException { List allTriggers = getAllTriggersForUser(); for (ChplRepeatableTrigger trigger : allTriggers) { diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/impl/CHPLFilesManagerImpl.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/impl/CHPLFilesManagerImpl.java index 265da48f01..908915c762 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/impl/CHPLFilesManagerImpl.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/impl/CHPLFilesManagerImpl.java @@ -27,7 +27,8 @@ public CHPLFilesManagerImpl(final CHPLFileDAO chplFileDAO) { @Override @Transactional - @PreAuthorize("hasAnyRole('ROLE_ADMIN', 'ROLE_ONC')") + @PreAuthorize("@permissions.hasAccess(T(gov.healthit.chpl.permissions.Permissions).API_DOCUMENTATION, " + + "T(gov.healthit.chpl.permissions.domains.ApiDocumentationDomainPermissions).CREATE)") public CHPLFileDTO addApiDocumentationFile(final CHPLFileDTO newFileDTO) throws EntityCreationException, EntityRetrievalException { // Need to delete the existing 'current' file diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/Permissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/Permissions.java index 999303b3c8..f766a307c1 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/Permissions.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/Permissions.java @@ -43,6 +43,7 @@ @Component public class Permissions { + public static final String API_DOCUMENTATION = "API_DOCUMENTATION"; public static final String CERTIFICATION_RESULTS = "CERTIFICATION_RESULTS"; public static final String CERTIFIED_PRODUCT = "CERTIFIED_PRODUCT"; public static final String CORRECTIVE_ACTION_PLAN = "CORRECTIVE_ACTION_PLAN"; diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/ApiDocumentationDomainPermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/ApiDocumentationDomainPermissions.java new file mode 100644 index 0000000000..a6148fa9d0 --- /dev/null +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/ApiDocumentationDomainPermissions.java @@ -0,0 +1,19 @@ +package gov.healthit.chpl.permissions.domains; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.stereotype.Component; + +import gov.healthit.chpl.permissions.domains.apiDocumentation.CreateActionPermissions; + +@Component +public class ApiDocumentationDomainPermissions extends DomainPermissions { + public static final String CREATE = "CREATE"; + + @Autowired + public ApiDocumentationDomainPermissions( + @Qualifier("apiDocumentationCreateActionPermissions") CreateActionPermissions createActionPermissions) { + getActionPermissions().put(CREATE, createActionPermissions); + } + +} diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/apiDocumentation/CreateActionPermissions.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/apiDocumentation/CreateActionPermissions.java new file mode 100644 index 0000000000..5b884f4580 --- /dev/null +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/permissions/domains/apiDocumentation/CreateActionPermissions.java @@ -0,0 +1,21 @@ +package gov.healthit.chpl.permissions.domains.apiDocumentation; + +import org.springframework.stereotype.Component; + +import gov.healthit.chpl.permissions.domains.ActionPermissions; + +@Component("apiDocumentationCreateActionPermissions") +public class CreateActionPermissions extends ActionPermissions { + + @Override + public boolean hasAccess() { + return getResourcePermissions().isUserRoleAdmin() + || getResourcePermissions().isUserRoleOnc(); + } + + @Override + public boolean hasAccess(Object obj) { + return false; + } + +} From 38862e2e01b0e34bf4ff7baf75119286fe848cc3 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Wed, 13 Nov 2024 12:37:30 -0500 Subject: [PATCH 04/10] fix: Use correct HQL when removing a testing lab from a listing [#OCD-4705] --- .../healthit/chpl/dao/CertifiedProductTestingLabDAO.java | 8 ++++---- .../healthit/chpl/manager/CertifiedProductManager.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/dao/CertifiedProductTestingLabDAO.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/dao/CertifiedProductTestingLabDAO.java index 90f73954ec..436439c728 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/dao/CertifiedProductTestingLabDAO.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/dao/CertifiedProductTestingLabDAO.java @@ -66,10 +66,10 @@ public CertifiedProductTestingLab lookupMapping(Long certifiedProductId, Long tl private CertifiedProductTestingLabMapEntity getEntityById(final Long id) throws EntityRetrievalException { CertifiedProductTestingLabMapEntity entity = null; Query query = entityManager.createQuery( - "SELECT tl from CertifiedProductTestingLabMapEntity tl " - + "LEFT OUTER JOIN FETCH tl.testingLab " - + "WHERE (NOT tl.deleted = true) " - + "AND (id = :entityid) ", + "SELECT tlm from CertifiedProductTestingLabMapEntity tlm " + + "LEFT OUTER JOIN FETCH tlm.testingLab " + + "WHERE (NOT tlm.deleted = true) " + + "AND (tlm.id = :entityid) ", CertifiedProductTestingLabMapEntity.class); query.setParameter("entityid", id); diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertifiedProductManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertifiedProductManager.java index 8e3cd9e956..1103a1d329 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertifiedProductManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/manager/CertifiedProductManager.java @@ -422,7 +422,7 @@ private void deleteCertifiedProductTestingLab(CertifiedProductTestingLab toDelet try { cpTestingLabDao.deleteCertifiedProductTestingLab(toDelete.getId()); } catch (Exception e) { - LOGGER.info("Could not delete CertifiedProductTestingLab with Id: {}, {}", toDelete.getId(), e.getMessage(), e); + LOGGER.error("Could not delete CertifiedProductTestingLab with Id: {}, {}", toDelete.getId(), e.getMessage(), e); } } @@ -430,7 +430,7 @@ private void addCertifiedProductTestingLab(CertifiedProductTestingLab toAdd, Lon try { cpTestingLabDao.createCertifiedProductTestingLab(toAdd, listingId); } catch (Exception e) { - LOGGER.info("Could not add CertifiedProductTestingLab with Id: {}, for Listing: {}, {}", toAdd.getTestingLab().getId(), listingId, e.getMessage(), e); + LOGGER.error("Could not add CertifiedProductTestingLab with Id: {}, for Listing: {}, {}", toAdd.getTestingLab().getId(), listingId, e.getMessage(), e); } } From 4b4b7675ac86e9ff40b1108ad4d75bbc44cb4922 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Wed, 13 Nov 2024 14:07:02 -0500 Subject: [PATCH 05/10] fix: Update reviewers to handle no Active status on a listing [#OCD-4705] --- .../BaselineStandardAsOfCertificationDayNormalizer.java | 2 +- .../reviewer/CodeSetAsOfCertificationDayReviewer.java | 2 +- .../validation/reviewer/FunctionalityTestedReviewer.java | 2 +- .../reviewer/StandardAsOfCertificationDayReviewer.java | 2 +- .../upload/listing/validation/reviewer/TestToolReviewer.java | 2 +- .../validation/reviewer/UnavailableCriteriaReviewer.java | 4 +++- .../chpl/validation/listing/reviewer/StandardReviewer.java | 2 +- .../listing/reviewer/UnavailableCriteriaReviewer.java | 4 +++- .../FunctionalityTestedAllowedByCriteriaReviewer.java | 2 +- .../edition2015/UnavailableCriteriaComparisonReviewer.java | 4 +++- .../UnavailableCriteriaTestTaskComparisonReviewer.java | 3 +-- .../edition2015/UnavailableCriteriaUcdComparisonReviewer.java | 4 +--- 12 files changed, 18 insertions(+), 15 deletions(-) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/normalizer/BaselineStandardAsOfCertificationDayNormalizer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/normalizer/BaselineStandardAsOfCertificationDayNormalizer.java index 3e7d728ebc..adea7a894d 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/normalizer/BaselineStandardAsOfCertificationDayNormalizer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/normalizer/BaselineStandardAsOfCertificationDayNormalizer.java @@ -18,6 +18,6 @@ public BaselineStandardAsOfCertificationDayNormalizer(BaselineStandardService ba } public LocalDate getStandardsCheckDate(CertifiedProductSearchDetails listing) { - return listing.getCertificationDay(); + return listing.getCertificationDay() == null ? LocalDate.MIN : listing.getCertificationDay(); } } \ No newline at end of file diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/CodeSetAsOfCertificationDayReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/CodeSetAsOfCertificationDayReviewer.java index 470ae0e149..d103149df0 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/CodeSetAsOfCertificationDayReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/CodeSetAsOfCertificationDayReviewer.java @@ -24,6 +24,6 @@ public CodeSetAsOfCertificationDayReviewer(CertificationResultRules certResultRu } public LocalDate getCodeSetCheckDate(CertifiedProductSearchDetails listing) { - return listing.getCertificationDay(); + return listing.getCertificationDay() == null ? LocalDate.MIN : listing.getCertificationDay(); } } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/FunctionalityTestedReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/FunctionalityTestedReviewer.java index 503a6d7a1f..bb613c0805 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/FunctionalityTestedReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/FunctionalityTestedReviewer.java @@ -158,7 +158,7 @@ private void reviewFunctionalityTestedAvailabilityAfterListingActiveDates(Certif } private boolean isFunctionalityTestedRetiredBeforeListingActiveDates(CertifiedProductSearchDetails listing, FunctionalityTested functionalityTested) { - LocalDate listingStartDay = listing.getCertificationDay(); + LocalDate listingStartDay = listing.getCertificationDay() == null ? LocalDate.MIN : listing.getCertificationDay(); LocalDate funcTestedEndDay = functionalityTested.getEndDay() == null ? LocalDate.MAX : functionalityTested.getEndDay(); return funcTestedEndDay.isBefore(listingStartDay); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/StandardAsOfCertificationDayReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/StandardAsOfCertificationDayReviewer.java index 78d4f32551..f2449d1cd3 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/StandardAsOfCertificationDayReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/StandardAsOfCertificationDayReviewer.java @@ -27,7 +27,7 @@ public StandardAsOfCertificationDayReviewer(CertificationResultRules certResultR } public LocalDate getStandardsCheckDate(CertifiedProductSearchDetails listing) { - return listing.getCertificationDay(); + return listing.getCertificationDay() == null ? LocalDate.MIN : listing.getCertificationDay(); } } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/TestToolReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/TestToolReviewer.java index 594216d9e9..4c7c03aaf7 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/TestToolReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/TestToolReviewer.java @@ -190,7 +190,7 @@ private void reviewTestToolAvailabilityAfterListingActiveDates(CertifiedProductS } private boolean isTestToolRetiredBeforeListingActiveDates(CertifiedProductSearchDetails listing, TestTool testTool) { - LocalDate listingStartDay = listing.getCertificationDay(); + LocalDate listingStartDay = listing.getCertificationDay() == null ? LocalDate.MIN : listing.getCertificationDay(); LocalDate testToolEndDay = testTool.getEndDay() == null ? LocalDate.MAX : testTool.getEndDay(); return testToolEndDay.isBefore(listingStartDay); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/UnavailableCriteriaReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/UnavailableCriteriaReviewer.java index e0d9952e4a..8449696ab5 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/UnavailableCriteriaReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/validation/reviewer/UnavailableCriteriaReviewer.java @@ -48,8 +48,10 @@ private boolean isCriterionAttested(CertificationResult certResult) { private boolean doCriterionDatesOverlapCertificationDay(CertifiedProductSearchDetails listing, CertificationResult certResult) { LocalDate listingEndDay = listing.getDecertificationDay() != null ? listing.getDecertificationDay() : LocalDate.now(); + LocalDate listingStartDay = listing.getCertificationDay() != null ? listing.getCertificationDay() : LocalDate.MIN; + return certResult.getCriterion() != null - && DateUtil.datesOverlap(Pair.of(listing.getCertificationDay(), listingEndDay), + && DateUtil.datesOverlap(Pair.of(listingStartDay, listingEndDay), Pair.of(certResult.getCriterion().getStartDay(), certResult.getCriterion().getEndDay())); } } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/StandardReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/StandardReviewer.java index 82a81a9fd3..3a3ff95ea6 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/StandardReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/StandardReviewer.java @@ -195,7 +195,7 @@ private void reviewStandardAvailabilityAfterListingActiveDates(CertifiedProductS } private boolean isStandardRetiredBeforeListingActiveDates(CertifiedProductSearchDetails listing, Standard standard) { - LocalDate listingStartDay = listing.getCertificationDay(); + LocalDate listingStartDay = listing.getCertificationDay() == null ? LocalDate.MIN : listing.getCertificationDay(); LocalDate standardEndDay = standard.getEndDay() == null ? LocalDate.MAX : standard.getEndDay(); return standardEndDay.isBefore(listingStartDay); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/UnavailableCriteriaReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/UnavailableCriteriaReviewer.java index f995fb7492..ce166d8fe8 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/UnavailableCriteriaReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/UnavailableCriteriaReviewer.java @@ -51,8 +51,10 @@ private boolean isCriterionAttested(CertificationResult certResult) { private boolean doCriterionDatesOverlapCertificationDay(CertifiedProductSearchDetails listing, CertificationResult certResult) { LocalDate listingEndDay = listing.getDecertificationDay() != null ? listing.getDecertificationDay() : LocalDate.now(); + LocalDate listingStartDay = listing.getCertificationDay() != null ? listing.getCertificationDay() : LocalDate.MIN; + return certResult.getCriterion() != null - && DateUtil.datesOverlap(Pair.of(listing.getCertificationDay(), listingEndDay), + && DateUtil.datesOverlap(Pair.of(listingStartDay, listingEndDay), Pair.of(certResult.getCriterion().getStartDay(), certResult.getCriterion().getEndDay())); } } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/FunctionalityTestedAllowedByCriteriaReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/FunctionalityTestedAllowedByCriteriaReviewer.java index 74e3c03408..c921d7c31d 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/FunctionalityTestedAllowedByCriteriaReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/FunctionalityTestedAllowedByCriteriaReviewer.java @@ -155,7 +155,7 @@ private void reviewFunctionalityTestedAvailabilityAfterListingActiveDates(Certif } private boolean isFunctionalityTestedRetiredBeforeListingActiveDates(CertifiedProductSearchDetails listing, FunctionalityTested functionalityTested) { - LocalDate listingStartDay = listing.getCertificationDay(); + LocalDate listingStartDay = listing.getCertificationDay() == null ? LocalDate.MIN : listing.getCertificationDay(); LocalDate funcTestedEndDay = functionalityTested.getEndDay() == null ? LocalDate.MAX : functionalityTested.getEndDay(); return funcTestedEndDay.isBefore(listingStartDay); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaComparisonReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaComparisonReviewer.java index 70e49bb77d..634c517ff0 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaComparisonReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaComparisonReviewer.java @@ -91,8 +91,10 @@ private void reviewCriterionAvailableAndEditable(CertifiedProductSearchDetails u private boolean doCriterionDatesOverlapCertificationDay(CertifiedProductSearchDetails listing, CertificationResult certResult) { LocalDate listingEndDay = listing.getDecertificationDay() != null ? listing.getDecertificationDay() : LocalDate.now(); + LocalDate listingStartDay = listing.getCertificationDay() != null ? listing.getCertificationDay() : LocalDate.MIN; + return certResult.getCriterion() != null - && DateUtil.datesOverlap(Pair.of(listing.getCertificationDay(), listingEndDay), + && DateUtil.datesOverlap(Pair.of(listingStartDay, listingEndDay), Pair.of(certResult.getCriterion().getStartDay(), certResult.getCriterion().getEndDay())); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaTestTaskComparisonReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaTestTaskComparisonReviewer.java index 77993746d5..a08f0a1fb9 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaTestTaskComparisonReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaTestTaskComparisonReviewer.java @@ -87,8 +87,7 @@ private Set getAddedCriteria(Set return newCriteria; } - private boolean isCriterionAvailable(CertifiedProductSearchDetails listing, - CertificationCriterion criterion) { + private boolean isCriterionAvailable(CertifiedProductSearchDetails listing, CertificationCriterion criterion) { return DateUtil.datesOverlap(Pair.of(listing.getCertificationDay(), listing.getDecertificationDay()), Pair.of(criterion.getStartDay(), criterion.getEndDay())); diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaUcdComparisonReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaUcdComparisonReviewer.java index 538ac93680..cda9b05802 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaUcdComparisonReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/UnavailableCriteriaUcdComparisonReviewer.java @@ -89,9 +89,7 @@ private Set getAddedCriteria(Set return newCriteria; } - private boolean isCriterionAvailable(CertifiedProductSearchDetails listing, - CertificationCriterion criterion) { - + private boolean isCriterionAvailable(CertifiedProductSearchDetails listing, CertificationCriterion criterion) { return DateUtil.datesOverlap(Pair.of(listing.getCertificationDay(), listing.getDecertificationDay()), Pair.of(criterion.getStartDay(), criterion.getEndDay())); } From 85dfdea5d575714afa467d8fe5b27be8a049081f Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Wed, 13 Nov 2024 18:25:49 -0500 Subject: [PATCH 06/10] fix: Set SSO user properties in JWTUserConverterFacade This was deleted (by accident??) in OCD-4660 and we really need it! A whole host of issues appear when it is not present. [#OCD-4705] --- .../authentication/JWTUserConverterFacade.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/JWTUserConverterFacade.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/JWTUserConverterFacade.java index 54953812b1..9db515712d 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/JWTUserConverterFacade.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/authentication/JWTUserConverterFacade.java @@ -8,8 +8,10 @@ import gov.healthit.chpl.auth.jwt.JWTConsumer; import gov.healthit.chpl.auth.user.JWTAuthenticatedUser; import gov.healthit.chpl.dao.auth.UserDAO; +import gov.healthit.chpl.domain.auth.User; import gov.healthit.chpl.exception.JWTValidationException; import gov.healthit.chpl.exception.MultipleUserAccountsException; +import gov.healthit.chpl.exception.UserRetrievalException; import gov.healthit.chpl.user.cognito.CognitoApiWrapper; import lombok.extern.log4j.Log4j2; @@ -38,7 +40,21 @@ public JWTAuthenticatedUser getAuthenticatedUser(String jwt) throws JWTValidatio //If SSO is on, try to validate the jwt using the Cognito converter if (ff4j.check(FeatureList.SSO)) { user = cognitoJwtUserConverter.getAuthenticatedUser(jwt); - } else { + if (user != null) { + try { + //Set some values not avail in the Cognito Access Token that were avail in the CHPL token + User cognitoUser = cognitoApiWrapper.getUserInfo(user.getCognitoId()); + user.setEmail(cognitoUser.getEmail()); + user.setSubjectName(cognitoUser.getEmail()); + user.setFullName(cognitoUser.getFullName()); + } catch (UserRetrievalException e) { + throw new JWTValidationException("Could not locate the Cognito user id"); + } + } + } + + //If SSO is off or jwt cannot be converted using the Cognito converter, use the CHP converter + if (user == null) { user = chplJwtUserConverter.getAuthenticatedUser(jwt); } return user; From d2f6209d350dd7f36d11fa4abd5e34fccdad5d7c Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Fri, 15 Nov 2024 15:56:55 -0500 Subject: [PATCH 07/10] feat: Leave non-editable removed criteria out of CSV if user is ACB When reviewing the checklist of things that should happen when a9 gets removed, I realized that certain criteria are considered "editable" - that is if they have been removed less than 1 year ago. I was including ALL criteria in the listing CSV File but I think that might be confusing for users actually because they would see some "original" criteria that actually could not be added anyway. This commit changes the criteria included in the download file - only those that are 1) attested or 2) editable to the user are included. If ADMIN/ONC gets the CSV file then they do get all the original criteria because they can do pretty much whatever they want. [#OCD-4705] --- .../CertificationCriteriaManager.java | 21 ++++++++++++++----- .../csv/ListingCsvDataWriter.java | 2 +- .../csv/ListingCsvHeadingWriter.java | 2 +- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certificationCriteria/CertificationCriteriaManager.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certificationCriteria/CertificationCriteriaManager.java index b5f00a2406..77ae8be90f 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certificationCriteria/CertificationCriteriaManager.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certificationCriteria/CertificationCriteriaManager.java @@ -16,6 +16,7 @@ import gov.healthit.chpl.certificationCriteria.CertificationCriterionWithAttributes.AllowedAttributes; import gov.healthit.chpl.dao.CertificationCriterionDAO; import gov.healthit.chpl.domain.CertifiedProductSearchDetails; +import gov.healthit.chpl.permissions.ResourcePermissionsFactory; import gov.healthit.chpl.util.CertificationResultRules; import gov.healthit.chpl.util.DateUtil; import lombok.extern.log4j.Log4j2; @@ -26,14 +27,17 @@ public class CertificationCriteriaManager { private CertificationCriterionDAO certificationCriterionDao; private CertificationResultRules rules; private CertificationCriterionComparator criterionComparator; + private ResourcePermissionsFactory resourcePermissionsFactory; @Autowired public CertificationCriteriaManager(CertificationCriterionDAO certificationCriterionDao, CertificationResultRules rules, - CertificationCriterionComparator criterionComparator) { + CertificationCriterionComparator criterionComparator, + ResourcePermissionsFactory resourcePermissionsFactory) { this.certificationCriterionDao = certificationCriterionDao; this.rules = rules; this.criterionComparator = criterionComparator; + this.resourcePermissionsFactory = resourcePermissionsFactory; } @Transactional @@ -65,10 +69,10 @@ public List getActive(String certificationEdition, Local .collect(Collectors.toList()); } - public List getCriteriaAvailableToListing(CertifiedProductSearchDetails listing) { + public List getCriteriaAvailableToListingAndUser(CertifiedProductSearchDetails listing) { List allCriteriaAvailableToListing = Stream.concat( getAttestedCriteria(listing).stream(), - getActiveCriteriaBasedOnEditionAndDate(listing).stream()) + getEditableCriteria(listing).stream()) .distinct() .sorted(criterionComparator) .collect(Collectors.toList()); @@ -82,11 +86,18 @@ private List getAttestedCriteria(CertifiedProductSearchD .collect(Collectors.toList()); } - private List getActiveCriteriaBasedOnEditionAndDate(CertifiedProductSearchDetails listing) { + private List getEditableCriteria(CertifiedProductSearchDetails listing) { String edition = listing.getEdition() != null && !StringUtils.isEmpty(listing.getEdition().getName()) ? listing.getEdition().getName() : null; - return this.getActive(edition, listing.getCertificationDay(), LocalDate.now()); + List allActiveCriteria = this.getActive(edition, listing.getCertificationDay(), LocalDate.now()); + if (!resourcePermissionsFactory.get().isUserRoleAdmin() + && !resourcePermissionsFactory.get().isUserRoleOnc()) { + return allActiveCriteria.stream() + .filter(criterion -> criterion.isEditable()) + .collect(Collectors.toList()); + } + return allActiveCriteria; } @Transactional diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/csv/ListingCsvDataWriter.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/csv/ListingCsvDataWriter.java index 5aa450cea5..9491d19a0f 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/csv/ListingCsvDataWriter.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/csv/ListingCsvDataWriter.java @@ -141,7 +141,7 @@ public List> getCsvData(CertifiedProductSearchDetails listing, int } private List getAllAvailableCriteriaAsCertResults(CertifiedProductSearchDetails listing) { - List allCriteriaAvailableToListing = criteriaManager.getCriteriaAvailableToListing(listing); + List allCriteriaAvailableToListing = criteriaManager.getCriteriaAvailableToListingAndUser(listing); List allAvailableCriteriaAsCertResults = new ArrayList(); allCriteriaAvailableToListing.stream() .forEach(criterion -> { diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/csv/ListingCsvHeadingWriter.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/csv/ListingCsvHeadingWriter.java index 1ab6112565..82279d4392 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/csv/ListingCsvHeadingWriter.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/csv/ListingCsvHeadingWriter.java @@ -111,7 +111,7 @@ public List getCsvHeadings(CertifiedProductSearchDetails listing) { private List getCriteriaHeadings(CertifiedProductSearchDetails listing) { List criteriaHeadings = new ArrayList(); - List allCriteriaAvailableToListing = criteriaManager.getCriteriaAvailableToListing(listing); + List allCriteriaAvailableToListing = criteriaManager.getCriteriaAvailableToListingAndUser(listing); allCriteriaAvailableToListing.stream() .forEach(certResult -> criteriaHeadings.addAll(getCriterionHeadings(certResult))); return criteriaHeadings; From 3a6891ac47933c8aab0ccf7122c01ea9293d1acf Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Fri, 15 Nov 2024 16:06:22 -0500 Subject: [PATCH 08/10] test: fix test compilation [#OCD-4705] --- .../CertificationCriteriaManagerTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/chpl/chpl-service/src/test/java/gov/healthit/chpl/certificationCriteria/CertificationCriteriaManagerTest.java b/chpl/chpl-service/src/test/java/gov/healthit/chpl/certificationCriteria/CertificationCriteriaManagerTest.java index f1e19998ec..1bf6dd7a85 100644 --- a/chpl/chpl-service/src/test/java/gov/healthit/chpl/certificationCriteria/CertificationCriteriaManagerTest.java +++ b/chpl/chpl-service/src/test/java/gov/healthit/chpl/certificationCriteria/CertificationCriteriaManagerTest.java @@ -12,6 +12,7 @@ import gov.healthit.chpl.dao.CertificationCriterionDAO; import gov.healthit.chpl.exception.EntityRetrievalException; +import gov.healthit.chpl.permissions.ResourcePermissionsFactory; import gov.healthit.chpl.util.CertificationResultRules; public class CertificationCriteriaManagerTest { @@ -54,7 +55,8 @@ public void setup() throws EntityRetrievalException { .endDay(LocalDate.parse("2024-01-01")) .build()) .toList()); - manager = new CertificationCriteriaManager(certificationCriterionDao, rules, criterionComparator); + ResourcePermissionsFactory resourcePermissionsFactory = Mockito.mock(ResourcePermissionsFactory.class); + manager = new CertificationCriteriaManager(certificationCriterionDao, rules, criterionComparator, resourcePermissionsFactory); } @Test From 44552236bb6a54bdcb788ca046eccd2ede008a1a Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 18 Nov 2024 14:01:39 -0500 Subject: [PATCH 09/10] fix: Ensure some reviewers run regardless of current listing status Certification Status reviewer must be run regardless of the (updated) status of the listing. If the "Active" status is removed, the updated listing is no longer Active but validation should still be run as that may have been the only status. [#OCD-4705] --- .../healthit/chpl/auth/jwt/JWTConsumer.java | 2 +- .../listing/AllowedListingValidator.java | 28 +++++++++---------- .../listing/Edition2015ListingValidator.java | 9 ++++-- .../chpl/validation/listing/Validator.java | 9 +++++- .../reviewer/CertificationStatusReviewer.java | 2 ++ .../edition2015/RequiredData2015Reviewer.java | 3 -- 6 files changed, 32 insertions(+), 21 deletions(-) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/jwt/JWTConsumer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/jwt/JWTConsumer.java index c72ca9b415..c6de42edd9 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/jwt/JWTConsumer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/auth/jwt/JWTConsumer.java @@ -43,7 +43,7 @@ public Map consume(String jwt) throws InvalidJwtException { JwtClaims jwtClaims = jwtConsumer.processToClaims(jwt); return jwtClaims.getClaimsMap(); } catch (InvalidJwtException e) { - LOGGER.error("Invalid JWT"); + LOGGER.error("Invalid JWT", e); throw e; } } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/AllowedListingValidator.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/AllowedListingValidator.java index 0d962f4af8..ce2fdfe02c 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/AllowedListingValidator.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/AllowedListingValidator.java @@ -7,36 +7,36 @@ import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Component; +import gov.healthit.chpl.validation.listing.reviewer.CertificationStatusReviewer; import gov.healthit.chpl.validation.listing.reviewer.ComparisonReviewer; import gov.healthit.chpl.validation.listing.reviewer.DeveloperStatusReviewer; import gov.healthit.chpl.validation.listing.reviewer.Reviewer; -/** - * A concrete validation implementation that does not check for any listing errors. - * @author kekey - * - */ @Component public class AllowedListingValidator extends Validator { - private DeveloperStatusReviewer devStatusReviewer; - + private List reviewersToAlwaysCheck; private List reviewers; private List comparisonReviewers; @Autowired - public AllowedListingValidator(@Qualifier("developerStatusReviewer") DeveloperStatusReviewer devStatusReviewer) { - this.devStatusReviewer = devStatusReviewer; - comparisonReviewers = new ArrayList(); + public AllowedListingValidator(@Qualifier("developerStatusReviewer") DeveloperStatusReviewer devStatusReviewer, + @Qualifier("certificationStatusReviewer") CertificationStatusReviewer certStatusReviewer) { + + this.reviewersToAlwaysCheck = new ArrayList(); + this.reviewersToAlwaysCheck.add(devStatusReviewer); + this.reviewersToAlwaysCheck.add(certStatusReviewer); + this.reviewers = new ArrayList(); + this.comparisonReviewers = new ArrayList(); } public List getReviewers() { - if (reviewers == null) { - reviewers = new ArrayList(); - reviewers.add(devStatusReviewer); - } return reviewers; } + public List getReviewersToAlwaysCheck() { + return reviewersToAlwaysCheck; + } + public List getComparisonReviewers() { return comparisonReviewers; } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/Edition2015ListingValidator.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/Edition2015ListingValidator.java index 19bf0caa56..b35caedb5b 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/Edition2015ListingValidator.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/Edition2015ListingValidator.java @@ -2,6 +2,8 @@ import java.util.ArrayList; import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; @@ -255,11 +257,15 @@ public class Edition2015ListingValidator extends Validator { @Autowired private CodeSetAsOfTodayReviewer codeSetReviewer; + @Override + public synchronized List getReviewersToAlwaysCheck() { + return Stream.of(devStatusReviewer, certStatusReviewer).collect(Collectors.toList()); + } + @Override public synchronized List getReviewers() { List reviewers = new ArrayList(); reviewers.add(chplNumberFormatReviewer); - reviewers.add(devStatusReviewer); reviewers.add(unsupportedCharacterReviewer); reviewers.add(fieldLengthReviewer); reviewers.add(requiredDataReviewer); @@ -271,7 +277,6 @@ public synchronized List getReviewers() { reviewers.add(sedReviewer); reviewers.add(ucdProcessReviewer); reviewers.add(oldCriteriaWithoutIcsReviewer); - reviewers.add(certStatusReviewer); reviewers.add(certDateReviewer); reviewers.add(optionalStandardReviewer); reviewers.add(conformanceMethodReviewer); diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/Validator.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/Validator.java index b6210f5678..420a9d23e9 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/Validator.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/Validator.java @@ -13,6 +13,7 @@ public abstract class Validator { public abstract List getReviewers(); + public abstract List getReviewersToAlwaysCheck(); public abstract List getComparisonReviewers(); public abstract List getComparisonReviewersToAlwaysCheck(); @@ -31,13 +32,19 @@ public synchronized void validate(CertifiedProductSearchDetails listing) { } } } + + //there are some reviewers we need to always run, no matter the listing status + for (Reviewer reviewer : getReviewersToAlwaysCheck()) { + reviewer.review(listing); + } } public void validate(CertifiedProductSearchDetails existingListing, CertifiedProductSearchDetails updatedListing) { + validate(updatedListing); + //validation only runs if the listing is Active. A user must be able to set a listing with errors to Withdrawn //without having to make changes to get it to be "valid" if (CollectionUtils.isEmpty(updatedListing.getCertificationEvents()) || updatedListing.isCertificateActive()) { - validate(updatedListing); for (ComparisonReviewer reviewer : getComparisonReviewers()) { reviewer.review(existingListing, updatedListing); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/CertificationStatusReviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/CertificationStatusReviewer.java index 0360d298a9..57e1a7e18b 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/CertificationStatusReviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/CertificationStatusReviewer.java @@ -32,6 +32,8 @@ public void review(CertifiedProductSearchDetails listing) { "listing.firstStatusNotActive", CertificationStatusType.Active.getName()); listing.addBusinessErrorMessage(msg); } + } else { + listing.addDataErrorMessage(msgUtil.getMessage("listing.noStatusProvided")); } doStatusesRepeat(listing); } diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/RequiredData2015Reviewer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/RequiredData2015Reviewer.java index d2e6c92ead..cf7348ff0d 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/RequiredData2015Reviewer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/validation/listing/reviewer/edition2015/RequiredData2015Reviewer.java @@ -175,9 +175,6 @@ private void reviewRequiredFieldsCommonToAllListings(CertifiedProductSearchDetai if (listing.getVersion() == null || StringUtils.isEmpty(listing.getVersion().getVersion())) { listing.addBusinessErrorMessage("A product version is required."); } - if (listing.getOldestStatus() == null) { - listing.addDataErrorMessage(msgUtil.getMessage("listing.noStatusProvided")); - } for (CertificationResult cert : listing.getCertificationResults()) { if (BooleanUtils.isTrue(cert.getSuccess()) From 58683fc4abe5aaf4ad369e661b6cd35cbfb23252 Mon Sep 17 00:00:00 2001 From: Katy Ekey Date: Mon, 18 Nov 2024 16:21:13 -0500 Subject: [PATCH 10/10] feat: Set listing status event reason to "null" if empty [#OCD-4705] --- .../CertificationStatusEventsService.java | 33 ++----------------- .../normalizer/ListingDetailsNormalizer.java | 9 +++++ 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/service/CertificationStatusEventsService.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/service/CertificationStatusEventsService.java index b84ac979c5..af6bf08ef4 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/service/CertificationStatusEventsService.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/certifiedproduct/service/CertificationStatusEventsService.java @@ -6,7 +6,6 @@ import java.util.function.Predicate; import java.util.stream.Collectors; -import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -62,11 +61,6 @@ public List getAddedCertificationStatusEvents(Certifie return subtractLists(updatedListing.getCertificationEvents(), existingListing.getCertificationEvents()); } - public List getAddedCertificationStatusEventsIgnoringReasonUpdates(CertifiedProductSearchDetails existingListing, - CertifiedProductSearchDetails updatedListing) { - return subtractListsIgnoringReasonUpdates(updatedListing.getCertificationEvents(), existingListing.getCertificationEvents()); - } - public List getRemovedCertificationStatusEvents(CertifiedProductSearchDetails existingListing, CertifiedProductSearchDetails updatedListing) { return subtractLists(existingListing.getCertificationEvents(), updatedListing.getCertificationEvents()); @@ -81,32 +75,11 @@ private List subtractLists(List subtractListsIgnoringReasonUpdates(List listA, List listB) { - Predicate notInListB = eventFromA -> !listB.stream() - .anyMatch(event -> doValuesMatchIgnoringReason(eventFromA, event)); - - return listA.stream() - .filter(notInListB) - .collect(Collectors.toList()); - } - private boolean doValuesMatch(CertificationStatusEvent event1, CertificationStatusEvent event2) { - return ((event1.getStatus() != null && event2.getStatus() != null - && StringUtils.equals(event1.getStatus().getName(), event2.getStatus().getName())) - || (ObjectUtils.allNotNull(event1.getCertificationStatusId(), event2.getCertificationStatusId()) - && Objects.equal(event1.getCertificationStatusId(), event2.getCertificationStatusId()))) - && (Objects.equal(event1.getEventDay(), event2.getEventDay()) - || Objects.equal(event1.getEventDate(), event2.getEventDate())) - && StringUtils.equalsIgnoreCase(event1.getReason(), event2.getReason()); - } - - private boolean doValuesMatchIgnoringReason(CertificationStatusEvent event1, CertificationStatusEvent event2) { - return ((event1.getStatus() != null && event2.getStatus() != null + return (event1.getStatus() != null && event2.getStatus() != null && StringUtils.equals(event1.getStatus().getName(), event2.getStatus().getName())) - || (ObjectUtils.allNotNull(event1.getCertificationStatusId(), event2.getCertificationStatusId()) - && Objects.equal(event1.getCertificationStatusId(), event2.getCertificationStatusId()))) - && (Objects.equal(event1.getEventDay(), event2.getEventDay()) - || Objects.equal(event1.getEventDate(), event2.getEventDate())); + && Objects.equal(event1.getEventDay(), event2.getEventDay()) + && StringUtils.equals(event1.getReason(), event2.getReason()); } private CertificationStatusEvent createCertificationStatusEvent(CertificationStatusEvent certStatusEvent) { diff --git a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/normalizer/ListingDetailsNormalizer.java b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/normalizer/ListingDetailsNormalizer.java index b7d0170b6a..4e920e619e 100644 --- a/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/normalizer/ListingDetailsNormalizer.java +++ b/chpl/chpl-service/src/main/java/gov/healthit/chpl/upload/listing/normalizer/ListingDetailsNormalizer.java @@ -88,6 +88,15 @@ public void normalize(CertifiedProductSearchDetails listing) { } private void setEmptyStringFieldsToNull(CertifiedProductSearchDetails listing) { + if (!CollectionUtils.isEmpty(listing.getCertificationEvents())) { + listing.getCertificationEvents().stream() + .forEach(event -> { + if (StringUtils.isEmpty(event.getReason())) { + event.setReason(null); + } + }); + } + if (StringUtils.isEmpty(listing.getSvapNoticeUrl())) { listing.setSvapNoticeUrl(null); }