From bedcd0c2a96a2fb9467a881ab1bd1f1aec1dd614 Mon Sep 17 00:00:00 2001 From: Damian Sobieralski Date: Tue, 19 Sep 2023 18:31:26 -0400 Subject: [PATCH] LMSA-7975 - last pass at feature before code review --- .../lms/crosslist/CrosslistConstants.java | 4 +- .../controller/CrosslistController.java | 92 ++++----- .../lms/crosslist/model/FindParentModel.java | 1 - .../crosslist/service/CrosslistService.java | 12 +- .../resources/static/css/crosslisting.css | 2 +- .../resources/templates/findParentCourse.html | 133 ++++++------- .../services/CrosslistServiceImplTest.java | 184 ++++++++++++++++++ 7 files changed, 285 insertions(+), 143 deletions(-) diff --git a/src/main/java/edu/iu/uits/lms/crosslist/CrosslistConstants.java b/src/main/java/edu/iu/uits/lms/crosslist/CrosslistConstants.java index 0c8b6ca..8a80667 100644 --- a/src/main/java/edu/iu/uits/lms/crosslist/CrosslistConstants.java +++ b/src/main/java/edu/iu/uits/lms/crosslist/CrosslistConstants.java @@ -52,8 +52,8 @@ public interface CrosslistConstants { String LOOKUP_SUCCESS_FOUND_MESSAGE = "Parent course is found"; String LOOKUP_FAILURE_COURSE_NOT_CROSSLISTED_MESSAGE = "This course has not been crosslisted"; - String LOOKUP_FAILURE_SECTION_NOT_FOUND_IN_CANVAS_MESSAGE = "This section has not been found in Canvas"; - String LOOKUP_FAILURE_SECTION_NOT_FOUND_IN_SIS_MESSAGE = "Section not found in SIS"; + String LOOKUP_FAILURE_NOT_FOUND_IN_CANVAS_MESSAGE = "Not found in Canvas"; + String LOOKUP_FAILURE_NOT_FOUND_IN_SIS_MESSAGE = "Not found in SIS"; String LOOKUP_SUCCESS_CSS = "rvt-color-green rvt-bg-green-100"; String LOOKUP_FAILURE_CSS = "rvt-orange-green rvt-bg-orange-100"; diff --git a/src/main/java/edu/iu/uits/lms/crosslist/controller/CrosslistController.java b/src/main/java/edu/iu/uits/lms/crosslist/controller/CrosslistController.java index a0b44af..f794d8a 100644 --- a/src/main/java/edu/iu/uits/lms/crosslist/controller/CrosslistController.java +++ b/src/main/java/edu/iu/uits/lms/crosslist/controller/CrosslistController.java @@ -879,25 +879,20 @@ public String endSelfImpersonation(@PathVariable("courseId") String courseId, @M @RequestMapping("/lookup-launch") @Secured({LTIConstants.ADMIN_AUTHORITY, LTIConstants.INSTRUCTOR_AUTHORITY}) - public String lookupLaunch(@ModelAttribute FindParentModel findParentModel, Model model, HttpServletRequest request) { - OidcAuthenticationToken token = getTokenWithoutContext(); - OidcTokenUtils oidcTokenUtils = new OidcTokenUtils(token); - String courseId = oidcTokenUtils.getCourseId(); + public String lookupLaunch(@ModelAttribute FindParentModel findParentModel, Model model, HttpSession session) { + getTokenWithoutContext(); - OidcAuthenticationToken sessionToken = courseSessionService.getAttributeFromSession(request.getSession(), courseId, OidcTokenAwareController.SESSION_TOKEN_KEY, OidcAuthenticationToken.class); + List terms = termService.getEnrollmentTerms() + .stream() + .filter(term -> term.getSisTermId().compareTo("4218") >= 0 && term.getSisTermId().charAt(0) == '4') + .sorted(Comparator.comparing(CanvasTerm::getSisTermId)) + .toList(); - if (sessionToken == null) { - courseSessionService.addAttributeToSession(request.getSession(), courseId, OidcTokenAwareController.SESSION_TOKEN_KEY, token); + if (courseSessionService.getAttributeFromSession(session, "all", "terms", List.class) == null) { + courseSessionService.addAttributeToSession(session, "all", "terms", terms); } -// model.addAttribute("courseId", courseId); -// model.addAttribute("hideFooter", true); - -// SubmissionStatus status = new SubmissionStatus(); -// status.setStatusClass(CrosslistConstants.STATUS_SUCCESS); -// status.setStatusMessage("Ta dah!"); -// -// model.addAttribute("submissionStatus", status); + model.addAttribute("terms", terms); return "findParentCourse"; } @@ -905,17 +900,24 @@ public String lookupLaunch(@ModelAttribute FindParentModel findParentModel, Mode @PostMapping(value = "/lookup-search-sisid") @Secured({LTIConstants.BASE_USER_AUTHORITY}) public String lookupSearchBySisId(@ModelAttribute FindParentModel findParentModel, Model model, HttpSession session) { - log.info("SIS search text = {}", findParentModel.getSisIdSearch()); + getTokenWithoutContext(); FindParentResult findParentResult = null; - if (findParentModel.getSisIdSearch() != null && ! findParentModel.getSisIdSearch().trim().isEmpty()) { -// Section section = sectionService.getSection(String.format("sis_section_id:%s", findParentModel.getSisIdSearch())); + List terms = courseSessionService.getAttributeFromSession(session, "all", + "terms", List.class); + if (findParentModel.getSisIdSearch() == null || findParentModel.getSisIdSearch().trim().isEmpty()) { + findParentResult = new FindParentResult(); + findParentResult.setStatusMessage("SIS ID needs to have a value"); + findParentResult.setStatusIconCssClasses(CrosslistConstants.LOOKUP_FAILURE_CSS); + } else { SisCourse sisCourse = sisService.getSisCourseBySiteId(findParentModel.getSisIdSearch().trim()); findParentResult = crosslistService.processSisLookup(sisCourse); } + model.addAttribute("terms", terms); + if (findParentResult != null) { model.addAttribute("findParentResult", findParentResult); } @@ -926,58 +928,32 @@ public String lookupSearchBySisId(@ModelAttribute FindParentModel findParentMode @PostMapping(value = "/lookup-search-termandclassnumber") @Secured({LTIConstants.BASE_USER_AUTHORITY}) public String lookupSearchByTermAndClassNUmber(@ModelAttribute FindParentModel findParentModel, Model model, HttpSession session) { - log.info("Term by Class Number = {}", findParentModel.getTermByClassNumberSearch()); - log.info("Class Number search text = {}", findParentModel.getClassNumberSearch()); + getTokenWithoutContext(); FindParentResult findParentResult = null; - if (findParentModel.getTermByClassNumberSearch() != null && ! findParentModel.getTermByClassNumberSearch().trim().isEmpty() && - findParentModel.getClassNumberSearch() != null && ! findParentModel.getClassNumberSearch().trim().isEmpty()) { - + List terms = courseSessionService.getAttributeFromSession(session, "all", + "terms", List.class); + + if (findParentModel.getTermByClassNumberSearch() == null || findParentModel.getTermByClassNumberSearch().trim().isEmpty()) { + findParentResult = new FindParentResult(); + findParentResult.setStatusMessage("Term needs to have a value"); + findParentResult.setStatusIconCssClasses(CrosslistConstants.LOOKUP_FAILURE_CSS); + } else if (findParentModel.getClassNumberSearch() == null || findParentModel.getClassNumberSearch().trim().isEmpty()) { + findParentResult = new FindParentResult(); + findParentResult.setStatusMessage("Class Number needs to have a value"); + findParentResult.setStatusIconCssClasses(CrosslistConstants.LOOKUP_FAILURE_CSS); + } else { final String strm = findParentModel.getTermByClassNumberSearch().trim(); final String classNumber = findParentModel.getClassNumberSearch().trim(); final String iuSiteId = sisService.getIuSiteIdFromStrmAndClassNumber(strm, classNumber); SisCourse sisCourse = sisService.getSisCourseBySiteId(iuSiteId); - -// Section section = sectionService.getSection(String.format("sis_section_id:%s", iuSiteId)); - findParentResult = crosslistService.processSisLookup(sisCourse); } - if (findParentResult != null) { - model.addAttribute("findParentResult", findParentResult); - } - - - return "findParentCourse"; - } - - @PostMapping(value = "/lookup-search-canvascourseid") - @Secured({LTIConstants.BASE_USER_AUTHORITY}) - public String lookupSearchByCourseId(@ModelAttribute FindParentModel findParentModel, Model model, HttpSession session) { - log.info("Canvas CourseId search text = {}", findParentModel.getCanvasCourseIdSearch()); - - FindParentResult findParentResult = null; - - if (findParentModel.getCanvasCourseIdSearch() != null && ! findParentModel.getCanvasCourseIdSearch().trim().isEmpty()) { - List
sections = courseService.getCourseSections(findParentModel.getCanvasCourseIdSearch().trim()); - - boolean isCrosslisted = false; - - for (Section section : sections) { - if (section.getSis_section_id() != null && ! section.getSis_section_id().isEmpty() && - section.getSis_course_id() != null && ! section.getSis_course_id().isEmpty() && - ! section.getSis_section_id().equals(section.getSis_course_id())) { - SisCourse sisCourse = new SisCourse(); - findParentResult = crosslistService.processSisLookup(sisCourse); - - } - - } - - } + model.addAttribute("terms", terms); if (findParentResult != null) { model.addAttribute("findParentResult", findParentResult); diff --git a/src/main/java/edu/iu/uits/lms/crosslist/model/FindParentModel.java b/src/main/java/edu/iu/uits/lms/crosslist/model/FindParentModel.java index 4e4fdc1..21201be 100644 --- a/src/main/java/edu/iu/uits/lms/crosslist/model/FindParentModel.java +++ b/src/main/java/edu/iu/uits/lms/crosslist/model/FindParentModel.java @@ -41,5 +41,4 @@ public class FindParentModel { private String sisIdSearch; private String termByClassNumberSearch; private String classNumberSearch; - private String canvasCourseIdSearch; } \ No newline at end of file diff --git a/src/main/java/edu/iu/uits/lms/crosslist/service/CrosslistService.java b/src/main/java/edu/iu/uits/lms/crosslist/service/CrosslistService.java index f633a00..9346cec 100644 --- a/src/main/java/edu/iu/uits/lms/crosslist/service/CrosslistService.java +++ b/src/main/java/edu/iu/uits/lms/crosslist/service/CrosslistService.java @@ -394,7 +394,8 @@ public FindParentResult processSisLookup(SisCourse sisCourse) { FindParentResult findParentResult = new FindParentResult(); if (sisCourse == null || sisCourse.getIuSiteId() == null) { - findParentResult.setStatusMessage(CrosslistConstants.LOOKUP_FAILURE_SECTION_NOT_FOUND_IN_SIS_MESSAGE); + findParentResult.setShowCourseInfo(false); + findParentResult.setStatusMessage(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_SIS_MESSAGE); findParentResult.setStatusIconCssClasses(CrosslistConstants.LOOKUP_FAILURE_CSS); return findParentResult; } @@ -402,13 +403,15 @@ public FindParentResult processSisLookup(SisCourse sisCourse) { Section section = sectionService.getSection(String.format("sis_section_id:%s", sisCourse.getIuSiteId())); if (section == null) { - findParentResult.setStatusMessage(CrosslistConstants.LOOKUP_FAILURE_SECTION_NOT_FOUND_IN_CANVAS_MESSAGE); + findParentResult.setShowCourseInfo(false); + findParentResult.setStatusMessage(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_CANVAS_MESSAGE); findParentResult.setStatusIconCssClasses(CrosslistConstants.LOOKUP_FAILURE_CSS); return findParentResult; } if (section.getSis_course_id() == null || section.getSis_section_id() == null) { - findParentResult.setStatusMessage(CrosslistConstants.LOOKUP_FAILURE_SECTION_NOT_FOUND_IN_CANVAS_MESSAGE); + findParentResult.setShowCourseInfo(false); + findParentResult.setStatusMessage(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_CANVAS_MESSAGE); findParentResult.setStatusIconCssClasses(CrosslistConstants.LOOKUP_FAILURE_CSS); return findParentResult; } @@ -416,7 +419,8 @@ public FindParentResult processSisLookup(SisCourse sisCourse) { Course course = courseService.getCourse(section.getCourse_id()); if (course == null) { - findParentResult.setStatusMessage(CrosslistConstants.LOOKUP_FAILURE_SECTION_NOT_FOUND_IN_CANVAS_MESSAGE); + findParentResult.setShowCourseInfo(false); + findParentResult.setStatusMessage(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_CANVAS_MESSAGE); findParentResult.setStatusIconCssClasses(CrosslistConstants.LOOKUP_FAILURE_CSS); return findParentResult; } diff --git a/src/main/resources/static/css/crosslisting.css b/src/main/resources/static/css/crosslisting.css index 014333f..f307f54 100644 --- a/src/main/resources/static/css/crosslisting.css +++ b/src/main/resources/static/css/crosslisting.css @@ -93,6 +93,6 @@ margin-bottom: 0.5rem; } -.alertThing { +.resultsWidth { width: max-content; } \ No newline at end of file diff --git a/src/main/resources/templates/findParentCourse.html b/src/main/resources/templates/findParentCourse.html index b5f7b58..9049735 100644 --- a/src/main/resources/templates/findParentCourse.html +++ b/src/main/resources/templates/findParentCourse.html @@ -55,54 +55,52 @@

Find parent course

  • -
  • -
    • -
  • -
    - - -
    -
-
+
- -
Example: SP22-BL-FOLK-E295-4441
+ +
Example: SP22-BL-FOLK-E295-4441
- +
+
-
+
@@ -116,31 +114,15 @@

Find parent course

-
- - -
- -
-
- -
Example: 2167456
-
- -
-
-
- +
-
- +

Parent course found

@@ -156,53 +138,50 @@

SP22-BL-FOLK-E295-1234

- - - - - - - - - - - - - - - - - - - - - - - - - - - -
diff --git a/src/test/java/edu/iu/uits/lms/crosslist/services/CrosslistServiceImplTest.java b/src/test/java/edu/iu/uits/lms/crosslist/services/CrosslistServiceImplTest.java index ab368ee..8b209d2 100644 --- a/src/test/java/edu/iu/uits/lms/crosslist/services/CrosslistServiceImplTest.java +++ b/src/test/java/edu/iu/uits/lms/crosslist/services/CrosslistServiceImplTest.java @@ -33,10 +33,14 @@ * #L% */ +import edu.iu.uits.lms.canvas.config.CanvasConfiguration; import edu.iu.uits.lms.canvas.model.CanvasTerm; import edu.iu.uits.lms.canvas.model.Course; import edu.iu.uits.lms.canvas.model.Section; import edu.iu.uits.lms.canvas.services.CourseService; +import edu.iu.uits.lms.canvas.services.SectionService; +import edu.iu.uits.lms.crosslist.CrosslistConstants; +import edu.iu.uits.lms.crosslist.model.FindParentResult; import edu.iu.uits.lms.crosslist.model.SectionUIDisplay; import edu.iu.uits.lms.crosslist.service.CrosslistService; import edu.iu.uits.lms.iuonly.model.SisCourse; @@ -63,10 +67,18 @@ public class CrosslistServiceImplTest { @InjectMocks private CrosslistService crosslistService = null; + @Autowired + @Mock + private CanvasConfiguration canvasConfiguration = null; + @Autowired @Mock private CourseService courseService = null; + @Autowired + @Mock + private SectionService sectionService = null; + @Autowired @Mock private SisServiceImpl sisService = null; @@ -706,6 +718,178 @@ public void first_sis_course_with_many_etexts_second_sis_course_with_many_etext_ Assertions.assertFalse(crosslistService.canCoursesBeCrosslistedBasedOnEtexts(sourceSisCourseSiteId1, sourceSisCourseSiteId2)); } + @Test + public void nullSisCourse_processSisLookup() { + FindParentResult findParentResult = crosslistService.processSisLookup(null); + + Assertions.assertFalse(findParentResult.isShowCourseInfo()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_SIS_MESSAGE, findParentResult.getStatusMessage()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_CSS, findParentResult.getStatusIconCssClasses()); + } + + @Test + public void nullIuSiteId_processSisLookup() { + SisCourse sisCourse = new SisCourse(); + + FindParentResult findParentResult = crosslistService.processSisLookup(sisCourse); + + Assertions.assertFalse(findParentResult.isShowCourseInfo()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_SIS_MESSAGE, findParentResult.getStatusMessage()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_CSS, findParentResult.getStatusIconCssClasses()); + } + + @Test + public void nullSection_processSisLookup() { + SisCourse sisCourse = new SisCourse(); + + final String iuSiteId = "TestIuSiteId"; + + sisCourse.setIuSiteId(iuSiteId); + + FindParentResult findParentResult = crosslistService.processSisLookup(sisCourse); + + Assertions.assertFalse(findParentResult.isShowCourseInfo()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_CANVAS_MESSAGE, findParentResult.getStatusMessage()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_CSS, findParentResult.getStatusIconCssClasses()); + } + + @Test + public void nullSectionSisCourseId_processSisLookup() { + SisCourse sisCourse = new SisCourse(); + + final String iuSiteId = "TestIuSiteId"; + + sisCourse.setIuSiteId(iuSiteId); + + Section section = new Section(); + section.setSis_section_id("123"); + + Mockito.when(sectionService.getSection(String.format("sis_section_id:%s", sisCourse.getIuSiteId()))).thenReturn(section); + + FindParentResult findParentResult = crosslistService.processSisLookup(sisCourse); + + Assertions.assertFalse(findParentResult.isShowCourseInfo()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_CANVAS_MESSAGE, findParentResult.getStatusMessage()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_CSS, findParentResult.getStatusIconCssClasses()); + } + + @Test + public void nullSectionSisSectionId_processSisLookup() { + SisCourse sisCourse = new SisCourse(); + + final String iuSiteId = "TestIuSiteId"; + + sisCourse.setIuSiteId(iuSiteId); + + Section section = new Section(); + section.setSis_course_id("123"); + + Mockito.when(sectionService.getSection(String.format("sis_section_id:%s", sisCourse.getIuSiteId()))).thenReturn(section); + + FindParentResult findParentResult = crosslistService.processSisLookup(sisCourse); + + Assertions.assertFalse(findParentResult.isShowCourseInfo()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_CANVAS_MESSAGE, findParentResult.getStatusMessage()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_CSS, findParentResult.getStatusIconCssClasses()); + } + + @Test + public void nullCourse_processSisLookup() { + SisCourse sisCourse = new SisCourse(); + + final String iuSiteId = "TestIuSiteId"; + + sisCourse.setIuSiteId(iuSiteId); + + Section section = new Section(); + section.setSis_course_id("123"); + section.setSis_section_id("123"); + + Mockito.when(sectionService.getSection(String.format("sis_section_id:%s", sisCourse.getIuSiteId()))).thenReturn(section); + + FindParentResult findParentResult = crosslistService.processSisLookup(sisCourse); + + Assertions.assertFalse(findParentResult.isShowCourseInfo()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_NOT_FOUND_IN_CANVAS_MESSAGE, findParentResult.getStatusMessage()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_CSS, findParentResult.getStatusIconCssClasses()); + } + + @Test + public void notCrosslistedBecauseSectionSisCourseIdEqualsSectionSisSectionId_processSisLookup() { + SisCourse sisCourse = new SisCourse(); + + final String iuSiteId = "TestIuSiteId"; + + sisCourse.setIuSiteId(iuSiteId); + + Section section = new Section(); + section.setSis_course_id("123"); + section.setSis_section_id("123"); + + final String baseUrl = "https://testsite.org"; + + final String courseName = "Course Name 1"; + final String sisCourseId = "123"; + + Course course = new Course(); + course.setId("1"); + course.setName(courseName); + course.setSisCourseId(sisCourseId); + + Mockito.when(sectionService.getSection(String.format("sis_section_id:%s", sisCourse.getIuSiteId()))).thenReturn(section); + Mockito.when(courseService.getCourse(section.getCourse_id())).thenReturn(course); + Mockito.when(canvasConfiguration.getBaseUrl()).thenReturn(baseUrl); + + FindParentResult findParentResult = crosslistService.processSisLookup(sisCourse); + + // findParentResult.setName(course.getName()); + // findParentResult.setSisCourseId(course.getSisCourseId()); + + Assertions.assertTrue(findParentResult.isShowCourseInfo()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_COURSE_NOT_CROSSLISTED_MESSAGE, findParentResult.getStatusMessage()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_FAILURE_CSS, findParentResult.getStatusIconCssClasses()); + Assertions.assertEquals(String.format("%s/courses/%s", canvasConfiguration.getBaseUrl(), course.getId()), findParentResult.getUrl()); + Assertions.assertEquals(courseName, findParentResult.getName()); + Assertions.assertEquals(sisCourseId, findParentResult.getSisCourseId()); + } + + @Test + public void parentFoundSuccess_processSisLookup() { + SisCourse sisCourse = new SisCourse(); + + final String iuSiteId = "TestIuSiteId"; + + sisCourse.setIuSiteId(iuSiteId); + + Section section = new Section(); + section.setSis_course_id("123"); + section.setSis_section_id("456"); + + final String baseUrl = "https://testsite.org"; + + final String courseName = "Course Name 1"; + final String sisCourseId = "123"; + + Course course = new Course(); + course.setId("1"); + course.setName(courseName); + course.setSisCourseId(sisCourseId); + + Mockito.when(sectionService.getSection(String.format("sis_section_id:%s", sisCourse.getIuSiteId()))).thenReturn(section); + Mockito.when(courseService.getCourse(section.getCourse_id())).thenReturn(course); + Mockito.when(canvasConfiguration.getBaseUrl()).thenReturn(baseUrl); + + FindParentResult findParentResult = crosslistService.processSisLookup(sisCourse); + + Assertions.assertTrue(findParentResult.isShowCourseInfo()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_SUCCESS_FOUND_MESSAGE, findParentResult.getStatusMessage()); + Assertions.assertEquals(CrosslistConstants.LOOKUP_SUCCESS_CSS, findParentResult.getStatusIconCssClasses()); + Assertions.assertEquals(String.format("%s/courses/%s", canvasConfiguration.getBaseUrl(), course.getId()), findParentResult.getUrl()); + Assertions.assertEquals(courseName, findParentResult.getName()); + Assertions.assertEquals(sisCourseId, findParentResult.getSisCourseId()); + } + + private Course createCourse(String courseId, String sisCourseId) { Course course = new Course(); course.setId(courseId);