Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LMSA-9020 - re-work etext checking #12

Merged
merged 3 commits into from
Sep 14, 2023
Merged

LMSA-9020 - re-work etext checking #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
59eb982
LMSA-9020 - re-work etext checking
iudsobiera Sep 13, 2023
ed0b15a
LMSA-9020 - remove unneeded import
iudsobiera Sep 13, 2023
cb4bf83
LMSA-9020 - changes made after code review
iudsobiera Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 19 additions & 48 deletions src/main/java/edu/iu/uits/lms/crosslist/controller/CrosslistController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,62 +400,33 @@ public String doContinue(@PathVariable("courseId") String courseId, @RequestPara
model.addAttribute("courseTitle", currentCourse.getName());
model.addAttribute("removeListSections", sectionWrapper.getRemoveList());

SisCourse sisCurrentCourse = sisService.getSisCourseBySiteId(currentCourse.getSisCourseId());

// if course has etexts, check to see if the crosslisted sections have the same etexts
if (sisCurrentCourse != null && sisCurrentCourse.getEtextIsbns() != null) {
List<String> missingEtextSections = new ArrayList<String>();

String[] currentCourseEtextIsbns = sisCurrentCourse.getEtextIsbns().split(",");

for (SectionUIDisplay sectionUIDisplay : sectionWrapper.getAddList()) {
String sectionUIDisplaySectionName = sectionUIDisplay.getSectionName();

// Needed in case of in impersonation mode the section name will read
// FA20-blah-blah-blah-1234 (FA20-blah-blah-blah-1234)
int indexOfParenthesis = sectionUIDisplaySectionName.indexOf("(");

if (indexOfParenthesis != -1) {
sectionUIDisplaySectionName = sectionUIDisplaySectionName.substring(0, indexOfParenthesis).trim();
}

SisCourse sisCrosslistCourse = sisService.getSisCourseBySiteId(sectionUIDisplaySectionName);

if (sisCrosslistCourse == null || sisCrosslistCourse.getEtextIsbns() == null) {
String sectionName = sectionUIDisplay.getSectionName();
missingEtextSections.add(sectionName);
// if the current course has etexts, check to see if the wanted crosslisted sections have the same etexts
List<String> missingEtextSections = new ArrayList<>();

List<SectionUIDisplay> addList = sectionWrapper.getAddList();
List<SectionUIDisplay> finalList = sectionWrapper.getFinalList();
for (SectionUIDisplay sectionUIDisplay : sectionWrapper.getAddList()) {
String sectionUIDisplaySectionName = sectionUIDisplay.getSectionName();

sectionWrapper.setAddList(removeSectionUiDisplayBySectionName(addList, sectionName));
sectionWrapper.setFinalList(removeSectionUiDisplayBySectionName(finalList, sectionName));
uncheckSectionUiDisplayBySectionId(sectionUIDisplay.getSectionId(), sectionList);
} else {
List<String> crosslistCourseEtextIsbns = Arrays.asList(sisCrosslistCourse.getEtextIsbns().split(","));
// Needed in case of in impersonation mode the section name will read
// FA20-blah-blah-blah-1234 (FA20-blah-blah-blah-1234)
int indexOfParenthesis = sectionUIDisplaySectionName.indexOf("(");

for (String currentCourseEtextIsbn : currentCourseEtextIsbns) {
boolean exists = crosslistCourseEtextIsbns.contains(currentCourseEtextIsbn);

if (! exists) {
String sectionName = sectionUIDisplay.getSectionName();
missingEtextSections.add(sectionName);
if (indexOfParenthesis != -1) {
sectionUIDisplaySectionName = sectionUIDisplaySectionName.substring(0, indexOfParenthesis).trim();
}

List<SectionUIDisplay> addList = sectionWrapper.getAddList();
List<SectionUIDisplay> finalList = sectionWrapper.getFinalList();
if (! crosslistService.canCoursesBeCrosslistedBasedOnEtexts(currentCourse.getSisCourseId(), sectionUIDisplaySectionName)) {
sectionWrapper.setAddList(removeSectionUiDisplayBySectionName(sectionWrapper.getAddList(), sectionUIDisplay.getSectionName()));
sectionWrapper.setFinalList(removeSectionUiDisplayBySectionName(sectionWrapper.getFinalList(), sectionUIDisplay.getSectionName()));
uncheckSectionUiDisplayBySectionId(sectionUIDisplay.getSectionId(), sectionList);

sectionWrapper.setAddList(removeSectionUiDisplayBySectionName(addList, sectionName));
sectionWrapper.setFinalList(removeSectionUiDisplayBySectionName(finalList, sectionName));
uncheckSectionUiDisplayBySectionId(sectionUIDisplay.getSectionId(), sectionList);
}
}
}
missingEtextSections.add(sectionUIDisplay.getSectionName());
}
}

if (missingEtextSections.size() > 0) {
model.addAttribute("missingEtextSections", missingEtextSections);
}
} // end etexts
if (! missingEtextSections.isEmpty()) {
model.addAttribute("missingEtextSections", missingEtextSections);
}

model.addAttribute("summaryListSections", sectionWrapper.getFinalList());
model.addAttribute("addListSections", sectionWrapper.getAddList());
Expand Down
33 changes: 33 additions & 0 deletions src/main/java/edu/iu/uits/lms/crosslist/service/CrosslistService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@
import edu.iu.uits.lms.common.session.CourseSessionService;
import edu.iu.uits.lms.crosslist.CrosslistConstants;
import edu.iu.uits.lms.crosslist.model.SectionUIDisplay;
import edu.iu.uits.lms.iuonly.model.SisCourse;
import edu.iu.uits.lms.iuonly.services.FeatureAccessServiceImpl;
import edu.iu.uits.lms.iuonly.services.SisServiceImpl;
import lombok.Data;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Autowired;
Expand All @@ -57,6 +59,7 @@
import java.time.LocalDate;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.Date;
import java.util.List;
Expand All @@ -82,6 +85,9 @@ public class CrosslistService {
@Autowired
private AccountService accountService = null;

@Autowired
private SisServiceImpl sisService;

// self reference so can use the cache for getCourseSections() from within this service
@Lazy
@Autowired
Expand Down Expand Up @@ -347,6 +353,33 @@ public List<Section> getCourseSections(String courseId) {
return courseService.getCourseSections(courseId);
}

public boolean canCoursesBeCrosslistedBasedOnEtexts(String sourceSisCourseSiteId, String destinationSisCourseSiteId) {
SisCourse sourceSisCourse = sisService.getSisCourseBySiteId(sourceSisCourseSiteId);
sourceSisCourse = sourceSisCourse == null ? new SisCourse() : sourceSisCourse;

SisCourse destinationSisCourse = sisService.getSisCourseBySiteId(destinationSisCourseSiteId);
destinationSisCourse = destinationSisCourse == null ? new SisCourse() : destinationSisCourse;

if (sourceSisCourse.getEtextIsbns() == null && destinationSisCourse.getEtextIsbns() != null) {
return false;
}

if (sourceSisCourse.getEtextIsbns() != null && destinationSisCourse.getEtextIsbns() == null) {
return false;
}

List<String> sourceCourseEtextIsbns = sourceSisCourse.getEtextIsbns() == null
? new ArrayList<>() : new ArrayList<>(List.of(sourceSisCourse.getEtextIsbns().split(",")));

List<String> destinationCourseEtextIsbns = destinationSisCourse.getEtextIsbns() == null
? new ArrayList<>() : new ArrayList<>(List.of(destinationSisCourse.getEtextIsbns().split(",")));

Collections.sort(sourceCourseEtextIsbns);
Collections.sort(destinationCourseEtextIsbns);

return sourceCourseEtextIsbns.equals(destinationCourseEtextIsbns);
}

@Data
private class CourseSisNaturalAndAlien {
String courseId;
Expand Down
113 changes: 113 additions & 0 deletions src/test/java/edu/iu/uits/lms/crosslist/services/CrosslistServiceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
import edu.iu.uits.lms.canvas.services.CourseService;
import edu.iu.uits.lms.crosslist.model.SectionUIDisplay;
import edu.iu.uits.lms.crosslist.service.CrosslistService;
import edu.iu.uits.lms.iuonly.model.SisCourse;
import edu.iu.uits.lms.iuonly.services.SisServiceImpl;
import lombok.extern.slf4j.Slf4j;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -65,6 +67,10 @@ public class CrosslistServiceImplTest {
@Mock
private CourseService courseService = null;

@Autowired
@Mock
private SisServiceImpl sisService = null;

@Autowired
@Mock
private CrosslistService self;
Expand Down Expand Up @@ -593,6 +599,113 @@ public void unavailable_true_true_true() {

}

@Test
public void both_not_siscourses_no_etext_canCoursesBeCrosslistedBasedOnEtexts() {
String sourceSisCourseSiteId1 = "course1";
String sourceSisCourseSiteId2 = "course2";

Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId1)).thenReturn(null);
Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId2)).thenReturn(null);

Assertions.assertTrue(crosslistService.canCoursesBeCrosslistedBasedOnEtexts(sourceSisCourseSiteId1, sourceSisCourseSiteId2));
}

@Test
public void first_not_sis_second_is_sis_courses_with_etext_canCoursesBeCrosslistedBasedOnEtexts() {
String sourceSisCourseSiteId1 = "course1";
String sourceSisCourseSiteId2 = "course2";

SisCourse sisCourse = new SisCourse();
sisCourse.setEtextIsbns("etext1");

Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId1)).thenReturn(null);
Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId2)).thenReturn(sisCourse);

