Skip to content

Commit f1744aa

Browse files
authored
SAK-52252 Lessons if a subpage was imported incorrectly in the past, it can no longer be re-imported (#14300)
1 parent 40e805e commit f1744aa

File tree

1 file changed

+216
-29
lines changed

1 file changed

+216
-29
lines changed

lessonbuilder/tool/src/java/org/sakaiproject/lessonbuildertool/service/LessonBuilderEntityProducer.java

Lines changed: 216 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -576,19 +576,24 @@ public String archive(String siteId, Document doc, Stack stack, String archivePa
576576
// Orphaned pages need not apply!
577577
SimplePageBean simplePageBean = makeSimplePageBean(siteId);
578578
OrphanPageFinder orphanFinder = simplePageBean.getOrphanFinder(siteId);
579+
580+
Map<Long, List<Long>> pageToReferencedPages = findReferencedPagesByItems(siteId);
579581

582+
Set<Long> originalSelectedPageIds = new HashSet<>();
580583
Set<Long> selectedPageIds = new HashSet<>();
581584
boolean hasSelection = selectedIds != null && !selectedIds.isEmpty();
582585
if (hasSelection) {
583586
for (String idStr : selectedIds) {
584587
try {
585-
selectedPageIds.add(Long.valueOf(idStr));
588+
Long pageId = Long.valueOf(idStr);
589+
originalSelectedPageIds.add(pageId);
590+
selectedPageIds.add(pageId);
586591
} catch (NumberFormatException e) {
587592
log.warn("Invalid page Id: {}", idStr);
588593
}
589594
}
590595
// Expand selection to include all descendant pages
591-
selectedPageIds = expandSelectionToIncludeDescendants(selectedPageIds, siteId);
596+
selectedPageIds = expandSelectionToIncludeDescendants(selectedPageIds, siteId, orphanFinder, pageToReferencedPages);
592597
}
593598

594599
try
@@ -608,7 +613,9 @@ public String archive(String siteId, Document doc, Stack stack, String archivePa
608613
List<SimplePage> sitePages = simplePageToolDao.getSitePages(siteId);
609614
if (sitePages != null && !sitePages.isEmpty()) {
610615
for (SimplePage page: sitePages) {
611-
if (orphanFinder.isOrphan(page.getPageId())) {
616+
// Skip orphaned pages unless they are in our selected set
617+
boolean isSelectedOrExpanded = hasSelection && selectedPageIds.contains(Long.valueOf(page.getPageId()));
618+
if (orphanFinder.isOrphan(page.getPageId()) && !isSelectedOrExpanded) {
612619
orphansSkipped++;
613620
continue;
614621
}
@@ -624,11 +631,12 @@ public String archive(String siteId, Document doc, Stack stack, String archivePa
624631

625632
int count = 0;
626633
if (hasSelection) {
627-
Set<Long> topLevelSelectedPages = findTopLevelSelectedPages(selectedPageIds, siteId);
634+
Set<Long> topLevelSelectedPages = findTopLevelSelectedPages(originalSelectedPageIds, selectedPageIds, siteId, pageToReferencedPages);
628635
// Filter out top-level selections that are orphans (not exported as pages)
629636
List<Long> orderedTopLevelPages = new ArrayList<>();
630637
for (Long id : topLevelSelectedPages) {
631-
if (orphanFinder.isOrphan(id)) {
638+
boolean wasOriginallySelected = originalSelectedPageIds.contains(id);
639+
if (orphanFinder.isOrphan(id) && !wasOriginallySelected) {
632640
selectionSkipped++;
633641
continue;
634642
}
@@ -701,15 +709,17 @@ public String archive(String siteId, Document doc, Stack stack, String archivePa
701709

702710
/**
703711
* Expands the selected page IDs to include all descendant pages
712+
* Only includes referenced subpages if the parent page is valid (not orphaned)
704713
*/
705-
private Set<Long> expandSelectionToIncludeDescendants(Set<Long> selectedPageIds, String siteId) {
714+
private Set<Long> expandSelectionToIncludeDescendants(Set<Long> selectedPageIds, String siteId, OrphanPageFinder orphanFinder, Map<Long, List<Long>> pageToReferencedPages) {
706715
if (selectedPageIds.isEmpty()) return selectedPageIds;
707716

708717
Set<Long> expandedIds = new HashSet<>(selectedPageIds);
709718
List<SimplePage> allPages = simplePageToolDao.getSitePages(siteId);
710719

711720
if (allPages == null || allPages.isEmpty()) return expandedIds;
712721

722+
// Build parent-child relationships based on the parent field
713723
Map<Long, List<Long>> parentToChildren = new HashMap<>();
714724
for (SimplePage page : allPages) {
715725
Long parentId = page.getParent();
@@ -723,34 +733,189 @@ private Set<Long> expandSelectionToIncludeDescendants(Set<Long> selectedPageIds,
723733

724734
while (!toProcess.isEmpty()) {
725735
Long currentPageId = toProcess.poll();
726-
List<Long> children = parentToChildren.get(currentPageId);
727736

737+
// Only process this page if it's not an orphan
738+
boolean isExplicitlySelected = selectedPageIds.contains(currentPageId);
739+
boolean isOrphan = orphanFinder.isOrphan(currentPageId);
740+
741+
if (isOrphan && !isExplicitlySelected) {
742+
log.debug("Skipping orphan page {} during expansion", currentPageId);
743+
continue;
744+
}
745+
746+
// Add direct children (based on parent field)
747+
List<Long> children = parentToChildren.get(currentPageId);
728748
if (children != null) {
729749
for (Long childId : children) {
730-
if (!expandedIds.contains(childId)) {
750+
if (!expandedIds.contains(childId) && !orphanFinder.isOrphan(childId)) {
731751
expandedIds.add(childId);
732752
toProcess.offer(childId);
733753
}
734754
}
735755
}
756+
757+
// Add pages referenced by items of type page (subpages)
758+
if (!isOrphan) {
759+
List<Long> referencedPages = pageToReferencedPages.get(currentPageId);
760+
if (referencedPages != null) {
761+
for (Long referencedPageId : referencedPages) {
762+
if (!expandedIds.contains(referencedPageId)) {
763+
// Only include the referenced page if it exists and is valid
764+
SimplePage referencedPage = simplePageToolDao.getPage(referencedPageId);
765+
if (referencedPage != null && siteId.equals(referencedPage.getSiteId())) {
766+
expandedIds.add(referencedPageId);
767+
toProcess.offer(referencedPageId);
768+
log.debug("Including referenced subpage {} from valid page {}", referencedPageId, currentPageId);
769+
}
770+
}
771+
}
772+
}
773+
}
736774
}
737775

738776
return expandedIds;
739777
}
740778

779+
/**
780+
* Handles orphaned subpages that weren't exported due to missing parent field
781+
* but are referenced by SimplePageItems of type PAGE.
782+
*/
783+
private Long handleOrphanedSubpage(Long oldSubpageId, String fromSiteId, String toSiteId, Map<Long, Long> pageMap, Long parentPageId) {
784+
try {
785+
SimplePage orphanedPage = simplePageToolDao.getPage(oldSubpageId);
786+
if (orphanedPage == null || !fromSiteId.equals(orphanedPage.getSiteId())) {
787+
log.debug("Orphaned subpage {} not found in source site {}", oldSubpageId, fromSiteId);
788+
return null;
789+
}
790+
791+
// Create a new page in the destination site
792+
SimplePage newPage = simplePageToolDao.makePage(toSiteId, null, orphanedPage.getTitle(), parentPageId, null);
793+
794+
// Copy essential properties
795+
if (orphanedPage.getCssSheet() != null) {
796+
// Update CSS sheet paths to reference the new site
797+
String newCssSheet = orphanedPage.getCssSheet().replace("/group/" + fromSiteId + "/", "/group/" + toSiteId + "/");
798+
newPage.setCssSheet(newCssSheet);
799+
}
800+
801+
// Copy other properties
802+
newPage.setHidden(orphanedPage.isHidden());
803+
newPage.setHiddenFromNavigation(orphanedPage.isHiddenFromNavigation());
804+
newPage.setReleaseDate(orphanedPage.getReleaseDate());
805+
newPage.setGradebookPoints(orphanedPage.getGradebookPoints());
806+
if (orphanedPage.getFolder() != null) {
807+
String newFolder = orphanedPage.getFolder().replace("/group/" + fromSiteId + "/", "/group/" + toSiteId + "/");
808+
newPage.setFolder(newFolder);
809+
}
810+
811+
// Set the parent to the current page being processed to establish hierarchy
812+
newPage.setParent(parentPageId);
813+
814+
// Find the top-level page for topparent
815+
SimplePage parentPage = simplePageToolDao.getPage(parentPageId);
816+
Long topParentId = parentPageId;
817+
if (parentPage != null) {
818+
// Traverse up to find the real top parent
819+
while (parentPage.getParent() != null) {
820+
parentPage = simplePageToolDao.getPage(parentPage.getParent());
821+
if (parentPage != null) {
822+
topParentId = parentPage.getPageId();
823+
} else {
824+
break;
825+
}
826+
}
827+
newPage.setTopParent(topParentId);
828+
829+
// Set the toolId from the top-level page
830+
SimplePage topParentPage = simplePageToolDao.getPage(topParentId);
831+
if (topParentPage != null && topParentPage.getToolId() != null) {
832+
newPage.setToolId(topParentPage.getToolId());
833+
}
834+
}
835+
836+
// Save the page
837+
simplePageToolDao.quickSaveItem(newPage);
838+
839+
// Add to the pageMap so other references to this page work
840+
pageMap.put(oldSubpageId, newPage.getPageId());
841+
842+
log.info("Created orphaned subpage recovery: old page {} -> new page {} with parent {} in site {}",
843+
oldSubpageId, newPage.getPageId(), parentPageId, toSiteId);
844+
845+
return newPage.getPageId();
846+
847+
} catch (Exception e) {
848+
log.error("Failed to handle orphaned subpage {}: {}", oldSubpageId, e.getMessage(), e);
849+
return null;
850+
}
851+
}
852+
853+
/**
854+
* Finds pages referenced by SimplePageItems of type PAGE (subpages)
855+
* This helps identify orphaned subpages that should be included in exports
856+
* but lack proper parent/topparent/toolid fields
857+
*/
858+
private Map<Long, List<Long>> findReferencedPagesByItems(String siteId) {
859+
Map<Long, List<Long>> pageToReferencedPages = new HashMap<>();
860+
861+
List<SimplePage> allPages = simplePageToolDao.getSitePages(siteId);
862+
if (allPages == null || allPages.isEmpty()) {
863+
return pageToReferencedPages;
864+
}
865+
866+
for (SimplePage page : allPages) {
867+
List<SimplePageItem> items = simplePageToolDao.findItemsOnPage(page.getPageId());
868+
if (items != null) {
869+
for (SimplePageItem item : items) {
870+
if (item.getType() == SimplePageItem.PAGE) {
871+
try {
872+
Long referencedPageId = Long.valueOf(item.getSakaiId());
873+
// Verify that the referenced page exists
874+
SimplePage referencedPage = simplePageToolDao.getPage(referencedPageId);
875+
if (referencedPage != null && siteId.equals(referencedPage.getSiteId())) {
876+
pageToReferencedPages.computeIfAbsent(page.getPageId(), k -> new ArrayList<>())
877+
.add(referencedPageId);
878+
log.debug("Found subpage reference: page {} references subpage {}", page.getPageId(), referencedPageId);
879+
}
880+
} catch (NumberFormatException e) {
881+
// Invalid sakaiId, skip this item
882+
log.debug("Invalid sakaiId for PAGE item: {}", item.getSakaiId());
883+
}
884+
}
885+
}
886+
}
887+
}
888+
889+
return pageToReferencedPages;
890+
}
891+
741892
/**
742893
* Finds the top-level pages that should become new Lessons from the selected pages
894+
* Only considers pages that were originally selected by the user, not those added automatically as references
743895
*/
744-
private Set<Long> findTopLevelSelectedPages(Set<Long> selectedPageIds, String siteId) {
896+
private Set<Long> findTopLevelSelectedPages(Set<Long> originalSelectedPageIds, Set<Long> allSelectedPageIds, String siteId, Map<Long, List<Long>> pageToReferencedPages) {
745897
Set<Long> topLevelPages = new HashSet<>();
898+
Set<Long> referencedBySelectedPages = new HashSet<>();
899+
900+
// Find all pages that are referenced by originally selected pages
901+
for (Long selectedPageId : originalSelectedPageIds) {
902+
List<Long> referencedPages = pageToReferencedPages.get(selectedPageId);
903+
if (referencedPages != null) {
904+
referencedBySelectedPages.addAll(referencedPages);
905+
}
906+
}
746907

747-
for (Long pageId : selectedPageIds) {
908+
for (Long pageId : originalSelectedPageIds) {
748909
SimplePage page = simplePageToolDao.getPage(pageId);
749910
if (page == null) continue;
750911

751-
// If this page has no parent or its parent is not selected, it's a top-level page
912+
// Check if this page has a parent relationship or is referenced by another selected page
752913
Long parentId = page.getParent();
753-
if (parentId == null || !selectedPageIds.contains(parentId)) {
914+
boolean hasSelectedParent = parentId != null && originalSelectedPageIds.contains(parentId);
915+
boolean isReferencedBySelected = referencedBySelectedPages.contains(pageId);
916+
917+
// It's a top-level page if it has no selected parent AND is not referenced by a selected page
918+
if (!hasSelectedParent && !isReferencedBySelected) {
754919
topLevelPages.add(pageId);
755920
}
756921
}
@@ -954,11 +1119,22 @@ private boolean mergePage(Element element, String oldServer, String siteId, Stri
9541119
}
9551120
} else if (type == SimplePageItem.PAGE) {
9561121
// sakaiId should be the new page ID
957-
Long newPageId = pageMap.get(Long.valueOf(sakaiId));
1122+
Long oldSubpageId = Long.valueOf(sakaiId);
1123+
Long newPageId = pageMap.get(oldSubpageId);
9581124
// we've seen a few cases where sakaiId of a subpage is 0. It won't be
9591125
// in the map, so this leaves it zero.
960-
if (newPageId != null)
1126+
if (newPageId != null) {
9611127
sakaiId = newPageId.toString();
1128+
} else {
1129+
// Try to find and import the orphaned subpage from the source site
1130+
newPageId = handleOrphanedSubpage(oldSubpageId, fromSiteId, siteId, pageMap, pageId);
1131+
if (newPageId != null) {
1132+
sakaiId = newPageId.toString();
1133+
log.info("Successfully recovered orphaned subpage {} as new page {}", oldSubpageId, newPageId);
1134+
} else {
1135+
log.warn("Could not recover orphaned subpage {} referenced from page {}", oldSubpageId, pageId);
1136+
}
1137+
}
9621138
}
9631139

9641140
if (type == SimplePageItem.ASSIGNMENT || type == SimplePageItem.ASSESSMENT || type == SimplePageItem.FORUM) {
@@ -1996,23 +2172,34 @@ public boolean willArchiveMerge()
19962172

19972173
@Override
19982174
public List<Map<String, String>> getEntityMap(String fromContext) {
1999-
try {
2000-
Set<String> navigationToolIds = siteService.getSite(fromContext).getOrderedPages()
2001-
.stream().map(SitePage::getId).collect(Collectors.toSet());
2002-
2003-
List<SimplePage> sitePages = simplePageToolDao.getSitePages(fromContext);
2004-
if (sitePages == null || sitePages.isEmpty()) {
2005-
return Collections.emptyList();
2175+
// Get orphan finder to identify problematic pages
2176+
SimplePageBean simplePageBean = makeSimplePageBean(fromContext);
2177+
OrphanPageFinder orphanFinder = simplePageBean.getOrphanFinder(fromContext);
2178+
2179+
// Find pages referenced by items, but only from valid (non-orphan) pages
2180+
Map<Long, List<Long>> referencedPages = findReferencedPagesByItems(fromContext);
2181+
Set<Long> validReferencedPageIds = new HashSet<>();
2182+
2183+
for (Map.Entry<Long, List<Long>> entry : referencedPages.entrySet()) {
2184+
Long referencingPageId = entry.getKey();
2185+
// Only include references from pages that are not orphans
2186+
if (!orphanFinder.isOrphan(referencingPageId)) {
2187+
validReferencedPageIds.addAll(entry.getValue());
20062188
}
2007-
2008-
return sitePages.stream()
2009-
.filter(page -> navigationToolIds.contains(page.getToolId()))
2010-
.map(p -> Map.of("id", Long.toString(p.getPageId()), "title", p.getTitle()))
2011-
.collect(Collectors.toList());
2012-
} catch (IdUnusedException e) {
2013-
log.warn("Could not find site {}: {}", fromContext, e);
2014-
return Collections.emptyList();
20152189
}
2190+
2191+
return simplePageToolDao.getSitePages(fromContext).stream()
2192+
.filter(p -> {
2193+
// Include page if it's not an orphan OR if it's referenced by a valid (non-orphan) page
2194+
boolean isOrphan = orphanFinder.isOrphan(p.getPageId());
2195+
boolean isValidlyReferenced = validReferencedPageIds.contains(p.getPageId());
2196+
return !isOrphan || isValidlyReferenced;
2197+
})
2198+
.map(p -> {
2199+
String title = p.getTitle();
2200+
return Map.of("id", Long.toString(p.getPageId()), "title", title);
2201+
})
2202+
.collect(Collectors.toList());
20162203
}
20172204

20182205
@Override

0 commit comments

Comments
 (0)