Merge branch 'fix-create-shared-editions-bug' of...

Merge branch 'fix-create-shared-editions-bug' of gitlab.ewi.tudelft.nl:eip/labrador/queue into 557-course-filter-approved-requests-broken

src/main/java/nl/tudelft/queue/controller/EditionController.java

@@ -18,12 +18,16 @@
 package nl.tudelft.queue.controller;
 
 import static java.time.LocalDateTime.now;
+import static nl.tudelft.labracore.api.dto.PersonDetailsDTO.DefaultRoleEnum.ADMIN;
+import static nl.tudelft.labracore.api.dto.PersonDetailsDTO.DefaultRoleEnum.TEACHER;
 import static nl.tudelft.labracore.lib.LabracoreApiUtil.fromPageable;
 import static nl.tudelft.queue.PageUtil.toPage;
 
 import java.io.IOException;
+import java.time.LocalDateTime;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Objects;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
@@ -153,6 +157,9 @@ public class EditionController {
 	public String getEditionList(@AuthenticatedPerson Person person,
 			@PageableDefault(sort = "id", direction = Sort.Direction.DESC) Pageable pageable,
 			Model model) {
+
+		PersonDetailsDTO pd = Objects.requireNonNull(pApi.getPersonById(person.getId()).block());
+
 		var filter = es.getFilter("/editions");
 		var editions = eApi
 				.getEditionsPageActiveOrTaughtBy(person.getId(), fromPageable(pageable), filter.getPrograms(),
@@ -171,6 +178,17 @@ public class EditionController {
 
 		model.addAttribute("page", "catalog");
 
+		if (pd.getDefaultRole() == ADMIN || pd.getDefaultRole() == TEACHER) {
+			model.addAttribute("allEditions", eCache.get(
+					Objects.requireNonNullElse(
+							eApi.getAllEditionsActiveDuringPeriod(
+									new Period().start(LocalDateTime.now())
+											.end(LocalDateTime.now().plusYears(1)))
+									.map(EditionSummaryDTO::getId)
+									.collectList().block(),
+							List.of())));
+		}
+
 		return "edition/index";
 	}
 

src/main/java/nl/tudelft/queue/controller/HomeController.java

@@ -17,8 +17,6 @@
  */
 package nl.tudelft.queue.controller;
 
-import static nl.tudelft.labracore.api.dto.PersonDetailsDTO.DefaultRoleEnum.ADMIN;
-import static nl.tudelft.labracore.api.dto.PersonDetailsDTO.DefaultRoleEnum.TEACHER;
 import static nl.tudelft.labracore.api.dto.RoleEditionDetailsDTO.TypeEnum.TEACHER_RO;
 
 import java.time.LocalDateTime;
@@ -199,17 +197,6 @@ public class HomeController {
 							.filter(qs -> ps.canEnqueueSelf(qs.getId()) || qs.getSlot().open())
 							.collect(Collectors.toList())));
 
-			if (pd.getDefaultRole() == ADMIN || pd.getDefaultRole() == TEACHER) {
-				model.addAttribute("allEditions", eCache.get(
-						Objects.requireNonNullElse(
-								eApi.getAllEditionsActiveDuringPeriod(
-										new Period().start(LocalDateTime.now())
-												.end(LocalDateTime.now().plusYears(1)))
-										.map(EditionSummaryDTO::getId)
-										.collectList().block(),
-								List.of())));
-			}
-
 			model.addAttribute("editions", editions);
 			model.addAttribute("sharedEditions", sharedEditions);
 			model.addAttribute("sharedLabs", sharedLabs);

src/test/java/nl/tudelft/queue/controller/EditionControllerTest.java

@@ -56,6 +56,7 @@ import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
 
+import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
 import test.TestDatabaseLoader;
 import test.labracore.*;
@@ -86,6 +87,9 @@ public class EditionControllerTest {
 	@Autowired
 	private EditionCollectionApiMocker ecApiMocker;
 
+	@Autowired
+	private PersonApiMocker pApiMocker;
+
 	@Autowired
 	private RoleApiMocker rApiMocker;
 
@@ -111,6 +115,7 @@ public class EditionControllerTest {
 		sApiMocker.mock();
 		sgApiMocker.mock();
 		ecApiMocker.mock();
+		pApiMocker.mock();
 
 		student100 = db.getStudents()[100];
 		student155 = db.getStudents()[155];
@@ -126,15 +131,20 @@ public class EditionControllerTest {
 		when(eApi.getEditionsPageActiveOrTaughtBy(any(), any(), any(), any()))
 				.thenReturn(Mono.just(new PageEditionDetailsDTO().content(List.of()).totalElements(0L)));
 
+		when(eApi.getAllEditionsActiveDuringPeriod(any())).thenReturn(Flux.just());
+
 		mvc.perform(get("/editions"))
 				.andExpect(status().isOk())
 				.andExpect(model().attribute("page", "catalog"))
-				.andExpect(model().attributeExists("editions"));
+				.andExpect(model().attributeExists("editions"))
+				.andExpect(model().attributeExists("allEditions"));
 	}
 
 	@Test
 	@WithUserDetails("teacher0")
 	void submitFiltersRedirectsBackToEditions() throws Exception {
+		when(eApi.getAllEditionsActiveDuringPeriod(any())).thenReturn(Flux.just());
+
 		MockHttpSession session = new MockHttpSession();
 
 		mvc.perform(post("/editions/filter").with(csrf())
@@ -150,6 +160,8 @@ public class EditionControllerTest {
 		when(eApi.getEditionsPageActiveOrTaughtBy(any(), any(), any(), any()))
 				.thenReturn(Mono.just(new PageEditionDetailsDTO().content(List.of()).totalElements(0L)));
 
+		when(eApi.getAllEditionsActiveDuringPeriod(any())).thenReturn(Flux.just());
+
 		MockHttpSession session = new MockHttpSession();
 
 		mvc.perform(post("/editions/filter").with(csrf())
@@ -171,6 +183,21 @@ public class EditionControllerTest {
 				.andExpect(view().name("course/request"));
 	}
 
+	@Test
+	@WithUserDetails("student155")
+	void allEditionsNotExposedToStudent() throws Exception {
+		when(eApi.getEditionsPageActiveOrTaughtBy(any(), any(), any(), any()))
+				.thenReturn(Mono.just(new PageEditionDetailsDTO().content(List.of()).totalElements(0L)));
+
+		when(eApi.getAllEditionsActiveDuringPeriod(any())).thenReturn(Flux.just());
+
+		mvc.perform(get("/editions"))
+				.andExpect(status().isOk())
+				.andExpect(model().attribute("page", "catalog"))
+				.andExpect(model().attributeExists("editions"))
+				.andExpect(model().attributeDoesNotExist("allEditions"));
+	}
+
 	@Test
 	@WithUserDetails("student155")
 	void getEditionEnrolViewContainsEdition() throws Exception {

src/test/java/nl/tudelft/queue/controller/HomeControllerTest.java

@@ -128,7 +128,6 @@ class HomeControllerTest {
 				.andExpect(model().attribute("user", matchPersonId(student50.getId())))
 				.andExpect(model().attributeExists("activeRoles", "editions", "labs"))
 				.andExpect(model().attributeExists("sharedEditions", "sharedLabs"))
-				.andExpect(model().attributeDoesNotExist("allEditions"))
 				.andExpect(view().name("home/dashboard"));
 
 		verify(personApi).getPersonById(student50.getId());
@@ -148,7 +147,6 @@ class HomeControllerTest {
 				.andExpect(model().attribute("user", matchPersonId(admin.getId())))
 				.andExpect(model().attribute("activeRoles", List.of()))
 				.andExpect(model().attribute("editions", Map.of()))
-				.andExpect(model().attributeExists("allEditions"))
 				.andExpect(view().name("home/dashboard"));
 
 		verify(personApi).getPersonById(admin.getId());
@@ -162,7 +160,6 @@ class HomeControllerTest {
 
 		mvc.perform(get("/"))
 				.andExpect(status().isOk())
-				.andExpect(model().attributeExists("allEditions"))
 				.andExpect(view().name("home/dashboard"));
 	}