Assertions.assertFalse(crosslistService.canCoursesBeCrosslistedBasedOnEtexts(sourceSisCourseSiteId1, sourceSisCourseSiteId2));
}

@Test
public void first_sis_with_etext_second_is_not_sis_courses_canCoursesBeCrosslistedBasedOnEtexts() {
String sourceSisCourseSiteId1 = "course1";
String sourceSisCourseSiteId2 = "course2";

SisCourse sisCourse = new SisCourse();
sisCourse.setEtextIsbns("etext1");

Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId1)).thenReturn(sisCourse);
Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId2)).thenReturn(null);

Assertions.assertFalse(crosslistService.canCoursesBeCrosslistedBasedOnEtexts(sourceSisCourseSiteId1, sourceSisCourseSiteId2));
}

@Test
public void first_sis_course_with_etext_second_sis_course_with_matching_etext_canCoursesBeCrosslistedBasedOnEtexts() {
String sourceSisCourseSiteId1 = "course1";
String sourceSisCourseSiteId2 = "course2";

SisCourse sisCourse1 = new SisCourse();
sisCourse1.setEtextIsbns("etext1");

SisCourse sisCourse2 = new SisCourse();
sisCourse2.setEtextIsbns("etext1");

Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId1)).thenReturn(sisCourse1);
Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId2)).thenReturn(sisCourse2);

