From 53b9faff247d14bcbfab95a03d3bfd577eebd73d Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Thu, 27 Aug 2026 17:07:11 -0400 Subject: [PATCH 1/2] Add fixed course end date certificate variable --- .../api/CertificateDefinition.java | 2 + .../certification/api/CertificateService.java | 22 ++++ .../certification/api/VariableResolver.java | 1 + .../certification/Messages.properties | 1 + .../impl/AbstractVariableResolver.java | 10 +- .../impl/AwardVariableResolver.java | 22 ++++ .../CertificateServiceHibernateImpl.java | 11 ++ .../impl/AwardVariableResolverTest.java | 70 ++++++++++ .../CertificateDefinitionMappingTest.java | 72 +++++++++++ .../impl/CertificateDefinition.hbm.xml | 1 + tool/pom.xml | 5 + .../certification/tool/Messages.properties | 4 + .../tool/CertificateEditController.java | 26 +++- .../tool/util/CertificateToolState.java | 33 ++++- .../CertificateDefinitionValidator.java | 14 ++ .../CertificateDefinitionValidatorTest.java | 122 ++++++++++++++++++ tool/src/webapp/jsp/createCertificateFour.jsp | 10 ++ tool/src/webapp/jsp/createCertificateOne.jsp | 16 +++ 18 files changed, 439 insertions(+), 3 deletions(-) create mode 100644 impl/src/test/java/org/sakaiproject/certification/impl/AwardVariableResolverTest.java create mode 100644 impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionMappingTest.java create mode 100644 tool/src/test/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidatorTest.java diff --git a/api/src/java/org/sakaiproject/certification/api/CertificateDefinition.java b/api/src/java/org/sakaiproject/certification/api/CertificateDefinition.java index ed2bd412ef7..a2500573b39 100644 --- a/api/src/java/org/sakaiproject/certification/api/CertificateDefinition.java +++ b/api/src/java/org/sakaiproject/certification/api/CertificateDefinition.java @@ -16,6 +16,7 @@ package org.sakaiproject.certification.api; +import java.time.LocalDate; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -51,6 +52,7 @@ public class CertificateDefinition { protected String description; protected String siteId; protected String expiryOffset; + protected LocalDate courseEndDate; protected Date createDate; /** * The status of a CertificateDefinition is one of: diff --git a/api/src/java/org/sakaiproject/certification/api/CertificateService.java b/api/src/java/org/sakaiproject/certification/api/CertificateService.java index bdf65145c8d..dbd6af2178c 100644 --- a/api/src/java/org/sakaiproject/certification/api/CertificateService.java +++ b/api/src/java/org/sakaiproject/certification/api/CertificateService.java @@ -17,6 +17,7 @@ package org.sakaiproject.certification.api; import java.io.InputStream; +import java.time.LocalDate; import java.util.Collection; import java.util.Date; import java.util.List; @@ -111,6 +112,27 @@ public CertificateDefinition createCertificateDefinition(String name, String des String mimeType, InputStream template) throws IdUsedException, UnsupportedTemplateTypeException, DocumentTemplateException; + /** + * Creates a new certificate definition with an optional date-only course end date. + * + * @param name the name of the certificate + * @param description a description of the certificate + * @param siteId the containing site + * @param progressHidden specifies whether site members can view their progress towards earning this certificate + * @param courseEndDate the course convening end date, or null if it is not configured + * @param fileName the filename associated with the template file + * @param mimeType the mimetype for the template file + * @param template an input stream containing the contents of the template file + * @return the new certificate definition + * @throws IdUsedException if the certificate name is already used in the site + * @throws UnsupportedTemplateTypeException if the template type is unsupported + * @throws DocumentTemplateException if the template cannot be stored + */ + public CertificateDefinition createCertificateDefinition(String name, String description, String siteId, + Boolean progressHidden, LocalDate courseEndDate, + String fileName, String mimeType, InputStream template) + throws IdUsedException, UnsupportedTemplateTypeException, DocumentTemplateException; + /** * Populates the DocumentTemplate object for this CertificateDefinition. * diff --git a/api/src/java/org/sakaiproject/certification/api/VariableResolver.java b/api/src/java/org/sakaiproject/certification/api/VariableResolver.java index 7798ef6d94e..ddbc00cfdfa 100644 --- a/api/src/java/org/sakaiproject/certification/api/VariableResolver.java +++ b/api/src/java/org/sakaiproject/certification/api/VariableResolver.java @@ -27,6 +27,7 @@ public interface VariableResolver { public static final String LAST_NAME = "recipient.lastname"; public static final String CERT_EXPIREDATE = "cert.expiredate"; public static final String CERT_AWARDDATE = "cert.date"; + public static final String CERT_ENDDATE = "cert.enddate"; public Set getVariableLabels(); diff --git a/impl/src/bundle/org/sakaiproject/certification/Messages.properties b/impl/src/bundle/org/sakaiproject/certification/Messages.properties index 8b0c07522dd..63500f73c39 100644 --- a/impl/src/bundle/org/sakaiproject/certification/Messages.properties +++ b/impl/src/bundle/org/sakaiproject/certification/Messages.properties @@ -47,6 +47,7 @@ variable.firstname=first name of the recipient variable.lastname=last name of the recipient variable.expiration=expiration date variable.issuedate=date of award +variable.courseEndDate=course end date variable.unassigned=unassigned report.table.header.duedate=Due Date for {0} diff --git a/impl/src/java/org/sakaiproject/certification/impl/AbstractVariableResolver.java b/impl/src/java/org/sakaiproject/certification/impl/AbstractVariableResolver.java index fabdad8b478..8874da9860f 100644 --- a/impl/src/java/org/sakaiproject/certification/impl/AbstractVariableResolver.java +++ b/impl/src/java/org/sakaiproject/certification/impl/AbstractVariableResolver.java @@ -24,9 +24,17 @@ public abstract class AbstractVariableResolver implements VariableResolver { - private final ResourceLoader messages = new ResourceLoader("org.sakaiproject.certification.Messages"); + private final ResourceLoader messages; private final HashMap descriptions = new HashMap<>(); + protected AbstractVariableResolver() { + this(new ResourceLoader("org.sakaiproject.certification.Messages")); + } + + protected AbstractVariableResolver(ResourceLoader messages) { + this.messages = messages; + } + public void addVariable (String variable, String description) { descriptions.put(variable, description); } diff --git a/impl/src/java/org/sakaiproject/certification/impl/AwardVariableResolver.java b/impl/src/java/org/sakaiproject/certification/impl/AwardVariableResolver.java index 29862492f51..9012627a71c 100644 --- a/impl/src/java/org/sakaiproject/certification/impl/AwardVariableResolver.java +++ b/impl/src/java/org/sakaiproject/certification/impl/AwardVariableResolver.java @@ -16,24 +16,46 @@ package org.sakaiproject.certification.impl; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; +import java.time.format.FormatStyle; + import org.sakaiproject.certification.api.CertificateDefinition; import org.sakaiproject.certification.api.VariableResolutionException; +import org.sakaiproject.util.ResourceLoader; public class AwardVariableResolver extends AbstractVariableResolver { private static final String MESSAGE_NAMEOFCERT = "variable.nameOfCert"; + private static final String MESSAGE_COURSE_END_DATE = "variable.courseEndDate"; private static final String MESSAGE_UNASSIGNED = "variable.unassigned"; public AwardVariableResolver() { + this(new ResourceLoader("org.sakaiproject.certification.Messages")); + } + + AwardVariableResolver(ResourceLoader messages) { + super(messages); String name = getMessages().getString(MESSAGE_NAMEOFCERT); + String courseEndDate = getMessages().getString(MESSAGE_COURSE_END_DATE); String unassigned = getMessages().getString(MESSAGE_UNASSIGNED); addVariable(CERT_NAME, name); + addVariable(CERT_ENDDATE, courseEndDate); addVariable (UNASSIGNED, unassigned); } public String getValue(CertificateDefinition certDef, String varLabel, String userId, boolean useCaching) throws VariableResolutionException { if (CERT_NAME.equals(varLabel)) { return certDef.getName(); + } else if (CERT_ENDDATE.equals(varLabel)) { + LocalDate courseEndDate = certDef.getCourseEndDate(); + if (courseEndDate == null) { + return ""; + } + + DateTimeFormatter dateFormatter = DateTimeFormatter.ofLocalizedDate(FormatStyle.LONG) + .withLocale(getMessages().getLocale()); + return dateFormatter.format(courseEndDate); } else if (UNASSIGNED.equals(varLabel)) { return ""; } diff --git a/impl/src/java/org/sakaiproject/certification/impl/hibernate/CertificateServiceHibernateImpl.java b/impl/src/java/org/sakaiproject/certification/impl/hibernate/CertificateServiceHibernateImpl.java index ae8c965fa60..28128cf57c7 100644 --- a/impl/src/java/org/sakaiproject/certification/impl/hibernate/CertificateServiceHibernateImpl.java +++ b/impl/src/java/org/sakaiproject/certification/impl/hibernate/CertificateServiceHibernateImpl.java @@ -19,6 +19,7 @@ import java.io.File; import java.io.InputStream; import java.text.DateFormat; +import java.time.LocalDate; import java.util.ArrayList; import java.util.Collection; import java.util.Date; @@ -234,6 +235,7 @@ public Object doInHibernate(Session session) { CertificateDefinition cdhi = (CertificateDefinition) q.list().get(0); cdhi.setName(cd.getName()); cdhi.setDescription(cd.getDescription()); + cdhi.setCourseEndDate(cd.getCourseEndDate()); cdhi.setProgressHidden(cd.getProgressHidden()); session.update(cdhi); return cdhi; @@ -250,6 +252,14 @@ public CertificateDefinition createCertificateDefinition (final String name, fin final String siteId, final Boolean progressHidden, final String fileName, final String mimeType, final InputStream template) throws IdUsedException, UnsupportedTemplateTypeException, DocumentTemplateException { + return createCertificateDefinition(name, description, siteId, progressHidden, null, fileName, mimeType, template); + } + + public CertificateDefinition createCertificateDefinition (final String name, final String description, + final String siteId, final Boolean progressHidden, + final LocalDate courseEndDate, final String fileName, + final String mimeType, final InputStream template) + throws IdUsedException, UnsupportedTemplateTypeException, DocumentTemplateException { CertificateDefinition cd = null; try { cd = (CertificateDefinition) getHibernateTemplate().execute(new HibernateCallback() { @@ -261,6 +271,7 @@ public Object doInHibernate(Session session) throws HibernateException { certificateDefinition.setDescription(description); certificateDefinition.setName(name); certificateDefinition.setSiteId(siteId); + certificateDefinition.setCourseEndDate(courseEndDate); certificateDefinition.setProgressHidden(progressHidden); certificateDefinition.setStatus(CertificateDefinitionStatus.UNPUBLISHED); session.save(certificateDefinition); diff --git a/impl/src/test/java/org/sakaiproject/certification/impl/AwardVariableResolverTest.java b/impl/src/test/java/org/sakaiproject/certification/impl/AwardVariableResolverTest.java new file mode 100644 index 00000000000..563ece480d0 --- /dev/null +++ b/impl/src/test/java/org/sakaiproject/certification/impl/AwardVariableResolverTest.java @@ -0,0 +1,70 @@ +/** + * Copyright (c) 2003-2026 The Apereo Foundation + * + * Licensed under the Educational Community License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://opensource.org/licenses/ecl2 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.sakaiproject.certification.impl; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; +import java.time.format.FormatStyle; +import java.util.Locale; + +import org.junit.Test; + +import org.sakaiproject.certification.api.CertificateDefinition; +import org.sakaiproject.certification.api.VariableResolver; +import org.sakaiproject.util.ResourceLoader; + +public class AwardVariableResolverTest { + + private static final Locale TEST_LOCALE = Locale.US; + + private final AwardVariableResolver resolver = new AwardVariableResolver(new ResourceLoader() { + @Override + public Locale getLocale() { + return TEST_LOCALE; + } + + @Override + public String getString(String key) { + return key; + } + }); + + @Test + public void exposesCourseEndDateVariable() { + assertTrue(resolver.getVariableLabels().contains(VariableResolver.CERT_ENDDATE)); + } + + @Test + public void resolvesCourseEndDateUsingCurrentLocale() throws Exception { + LocalDate courseEndDate = LocalDate.of(2026, 8, 27); + CertificateDefinition definition = new CertificateDefinition(); + definition.setCourseEndDate(courseEndDate); + String expected = DateTimeFormatter.ofLocalizedDate(FormatStyle.LONG) + .withLocale(resolver.getMessages().getLocale()) + .format(courseEndDate); + + assertEquals(expected, resolver.getValue(definition, VariableResolver.CERT_ENDDATE, "user", false)); + } + + @Test + public void resolvesUnsetCourseEndDateToEmptyString() throws Exception { + assertEquals("", resolver.getValue(new CertificateDefinition(), VariableResolver.CERT_ENDDATE, "user", false)); + } +} diff --git a/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionMappingTest.java b/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionMappingTest.java new file mode 100644 index 00000000000..9de8c8f3494 --- /dev/null +++ b/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionMappingTest.java @@ -0,0 +1,72 @@ +/** + * Copyright (c) 2003-2026 The Apereo Foundation + * + * Licensed under the Educational Community License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://opensource.org/licenses/ecl2 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.sakaiproject.certification.impl.hibernate; + +import static org.junit.Assert.assertEquals; + +import java.time.LocalDate; + +import org.hibernate.boot.Metadata; +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.mapping.PersistentClass; +import org.junit.Test; + +import org.sakaiproject.certification.api.CertificateDefinition; + +public class CertificateDefinitionMappingTest { + + private static final String MAPPING_ROOT = "org/sakaiproject/certification/impl/"; + private static final String[] MAPPING_RESOURCES = { + MAPPING_ROOT + "CertificateDefinition.hbm.xml", + MAPPING_ROOT + "DocumentTemplate.hbm.xml", + MAPPING_ROOT + "criteria/AbstractCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/GreaterThanScoreCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/DueDatePassedCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/FinalGradeScoreCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/WillExpireCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/CertAssignment.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/CertCategory.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/CertGradebook.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/CertGradeRecord.hbm.xml" + }; + + @Test + public void courseEndDateIsMappedAsLocalDate() { + StandardServiceRegistry registry = new StandardServiceRegistryBuilder() + .applySetting("hibernate.dialect", "org.hibernate.dialect.H2Dialect") + .applySetting("hibernate.connection.provider_class", + "org.hibernate.engine.jdbc.connections.internal.UserSuppliedConnectionProviderImpl") + .applySetting("hibernate.temp.use_jdbc_metadata_defaults", "false") + .build(); + + try { + MetadataSources metadataSources = new MetadataSources(registry); + for (String resource : MAPPING_RESOURCES) { + metadataSources.addResource(resource); + } + + Metadata metadata = metadataSources.buildMetadata(); + PersistentClass mapping = metadata.getEntityBinding(CertificateDefinition.class.getName()); + + assertEquals(LocalDate.class, mapping.getProperty("courseEndDate").getType().getReturnedClass()); + } finally { + StandardServiceRegistryBuilder.destroy(registry); + } + } +} diff --git a/model/src/java/org/sakaiproject/certification/impl/CertificateDefinition.hbm.xml b/model/src/java/org/sakaiproject/certification/impl/CertificateDefinition.hbm.xml index c3296370105..8c5885daba7 100644 --- a/model/src/java/org/sakaiproject/certification/impl/CertificateDefinition.hbm.xml +++ b/model/src/java/org/sakaiproject/certification/impl/CertificateDefinition.hbm.xml @@ -16,6 +16,7 @@ + diff --git a/tool/pom.xml b/tool/pom.xml index d5f2121571e..2cd0cc2b000 100644 --- a/tool/pom.xml +++ b/tool/pom.xml @@ -80,6 +80,11 @@ org.apache.commons commons-lang3 + + junit + junit + test + diff --git a/tool/src/bundle/org/sakaiproject/certification/tool/Messages.properties b/tool/src/bundle/org/sakaiproject/certification/tool/Messages.properties index cbfd652df89..fce7f32b6b7 100644 --- a/tool/src/bundle/org/sakaiproject/certification/tool/Messages.properties +++ b/tool/src/bundle/org/sakaiproject/certification/tool/Messages.properties @@ -4,6 +4,8 @@ form.error.noneselected=Please select one item before proceeding. form.error.multipleselect=Select only one item for this operation. form.error.namefield=Please provide a name for the certificate. form.error.fieldValue=Not a valid variable. +form.error.courseEndDate.required=Enter a course end date because this certificate uses the course end date variable. +typeMismatch.certificateToolState.certificateDefinition.courseEndDate=Enter a valid course end date. form.error.invalidTemplate=Invalid Template File. form.error.templateField=Please select a valid template file. form.error.templateProcessingError=An Error occurred processing the template. @@ -26,6 +28,7 @@ form.label.status=Status form.label.created=Created form.label.criteria=Requirements: form.label.description=Description +form.label.courseEndDate=Course end date form.label.field=Field in the PDF form.label.value=Value to be substituted form.label.overwrite=Overwrite unassigned @@ -61,6 +64,7 @@ form.text.criteria.remove=Remove this requeriment #form one in create process form.text.create.title=Create Certificate Definition (Step 1) form.text.create.description=Use the form below to create a new certificate. Supply a name, a description, and a file to use as the template for printable certificates for awardees. +form.text.courseEndDate.description=Optional. This fixed date can be added to the certificate by selecting the course end date variable when assigning template fields. #form two in create process form.text.fields.title=Fill In Certificate Fields (Step 2) form.text.fields.description=The template you have provided has fields which will be customized for each recipient. Fill in the values for the template fields below. Values will be replaced dynamically when the certificate is generated. diff --git a/tool/src/java/org/sakaiproject/certification/tool/CertificateEditController.java b/tool/src/java/org/sakaiproject/certification/tool/CertificateEditController.java index 1d850a1728d..ea00a0351e5 100644 --- a/tool/src/java/org/sakaiproject/certification/tool/CertificateEditController.java +++ b/tool/src/java/org/sakaiproject/certification/tool/CertificateEditController.java @@ -18,8 +18,11 @@ import com.fasterxml.jackson.databind.ObjectMapper; +import java.beans.PropertyEditorSupport; import java.io.ByteArrayOutputStream; import java.io.InputStream; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -36,6 +39,8 @@ import org.springframework.stereotype.Controller; import org.springframework.validation.BindingResult; +import org.springframework.web.bind.WebDataBinder; +import org.springframework.web.bind.annotation.InitBinder; import org.springframework.web.bind.annotation.ModelAttribute; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; @@ -113,11 +118,29 @@ public class CertificateEditController extends BaseCertificateController { private final ObjectMapper mapper = new ObjectMapper(); + private static final DateTimeFormatter ISO_LOCAL_DATE = DateTimeFormatter.ISO_LOCAL_DATE; + private final int CONSTRAINT_DESCRIPTION_LENGTH = 500; private final int CONSTRAINT_NAME_LENGTH = 255; public final int ERROR_BAD_REQUEST = 400; + @InitBinder + public void bindLocalDates(WebDataBinder binder) { + binder.registerCustomEditor(LocalDate.class, new PropertyEditorSupport() { + @Override + public void setAsText(String text) { + setValue(StringUtils.isBlank(text) ? null : LocalDate.parse(text, ISO_LOCAL_DATE)); + } + + @Override + public String getAsText() { + LocalDate value = (LocalDate) getValue(); + return value == null ? "" : ISO_LOCAL_DATE.format(value); + } + }); + } + /** * This allows other methods to use @ModelAttribute(MOD_ATTR). * Using this model attribute adds the certId request param, and populates @@ -581,7 +604,8 @@ protected ModelAndView createCertHandlerFourth(@ModelAttribute(MOD_ATTR) Certifi if (StringUtils.isEmpty(certDef.getId())) { //create a hibernate impl certificateService.createCertificateDefinition(certDef.getName(), certDef.getDescription(), - siteId(), certDef.getProgressHidden(), certificateToolState.getTemplateFilename(), certificateToolState.getTemplateMimeType(), + siteId(), certDef.getProgressHidden(), certDef.getCourseEndDate(), + certificateToolState.getTemplateFilename(), certificateToolState.getTemplateMimeType(), certificateToolState.getTemplateInputStream()); //gets the hibernateImpl diff --git a/tool/src/java/org/sakaiproject/certification/tool/util/CertificateToolState.java b/tool/src/java/org/sakaiproject/certification/tool/util/CertificateToolState.java index 36884fc4bb0..abb6f7c242e 100644 --- a/tool/src/java/org/sakaiproject/certification/tool/util/CertificateToolState.java +++ b/tool/src/java/org/sakaiproject/certification/tool/util/CertificateToolState.java @@ -18,6 +18,9 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; +import java.time.format.FormatStyle; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -37,6 +40,7 @@ import org.sakaiproject.certification.api.criteria.gradebook.WillExpireCriterion; import org.sakaiproject.tool.api.ToolSession; import org.sakaiproject.tool.cover.SessionManager; +import org.sakaiproject.util.ResourceLoader; @Slf4j public class CertificateToolState { @@ -183,6 +187,18 @@ public Map getFieldToDescription() return fieldToDesc; } + public String getFormattedCourseEndDate() { + LocalDate courseEndDate = certificateDefinition.getCourseEndDate(); + if (courseEndDate == null) { + return null; + } + + ResourceLoader resourceLoader = new ResourceLoader(); + DateTimeFormatter dateFormatter = DateTimeFormatter.ofLocalizedDate(FormatStyle.LONG) + .withLocale(resourceLoader.getLocale()); + return dateFormatter.format(courseEndDate); + } + /** * * @return a map of ${} format to description @@ -255,6 +271,9 @@ public void setPredifinedFields(Map predifinedFields) { //hate to hard code this. I'd grab it from GradebookVariableResolver, but we can't access impl String expireDate = "${cert.expiredate}"; boolean fieldValuesContainWechi = certDef.getFieldValues().values().contains(expireDate); + String courseEndDate = "${" + VariableResolver.CERT_ENDDATE + "}"; + boolean fieldValuesContainCourseEndDate = containsVariable(certDef.getFieldValues(), courseEndDate) + || containsVariable(getTemplateFields(), courseEndDate); Map temp = null; if(predifinedFields != null) { @@ -272,7 +291,14 @@ public void setPredifinedFields(Map predifinedFields) { * or if the field values contain wechi (for whatever reason). * So we add it if * key != expiry date OR criteriacontainswechi OR fieldvaluescontainwechi*/ - if (!expireDate.equals(newKey) || criteriaContainWechi || fieldValuesContainWechi) { + boolean includeField = !expireDate.equals(newKey) || criteriaContainWechi || fieldValuesContainWechi; + if (courseEndDate.equals(newKey) + && certDef.getCourseEndDate() == null + && !fieldValuesContainCourseEndDate) { + includeField = false; + } + + if (includeField) { temp.put(newKey, predifinedFields.get(key)); } } @@ -281,6 +307,11 @@ public void setPredifinedFields(Map predifinedFields) { this.predifinedFields = temp; } + private boolean containsVariable(Map fields, String variable) { + return fields != null + && (fields.containsValue(variable) || fields.containsValue(variable.substring(1))); + } + /** * @return a map from the PDF's fields to their selected values' descriptions */ diff --git a/tool/src/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidator.java b/tool/src/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidator.java index 7ac179c66a9..ab015fc1566 100644 --- a/tool/src/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidator.java +++ b/tool/src/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidator.java @@ -28,6 +28,7 @@ import org.sakaiproject.certification.api.CertificateService; import org.sakaiproject.certification.api.DocumentTemplateException; +import org.sakaiproject.certification.api.VariableResolver; import org.sakaiproject.certification.tool.util.CertificateToolState; public class CertificateDefinitionValidator { @@ -35,6 +36,14 @@ public class CertificateDefinitionValidator { private final Pattern variablePattern = Pattern.compile ("\\$\\{(.+)\\}"); public void validateFirst(CertificateToolState certificateToolState, Errors errors, CertificateService service) { + String courseEndDateVariable = "${" + VariableResolver.CERT_ENDDATE + "}"; + Map fieldValues = certificateToolState.getCertificateDefinition().getFieldValues(); + if (certificateToolState.getCertificateDefinition().getCourseEndDate() == null + && (containsVariable(fieldValues, courseEndDateVariable) + || containsVariable(certificateToolState.getTemplateFields(), courseEndDateVariable))) { + errors.rejectValue("certificateDefinition.courseEndDate", "form.error.courseEndDate.required"); + } + CommonsMultipartFile newTemplate = certificateToolState.getNewTemplate(); if (newTemplate != null && newTemplate.getSize() > 0) { if(!certificateToolState.getMimeTypes().contains( newTemplate.getContentType() )) { @@ -51,6 +60,11 @@ public void validateFirst(CertificateToolState certificateToolState, Errors erro } } + private boolean containsVariable(Map fields, String variable) { + return fields != null + && (fields.containsValue(variable) || fields.containsValue(variable.substring(1))); + } + public void validateSecond(CertificateToolState certificateToolState, Errors errors) { // The only invalid case is when the expiry date is your only criterion. // This case is handled in CertificateEditController diff --git a/tool/src/test/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidatorTest.java b/tool/src/test/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidatorTest.java new file mode 100644 index 00000000000..f211d3e124b --- /dev/null +++ b/tool/src/test/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidatorTest.java @@ -0,0 +1,122 @@ +/** + * Copyright (c) 2003-2026 The Apereo Foundation + * + * Licensed under the Educational Community License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://opensource.org/licenses/ecl2 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.sakaiproject.certification.tool.validator; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.time.LocalDate; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.junit.Test; + +import org.springframework.validation.BeanPropertyBindingResult; + +import org.sakaiproject.certification.api.CertificateDefinition; +import org.sakaiproject.certification.api.VariableResolver; +import org.sakaiproject.certification.tool.util.CertificateToolState; + +public class CertificateDefinitionValidatorTest { + + private static final String COURSE_END_DATE_VARIABLE = "${" + VariableResolver.CERT_ENDDATE + "}"; + + private final CertificateDefinitionValidator validator = new CertificateDefinitionValidator(); + + @Test + public void missingCourseEndDateIsRejectedWhenCertificateUsesVariable() { + CertificateToolState state = stateWithCourseEndDate(null); + state.getCertificateDefinition().getFieldValues().put("date", COURSE_END_DATE_VARIABLE); + BeanPropertyBindingResult errors = validateFirst(state); + + assertTrue(errors.hasFieldErrors("certificateDefinition.courseEndDate")); + } + + @Test + public void missingCourseEndDateIsRejectedWhenEscapedVariableIsPosted() { + CertificateToolState state = stateWithCourseEndDate(null); + state.getCertificateDefinition().getFieldValues().put("date", COURSE_END_DATE_VARIABLE.substring(1)); + BeanPropertyBindingResult errors = validateFirst(state); + + assertTrue(errors.hasFieldErrors("certificateDefinition.courseEndDate")); + } + + @Test + public void configuredCourseEndDateAllowsVariable() { + CertificateToolState state = stateWithCourseEndDate(LocalDate.of(2026, 8, 27)); + state.getCertificateDefinition().getFieldValues().put("date", COURSE_END_DATE_VARIABLE); + BeanPropertyBindingResult errors = validateFirst(state); + + assertFalse(errors.hasFieldErrors("certificateDefinition.courseEndDate")); + } + + @Test + public void missingCourseEndDateIsRejectedWhenInProgressMappingUsesVariable() { + CertificateToolState state = stateWithCourseEndDate(null); + state.setTemplateFields(new LinkedHashMap<>()); + state.getTemplateFields().put("date", COURSE_END_DATE_VARIABLE.substring(1)); + BeanPropertyBindingResult errors = validateFirst(state); + + assertTrue(errors.hasFieldErrors("certificateDefinition.courseEndDate")); + } + + @Test + public void courseEndDateVariableIsHiddenUntilDateIsConfigured() { + CertificateToolState state = stateWithCourseEndDate(null); + state.setPredifinedFields(predefinedFields()); + + assertFalse(state.getPredifinedFields().containsKey(COURSE_END_DATE_VARIABLE)); + + state.getCertificateDefinition().setCourseEndDate(LocalDate.of(2026, 8, 27)); + state.setPredifinedFields(predefinedFields()); + + assertTrue(state.getPredifinedFields().containsKey(COURSE_END_DATE_VARIABLE)); + } + + @Test + public void selectedCourseEndDateVariableRemainsVisibleIfDateIsCleared() { + CertificateToolState state = stateWithCourseEndDate(null); + state.setTemplateFields(new LinkedHashMap<>()); + state.getTemplateFields().put("date", COURSE_END_DATE_VARIABLE.substring(1)); + state.setPredifinedFields(predefinedFields()); + + assertTrue(state.getPredifinedFields().containsKey(COURSE_END_DATE_VARIABLE)); + } + + private BeanPropertyBindingResult validateFirst(CertificateToolState state) { + BeanPropertyBindingResult errors = new BeanPropertyBindingResult(state, "certificateToolState"); + validator.validateFirst(state, errors, null); + return errors; + } + + private CertificateToolState stateWithCourseEndDate(LocalDate courseEndDate) { + CertificateDefinition definition = new CertificateDefinition(); + definition.setCourseEndDate(courseEndDate); + + CertificateToolState state = new CertificateToolState(); + state.setCertificateDefinition(definition); + return state; + } + + private Map predefinedFields() { + Map fields = new LinkedHashMap<>(); + fields.put(VariableResolver.UNASSIGNED, "unassigned"); + fields.put(VariableResolver.CERT_AWARDDATE, "date of award"); + fields.put(VariableResolver.CERT_ENDDATE, "course end date"); + return fields; + } +} diff --git a/tool/src/webapp/jsp/createCertificateFour.jsp b/tool/src/webapp/jsp/createCertificateFour.jsp index 74aa098d411..1bb22f73e80 100644 --- a/tool/src/webapp/jsp/createCertificateFour.jsp +++ b/tool/src/webapp/jsp/createCertificateFour.jsp @@ -43,6 +43,16 @@ + +
+ + + + +
+
+
+ + + +
+ +
+ +
+ +
+
From d883682ed8c8594cfd349c9a48b2b49275cbbf43 Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Fri, 28 Aug 2026 10:13:46 -0400 Subject: [PATCH 2/2] Enforce course end date persistence invariants --- README.md | 7 +- .../api/CertificateDefinitionConstraints.java | 43 ++++ .../certification/api/CertificateService.java | 9 +- conversion/add-course-end-date.sql | 1 + impl/pom.xml | 5 + .../CertificateServiceHibernateImpl.java | 26 +- .../CertificateDefinitionMappingTest.java | 72 ------ .../CertificateDefinitionPersistenceTest.java | 243 ++++++++++++++++++ .../tool/CertificateEditController.java | 12 +- .../CertificateDefinitionValidator.java | 20 +- .../CertificateDefinitionValidatorTest.java | 32 +-- 11 files changed, 356 insertions(+), 114 deletions(-) create mode 100644 api/src/java/org/sakaiproject/certification/api/CertificateDefinitionConstraints.java create mode 100644 conversion/add-course-end-date.sql delete mode 100644 impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionMappingTest.java create mode 100644 impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionPersistenceTest.java diff --git a/README.md b/README.md index de0b7fe8b7c..fe471de23c0 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,12 @@ This is a boolean value, whose default is false, which controls whether or not s `certification.extraUserProperties.enable = true` +# Conversion (Course end date) + +When upgrading an existing installation to a version that supports the fixed course end date certificate variable, +apply [`conversion/add-course-end-date.sql`](conversion/add-course-end-date.sql) before starting Sakai if automatic +database updates are disabled. + # Conversion (Users of versions older than 12.0) Due to the tool refactor, some tables were renamed and some classes were refactored, a conversion script is required to make it work in the 12.x version and newer. @@ -44,4 +50,3 @@ The tool id has been changed for consistency: ``` UPDATE sakai_site_tool SET registration = 'sakai.certification' WHERE registration = 'com.rsmart.certification'; ``` - diff --git a/api/src/java/org/sakaiproject/certification/api/CertificateDefinitionConstraints.java b/api/src/java/org/sakaiproject/certification/api/CertificateDefinitionConstraints.java new file mode 100644 index 00000000000..3ac1e7cc3b8 --- /dev/null +++ b/api/src/java/org/sakaiproject/certification/api/CertificateDefinitionConstraints.java @@ -0,0 +1,43 @@ +/** + * Copyright (c) 2003-2026 The Apereo Foundation + * + * Licensed under the Educational Community License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://opensource.org/licenses/ecl2 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.sakaiproject.certification.api; + +import java.time.LocalDate; +import java.util.Map; + +/** + * Cross-field constraints for certificate definitions. + */ +public final class CertificateDefinitionConstraints { + + private static final String COURSE_END_DATE_VARIABLE = "${" + VariableResolver.CERT_ENDDATE + "}"; + private static final String ESCAPED_COURSE_END_DATE_VARIABLE = COURSE_END_DATE_VARIABLE.substring(1); + + private CertificateDefinitionConstraints() { + } + + public static boolean isCourseEndDateConfigurationValid(LocalDate courseEndDate, + Map fieldValues) { + return courseEndDate != null || !usesCourseEndDateVariable(fieldValues); + } + + public static boolean usesCourseEndDateVariable(Map fieldValues) { + return fieldValues != null + && (fieldValues.containsValue(COURSE_END_DATE_VARIABLE) + || fieldValues.containsValue(ESCAPED_COURSE_END_DATE_VARIABLE)); + } +} diff --git a/api/src/java/org/sakaiproject/certification/api/CertificateService.java b/api/src/java/org/sakaiproject/certification/api/CertificateService.java index dbd6af2178c..6b779f866be 100644 --- a/api/src/java/org/sakaiproject/certification/api/CertificateService.java +++ b/api/src/java/org/sakaiproject/certification/api/CertificateService.java @@ -52,8 +52,11 @@ public interface CertificateService { * @param cd * @return * @throws IdUnusedException + * @throws IncompleteCertificateDefinitionException if the course end date variable is mapped without a course + * end date */ - public CertificateDefinition updateCertificateDefinition (CertificateDefinition cd) throws IdUnusedException; + public CertificateDefinition updateCertificateDefinition(CertificateDefinition cd) + throws IdUnusedException, IncompleteCertificateDefinitionException; public void setDocumentTemplateService (DocumentTemplateService dts); @@ -180,9 +183,11 @@ public InputStream getTemplateFileInputStream(String resourceId) * @param certificateDefinitionId * @param fieldValues * @throws IdUnusedException + * @throws IncompleteCertificateDefinitionException if the course end date variable is mapped without a course + * end date */ public void setFieldValues(String certificateDefinitionId, Map fieldValues) - throws IdUnusedException; + throws IdUnusedException, IncompleteCertificateDefinitionException; /** * This sets the CertificateDefinitionStatus to ACTIVE or INACTIVE depending on the value of the boolean 'active' diff --git a/conversion/add-course-end-date.sql b/conversion/add-course-end-date.sql new file mode 100644 index 00000000000..cb36b88572b --- /dev/null +++ b/conversion/add-course-end-date.sql @@ -0,0 +1 @@ +ALTER TABLE certificate_definition ADD course_end_date DATE; diff --git a/impl/pom.xml b/impl/pom.xml index c67a2054edd..b5fd16c24fb 100644 --- a/impl/pom.xml +++ b/impl/pom.xml @@ -80,6 +80,11 @@ junit test + + org.hsqldb + hsqldb + test + javax.servlet javax.servlet-api diff --git a/impl/src/java/org/sakaiproject/certification/impl/hibernate/CertificateServiceHibernateImpl.java b/impl/src/java/org/sakaiproject/certification/impl/hibernate/CertificateServiceHibernateImpl.java index 28128cf57c7..9620b4d4927 100644 --- a/impl/src/java/org/sakaiproject/certification/impl/hibernate/CertificateServiceHibernateImpl.java +++ b/impl/src/java/org/sakaiproject/certification/impl/hibernate/CertificateServiceHibernateImpl.java @@ -54,6 +54,7 @@ import org.sakaiproject.authz.api.SecurityAdvisor; import org.sakaiproject.authz.api.SecurityService; import org.sakaiproject.certification.api.CertificateDefinition; +import org.sakaiproject.certification.api.CertificateDefinitionConstraints; import org.sakaiproject.certification.api.CertificateDefinitionStatus; import org.sakaiproject.certification.api.CertificateService; import org.sakaiproject.certification.api.DocumentTemplate; @@ -221,7 +222,10 @@ public Object doInHibernate(Session session) throws HibernateException { deleteTemplateFile(cd.getDocumentTemplate().getResourceId()); } - public CertificateDefinition updateCertificateDefinition(final CertificateDefinition cd) throws IdUnusedException { + public CertificateDefinition updateCertificateDefinition(final CertificateDefinition cd) + throws IdUnusedException, IncompleteCertificateDefinitionException { + validateCourseEndDateConfiguration(cd.getCourseEndDate(), cd.getFieldValues()); + CertificateDefinition retVal = null; if (cd instanceof CertificateDefinition) { retVal = (CertificateDefinition) cd; @@ -236,6 +240,7 @@ public Object doInHibernate(Session session) { cdhi.setName(cd.getName()); cdhi.setDescription(cd.getDescription()); cdhi.setCourseEndDate(cd.getCourseEndDate()); + cdhi.setFieldValues(copyFieldValues(cd.getFieldValues())); cdhi.setProgressHidden(cd.getProgressHidden()); session.update(cdhi); return cdhi; @@ -548,14 +553,17 @@ private Object doSecureCertificateService(SecureCertificateServiceCallback callb } } - public void setFieldValues(String certificateDefinitionId, Map fieldValues) throws IdUnusedException { + public void setFieldValues(String certificateDefinitionId, Map fieldValues) + throws IdUnusedException, IncompleteCertificateDefinitionException { CertificateDefinition cd = (CertificateDefinition)getCertificateDefinition(certificateDefinitionId); - cd.setFieldValues(fieldValues); + validateCourseEndDateConfiguration(cd.getCourseEndDate(), fieldValues); + cd.setFieldValues(copyFieldValues(fieldValues)); getHibernateTemplate().update(cd); } public void activateCertificateDefinition(String certificateDefinitionId, boolean active) throws IncompleteCertificateDefinitionException, IdUnusedException { CertificateDefinition cd = (CertificateDefinition)getCertificateDefinition(certificateDefinitionId); + validateCourseEndDateConfiguration(cd.getCourseEndDate(), cd.getFieldValues()); if (cd.getDocumentTemplate() == null || cd.getName() == null || cd.getAwardCriteria() == null || cd.getFieldValues() == null) { throw new IncompleteCertificateDefinitionException ("incomplete certificate definition"); @@ -565,6 +573,18 @@ public void activateCertificateDefinition(String certificateDefinitionId, boolea getHibernateTemplate().update(cd); } + private void validateCourseEndDateConfiguration(LocalDate courseEndDate, Map fieldValues) + throws IncompleteCertificateDefinitionException { + if (!CertificateDefinitionConstraints.isCourseEndDateConfigurationValid(courseEndDate, fieldValues)) { + throw new IncompleteCertificateDefinitionException( + "course end date is required when the course end date variable is mapped"); + } + } + + private Map copyFieldValues(Map fieldValues) { + return fieldValues == null ? null : new HashMap<>(fieldValues); + } + private void setCriteriaFactoryOnCriteria(CertificateDefinition certDef) { Set criteria = certDef.getAwardCriteria(); if (criteria != null) { diff --git a/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionMappingTest.java b/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionMappingTest.java deleted file mode 100644 index 9de8c8f3494..00000000000 --- a/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionMappingTest.java +++ /dev/null @@ -1,72 +0,0 @@ -/** - * Copyright (c) 2003-2026 The Apereo Foundation - * - * Licensed under the Educational Community License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://opensource.org/licenses/ecl2 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.sakaiproject.certification.impl.hibernate; - -import static org.junit.Assert.assertEquals; - -import java.time.LocalDate; - -import org.hibernate.boot.Metadata; -import org.hibernate.boot.MetadataSources; -import org.hibernate.boot.registry.StandardServiceRegistry; -import org.hibernate.boot.registry.StandardServiceRegistryBuilder; -import org.hibernate.mapping.PersistentClass; -import org.junit.Test; - -import org.sakaiproject.certification.api.CertificateDefinition; - -public class CertificateDefinitionMappingTest { - - private static final String MAPPING_ROOT = "org/sakaiproject/certification/impl/"; - private static final String[] MAPPING_RESOURCES = { - MAPPING_ROOT + "CertificateDefinition.hbm.xml", - MAPPING_ROOT + "DocumentTemplate.hbm.xml", - MAPPING_ROOT + "criteria/AbstractCriterion.hbm.xml", - MAPPING_ROOT + "criteria/gradebook/GreaterThanScoreCriterion.hbm.xml", - MAPPING_ROOT + "criteria/gradebook/DueDatePassedCriterion.hbm.xml", - MAPPING_ROOT + "criteria/gradebook/FinalGradeScoreCriterion.hbm.xml", - MAPPING_ROOT + "criteria/gradebook/WillExpireCriterion.hbm.xml", - MAPPING_ROOT + "criteria/gradebook/CertAssignment.hbm.xml", - MAPPING_ROOT + "criteria/gradebook/CertCategory.hbm.xml", - MAPPING_ROOT + "criteria/gradebook/CertGradebook.hbm.xml", - MAPPING_ROOT + "criteria/gradebook/CertGradeRecord.hbm.xml" - }; - - @Test - public void courseEndDateIsMappedAsLocalDate() { - StandardServiceRegistry registry = new StandardServiceRegistryBuilder() - .applySetting("hibernate.dialect", "org.hibernate.dialect.H2Dialect") - .applySetting("hibernate.connection.provider_class", - "org.hibernate.engine.jdbc.connections.internal.UserSuppliedConnectionProviderImpl") - .applySetting("hibernate.temp.use_jdbc_metadata_defaults", "false") - .build(); - - try { - MetadataSources metadataSources = new MetadataSources(registry); - for (String resource : MAPPING_RESOURCES) { - metadataSources.addResource(resource); - } - - Metadata metadata = metadataSources.buildMetadata(); - PersistentClass mapping = metadata.getEntityBinding(CertificateDefinition.class.getName()); - - assertEquals(LocalDate.class, mapping.getProperty("courseEndDate").getType().getReturnedClass()); - } finally { - StandardServiceRegistryBuilder.destroy(registry); - } - } -} diff --git a/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionPersistenceTest.java b/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionPersistenceTest.java new file mode 100644 index 00000000000..08050ee8af3 --- /dev/null +++ b/impl/src/test/java/org/sakaiproject/certification/impl/hibernate/CertificateDefinitionPersistenceTest.java @@ -0,0 +1,243 @@ +/** + * Copyright (c) 2003-2026 The Apereo Foundation + * + * Licensed under the Educational Community License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://opensource.org/licenses/ecl2 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.sakaiproject.certification.impl.hibernate; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + +import java.time.LocalDate; +import java.util.Collections; +import java.util.Date; +import java.util.Map; +import java.util.UUID; + +import org.hibernate.Session; +import org.hibernate.SessionFactory; +import org.hibernate.Transaction; +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import org.hsqldb.jdbc.JDBCDataSource; + +import org.springframework.orm.hibernate5.HibernateTransactionManager; +import org.springframework.transaction.TransactionStatus; +import org.springframework.transaction.support.DefaultTransactionDefinition; + +import org.sakaiproject.certification.api.CertificateDefinition; +import org.sakaiproject.certification.api.CertificateDefinitionStatus; +import org.sakaiproject.certification.api.CertificateService; +import org.sakaiproject.certification.api.IncompleteCertificateDefinitionException; +import org.sakaiproject.certification.api.VariableResolver; + +public class CertificateDefinitionPersistenceTest { + + private static final String MAPPING_ROOT = "org/sakaiproject/certification/impl/"; + private static final String COURSE_END_DATE_VARIABLE = "${" + VariableResolver.CERT_ENDDATE + "}"; + private static final String CERTIFICATE_NAME_VARIABLE = "${" + VariableResolver.CERT_NAME + "}"; + private static final String[] MAPPING_RESOURCES = { + MAPPING_ROOT + "CertificateDefinition.hbm.xml", + MAPPING_ROOT + "DocumentTemplate.hbm.xml", + MAPPING_ROOT + "criteria/AbstractCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/GreaterThanScoreCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/DueDatePassedCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/FinalGradeScoreCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/WillExpireCriterion.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/CertAssignment.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/CertCategory.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/CertGradebook.hbm.xml", + MAPPING_ROOT + "criteria/gradebook/CertGradeRecord.hbm.xml" + }; + + private StandardServiceRegistry registry; + private SessionFactory sessionFactory; + private HibernateTransactionManager transactionManager; + private CertificateService certificateService; + + @Before + public void setUp() { + JDBCDataSource dataSource = new JDBCDataSource(); + dataSource.setUrl("jdbc:hsqldb:mem:certification-" + UUID.randomUUID()); + dataSource.setUser("sa"); + dataSource.setPassword(""); + + registry = new StandardServiceRegistryBuilder() + .applySetting("hibernate.dialect", "org.hibernate.dialect.HSQLDialect") + .applySetting("hibernate.connection.datasource", dataSource) + .applySetting("hibernate.current_session_context_class", + "org.springframework.orm.hibernate5.SpringSessionContext") + .applySetting("hibernate.hbm2ddl.auto", "create-drop") + .applySetting("hibernate.show_sql", "false") + .build(); + + MetadataSources metadataSources = new MetadataSources(registry); + for (String resource : MAPPING_RESOURCES) { + metadataSources.addResource(resource); + } + sessionFactory = metadataSources.buildMetadata().buildSessionFactory(); + + transactionManager = new HibernateTransactionManager(sessionFactory); + CertificateServiceHibernateImpl service = new CertificateServiceHibernateImpl(); + service.setSessionFactory(sessionFactory); + certificateService = service; + } + + @After + public void tearDown() { + if (sessionFactory != null) { + sessionFactory.close(); + } + if (registry != null) { + StandardServiceRegistryBuilder.destroy(registry); + } + } + + @Test + public void updatePersistsCourseEndDateAndFieldMappingsTogether() throws Exception { + LocalDate originalDate = LocalDate.of(2026, 8, 27); + String id = saveDefinition(originalDate, fieldValues(COURSE_END_DATE_VARIABLE)); + + CertificateDefinition definition = getDefinition(id); + definition.setCourseEndDate(null); + definition.setFieldValues(fieldValues(CERTIFICATE_NAME_VARIABLE)); + assertNull(definition.getCourseEndDate()); + CertificateDefinition updated = + inTransaction(() -> certificateService.updateCertificateDefinition(definition)); + assertNull(updated.getCourseEndDate()); + + CertificateDefinition withoutCourseEndDate = getDefinition(id); + assertNull(withoutCourseEndDate.getCourseEndDate()); + assertEquals(CERTIFICATE_NAME_VARIABLE, withoutCourseEndDate.getFieldValues().get("date")); + + LocalDate revisedDate = LocalDate.of(2026, 9, 30); + withoutCourseEndDate.setCourseEndDate(revisedDate); + withoutCourseEndDate.setFieldValues(fieldValues(COURSE_END_DATE_VARIABLE)); + inTransaction(() -> certificateService.updateCertificateDefinition(withoutCourseEndDate)); + + CertificateDefinition withCourseEndDate = getDefinition(id); + assertEquals(revisedDate, withCourseEndDate.getCourseEndDate()); + assertEquals(COURSE_END_DATE_VARIABLE, withCourseEndDate.getFieldValues().get("date")); + } + + @Test + public void setFieldValuesRejectsCourseEndDateVariableWithoutDate() throws Exception { + String id = saveDefinition(null, Collections.emptyMap()); + + assertIncomplete(() -> { + certificateService.setFieldValues(id, fieldValues(COURSE_END_DATE_VARIABLE)); + return null; + }); + + assertEquals(Collections.emptyMap(), getDefinition(id).getFieldValues()); + } + + @Test + public void setFieldValuesPersistsCourseEndDateVariableWhenDateIsConfigured() throws Exception { + String id = saveDefinition(LocalDate.of(2026, 8, 27), Collections.emptyMap()); + + inTransaction(() -> { + certificateService.setFieldValues(id, fieldValues(COURSE_END_DATE_VARIABLE)); + return null; + }); + + assertEquals(COURSE_END_DATE_VARIABLE, getDefinition(id).getFieldValues().get("date")); + } + + @Test + public void updateRejectsAndRollsBackInvalidCourseEndDateConfiguration() throws Exception { + LocalDate originalDate = LocalDate.of(2026, 8, 27); + String id = saveDefinition(originalDate, fieldValues(COURSE_END_DATE_VARIABLE)); + + CertificateDefinition definition = getDefinition(id); + definition.setCourseEndDate(null); + assertIncomplete(() -> certificateService.updateCertificateDefinition(definition)); + + CertificateDefinition persisted = getDefinition(id); + assertEquals(originalDate, persisted.getCourseEndDate()); + assertEquals(COURSE_END_DATE_VARIABLE, persisted.getFieldValues().get("date")); + } + + @Test + public void activationRejectsPersistedInvalidCourseEndDateConfiguration() throws Exception { + String id = saveDefinition(null, fieldValues(COURSE_END_DATE_VARIABLE)); + + assertIncomplete(() -> { + certificateService.activateCertificateDefinition(id, true); + return null; + }); + } + + private String saveDefinition(LocalDate courseEndDate, Map fieldValues) { + CertificateDefinition definition = new CertificateDefinition(); + definition.setName("Certificate " + UUID.randomUUID()); + definition.setDescription("description"); + definition.setCreateDate(new Date()); + definition.setCreatorUserId("creator"); + definition.setSiteId("site"); + definition.setCourseEndDate(courseEndDate); + definition.setProgressHidden(false); + definition.setStatus(CertificateDefinitionStatus.UNPUBLISHED); + definition.setFieldValues(fieldValues); + + try (Session session = sessionFactory.openSession()) { + Transaction transaction = session.beginTransaction(); + session.save(definition); + transaction.commit(); + } + return definition.getId(); + } + + private CertificateDefinition getDefinition(String id) throws Exception { + return inTransaction(() -> certificateService.getCertificateDefinition(id)); + } + + private Map fieldValues(String value) { + return Collections.singletonMap("date", value); + } + + private void assertIncomplete(TransactionalOperation operation) throws Exception { + try { + inTransaction(operation); + fail("Expected IncompleteCertificateDefinitionException"); + } catch (IncompleteCertificateDefinitionException expected) { + // Expected. + } + } + + private T inTransaction(TransactionalOperation operation) throws Exception { + TransactionStatus status = transactionManager.getTransaction(new DefaultTransactionDefinition()); + T result; + try { + result = operation.execute(); + } catch (Exception | Error e) { + transactionManager.rollback(status); + throw e; + } + transactionManager.commit(status); + return result; + } + + @FunctionalInterface + private interface TransactionalOperation { + T execute() throws Exception; + } +} diff --git a/tool/src/java/org/sakaiproject/certification/tool/CertificateEditController.java b/tool/src/java/org/sakaiproject/certification/tool/CertificateEditController.java index ea00a0351e5..64118d8b6a9 100644 --- a/tool/src/java/org/sakaiproject/certification/tool/CertificateEditController.java +++ b/tool/src/java/org/sakaiproject/certification/tool/CertificateEditController.java @@ -551,7 +551,12 @@ protected ModelAndView createCertHandlerThird(@ModelAttribute(MOD_ATTR) Certific } else { model.put(STATUS_MESSAGE_KEY, FORM_ERR); model.put(MOD_ATTR, certificateToolState); - model.put(ERROR_MESSAGE, PREDEFINED_VAR_EXCEPTION); + if (result.hasFieldErrors("certificateDefinition.courseEndDate")) { + model.put(ERROR_MESSAGE, + result.getFieldError("certificateDefinition.courseEndDate").getCode()); + } else { + model.put(ERROR_MESSAGE, PREDEFINED_VAR_EXCEPTION); + } return new ModelAndView(VIEW_CREATE_CERTIFICATE_THREE, model); } @@ -633,11 +638,6 @@ protected ModelAndView createCertHandlerFourth(@ModelAttribute(MOD_ATTR) Certifi } certificateService.setAwardCriteria(certDef.getId(), awardCriteria); - - certDef = certificateService.getCertificateDefinition(certDef.getId()); - certificateService.setFieldValues(certDef.getId(), certificateToolState.getTemplateFields()); - - certDef = certificateService.getCertificateDefinition(certDef.getId()); certificateService.activateCertificateDefinition(certDef.getId(), true); } diff --git a/tool/src/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidator.java b/tool/src/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidator.java index ab015fc1566..c32c48559ad 100644 --- a/tool/src/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidator.java +++ b/tool/src/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidator.java @@ -26,9 +26,9 @@ import org.springframework.validation.Errors; import org.springframework.web.multipart.commons.CommonsMultipartFile; +import org.sakaiproject.certification.api.CertificateDefinitionConstraints; import org.sakaiproject.certification.api.CertificateService; import org.sakaiproject.certification.api.DocumentTemplateException; -import org.sakaiproject.certification.api.VariableResolver; import org.sakaiproject.certification.tool.util.CertificateToolState; public class CertificateDefinitionValidator { @@ -36,14 +36,6 @@ public class CertificateDefinitionValidator { private final Pattern variablePattern = Pattern.compile ("\\$\\{(.+)\\}"); public void validateFirst(CertificateToolState certificateToolState, Errors errors, CertificateService service) { - String courseEndDateVariable = "${" + VariableResolver.CERT_ENDDATE + "}"; - Map fieldValues = certificateToolState.getCertificateDefinition().getFieldValues(); - if (certificateToolState.getCertificateDefinition().getCourseEndDate() == null - && (containsVariable(fieldValues, courseEndDateVariable) - || containsVariable(certificateToolState.getTemplateFields(), courseEndDateVariable))) { - errors.rejectValue("certificateDefinition.courseEndDate", "form.error.courseEndDate.required"); - } - CommonsMultipartFile newTemplate = certificateToolState.getNewTemplate(); if (newTemplate != null && newTemplate.getSize() > 0) { if(!certificateToolState.getMimeTypes().contains( newTemplate.getContentType() )) { @@ -60,11 +52,6 @@ public void validateFirst(CertificateToolState certificateToolState, Errors erro } } - private boolean containsVariable(Map fields, String variable) { - return fields != null - && (fields.containsValue(variable) || fields.containsValue(variable.substring(1))); - } - public void validateSecond(CertificateToolState certificateToolState, Errors errors) { // The only invalid case is when the expiry date is your only criterion. // This case is handled in CertificateEditController @@ -92,6 +79,11 @@ public void validateThird(CertificateToolState certificateToolState, Errors erro } certificateToolState.setTemplateFields(currentFields); + if (!CertificateDefinitionConstraints.isCourseEndDateConfigurationValid( + certificateToolState.getCertificateDefinition().getCourseEndDate(), currentFields)) { + errors.rejectValue("certificateDefinition.courseEndDate", "form.error.courseEndDate.required"); + } + Map preDefFields = certificateToolState.getPredifinedFields(); Set keySet = preDefFields.keySet(); for(String val : currentFields.values()) { diff --git a/tool/src/test/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidatorTest.java b/tool/src/test/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidatorTest.java index f211d3e124b..1f8de92f33a 100644 --- a/tool/src/test/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidatorTest.java +++ b/tool/src/test/java/org/sakaiproject/certification/tool/validator/CertificateDefinitionValidatorTest.java @@ -38,19 +38,21 @@ public class CertificateDefinitionValidatorTest { private final CertificateDefinitionValidator validator = new CertificateDefinitionValidator(); @Test - public void missingCourseEndDateIsRejectedWhenCertificateUsesVariable() { + public void courseEndDateCanBeClearedBeforeExistingMappingIsChanged() { CertificateToolState state = stateWithCourseEndDate(null); state.getCertificateDefinition().getFieldValues().put("date", COURSE_END_DATE_VARIABLE); BeanPropertyBindingResult errors = validateFirst(state); - assertTrue(errors.hasFieldErrors("certificateDefinition.courseEndDate")); + assertFalse(errors.hasFieldErrors("certificateDefinition.courseEndDate")); } @Test - public void missingCourseEndDateIsRejectedWhenEscapedVariableIsPosted() { + public void missingCourseEndDateIsRejectedWhenFinalMappingUsesVariable() { CertificateToolState state = stateWithCourseEndDate(null); - state.getCertificateDefinition().getFieldValues().put("date", COURSE_END_DATE_VARIABLE.substring(1)); - BeanPropertyBindingResult errors = validateFirst(state); + state.setTemplateFields(new LinkedHashMap<>()); + state.getTemplateFields().put("date", COURSE_END_DATE_VARIABLE.substring(1)); + state.setPredifinedFields(predefinedFields()); + BeanPropertyBindingResult errors = validateThird(state); assertTrue(errors.hasFieldErrors("certificateDefinition.courseEndDate")); } @@ -58,20 +60,12 @@ public void missingCourseEndDateIsRejectedWhenEscapedVariableIsPosted() { @Test public void configuredCourseEndDateAllowsVariable() { CertificateToolState state = stateWithCourseEndDate(LocalDate.of(2026, 8, 27)); - state.getCertificateDefinition().getFieldValues().put("date", COURSE_END_DATE_VARIABLE); - BeanPropertyBindingResult errors = validateFirst(state); - - assertFalse(errors.hasFieldErrors("certificateDefinition.courseEndDate")); - } - - @Test - public void missingCourseEndDateIsRejectedWhenInProgressMappingUsesVariable() { - CertificateToolState state = stateWithCourseEndDate(null); state.setTemplateFields(new LinkedHashMap<>()); state.getTemplateFields().put("date", COURSE_END_DATE_VARIABLE.substring(1)); - BeanPropertyBindingResult errors = validateFirst(state); + state.setPredifinedFields(predefinedFields()); + BeanPropertyBindingResult errors = validateThird(state); - assertTrue(errors.hasFieldErrors("certificateDefinition.courseEndDate")); + assertFalse(errors.hasFieldErrors("certificateDefinition.courseEndDate")); } @Test @@ -103,6 +97,12 @@ private BeanPropertyBindingResult validateFirst(CertificateToolState state) { return errors; } + private BeanPropertyBindingResult validateThird(CertificateToolState state) { + BeanPropertyBindingResult errors = new BeanPropertyBindingResult(state, "certificateToolState"); + validator.validateThird(state, errors); + return errors; + } + private CertificateToolState stateWithCourseEndDate(LocalDate courseEndDate) { CertificateDefinition definition = new CertificateDefinition(); definition.setCourseEndDate(courseEndDate);