From ec666159fa5704ec7dfdd5e996aa1c1f310d24ef Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Tue, 26 Sep 2017 08:45:45 +0200
Subject: [PATCH] OO-3040: jump missing page while deleting a section

---
 .../modules/portfolio/manager/BinderDAO.java  |  6 ++-
 .../modules/portfolio/manager/PageDAO.java    |  2 +
 .../manager/PortfolioServiceTest.java         | 43 +++++++++++++++++++
 3 files changed, 49 insertions(+), 2 deletions(-)

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 da3e23c0283..aa443fc4d0e 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 59e61ac4210..b0af0a33ae9 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 54ff516a238..f0259a79767 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());
+	}
 }
-- 
GitLab