Assertions.assertTrue(crosslistService.canCoursesBeCrosslistedBasedOnEtexts(sourceSisCourseSiteId1, sourceSisCourseSiteId2));
}

@Test
public void first_sis_course_with_etext_second_sis_course_with_different_etext_canCoursesBeCrosslistedBasedOnEtexts() {
String sourceSisCourseSiteId1 = "course1";
String sourceSisCourseSiteId2 = "course2";

SisCourse sisCourse1 = new SisCourse();
sisCourse1.setEtextIsbns("etext1");

SisCourse sisCourse2 = new SisCourse();
sisCourse2.setEtextIsbns("etext2");

Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId1)).thenReturn(sisCourse1);
Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId2)).thenReturn(sisCourse2);

Assertions.assertFalse(crosslistService.canCoursesBeCrosslistedBasedOnEtexts(sourceSisCourseSiteId1, sourceSisCourseSiteId2));
}

@Test
public void first_sis_course_with_many_etexts_second_sis_course_with_only_one_matching_etext_canCoursesBeCrosslistedBasedOnEtexts() {
String sourceSisCourseSiteId1 = "course1";
String sourceSisCourseSiteId2 = "course2";

SisCourse sisCourse1 = new SisCourse();
sisCourse1.setEtextIsbns("etext1,etext2");

SisCourse sisCourse2 = new SisCourse();
sisCourse2.setEtextIsbns("etext2");

Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId1)).thenReturn(sisCourse1);
Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId2)).thenReturn(sisCourse2);

Assertions.assertFalse(crosslistService.canCoursesBeCrosslistedBasedOnEtexts(sourceSisCourseSiteId1, sourceSisCourseSiteId2));
}

@Test
public void first_sis_course_with_many_etexts_second_sis_course_with_many_etext_only_one_matching_etext_canCoursesBeCrosslistedBasedOnEtexts() {
String sourceSisCourseSiteId1 = "course1";
String sourceSisCourseSiteId2 = "course2";

SisCourse sisCourse1 = new SisCourse();
sisCourse1.setEtextIsbns("etext1,etext2");

SisCourse sisCourse2 = new SisCourse();
sisCourse2.setEtextIsbns("etext2,etext3");

Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId1)).thenReturn(sisCourse1);
Mockito.when(sisService.getSisCourseBySiteId(sourceSisCourseSiteId2)).thenReturn(sisCourse2);

Assertions.assertFalse(crosslistService.canCoursesBeCrosslistedBasedOnEtexts(sourceSisCourseSiteId1, sourceSisCourseSiteId2));
}

private Course createCourse(String courseId, String sisCourseId) {
Course course = new Course();
course.setId(courseId);
Expand Down