diff --git a/src/main/java/org/olat/modules/portfolio/manager/BinderDAO.java b/src/main/java/org/olat/modules/portfolio/manager/BinderDAO.java index da3e23c028366753f10279fe73d7c8c3166f69e0..aa443fc4d0efc246cc343804bfa811523521f692 100644 --- a/src/main/java/org/olat/modules/portfolio/manager/BinderDAO.java +++ b/src/main/java/org/olat/modules/portfolio/manager/BinderDAO.java @@ -577,8 +577,10 @@ public class BinderDAO { List<Page> pages = new ArrayList<>(section.getPages()); //delete pages for(Page page:pages) { - pageDao.deletePage(page); - section.getPages().remove(page); + if(page != null) { + pageDao.deletePage(page); + section.getPages().remove(page); + } } List<Assignment> assignments = new ArrayList<>(((SectionImpl)section).getAssignments()); diff --git a/src/main/java/org/olat/modules/portfolio/manager/PageDAO.java b/src/main/java/org/olat/modules/portfolio/manager/PageDAO.java index 59e61ac421042606a204620c37d5babfac72a514..b0af0a33ae9df2308bbf4142c780e4f8c6fe1488 100644 --- a/src/main/java/org/olat/modules/portfolio/manager/PageDAO.java +++ b/src/main/java/org/olat/modules/portfolio/manager/PageDAO.java @@ -443,6 +443,8 @@ public class PageDAO { * @return */ public int deletePage(Page page) { + if(page == null || page.getKey() == null) return 0;//nothing to do + OLATResourceable ores = OresHelper.createOLATResourceableInstance(Page.class, page.getKey()); PageBody body = page.getBody(); diff --git a/src/test/java/org/olat/modules/portfolio/manager/PortfolioServiceTest.java b/src/test/java/org/olat/modules/portfolio/manager/PortfolioServiceTest.java index 54ff516a2383c1917e5b9039c8750545062f9bf7..f0259a7976778f692016608a9ac07a7e31b04408 100644 --- a/src/test/java/org/olat/modules/portfolio/manager/PortfolioServiceTest.java +++ b/src/test/java/org/olat/modules/portfolio/manager/PortfolioServiceTest.java @@ -41,6 +41,7 @@ import org.olat.modules.portfolio.Section; import org.olat.modules.portfolio.SectionRef; import org.olat.modules.portfolio.model.AccessRights; import org.olat.modules.portfolio.model.BinderStatistics; +import org.olat.modules.portfolio.model.PageImpl; import org.olat.modules.portfolio.model.SectionImpl; import org.olat.modules.portfolio.model.SynchedBinder; import org.olat.repository.RepositoryEntry; @@ -1028,4 +1029,46 @@ public class PortfolioServiceTest extends OlatTestCase { Assert.assertNull(deletedPage); } } + + /** + * Check if we can delete a section where there is an error of numbering + * in the list of pages. + * + */ + @Test + public void deleteSectionWithPages_errorInNumbering() { + // prepare a binder with 2 sections and some pages + Identity owner = JunitTestHelper.createAndPersistIdentityAsRndUser("del-binder-"); + Binder binder = portfolioService.createNewBinder("Binder to delete", "Deletion", "", owner); + SectionRef sectionRef = portfolioService.appendNewSection("1. section ", "Section 1", null, null, binder); + dbInstance.commit(); + portfolioService.updateBinderUserInformations(binder, owner); + dbInstance.commit(); + + Section reloadedSection = portfolioService.getSection(sectionRef); + List<Page> pagesSection = new ArrayList<>(); + for(int i=0; i<10; i++) { + Page page = portfolioService.appendNewPage(owner, "New page", "A brand new page.", null, null, reloadedSection); + pagesSection.add(page); + } + dbInstance.commitAndCloseSession(); + + //simulate a gap in numbering of the list + Page pageToDelete = dbInstance.getCurrentEntityManager().getReference(PageImpl.class, pagesSection.get(4).getKey()); + dbInstance.getCurrentEntityManager().remove(pageToDelete); + dbInstance.commitAndCloseSession(); + + // delete the section + portfolioService.deleteSection(binder, reloadedSection); + dbInstance.commit(); + + //check if the section and the pages are deleted + Section deletedSection = binderDao.loadSectionByKey(sectionRef.getKey()); + Assert.assertNull(deletedSection); + Page deletedPage = pageDao.loadByKey(pagesSection.get(4).getKey()); + Assert.assertNull(deletedPage); + List<Page> deletedPages = pageDao.getPages(sectionRef); + Assert.assertNotNull(deletedPages); + Assert.assertTrue(deletedPages.isEmpty()); + } }