From cd9d1825880aff8d68a996aef2eff34c0a534f0c Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Mon, 18 May 2026 09:16:08 -0400 Subject: [PATCH 1/3] SAK-52547 Assignments preserve Gradebook category on import --- .../assignment/api/AssignmentConstants.java | 1 + .../impl/AssignmentServiceImpl.java | 59 ++++++++++++- .../AssignmentTransferCopyEntitiesTest.java | 84 +++++++++++++++++++ 3 files changed, 140 insertions(+), 4 deletions(-) diff --git a/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java b/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java index dca16263fd84..c1f086e8120b 100644 --- a/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java +++ b/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java @@ -366,6 +366,7 @@ public enum IMSGradingProgress { public static final String PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT = "prop_new_assignment_add_to_gradebook"; public static final String NEW_ASSIGNMENT_ADD_TO_GRADEBOOK = "new_assignment_add_to_gradebook"; + public static final String NEW_ASSIGNMENT_CATEGORY = "new_assignment_category"; /** * Sakai property key to change the default value for the 'Add due date to calendar' checkbox diff --git a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java index 11876c956aac..43d475b10d9d 100644 --- a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java +++ b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java @@ -4704,6 +4704,11 @@ public Map transferCopyEntities(String fromContext, String toCon nProperties.remove(PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT); final String assignmentAddToGradebookChoice = nProperties.get(NEW_ASSIGNMENT_ADD_TO_GRADEBOOK); final String nAssignmentRef = AssignmentReferenceReckoner.reckoner().assignment(nAssignment).reckon().getReference(); + if (GRADEBOOK_INTEGRATION_ADD.equals(assignmentAddToGradebookChoice)) { + remapImportedAssignmentCategoryProperty(nProperties, fromContext, toContext); + } else { + nProperties.remove(NEW_ASSIGNMENT_CATEGORY); + } switch (assignmentAddToGradebookChoice) { case GRADEBOOK_INTEGRATION_ADD, GRADEBOOK_INTEGRATION_ASSOCIATE -> { @@ -4711,7 +4716,9 @@ public Map transferCopyEntities(String fromContext, String toCon org.sakaiproject.grading.api.Assignment newGbAssignment = null; boolean isOriginalAssignmentExternal = gradingService.isExternalAssignmentDefined(oAssignment.getContext(), associatedGradebookAssignment); - if (!isOriginalAssignmentExternal) { + if (isOriginalAssignmentExternal) { + originalGBAssignment = gradingService.getExternalAssignment(oAssignment.getContext(), associatedGradebookAssignment); + } else if (StringUtils.isNotBlank(associatedGradebookAssignment)) { // load the assignment for internal gb link try { originalGBAssignment = gradingService.getAssignmentByNameOrId( @@ -4735,16 +4742,18 @@ public Map transferCopyEntities(String fromContext, String toCon log.debug("Assignment {} not found in gradebook for site {}", originalGBAssignment.getName(), nAssignment.getContext(), anfe); } } - } else { - originalGBAssignment = gradingService.getExternalAssignment(oAssignment.getContext(), associatedGradebookAssignment); } if (nAssignment.getDraft()) { // assignment is in the draft state - if (isOriginalAssignmentExternal) { + if (isOriginalAssignmentExternal || GRADEBOOK_INTEGRATION_ADD.equals(assignmentAddToGradebookChoice)) { // if this is an external defined assignments will create when publishing nProperties.remove(PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT); nProperties.put(NEW_ASSIGNMENT_ADD_TO_GRADEBOOK, GRADEBOOK_INTEGRATION_ADD); + if (isOriginalAssignmentExternal && originalGBAssignment != null) { + createCategoryForGbAssignmentIfNecessary(originalGBAssignment, oAssignment.getContext(), nAssignment.getContext()) + .ifPresent(categoryId -> nProperties.put(NEW_ASSIGNMENT_CATEGORY, categoryId.toString())); + } } else { if (newGbAssignment != null) { if (StringUtils.isNotBlank(newGbAssignment.getId().toString())) { @@ -5641,6 +5650,48 @@ private String generateUniqueAssignmentTitle(String originalTitle, String contex return uniqueTitle; } + private void remapImportedAssignmentCategoryProperty(Map properties, String fromGradebookId, String toGradebookId) { + + String categoryIds = StringUtils.trimToNull(properties.get(NEW_ASSIGNMENT_CATEGORY)); + if (categoryIds == null) { + return; + } + + Optional destinationCategoryId = createCategoryForAssignmentPropertyIfNecessary(categoryIds, fromGradebookId, toGradebookId); + if (destinationCategoryId.isPresent()) { + properties.put(NEW_ASSIGNMENT_CATEGORY, destinationCategoryId.get().toString()); + } else { + properties.remove(NEW_ASSIGNMENT_CATEGORY); + } + } + + private Optional createCategoryForAssignmentPropertyIfNecessary(String categoryIds, String fromGradebookId, String toGradebookId) { + + List selectedCategories = Arrays.stream(categoryIds.split(",")) + .map(StringUtils::trimToNull) + .filter(Objects::nonNull) + .filter(categoryId -> !"-1".equals(categoryId)) + .collect(Collectors.toList()); + + if (selectedCategories.isEmpty()) { + return Optional.empty(); + } + + // Category ids are gradebook-specific. Resolve the source id to a category name, + // then match or create the equivalent category in the destination gradebook. + Optional sourceCategory = gradingService.getCategoryDefinitions(fromGradebookId, fromGradebookId).stream() + .filter(category -> category.getId() != null && selectedCategories.contains(category.getId().toString())) + .findFirst(); + + if (!sourceCategory.isPresent()) { + return Optional.empty(); + } + + org.sakaiproject.grading.api.Assignment sourceAssignment = new org.sakaiproject.grading.api.Assignment(); + sourceAssignment.setCategoryName(sourceCategory.get().getName()); + return createCategoryForGbAssignmentIfNecessary(sourceAssignment, fromGradebookId, toGradebookId); + } + private Optional createCategoryForGbAssignmentIfNecessary( org.sakaiproject.grading.api.Assignment gbAssignment, String fromGradebookId, String toGradebookId) { diff --git a/assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTransferCopyEntitiesTest.java b/assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTransferCopyEntitiesTest.java index 46084807c9ce..e7d44a5a1195 100644 --- a/assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTransferCopyEntitiesTest.java +++ b/assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTransferCopyEntitiesTest.java @@ -64,6 +64,10 @@ import org.sakaiproject.entity.api.ResourcePropertiesEdit; import org.sakaiproject.entity.api.EntityTransferrer; import org.sakaiproject.entity.api.ResourceProperties; +import org.sakaiproject.grading.api.CategoryDefinition; +import org.sakaiproject.grading.api.GradebookInformation; +import org.sakaiproject.grading.api.GradingConstants; +import org.sakaiproject.grading.api.GradingService; import org.sakaiproject.site.api.SiteService; import org.sakaiproject.time.api.UserTimeService; import org.sakaiproject.tool.api.SessionManager; @@ -89,6 +93,7 @@ public class AssignmentTransferCopyEntitiesTest extends AbstractTransactionalJUn @Autowired private SecurityService securityService; @Autowired private SessionManager sessionManager; @Autowired private ServerConfigurationService serverConfigurationService; + @Autowired private GradingService gradingService; @Resource(name = "org.sakaiproject.time.api.UserTimeService") private UserTimeService userTimeService; @Autowired private UserDirectoryService userDirectoryService; @@ -204,6 +209,66 @@ public void transferCopyEntitiesRecreatesDraftOpenDateAnnouncementWhenImportDefa verify(announcementChannel).commitMessage(message, 0, "org.sakaiproject.announcement.impl.SiteEmailNotificationAnnc"); } + @Test + public void transferCopyEntitiesStoresDestinationCategoryForImportedDraftFromExternalGradebookItem() throws Exception { + + String fromContext = UUID.randomUUID().toString(); + String toContext = UUID.randomUUID().toString(); + Assignment sourceAssignment = createPublishedAssignment(fromContext); + sourceAssignment.setTypeOfGrade(Assignment.GradeType.SCORE_GRADE_TYPE); + String sourceAssignmentRef = AssignmentReferenceReckoner.reckoner().assignment(sourceAssignment).reckon().getReference(); + sourceAssignment.getProperties().put(AssignmentConstants.NEW_ASSIGNMENT_ADD_TO_GRADEBOOK, + AssignmentConstants.GRADEBOOK_INTEGRATION_ASSOCIATE); + sourceAssignment.getProperties().put(AssignmentConstants.PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT, + sourceAssignmentRef); + updateAssignment(sourceAssignment); + + stubContextPermissions(toContext); + stubCategoryImport(fromContext, toContext, 11L, 22L, "Essays"); + + org.sakaiproject.grading.api.Assignment sourceGradebookAssignment = new org.sakaiproject.grading.api.Assignment(); + sourceGradebookAssignment.setCategoryName("Essays"); + when(gradingService.isExternalAssignmentDefined(fromContext, sourceAssignmentRef)).thenReturn(true); + when(gradingService.getExternalAssignment(fromContext, sourceAssignmentRef)).thenReturn(sourceGradebookAssignment); + + getAssignmentServiceImpl().transferCopyEntities(fromContext, toContext, null, null); + + Collection importedAssignments = assignmentService.getAssignmentsForContext(toContext); + assertEquals(1, importedAssignments.size()); + Assignment importedAssignment = importedAssignments.iterator().next(); + assertTrue(importedAssignment.getDraft()); + assertEquals(AssignmentConstants.GRADEBOOK_INTEGRATION_ADD, + importedAssignment.getProperties().get(AssignmentConstants.NEW_ASSIGNMENT_ADD_TO_GRADEBOOK)); + assertEquals("22", importedAssignment.getProperties().get(AssignmentConstants.NEW_ASSIGNMENT_CATEGORY)); + } + + @Test + public void transferCopyEntitiesRemapsDraftAssignmentCategoryProperty() throws Exception { + + String fromContext = UUID.randomUUID().toString(); + String toContext = UUID.randomUUID().toString(); + Assignment sourceAssignment = createPublishedAssignment(fromContext); + sourceAssignment.setDraft(true); + sourceAssignment.setTypeOfGrade(Assignment.GradeType.SCORE_GRADE_TYPE); + sourceAssignment.getProperties().put(AssignmentConstants.NEW_ASSIGNMENT_ADD_TO_GRADEBOOK, + AssignmentConstants.GRADEBOOK_INTEGRATION_ADD); + sourceAssignment.getProperties().put(AssignmentConstants.NEW_ASSIGNMENT_CATEGORY, "11"); + updateAssignment(sourceAssignment); + + stubContextPermissions(toContext); + stubCategoryImport(fromContext, toContext, 11L, 22L, "Essays"); + + getAssignmentServiceImpl().transferCopyEntities(fromContext, toContext, null, null); + + Collection importedAssignments = assignmentService.getAssignmentsForContext(toContext); + assertEquals(1, importedAssignments.size()); + Assignment importedAssignment = importedAssignments.iterator().next(); + assertTrue(importedAssignment.getDraft()); + assertEquals(AssignmentConstants.GRADEBOOK_INTEGRATION_ADD, + importedAssignment.getProperties().get(AssignmentConstants.NEW_ASSIGNMENT_ADD_TO_GRADEBOOK)); + assertEquals("22", importedAssignment.getProperties().get(AssignmentConstants.NEW_ASSIGNMENT_CATEGORY)); + } + private Assignment createPublishedAssignment(String context) { stubContextPermissions(context); @@ -250,6 +315,25 @@ private void stubContextPermissions(String context) { when(securityService.unlock(AssignmentServiceConstants.SECURE_UPDATE_ASSIGNMENT, contextReference)).thenReturn(true); } + private void stubCategoryImport(String fromContext, String toContext, Long sourceCategoryId, Long destinationCategoryId, + String categoryName) { + + CategoryDefinition sourceCategory = new CategoryDefinition(); + sourceCategory.setId(sourceCategoryId); + sourceCategory.setName(categoryName); + + CategoryDefinition destinationCategory = new CategoryDefinition(); + destinationCategory.setId(destinationCategoryId); + destinationCategory.setName(categoryName); + + GradebookInformation destinationGradebookInformation = new GradebookInformation(); + destinationGradebookInformation.setCategoryType(GradingConstants.CATEGORY_TYPE_ONLY_CATEGORY); + + when(gradingService.getGradebookInformation(toContext, toContext)).thenReturn(destinationGradebookInformation); + when(gradingService.getCategoryDefinitions(fromContext, fromContext)).thenReturn(List.of(sourceCategory)); + when(gradingService.getCategoryDefinitions(toContext, toContext)).thenReturn(List.of(destinationCategory)); + } + private AssignmentServiceImpl getAssignmentServiceImpl() { return (AssignmentServiceImpl) AopTestUtils.getTargetObject(assignmentService); From e898810078f1f5c6dad69da86ad41c21b04c0ec6 Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Mon, 18 May 2026 09:29:09 -0400 Subject: [PATCH 2/3] Coderabbit --- .../impl/AssignmentServiceImpl.java | 21 ++++++++++----- .../AssignmentTransferCopyEntitiesTest.java | 26 +++++++++++++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java index 43d475b10d9d..3322e6e89e22 100644 --- a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java +++ b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java @@ -4704,11 +4704,7 @@ public Map transferCopyEntities(String fromContext, String toCon nProperties.remove(PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT); final String assignmentAddToGradebookChoice = nProperties.get(NEW_ASSIGNMENT_ADD_TO_GRADEBOOK); final String nAssignmentRef = AssignmentReferenceReckoner.reckoner().assignment(nAssignment).reckon().getReference(); - if (GRADEBOOK_INTEGRATION_ADD.equals(assignmentAddToGradebookChoice)) { - remapImportedAssignmentCategoryProperty(nProperties, fromContext, toContext); - } else { - nProperties.remove(NEW_ASSIGNMENT_CATEGORY); - } + boolean categoryCreatedFromGradebookAssignment = false; switch (assignmentAddToGradebookChoice) { case GRADEBOOK_INTEGRATION_ADD, GRADEBOOK_INTEGRATION_ASSOCIATE -> { @@ -4751,8 +4747,11 @@ public Map transferCopyEntities(String fromContext, String toCon nProperties.remove(PROP_ASSIGNMENT_ASSOCIATE_GRADEBOOK_ASSIGNMENT); nProperties.put(NEW_ASSIGNMENT_ADD_TO_GRADEBOOK, GRADEBOOK_INTEGRATION_ADD); if (isOriginalAssignmentExternal && originalGBAssignment != null) { - createCategoryForGbAssignmentIfNecessary(originalGBAssignment, oAssignment.getContext(), nAssignment.getContext()) - .ifPresent(categoryId -> nProperties.put(NEW_ASSIGNMENT_CATEGORY, categoryId.toString())); + Optional categoryId = createCategoryForGbAssignmentIfNecessary(originalGBAssignment, oAssignment.getContext(), nAssignment.getContext()); + if (categoryId.isPresent()) { + nProperties.put(NEW_ASSIGNMENT_CATEGORY, categoryId.get().toString()); + categoryCreatedFromGradebookAssignment = true; + } } } else { if (newGbAssignment != null) { @@ -4837,6 +4836,14 @@ public Map transferCopyEntities(String fromContext, String toCon } } + if (GRADEBOOK_INTEGRATION_ADD.equals(nProperties.get(NEW_ASSIGNMENT_ADD_TO_GRADEBOOK))) { + if (!categoryCreatedFromGradebookAssignment) { + remapImportedAssignmentCategoryProperty(nProperties, fromContext, toContext); + } + } else { + nProperties.remove(NEW_ASSIGNMENT_CATEGORY); + } + // review service if (oAssignment.getContentReview()) { nAssignment.setContentReview(true); diff --git a/assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTransferCopyEntitiesTest.java b/assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTransferCopyEntitiesTest.java index e7d44a5a1195..1a8dc16f19a6 100644 --- a/assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTransferCopyEntitiesTest.java +++ b/assignment/impl/src/test/org/sakaiproject/assignment/impl/AssignmentTransferCopyEntitiesTest.java @@ -269,6 +269,32 @@ public void transferCopyEntitiesRemapsDraftAssignmentCategoryProperty() throws E assertEquals("22", importedAssignment.getProperties().get(AssignmentConstants.NEW_ASSIGNMENT_CATEGORY)); } + @Test + public void transferCopyEntitiesRemovesCategoryWhenFinalGradebookIntegrationIsNo() throws Exception { + + String fromContext = UUID.randomUUID().toString(); + String toContext = UUID.randomUUID().toString(); + Assignment sourceAssignment = createPublishedAssignment(fromContext); + sourceAssignment.setTypeOfGrade(Assignment.GradeType.SCORE_GRADE_TYPE); + sourceAssignment.getProperties().put(AssignmentConstants.NEW_ASSIGNMENT_ADD_TO_GRADEBOOK, + AssignmentConstants.GRADEBOOK_INTEGRATION_ADD); + sourceAssignment.getProperties().put(AssignmentConstants.NEW_ASSIGNMENT_CATEGORY, "11"); + updateAssignment(sourceAssignment); + + stubContextPermissions(toContext); + stubCategoryImport(fromContext, toContext, 11L, 22L, "Essays"); + + getAssignmentServiceImpl().transferCopyEntities(fromContext, toContext, null, + List.of(EntityTransferrer.PUBLISH_OPTION)); + + Collection importedAssignments = assignmentService.getAssignmentsForContext(toContext); + assertEquals(1, importedAssignments.size()); + Assignment importedAssignment = importedAssignments.iterator().next(); + assertEquals(AssignmentConstants.GRADEBOOK_INTEGRATION_NO, + importedAssignment.getProperties().get(AssignmentConstants.NEW_ASSIGNMENT_ADD_TO_GRADEBOOK)); + Assert.assertNull(importedAssignment.getProperties().get(AssignmentConstants.NEW_ASSIGNMENT_CATEGORY)); + } + private Assignment createPublishedAssignment(String context) { stubContextPermissions(context); From 4b674041aa8061bde632ddb37e8c15bdcad241a0 Mon Sep 17 00:00:00 2001 From: Sam Ottenhoff Date: Tue, 19 May 2026 09:20:38 -0400 Subject: [PATCH 3/3] coderabbit suggestion --- .../sakaiproject/assignment/impl/AssignmentServiceImpl.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java index 3322e6e89e22..6f30e9c111c5 100644 --- a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java +++ b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java @@ -5686,7 +5686,11 @@ private Optional createCategoryForAssignmentPropertyIfNecessary(String cat // Category ids are gradebook-specific. Resolve the source id to a category name, // then match or create the equivalent category in the destination gradebook. - Optional sourceCategory = gradingService.getCategoryDefinitions(fromGradebookId, fromGradebookId).stream() + List categoryDefs = gradingService.getCategoryDefinitions(fromGradebookId, fromGradebookId); + if (categoryDefs == null) { + categoryDefs = Collections.emptyList(); + } + Optional sourceCategory = categoryDefs.stream() .filter(category -> category.getId() != null && selectedCategories.contains(category.getId().toString())) .findFirst();