From 5c184cee6d846f017c5b750c8611606ec643783c Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Thu, 8 Oct 2026 11:09:48 -0700 Subject: [PATCH 01/12] feat: add TeacherPresentation config and reflection answers API --- .../TeacherPresentationConfigDao.java | 25 ++ ...HibernateTeacherPresentationConfigDao.java | 74 ++++++ ...eacherPresentationConfigAPIController.java | 172 ++++++++++++++ .../TeacherPresentationConfigService.java | 46 ++++ .../TeacherPresentationConfigServiceImpl.java | 218 ++++++++++++++++++ .../TeacherPresentationConfig.java | 89 +++++++ .../TeacherPresentationReflectionAnswer.java | 64 +++++ ...V2__create_teacher_presentation_tables.sql | 35 +++ ...erPresentationConfigAPIControllerTest.java | 165 +++++++++++++ ...cherPresentationConfigServiceImplTest.java | 166 +++++++++++++ 10 files changed, 1054 insertions(+) create mode 100644 src/main/java/org/wise/portal/dao/teacherpresentation/TeacherPresentationConfigDao.java create mode 100644 src/main/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDao.java create mode 100644 src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java create mode 100644 src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java create mode 100644 src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationReflectionAnswer.java create mode 100644 src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql create mode 100644 src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java create mode 100644 src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java diff --git a/src/main/java/org/wise/portal/dao/teacherpresentation/TeacherPresentationConfigDao.java b/src/main/java/org/wise/portal/dao/teacherpresentation/TeacherPresentationConfigDao.java new file mode 100644 index 000000000..57a4ef54a --- /dev/null +++ b/src/main/java/org/wise/portal/dao/teacherpresentation/TeacherPresentationConfigDao.java @@ -0,0 +1,25 @@ +package org.wise.portal.dao.teacherpresentation; + +import java.util.List; + +import org.wise.portal.dao.SimpleDao; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; + +public interface TeacherPresentationConfigDao + extends SimpleDao { + + /** + * @return the config for the given run, period, node and component, or null if none exists + */ + TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, String componentId); + + List getAnswers(TeacherPresentationConfig config); + + TeacherPresentationReflectionAnswer getAnswer(TeacherPresentationConfig config, + String questionId); + + void saveAnswer(TeacherPresentationReflectionAnswer answer); +} diff --git a/src/main/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDao.java b/src/main/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDao.java new file mode 100644 index 000000000..5bf015b53 --- /dev/null +++ b/src/main/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDao.java @@ -0,0 +1,74 @@ +package org.wise.portal.dao.teacherpresentation.impl; + +import java.util.List; + +import jakarta.persistence.TypedQuery; +import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; +import jakarta.persistence.criteria.Root; + +import org.springframework.stereotype.Repository; +import org.wise.portal.dao.impl.AbstractHibernateDao; +import org.wise.portal.dao.teacherpresentation.TeacherPresentationConfigDao; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; + +@Repository("teacherPresentationConfigDao") +public class HibernateTeacherPresentationConfigDao + extends AbstractHibernateDao + implements TeacherPresentationConfigDao { + + @Override + protected Class getDataObjectClass() { + return TeacherPresentationConfig.class; + } + + @Override + public TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, + String componentId) { + CriteriaBuilder cb = getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(TeacherPresentationConfig.class); + Root root = cq.from(TeacherPresentationConfig.class); + cq.select(root).where(cb.equal(root.get("run"), run), cb.equal(root.get("period"), period), + cb.equal(root.get("nodeId"), nodeId), cb.equal(root.get("componentId"), componentId)); + List results = entityManager.createQuery(cq).getResultList(); + return results.isEmpty() ? null : results.get(0); + } + + @Override + public List getAnswers(TeacherPresentationConfig config) { + CriteriaBuilder cb = getCriteriaBuilder(); + CriteriaQuery cq = cb + .createQuery(TeacherPresentationReflectionAnswer.class); + Root root = cq + .from(TeacherPresentationReflectionAnswer.class); + cq.select(root).where(cb.equal(root.get("teacherPresentationConfig"), config)); + return entityManager.createQuery(cq).getResultList(); + } + + @Override + public TeacherPresentationReflectionAnswer getAnswer(TeacherPresentationConfig config, + String questionId) { + CriteriaBuilder cb = getCriteriaBuilder(); + CriteriaQuery cq = cb + .createQuery(TeacherPresentationReflectionAnswer.class); + Root root = cq + .from(TeacherPresentationReflectionAnswer.class); + cq.select(root).where(cb.equal(root.get("teacherPresentationConfig"), config), + cb.equal(root.get("questionId"), questionId)); + TypedQuery query = entityManager.createQuery(cq); + List results = query.getResultList(); + return results.isEmpty() ? null : results.get(0); + } + + @Override + public void saveAnswer(TeacherPresentationReflectionAnswer answer) { + if (answer.getId() == null) { + entityManager.persist(answer); + } else { + entityManager.merge(answer); + } + } +} diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java new file mode 100644 index 000000000..30aba3822 --- /dev/null +++ b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java @@ -0,0 +1,172 @@ +package org.wise.portal.presentation.web.controllers.teacher; + +import java.util.ArrayList; +import java.util.List; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.Authentication; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PutMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.server.ResponseStatusException; +import org.wise.portal.dao.ObjectNotFoundException; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.portal.domain.user.User; +import org.wise.portal.domain.workgroup.Workgroup; +import org.wise.portal.service.run.RunService; +import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; +import org.wise.portal.service.user.UserService; +import org.wise.portal.service.workgroup.WorkgroupService; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; + +/** + * REST API for TeacherPresentation: the teacher's selected student work, student name display + * setting, prompt and reflection answers for a component in a period. + */ +@RestController +@RequestMapping("/api/teacher/presentation-config") +@PreAuthorize("hasRole('ROLE_TEACHER')") +public class TeacherPresentationConfigAPIController { + + @Autowired + private TeacherPresentationConfigService teacherPresentationConfigService; + + @Autowired + private RunService runService; + + @Autowired + private UserService userService; + + @Autowired + private WorkgroupService workgroupService; + + public record Item(Integer studentWorkId) { + } + + public record ConfigRequest(Long runId, Long periodId, String nodeId, String componentId, + String componentType, List items, String studentNamesDisplay, String prompt) { + } + + public record AnswerRequest(Long runId, Long periodId, String nodeId, String componentId, + String componentType, String questionId, String questionText, String answerText) { + } + + public record AnswerResponse(String questionId, String questionText, String answerText, + Long answeredByWorkgroupId, Long updatedAt) { + } + + public record ConfigResponse(Long id, Long runId, Long periodId, String nodeId, + String componentId, String componentType, List items, String studentNamesDisplay, + String prompt, Long updatedByWorkgroupId, Long updatedAt, List answers) { + } + + @GetMapping + ConfigResponse getConfig(Authentication auth, @RequestParam Long runId, + @RequestParam Long periodId, @RequestParam String nodeId, + @RequestParam String componentId) throws ObjectNotFoundException { + Run run = runService.retrieveById(runId); + User user = userService.retrieveUserByUsername(auth.getName()); + if (!runService.isAllowedToViewStudentWork(run, user)) { + throw new AccessDeniedException("Not permitted"); + } + Group period = getPeriod(run, periodId); + TeacherPresentationConfig config = teacherPresentationConfigService.getConfig(run, period, + nodeId, componentId); + if (config == null) { + return new ConfigResponse(null, runId, periodId, nodeId, componentId, null, new ArrayList<>(), + TeacherPresentationConfig.NAMES_HIDE, null, null, null, new ArrayList<>()); + } + return toResponse(config); + } + + @PutMapping + ConfigResponse saveConfig(Authentication auth, @RequestBody ConfigRequest request) + throws ObjectNotFoundException { + Run run = runService.retrieveById(request.runId()); + User user = userService.retrieveUserByUsername(auth.getName()); + if (!runService.isAllowedToGradeStudentWork(run, user)) { + throw new AccessDeniedException("Not permitted"); + } + Group period = getPeriod(run, request.periodId()); + List studentWorkIds = new ArrayList<>(); + if (request.items() != null) { + for (Item item : request.items()) { + studentWorkIds.add(item == null ? null : item.studentWorkId()); + } + } + try { + TeacherPresentationConfig config = teacherPresentationConfigService.saveConfig(run, period, + request.nodeId(), request.componentId(), request.componentType(), studentWorkIds, + request.studentNamesDisplay(), request.prompt(), getTeacherWorkgroup(run, user)); + return toResponse(config); + } catch (IllegalArgumentException e) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage()); + } + } + + @PutMapping("/answers") + AnswerResponse saveAnswer(Authentication auth, @RequestBody AnswerRequest request) + throws ObjectNotFoundException { + Run run = runService.retrieveById(request.runId()); + User user = userService.retrieveUserByUsername(auth.getName()); + if (!runService.isAllowedToGradeStudentWork(run, user)) { + throw new AccessDeniedException("Not permitted"); + } + Group period = getPeriod(run, request.periodId()); + try { + TeacherPresentationReflectionAnswer answer = teacherPresentationConfigService.saveAnswer(run, + period, request.nodeId(), request.componentId(), request.componentType(), + request.questionId(), request.questionText(), request.answerText(), + getTeacherWorkgroup(run, user)); + return toAnswerResponse(answer); + } catch (IllegalArgumentException e) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage()); + } + } + + private Group getPeriod(Run run, Long periodId) { + for (Group period : run.getPeriods()) { + if (period.getId().equals(periodId)) { + return period; + } + } + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Period does not belong to the run"); + } + + private Workgroup getTeacherWorkgroup(Run run, User user) { + List workgroups = workgroupService.getWorkgroupListByRunAndUser(run, user); + return workgroups.isEmpty() ? null : workgroups.get(0); + } + + private ConfigResponse toResponse(TeacherPresentationConfig config) { + List items = new ArrayList<>(); + for (Integer id : teacherPresentationConfigService.getStudentWorkIds(config)) { + items.add(new Item(id)); + } + List answers = new ArrayList<>(); + for (TeacherPresentationReflectionAnswer answer : teacherPresentationConfigService + .getAnswers(config)) { + answers.add(toAnswerResponse(answer)); + } + Workgroup updatedBy = config.getUpdatedByWorkgroup(); + return new ConfigResponse(config.getId(), config.getRun().getId(), config.getPeriod().getId(), + config.getNodeId(), config.getComponentId(), config.getComponentType(), items, + config.getStudentNamesDisplay(), config.getPrompt(), + updatedBy == null ? null : updatedBy.getId(), config.getUpdatedAt().getTime(), answers); + } + + private AnswerResponse toAnswerResponse(TeacherPresentationReflectionAnswer answer) { + Workgroup answeredBy = answer.getAnsweredByWorkgroup(); + return new AnswerResponse(answer.getQuestionId(), answer.getQuestionText(), + answer.getAnswerText(), answeredBy == null ? null : answeredBy.getId(), + answer.getUpdatedAt().getTime()); + } +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java new file mode 100644 index 000000000..6eef6a227 --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java @@ -0,0 +1,46 @@ +package org.wise.portal.service.teacherpresentation; + +import java.util.List; + +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.portal.domain.workgroup.Workgroup; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; + +public interface TeacherPresentationConfigService { + + int MAX_ITEMS = 500; + int MAX_PROMPT_LENGTH = 1000; + + /** + * @return the config, or null if the teacher has not saved one yet + */ + TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, String componentId); + + List getAnswers(TeacherPresentationConfig config); + + List getStudentWorkIds(TeacherPresentationConfig config); + + /** + * Creates or updates the config for the run, period, node and component. + * + * @param prompt the teacher's prompt. null or blank resets to the component default. + * @param studentNamesDisplay one of show, hide or anonymize. null keeps the current value (or + * hide for a new config). + * @throws IllegalArgumentException if the input is invalid + */ + TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, + String componentType, List studentWorkIds, String studentNamesDisplay, + String prompt, Workgroup teacherWorkgroup); + + /** + * Creates or updates the answer to a reflection question. Creates the config if it does not + * exist yet. + * + * @throws IllegalArgumentException if the input is invalid + */ + TeacherPresentationReflectionAnswer saveAnswer(Run run, Group period, String nodeId, + String componentId, String componentType, String questionId, String questionText, + String answerText, Workgroup teacherWorkgroup); +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java new file mode 100644 index 000000000..1c9bcf165 --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java @@ -0,0 +1,218 @@ +package org.wise.portal.service.teacherpresentation.impl; + +import java.sql.Timestamp; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; +import org.wise.portal.dao.ObjectNotFoundException; +import org.wise.portal.dao.teacherpresentation.TeacherPresentationConfigDao; +import org.wise.portal.dao.work.StudentWorkDao; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.portal.domain.workgroup.Workgroup; +import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; +import org.wise.vle.domain.work.StudentWork; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; + +@Service +public class TeacherPresentationConfigServiceImpl implements TeacherPresentationConfigService { + + private static final int MAX_ID_LENGTH = 30; + private static final int MAX_QUESTION_TEXT_LENGTH = 5000; + private static final int MAX_ANSWER_TEXT_LENGTH = 20000; + private static final Set NAMES_DISPLAY_OPTIONS = Set.of( + TeacherPresentationConfig.NAMES_SHOW, TeacherPresentationConfig.NAMES_HIDE, + TeacherPresentationConfig.NAMES_ANONYMIZE); + + private final ObjectMapper objectMapper = new ObjectMapper(); + + @Autowired + private TeacherPresentationConfigDao teacherPresentationConfigDao; + + @Autowired + private StudentWorkDao studentWorkDao; + + @Override + @Transactional(readOnly = true) + public TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, + String componentId) { + return teacherPresentationConfigDao.getConfig(run, period, nodeId, componentId); + } + + @Override + @Transactional(readOnly = true) + public List getAnswers(TeacherPresentationConfig config) { + return teacherPresentationConfigDao.getAnswers(config); + } + + @Override + public List getStudentWorkIds(TeacherPresentationConfig config) { + try { + List> items = objectMapper.readValue(config.getItems(), + new TypeReference>>() { + }); + List ids = new ArrayList<>(); + for (Map item : items) { + ids.add(item.get("studentWorkId")); + } + return ids; + } catch (JsonProcessingException e) { + return new ArrayList<>(); + } + } + + @Override + @Transactional + public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, + String componentId, String componentType, List studentWorkIds, + String studentNamesDisplay, String prompt, Workgroup teacherWorkgroup) { + validateComponent(nodeId, componentId, componentType); + if (studentNamesDisplay != null && !NAMES_DISPLAY_OPTIONS.contains(studentNamesDisplay)) { + throw new IllegalArgumentException("Invalid studentNamesDisplay: " + studentNamesDisplay); + } + String normalizedPrompt = normalizePrompt(prompt); + Set uniqueIds = validateStudentWorkIds(run, period, studentWorkIds); + TeacherPresentationConfig config = getOrCreateConfig(run, period, nodeId, componentId, + componentType); + config.setItems(serializeItems(uniqueIds)); + if (studentNamesDisplay != null) { + config.setStudentNamesDisplay(studentNamesDisplay); + } + config.setPrompt(normalizedPrompt); + config.setUpdatedByWorkgroup(teacherWorkgroup); + config.setUpdatedAt(new Timestamp(System.currentTimeMillis())); + teacherPresentationConfigDao.save(config); + return config; + } + + @Override + @Transactional + public TeacherPresentationReflectionAnswer saveAnswer(Run run, Group period, String nodeId, + String componentId, String componentType, String questionId, String questionText, + String answerText, Workgroup teacherWorkgroup) { + validateComponent(nodeId, componentId, componentType); + if (questionId == null || questionId.isBlank() || questionId.length() > MAX_ID_LENGTH) { + throw new IllegalArgumentException("Invalid questionId"); + } + if (questionText == null || questionText.length() > MAX_QUESTION_TEXT_LENGTH) { + throw new IllegalArgumentException("Invalid questionText"); + } + if (answerText != null && answerText.length() > MAX_ANSWER_TEXT_LENGTH) { + throw new IllegalArgumentException("answerText is too long"); + } + TeacherPresentationConfig config = getOrCreateConfig(run, period, nodeId, componentId, + componentType); + if (config.getId() == null) { + teacherPresentationConfigDao.save(config); + } + Timestamp now = new Timestamp(System.currentTimeMillis()); + TeacherPresentationReflectionAnswer answer = teacherPresentationConfigDao.getAnswer(config, + questionId); + if (answer == null) { + answer = new TeacherPresentationReflectionAnswer(); + answer.setTeacherPresentationConfig(config); + answer.setQuestionId(questionId); + answer.setCreatedAt(now); + } + answer.setQuestionText(questionText); + answer.setAnswerText(answerText); + answer.setAnsweredByWorkgroup(teacherWorkgroup); + answer.setUpdatedAt(now); + teacherPresentationConfigDao.saveAnswer(answer); + return answer; + } + + private TeacherPresentationConfig getOrCreateConfig(Run run, Group period, String nodeId, + String componentId, String componentType) { + TeacherPresentationConfig config = teacherPresentationConfigDao.getConfig(run, period, nodeId, + componentId); + if (config == null) { + Timestamp now = new Timestamp(System.currentTimeMillis()); + config = new TeacherPresentationConfig(); + config.setRun(run); + config.setPeriod(period); + config.setNodeId(nodeId); + config.setComponentId(componentId); + config.setComponentType(componentType); + config.setCreatedAt(now); + config.setUpdatedAt(now); + } + return config; + } + + private void validateComponent(String nodeId, String componentId, String componentType) { + if (nodeId == null || nodeId.isBlank() || nodeId.length() > MAX_ID_LENGTH) { + throw new IllegalArgumentException("Invalid nodeId"); + } + if (componentId == null || componentId.isBlank() || componentId.length() > MAX_ID_LENGTH) { + throw new IllegalArgumentException("Invalid componentId"); + } + if (componentType == null || componentType.isBlank() + || componentType.length() > MAX_ID_LENGTH) { + throw new IllegalArgumentException("Invalid componentType"); + } + } + + private String normalizePrompt(String prompt) { + if (prompt == null || prompt.isBlank()) { + return null; + } + if (prompt.length() > MAX_PROMPT_LENGTH) { + throw new IllegalArgumentException("prompt is too long"); + } + return prompt; + } + + /** + * Each student work must belong to the run and period. It is not required to belong to the + * component, because discussion posts can come from connected components. + */ + private Set validateStudentWorkIds(Run run, Group period, List ids) { + Set uniqueIds = new LinkedHashSet<>(); + if (ids != null) { + uniqueIds.addAll(ids); + } + if (uniqueIds.size() > MAX_ITEMS) { + throw new IllegalArgumentException("Too many items. The maximum is " + MAX_ITEMS); + } + for (Integer id : uniqueIds) { + if (id == null) { + throw new IllegalArgumentException("Invalid studentWorkId"); + } + StudentWork work; + try { + work = studentWorkDao.getById(id); + } catch (ObjectNotFoundException e) { + throw new IllegalArgumentException("Unknown studentWorkId: " + id); + } + if (work == null || !work.getRun().getId().equals(run.getId()) + || !work.getPeriod().getId().equals(period.getId())) { + throw new IllegalArgumentException("Unknown studentWorkId: " + id); + } + } + return uniqueIds; + } + + private String serializeItems(Set studentWorkIds) { + List> items = new ArrayList<>(); + for (Integer id : studentWorkIds) { + items.add(Map.of("studentWorkId", id)); + } + try { + return objectMapper.writeValueAsString(items); + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); + } + } +} diff --git a/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java new file mode 100644 index 000000000..34e07aa6e --- /dev/null +++ b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java @@ -0,0 +1,89 @@ +package org.wise.vle.domain.teacherpresentation; + +import java.sql.Timestamp; + +import com.fasterxml.jackson.annotation.JsonIgnore; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Index; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; +import jakarta.persistence.UniqueConstraint; +import lombok.Getter; +import lombok.Setter; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.group.impl.PersistentGroup; +import org.wise.portal.domain.run.Run; +import org.wise.portal.domain.run.impl.RunImpl; +import org.wise.portal.domain.workgroup.Workgroup; +import org.wise.portal.domain.workgroup.impl.WorkgroupImpl; + +/** + * A teacher's TeacherPresentation configuration for one component in one period of a run. It + * stores which student work items (e.g. discussion posts and comments) the teacher chose to + * present, how student names are displayed, and an optional override of the default prompt. + */ +@Entity(name = "teacherPresentationConfig") +@Table(name = "teacher_presentation_config", + uniqueConstraints = @UniqueConstraint(name = "teacherPresentationConfigUniqueKey", + columnNames = { "runId", "periodId", "nodeId", "componentId" }), + indexes = @Index(columnList = "runId", name = "teacherPresentationConfigRunIdIndex")) +@Getter +@Setter +public class TeacherPresentationConfig { + + public static final String NAMES_SHOW = "show"; + public static final String NAMES_HIDE = "hide"; + public static final String NAMES_ANONYMIZE = "anonymize"; + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @ManyToOne(targetEntity = RunImpl.class, fetch = FetchType.LAZY) + @JoinColumn(name = "runId", nullable = false) + @JsonIgnore + private Run run; + + @ManyToOne(targetEntity = PersistentGroup.class, fetch = FetchType.LAZY) + @JoinColumn(name = "periodId", nullable = false) + @JsonIgnore + private Group period; + + @Column(name = "nodeId", length = 30, nullable = false) + private String nodeId; + + @Column(name = "componentId", length = 30, nullable = false) + private String componentId; + + @Column(name = "componentType", length = 30, nullable = false) + private String componentType; + + /** JSON array of the form [{"studentWorkId": 123}, ...] */ + @Column(name = "items", length = 65536, columnDefinition = "text", nullable = false) + private String items = "[]"; + + @Column(name = "studentNamesDisplay", length = 30, nullable = false) + private String studentNamesDisplay = NAMES_HIDE; + + /** The teacher's edited prompt. null means use the component's default prompt. */ + @Column(name = "prompt", length = 65536, columnDefinition = "text") + private String prompt; + + @ManyToOne(targetEntity = WorkgroupImpl.class, fetch = FetchType.LAZY) + @JoinColumn(name = "updatedByWorkgroupId") + @JsonIgnore + private Workgroup updatedByWorkgroup; + + @Column(name = "createdAt", nullable = false) + private Timestamp createdAt; + + @Column(name = "updatedAt", nullable = false) + private Timestamp updatedAt; +} diff --git a/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationReflectionAnswer.java b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationReflectionAnswer.java new file mode 100644 index 000000000..e44e54403 --- /dev/null +++ b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationReflectionAnswer.java @@ -0,0 +1,64 @@ +package org.wise.vle.domain.teacherpresentation; + +import java.sql.Timestamp; + +import com.fasterxml.jackson.annotation.JsonIgnore; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; +import jakarta.persistence.UniqueConstraint; +import lombok.Getter; +import lombok.Setter; +import org.wise.portal.domain.workgroup.Workgroup; +import org.wise.portal.domain.workgroup.impl.WorkgroupImpl; + +/** + * A teacher's answer to one reflection question of a TeacherPresentation. Only the latest answer + * per question is kept. The question text is copied at answer time so research data survives later + * edits to the question. + */ +@Entity(name = "teacherPresentationReflectionAnswer") +@Table(name = "teacher_presentation_reflection_answers", + uniqueConstraints = @UniqueConstraint( + name = "teacherPresentationReflectionAnswersUniqueKey", + columnNames = { "teacherPresentationConfigId", "questionId" })) +@Getter +@Setter +public class TeacherPresentationReflectionAnswer { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "teacherPresentationConfigId", nullable = false) + @JsonIgnore + private TeacherPresentationConfig teacherPresentationConfig; + + @Column(name = "questionId", length = 30, nullable = false) + private String questionId; + + @Column(name = "questionText", length = 65536, columnDefinition = "text", nullable = false) + private String questionText; + + @Column(name = "answerText", length = 65536, columnDefinition = "text") + private String answerText; + + @ManyToOne(targetEntity = WorkgroupImpl.class, fetch = FetchType.LAZY) + @JoinColumn(name = "answeredByWorkgroupId") + @JsonIgnore + private Workgroup answeredByWorkgroup; + + @Column(name = "createdAt", nullable = false) + private Timestamp createdAt; + + @Column(name = "updatedAt", nullable = false) + private Timestamp updatedAt; +} diff --git a/src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql b/src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql new file mode 100644 index 000000000..157005495 --- /dev/null +++ b/src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql @@ -0,0 +1,35 @@ +create table teacher_presentation_config ( + id bigint not null auto_increment, + runId bigint not null, + periodId bigint not null, + nodeId varchar(30) not null, + componentId varchar(30) not null, + componentType varchar(30) not null, + items text not null, + studentNamesDisplay varchar(30) not null default 'hide', + prompt text, + updatedByWorkgroupId bigint, + createdAt datetime(3) not null, + updatedAt datetime(3) not null, + primary key (id), + unique key teacherPresentationConfigUniqueKey (runId, periodId, nodeId, componentId), + index teacherPresentationConfigRunIdIndex (runId), + constraint teacherPresentationConfigRunIdFK foreign key (runId) references runs (id), + constraint teacherPresentationConfigPeriodIdFK foreign key (periodId) references `groups` (id), + constraint teacherPresentationConfigUpdatedByWorkgroupIdFK foreign key (updatedByWorkgroupId) references workgroups (id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; + +create table teacher_presentation_reflection_answers ( + id bigint not null auto_increment, + teacherPresentationConfigId bigint not null, + questionId varchar(30) not null, + questionText text not null, + answerText text, + answeredByWorkgroupId bigint, + createdAt datetime(3) not null, + updatedAt datetime(3) not null, + primary key (id), + unique key teacherPresentationReflectionAnswersUniqueKey (teacherPresentationConfigId, questionId), + constraint teacherPresentationReflectionAnswersConfigIdFK foreign key (teacherPresentationConfigId) references teacher_presentation_config (id) on delete cascade, + constraint teacherPresentationReflectionAnswersWorkgroupIdFK foreign key (answeredByWorkgroupId) references workgroups (id) +) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java new file mode 100644 index 000000000..9c5ba41ae --- /dev/null +++ b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java @@ -0,0 +1,165 @@ +package org.wise.portal.presentation.web.controllers.teacher; + +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.eq; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.isNull; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.Timestamp; +import java.util.Arrays; +import java.util.List; + +import org.easymock.EasyMockExtension; +import org.easymock.Mock; +import org.easymock.TestSubject; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.springframework.http.HttpStatus; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.web.server.ResponseStatusException; +import org.wise.portal.dao.ObjectNotFoundException; +import org.wise.portal.presentation.web.controllers.APIControllerTest; +import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.AnswerRequest; +import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.AnswerResponse; +import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.ConfigRequest; +import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.ConfigResponse; +import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.Item; +import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; + +@ExtendWith(EasyMockExtension.class) +public class TeacherPresentationConfigAPIControllerTest extends APIControllerTest { + + @TestSubject + private TeacherPresentationConfigAPIController controller = new TeacherPresentationConfigAPIController(); + + @Mock + private TeacherPresentationConfigService teacherPresentationConfigService; + + private void expectTeacherWithPermissions(boolean canView, boolean canGrade) + throws ObjectNotFoundException { + expect(runService.retrieveById(runId1)).andReturn(run1).anyTimes(); + expect(userService.retrieveUserByUsername(teacherAuth.getName())).andReturn(teacher1) + .anyTimes(); + expect(runService.isAllowedToViewStudentWork(run1, teacher1)).andReturn(canView).anyTimes(); + expect(runService.isAllowedToGradeStudentWork(run1, teacher1)).andReturn(canGrade).anyTimes(); + expect(workgroupService.getWorkgroupListByRunAndUser(run1, teacher1)) + .andReturn(Arrays.asList(teacher1Run1Workgroup)).anyTimes(); + } + + private ConfigRequest request(Long periodId, List items, String namesDisplay, + String prompt) { + return new ConfigRequest(runId1, periodId, run1Node1Id, run1Component1Id, "Discussion", items, + namesDisplay, prompt); + } + + @Test + public void getConfig_NoConfigSaved_ReturnDefaults() throws Exception { + expectTeacherWithPermissions(true, true); + expect(teacherPresentationConfigService.getConfig(run1, run1Period1, run1Node1Id, + run1Component1Id)).andReturn(null); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + ConfigResponse response = controller.getConfig(teacherAuth, runId1, run1Period1Id, + run1Node1Id, run1Component1Id); + assertTrue(response.items().isEmpty()); + assertEquals("hide", response.studentNamesDisplay()); + assertNull(response.prompt()); + assertTrue(response.answers().isEmpty()); + verify(teacherPresentationConfigService); + } + + @Test + public void getConfig_NoViewPermission_ThrowAccessDenied() throws Exception { + expectTeacherWithPermissions(false, false); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + assertThrows(AccessDeniedException.class, () -> controller.getConfig(teacherAuth, runId1, + run1Period1Id, run1Node1Id, run1Component1Id)); + } + + @Test + public void getConfig_PeriodNotInRun_ThrowBadRequest() throws Exception { + expectTeacherWithPermissions(true, true); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + ResponseStatusException e = assertThrows(ResponseStatusException.class, + () -> controller.getConfig(teacherAuth, runId1, 99999L, run1Node1Id, run1Component1Id)); + assertEquals(HttpStatus.BAD_REQUEST, e.getStatusCode()); + } + + @Test + public void saveConfig_NoGradePermission_ThrowAccessDenied() throws Exception { + expectTeacherWithPermissions(true, false); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + assertThrows(AccessDeniedException.class, () -> controller.saveConfig(teacherAuth, + request(run1Period1Id, Arrays.asList(new Item(1)), "hide", null))); + } + + @Test + public void saveConfig_ServiceRejectsInput_ThrowBadRequest() throws Exception { + expectTeacherWithPermissions(true, true); + expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), anyObject(), eq("bogus"), + isNull(), eq(teacher1Run1Workgroup))).andThrow(new IllegalArgumentException("Invalid")); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + ResponseStatusException e = assertThrows(ResponseStatusException.class, () -> controller + .saveConfig(teacherAuth, request(run1Period1Id, Arrays.asList(new Item(1)), "bogus", null))); + assertEquals(HttpStatus.BAD_REQUEST, e.getStatusCode()); + } + + @Test + public void saveConfig_ValidRequest_SaveWithTeacherWorkgroupAndReturnConfig() throws Exception { + expectTeacherWithPermissions(true, true); + TeacherPresentationConfig saved = new TeacherPresentationConfig(); + saved.setId(5L); + saved.setRun(run1); + saved.setPeriod(run1Period1); + saved.setNodeId(run1Node1Id); + saved.setComponentId(run1Component1Id); + saved.setComponentType("Discussion"); + saved.setUpdatedAt(new Timestamp(1000)); + saved.setPrompt("My prompt"); + saved.setStudentNamesDisplay("anonymize"); + saved.setUpdatedByWorkgroup(teacher1Run1Workgroup); + expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(Arrays.asList(7, 8)), + eq("anonymize"), eq("My prompt"), eq(teacher1Run1Workgroup))).andReturn(saved); + expect(teacherPresentationConfigService.getStudentWorkIds(saved)) + .andReturn(Arrays.asList(7, 8)); + expect(teacherPresentationConfigService.getAnswers(saved)) + .andReturn(Arrays.asList()); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + ConfigResponse response = controller.saveConfig(teacherAuth, + request(run1Period1Id, Arrays.asList(new Item(7), new Item(8)), "anonymize", "My prompt")); + assertEquals(5L, response.id()); + assertEquals(2, response.items().size()); + assertEquals("My prompt", response.prompt()); + assertEquals(teacher1Run1Workgroup.getId(), response.updatedByWorkgroupId()); + verify(teacherPresentationConfigService); + } + + @Test + public void saveAnswer_ValidRequest_ReturnAnswer() throws Exception { + expectTeacherWithPermissions(true, true); + TeacherPresentationReflectionAnswer answer = new TeacherPresentationReflectionAnswer(); + answer.setQuestionId("q1"); + answer.setQuestionText("Why?"); + answer.setAnswerText("Because"); + answer.setAnsweredByWorkgroup(teacher1Run1Workgroup); + answer.setUpdatedAt(new Timestamp(2000)); + expect(teacherPresentationConfigService.saveAnswer(run1, run1Period1, run1Node1Id, + run1Component1Id, "Discussion", "q1", "Why?", "Because", teacher1Run1Workgroup)) + .andReturn(answer); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + AnswerResponse response = controller.saveAnswer(teacherAuth, new AnswerRequest(runId1, + run1Period1Id, run1Node1Id, run1Component1Id, "Discussion", "q1", "Why?", "Because")); + assertEquals("q1", response.questionId()); + assertEquals("Because", response.answerText()); + verify(teacherPresentationConfigService); + } +} diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java new file mode 100644 index 000000000..9bfbe86c3 --- /dev/null +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java @@ -0,0 +1,166 @@ +package org.wise.portal.service.teacherpresentation.impl; + +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.replay; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.easymock.EasyMockExtension; +import org.easymock.Mock; +import org.easymock.TestSubject; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.wise.portal.dao.ObjectNotFoundException; +import org.wise.portal.dao.teacherpresentation.TeacherPresentationConfigDao; +import org.wise.portal.dao.work.StudentWorkDao; +import org.wise.portal.domain.group.impl.PersistentGroup; +import org.wise.portal.domain.run.impl.RunImpl; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; +import org.wise.vle.domain.work.StudentWork; + +@ExtendWith(EasyMockExtension.class) +public class TeacherPresentationConfigServiceImplTest { + + @TestSubject + private TeacherPresentationConfigServiceImpl service = new TeacherPresentationConfigServiceImpl(); + + @Mock + private TeacherPresentationConfigDao teacherPresentationConfigDao; + + @Mock + private StudentWorkDao studentWorkDao; + + private RunImpl run = new RunImpl(); + private RunImpl otherRun = new RunImpl(); + private PersistentGroup period = new PersistentGroup(); + private PersistentGroup otherPeriod = new PersistentGroup(); + + @BeforeEach + public void setUp() { + run.setId(1L); + otherRun.setId(2L); + period.setId(10L); + otherPeriod.setId(11L); + } + + private StudentWork work(RunImpl workRun, PersistentGroup workPeriod) { + StudentWork work = new StudentWork(); + work.setRun(workRun); + work.setPeriod(workPeriod); + return work; + } + + private TeacherPresentationConfig save(List ids, String namesDisplay, String prompt) { + return service.saveConfig(run, period, "node1", "comp1", "Discussion", ids, namesDisplay, + prompt, null); + } + + @Test + public void saveConfig_InvalidNamesDisplay_ThrowIllegalArgument() { + replay(teacherPresentationConfigDao, studentWorkDao); + assertThrows(IllegalArgumentException.class, () -> save(new ArrayList<>(), "bogus", null)); + } + + @Test + public void saveConfig_MoreThan100Items_ThrowIllegalArgument() { + List ids = new ArrayList<>(); + for (int i = 0; i < 101; i++) { + ids.add(i); + } + replay(teacherPresentationConfigDao, studentWorkDao); + assertThrows(IllegalArgumentException.class, () -> save(ids, "hide", null)); + } + + @Test + public void saveConfig_PromptTooLong_ThrowIllegalArgument() { + replay(teacherPresentationConfigDao, studentWorkDao); + assertThrows(IllegalArgumentException.class, + () -> save(new ArrayList<>(), "hide", "x".repeat(1001))); + } + + @Test + public void saveConfig_StudentWorkFromOtherPeriod_ThrowIllegalArgument() throws Exception { + expect(studentWorkDao.getById(5)).andReturn(work(run, otherPeriod)); + replay(teacherPresentationConfigDao, studentWorkDao); + assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); + } + + @Test + public void saveConfig_StudentWorkFromOtherRun_ThrowIllegalArgument() throws Exception { + expect(studentWorkDao.getById(5)).andReturn(work(otherRun, period)); + replay(teacherPresentationConfigDao, studentWorkDao); + assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); + } + + @Test + public void saveConfig_UnknownStudentWork_ThrowIllegalArgument() throws Exception { + expect(studentWorkDao.getById(5)).andThrow(new ObjectNotFoundException(5L, StudentWork.class)); + replay(teacherPresentationConfigDao, studentWorkDao); + assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); + } + + @Test + public void saveConfig_NewConfig_CreateWithDefaultsAndDeduplicateItems() throws Exception { + expect(studentWorkDao.getById(5)).andReturn(work(run, period)); + expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")).andReturn(null); + teacherPresentationConfigDao.save(anyObject(TeacherPresentationConfig.class)); + expectLastCall(); + replay(teacherPresentationConfigDao, studentWorkDao); + TeacherPresentationConfig config = save(Arrays.asList(5, 5), null, " "); + assertEquals("hide", config.getStudentNamesDisplay()); + assertNull(config.getPrompt()); + assertEquals(Arrays.asList(5), service.getStudentWorkIds(config)); + assertEquals("Discussion", config.getComponentType()); + } + + @Test + public void saveConfig_ExistingConfig_UpdatePromptAndNames() throws Exception { + TeacherPresentationConfig existing = new TeacherPresentationConfig(); + existing.setStudentNamesDisplay("show"); + expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")) + .andReturn(existing); + teacherPresentationConfigDao.save(existing); + expectLastCall(); + replay(teacherPresentationConfigDao, studentWorkDao); + TeacherPresentationConfig config = save(new ArrayList<>(), "anonymize", "My prompt"); + assertEquals("anonymize", config.getStudentNamesDisplay()); + assertEquals("My prompt", config.getPrompt()); + assertEquals(0, service.getStudentWorkIds(config).size()); + } + + @Test + public void saveAnswer_ExistingAnswer_UpdateInsteadOfCreatingHistory() { + TeacherPresentationConfig config = new TeacherPresentationConfig(); + config.setId(3L); + TeacherPresentationReflectionAnswer existing = new TeacherPresentationReflectionAnswer(); + existing.setId(9L); + existing.setQuestionId("q1"); + expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")) + .andReturn(config); + expect(teacherPresentationConfigDao.getAnswer(config, "q1")).andReturn(existing); + teacherPresentationConfigDao.saveAnswer(existing); + expectLastCall(); + replay(teacherPresentationConfigDao, studentWorkDao); + TeacherPresentationReflectionAnswer answer = service.saveAnswer(run, period, "node1", "comp1", + "Discussion", "q1", "Why?", "New answer", null); + assertEquals(9L, answer.getId()); + assertEquals("New answer", answer.getAnswerText()); + assertEquals("Why?", answer.getQuestionText()); + } + + @Test + public void saveAnswer_InvalidQuestionId_ThrowIllegalArgument() { + replay(teacherPresentationConfigDao, studentWorkDao); + assertThrows(IllegalArgumentException.class, () -> service.saveAnswer(run, period, "node1", + "comp1", "Discussion", " ", "Why?", "a", null)); + } +} From e719ee3accd0c8e9b54b5e4679c0f7f6bb7fb1f3 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Thu, 8 Oct 2026 17:07:58 -0700 Subject: [PATCH 02/12] test: update maximum item limit to 500 in TeacherPresentationConfigServiceImplTest --- .../impl/TeacherPresentationConfigServiceImplTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java index 9bfbe86c3..7996aef45 100644 --- a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java @@ -71,9 +71,9 @@ public void saveConfig_InvalidNamesDisplay_ThrowIllegalArgument() { } @Test - public void saveConfig_MoreThan100Items_ThrowIllegalArgument() { + public void saveConfig_MoreThan500Items_ThrowIllegalArgument() { List ids = new ArrayList<>(); - for (int i = 0; i < 101; i++) { + for (int i = 0; i < 501; i++) { ids.add(i); } replay(teacherPresentationConfigDao, studentWorkDao); @@ -144,8 +144,7 @@ public void saveAnswer_ExistingAnswer_UpdateInsteadOfCreatingHistory() { TeacherPresentationReflectionAnswer existing = new TeacherPresentationReflectionAnswer(); existing.setId(9L); existing.setQuestionId("q1"); - expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")) - .andReturn(config); + expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")).andReturn(config); expect(teacherPresentationConfigDao.getAnswer(config, "q1")).andReturn(existing); teacherPresentationConfigDao.saveAnswer(existing); expectLastCall(); From 5d571c2f51db58084831320699c294a15d80c70e Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Fri, 9 Oct 2026 09:32:48 -0700 Subject: [PATCH 03/12] test: add unit tests for HibernateTeacherPresentationConfigDao --- ...rnateTeacherPresentationConfigDaoTest.java | 221 ++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java diff --git a/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java b/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java new file mode 100644 index 000000000..4cbf3a21c --- /dev/null +++ b/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java @@ -0,0 +1,221 @@ +package org.wise.portal.dao.teacherpresentation.impl; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.sql.Timestamp; +import java.util.Calendar; +import java.util.Date; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.wise.portal.domain.authentication.Schoollevel; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.portal.domain.user.User; +import org.wise.portal.domain.workgroup.Workgroup; +import org.wise.portal.junit.AbstractTransactionalDbTests; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; + +@SpringBootTest +public class HibernateTeacherPresentationConfigDaoTest extends AbstractTransactionalDbTests { + + private Run run; + private Run otherRun; + private Group period1; + private Group period2; + private Workgroup teacherWorkgroup; + + @Autowired + private HibernateTeacherPresentationConfigDao teacherPresentationConfigDao; + + @BeforeEach + public void setUp() throws Exception { + super.setUp(); + + Long projectId1 = getNextAvailableProjectId(); + Long projectId2 = getNextAvailableProjectId(); + Date startTime = Calendar.getInstance().getTime(); + + User teacher = createTeacherUser("Albus", "Dumbledore", "adumbledore", "Prof. Dumbledore", + "secret", "Hogwarts", "Scotland", "UK", "albus@hogwarts.edu", "Hogwarts School", + Schoollevel.COLLEGE, "google-teacher-1"); + + run = createProjectAndRun(projectId1, "Potions 101", teacher, startTime, "POT101"); + otherRun = createProjectAndRun(projectId2, "Defense Against the Dark Arts", teacher, startTime, + "DADA101"); + + period1 = createPeriod("Period 1"); + period2 = createPeriod("Period 2"); + + Set periods = new TreeSet<>(); + periods.add(period1); + periods.add(period2); + run.setPeriods(periods); + + teacherWorkgroup = addUserToRun(teacher, run, period1); + } + + private TeacherPresentationConfig createConfig(Run run, Group period, String nodeId, + String componentId, String componentType) { + TeacherPresentationConfig config = new TeacherPresentationConfig(); + config.setRun(run); + config.setPeriod(period); + config.setNodeId(nodeId); + config.setComponentId(componentId); + config.setComponentType(componentType); + config.setItems("[{\"studentWorkId\":101}]"); + config.setStudentNamesDisplay(TeacherPresentationConfig.NAMES_HIDE); + config.setPrompt("Sample discussion prompt"); + config.setUpdatedByWorkgroup(teacherWorkgroup); + Timestamp now = new Timestamp(System.currentTimeMillis()); + config.setCreatedAt(now); + config.setUpdatedAt(now); + teacherPresentationConfigDao.save(config); + return config; + } + + private TeacherPresentationReflectionAnswer createAnswer(TeacherPresentationConfig config, + String questionId, String questionText, String answerText) { + TeacherPresentationReflectionAnswer answer = new TeacherPresentationReflectionAnswer(); + answer.setTeacherPresentationConfig(config); + answer.setQuestionId(questionId); + answer.setQuestionText(questionText); + answer.setAnswerText(answerText); + answer.setAnsweredByWorkgroup(teacherWorkgroup); + Timestamp now = new Timestamp(System.currentTimeMillis()); + answer.setCreatedAt(now); + answer.setUpdatedAt(now); + teacherPresentationConfigDao.saveAnswer(answer); + return answer; + } + + @Test + public void getConfig_WhenNoneExists_ReturnsNull() { + TeacherPresentationConfig config = teacherPresentationConfigDao.getConfig(run, period1, + "node1", "component1"); + assertNull(config); + } + + @Test + public void saveAndGetConfig_WhenExists_ReturnsMatchingConfig() { + TeacherPresentationConfig saved = createConfig(run, period1, "node1", "component1", + "Discussion"); + + TeacherPresentationConfig retrieved = teacherPresentationConfigDao.getConfig(run, period1, + "node1", "component1"); + assertNotNull(retrieved); + assertEquals(saved.getId(), retrieved.getId()); + assertEquals(run.getId(), retrieved.getRun().getId()); + assertEquals(period1.getId(), retrieved.getPeriod().getId()); + assertEquals("node1", retrieved.getNodeId()); + assertEquals("component1", retrieved.getComponentId()); + assertEquals("Discussion", retrieved.getComponentType()); + assertEquals("[{\"studentWorkId\":101}]", retrieved.getItems()); + assertEquals(TeacherPresentationConfig.NAMES_HIDE, retrieved.getStudentNamesDisplay()); + assertEquals("Sample discussion prompt", retrieved.getPrompt()); + assertEquals(teacherWorkgroup.getId(), retrieved.getUpdatedByWorkgroup().getId()); + } + + @Test + public void getConfig_WithDifferentParameters_ReturnsNull() { + createConfig(run, period1, "node1", "component1", "Discussion"); + + assertNull(teacherPresentationConfigDao.getConfig(otherRun, period1, "node1", "component1")); + assertNull(teacherPresentationConfigDao.getConfig(run, period2, "node1", "component1")); + assertNull(teacherPresentationConfigDao.getConfig(run, period1, "otherNode", "component1")); + assertNull(teacherPresentationConfigDao.getConfig(run, period1, "node1", "otherComponent")); + } + + @Test + public void saveAnswer_NewAnswer_PersistsAndCanBeRetrievedByQuestionId() { + TeacherPresentationConfig config = createConfig(run, period1, "node1", "component1", + "Discussion"); + + TeacherPresentationReflectionAnswer answer = createAnswer(config, "q1", + "What patterns did you notice?", "Students struggled with step 2."); + assertNotNull(answer.getId()); + + TeacherPresentationReflectionAnswer retrieved = teacherPresentationConfigDao.getAnswer(config, + "q1"); + assertNotNull(retrieved); + assertEquals(answer.getId(), retrieved.getId()); + assertEquals("q1", retrieved.getQuestionId()); + assertEquals("What patterns did you notice?", retrieved.getQuestionText()); + assertEquals("Students struggled with step 2.", retrieved.getAnswerText()); + assertEquals(teacherWorkgroup.getId(), retrieved.getAnsweredByWorkgroup().getId()); + } + + @Test + public void saveAnswer_ExistingAnswer_UpdatesAnswer() { + TeacherPresentationConfig config = createConfig(run, period1, "node1", "component1", + "Discussion"); + TeacherPresentationReflectionAnswer answer = createAnswer(config, "q1", + "Initial question", "Initial answer"); + Long answerId = answer.getId(); + + answer.setAnswerText("Updated answer"); + answer.setUpdatedAt(new Timestamp(System.currentTimeMillis())); + teacherPresentationConfigDao.saveAnswer(answer); + flush(); + + TeacherPresentationReflectionAnswer updated = teacherPresentationConfigDao.getAnswer(config, + "q1"); + assertNotNull(updated); + assertEquals(answerId, updated.getId()); + assertEquals("Updated answer", updated.getAnswerText()); + } + + @Test + public void getAnswers_MultipleAnswersForConfig_ReturnsOnlyAnswersForThatConfig() { + TeacherPresentationConfig config1 = createConfig(run, period1, "node1", "component1", + "Discussion"); + TeacherPresentationConfig config2 = createConfig(run, period1, "node2", "component2", + "Discussion"); + + createAnswer(config1, "q1", "Q1 text", "Answer 1"); + createAnswer(config1, "q2", "Q2 text", "Answer 2"); + createAnswer(config2, "q1", "Q1 for config2", "Answer other"); + + List answersConfig1 = teacherPresentationConfigDao + .getAnswers(config1); + assertEquals(2, answersConfig1.size()); + assertTrue(answersConfig1.stream().anyMatch(a -> "q1".equals(a.getQuestionId()))); + assertTrue(answersConfig1.stream().anyMatch(a -> "q2".equals(a.getQuestionId()))); + + List answersConfig2 = teacherPresentationConfigDao + .getAnswers(config2); + assertEquals(1, answersConfig2.size()); + assertEquals("q1", answersConfig2.get(0).getQuestionId()); + assertEquals("Answer other", answersConfig2.get(0).getAnswerText()); + } + + @Test + public void getAnswer_NonExistentQuestionId_ReturnsNull() { + TeacherPresentationConfig config = createConfig(run, period1, "node1", "component1", + "Discussion"); + createAnswer(config, "q1", "Question 1", "Answer 1"); + + assertNull(teacherPresentationConfigDao.getAnswer(config, "nonExistentQuestion")); + } + + @Test + public void delete_Config_RemovesConfig() { + TeacherPresentationConfig config = createConfig(run, period1, "node1", "component1", + "Discussion"); + flush(); + + teacherPresentationConfigDao.delete(config); + flush(); + + assertNull(teacherPresentationConfigDao.getConfig(run, period1, "node1", "component1")); + } +} From 97663419f1d01b6b034c438fa60446c505d1c954 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Fri, 9 Oct 2026 09:43:25 -0700 Subject: [PATCH 04/12] test: add unit tests for TeacherPresentationConfigAPIController error handling and configuration methods --- ...erPresentationConfigAPIControllerTest.java | 147 ++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java index 9c5ba41ae..4b5ddf5b5 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java @@ -12,6 +12,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.sql.Timestamp; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -162,4 +163,150 @@ public void saveAnswer_ValidRequest_ReturnAnswer() throws Exception { assertEquals("Because", response.answerText()); verify(teacherPresentationConfigService); } + + @Test + public void saveAnswer_ServiceRejectsInput_ThrowBadRequest() throws Exception { + expectTeacherWithPermissions(true, true); + expect(teacherPresentationConfigService.saveAnswer(eq(run1), eq(run1Period1), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(""), anyObject(), + anyObject(), eq(teacher1Run1Workgroup))) + .andThrow(new IllegalArgumentException("Invalid questionId")); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + + ResponseStatusException e = assertThrows(ResponseStatusException.class, () -> controller + .saveAnswer(teacherAuth, new AnswerRequest(runId1, run1Period1Id, run1Node1Id, + run1Component1Id, "Discussion", "", "Why?", "Because"))); + assertEquals(HttpStatus.BAD_REQUEST, e.getStatusCode()); + } + + @Test + public void saveAnswer_NoGradePermission_ThrowAccessDenied() throws Exception { + expectTeacherWithPermissions(true, false); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + + assertThrows(AccessDeniedException.class, () -> controller.saveAnswer(teacherAuth, + new AnswerRequest(runId1, run1Period1Id, run1Node1Id, run1Component1Id, + "Discussion", "q1", "Why?", "Because"))); + } + + @Test + public void saveAnswer_PeriodNotInRun_ThrowBadRequest() throws Exception { + expectTeacherWithPermissions(true, true); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + + ResponseStatusException e = assertThrows(ResponseStatusException.class, () -> controller + .saveAnswer(teacherAuth, new AnswerRequest(runId1, 99999L, run1Node1Id, + run1Component1Id, "Discussion", "q1", "Why?", "Because"))); + assertEquals(HttpStatus.BAD_REQUEST, e.getStatusCode()); + } + + @Test + public void getConfig_ExistingConfigWithAnswers_ReturnPopulatedResponse() throws Exception { + expectTeacherWithPermissions(true, true); + + TeacherPresentationConfig config = new TeacherPresentationConfig(); + config.setId(10L); + config.setRun(run1); + config.setPeriod(run1Period1); + config.setNodeId(run1Node1Id); + config.setComponentId(run1Component1Id); + config.setComponentType("Discussion"); + config.setPrompt("Prompt override"); + config.setStudentNamesDisplay("show"); + config.setUpdatedAt(new Timestamp(1000)); + config.setUpdatedByWorkgroup(teacher1Run1Workgroup); + + TeacherPresentationReflectionAnswer answer = new TeacherPresentationReflectionAnswer(); + answer.setQuestionId("q1"); + answer.setQuestionText("Reflection question"); + answer.setAnswerText("Reflection answer"); + answer.setUpdatedAt(new Timestamp(2000)); + answer.setAnsweredByWorkgroup(teacher1Run1Workgroup); + + expect(teacherPresentationConfigService.getConfig(run1, run1Period1, run1Node1Id, + run1Component1Id)).andReturn(config); + expect(teacherPresentationConfigService.getStudentWorkIds(config)) + .andReturn(Arrays.asList(101, 102)); + expect(teacherPresentationConfigService.getAnswers(config)) + .andReturn(Arrays.asList(answer)); + + replay(runService, userService, workgroupService, teacherPresentationConfigService); + + ConfigResponse response = controller.getConfig(teacherAuth, runId1, run1Period1Id, + run1Node1Id, run1Component1Id); + + assertEquals(10L, response.id()); + assertEquals(2, response.items().size()); + assertEquals(101, response.items().get(0).studentWorkId()); + assertEquals("Prompt override", response.prompt()); + assertEquals("show", response.studentNamesDisplay()); + assertEquals(1, response.answers().size()); + assertEquals("q1", response.answers().get(0).questionId()); + assertEquals("Reflection answer", response.answers().get(0).answerText()); + verify(teacherPresentationConfigService); + } + + @Test + public void saveConfig_NullItems_SaveWithEmptyList() throws Exception { + expectTeacherWithPermissions(true, true); + TeacherPresentationConfig saved = new TeacherPresentationConfig(); + saved.setId(6L); + saved.setRun(run1); + saved.setPeriod(run1Period1); + saved.setNodeId(run1Node1Id); + saved.setComponentId(run1Component1Id); + saved.setComponentType("Discussion"); + saved.setUpdatedAt(new Timestamp(1000)); + saved.setStudentNamesDisplay("hide"); + expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(new ArrayList<>()), + eq("hide"), isNull(), eq(teacher1Run1Workgroup))).andReturn(saved); + expect(teacherPresentationConfigService.getStudentWorkIds(saved)) + .andReturn(new ArrayList<>()); + expect(teacherPresentationConfigService.getAnswers(saved)) + .andReturn(new ArrayList<>()); + replay(runService, userService, workgroupService, teacherPresentationConfigService); + ConfigResponse response = controller.saveConfig(teacherAuth, + request(run1Period1Id, null, "hide", null)); + assertEquals(6L, response.id()); + assertTrue(response.items().isEmpty()); + verify(teacherPresentationConfigService); + } + + @Test + public void saveConfig_TeacherWithNoWorkgroup_SaveWithNullWorkgroup() throws Exception { + expect(runService.retrieveById(runId1)).andReturn(run1).anyTimes(); + expect(userService.retrieveUserByUsername(teacherAuth.getName())).andReturn(teacher1) + .anyTimes(); + expect(runService.isAllowedToViewStudentWork(run1, teacher1)).andReturn(true).anyTimes(); + expect(runService.isAllowedToGradeStudentWork(run1, teacher1)).andReturn(true).anyTimes(); + expect(workgroupService.getWorkgroupListByRunAndUser(run1, teacher1)) + .andReturn(new ArrayList<>()).anyTimes(); + + TeacherPresentationConfig saved = new TeacherPresentationConfig(); + saved.setId(7L); + saved.setRun(run1); + saved.setPeriod(run1Period1); + saved.setNodeId(run1Node1Id); + saved.setComponentId(run1Component1Id); + saved.setComponentType("Discussion"); + saved.setUpdatedAt(new Timestamp(1000)); + saved.setStudentNamesDisplay("hide"); + saved.setUpdatedByWorkgroup(null); + + expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), anyObject(), + eq("hide"), isNull(), isNull())).andReturn(saved); + expect(teacherPresentationConfigService.getStudentWorkIds(saved)) + .andReturn(new ArrayList<>()); + expect(teacherPresentationConfigService.getAnswers(saved)) + .andReturn(new ArrayList<>()); + + replay(runService, userService, workgroupService, teacherPresentationConfigService); + ConfigResponse response = controller.saveConfig(teacherAuth, + request(run1Period1Id, new ArrayList<>(), "hide", null)); + assertEquals(7L, response.id()); + assertNull(response.updatedByWorkgroupId()); + verify(teacherPresentationConfigService); + } } From cdbe40e6671746f428b02bb5161838d7bc6a94bf Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Fri, 9 Oct 2026 13:34:30 -0700 Subject: [PATCH 05/12] refactor(teacher-presentation): strip reflection answers from presentation feature --- .../TeacherPresentationConfigDao.java | 10 --- ...HibernateTeacherPresentationConfigDao.java | 37 -------- ...eacherPresentationConfigAPIController.java | 47 +--------- .../TeacherPresentationConfigService.java | 13 --- .../TeacherPresentationConfigServiceImpl.java | 46 ---------- .../TeacherPresentationReflectionAnswer.java | 64 ------------- ...V2__create_teacher_presentation_tables.sql | 15 ---- ...rnateTeacherPresentationConfigDaoTest.java | 90 ------------------- ...erPresentationConfigAPIControllerTest.java | 80 +---------------- ...cherPresentationConfigServiceImplTest.java | 27 ------ 10 files changed, 4 insertions(+), 425 deletions(-) delete mode 100644 src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationReflectionAnswer.java diff --git a/src/main/java/org/wise/portal/dao/teacherpresentation/TeacherPresentationConfigDao.java b/src/main/java/org/wise/portal/dao/teacherpresentation/TeacherPresentationConfigDao.java index 57a4ef54a..863671815 100644 --- a/src/main/java/org/wise/portal/dao/teacherpresentation/TeacherPresentationConfigDao.java +++ b/src/main/java/org/wise/portal/dao/teacherpresentation/TeacherPresentationConfigDao.java @@ -1,12 +1,9 @@ package org.wise.portal.dao.teacherpresentation; -import java.util.List; - import org.wise.portal.dao.SimpleDao; import org.wise.portal.domain.group.Group; import org.wise.portal.domain.run.Run; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; -import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; public interface TeacherPresentationConfigDao extends SimpleDao { @@ -15,11 +12,4 @@ public interface TeacherPresentationConfigDao getAnswers(TeacherPresentationConfig config); - - TeacherPresentationReflectionAnswer getAnswer(TeacherPresentationConfig config, - String questionId); - - void saveAnswer(TeacherPresentationReflectionAnswer answer); } diff --git a/src/main/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDao.java b/src/main/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDao.java index 5bf015b53..234126d62 100644 --- a/src/main/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDao.java +++ b/src/main/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDao.java @@ -2,7 +2,6 @@ import java.util.List; -import jakarta.persistence.TypedQuery; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Root; @@ -13,7 +12,6 @@ import org.wise.portal.domain.group.Group; import org.wise.portal.domain.run.Run; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; -import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; @Repository("teacherPresentationConfigDao") public class HibernateTeacherPresentationConfigDao @@ -36,39 +34,4 @@ public TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, List results = entityManager.createQuery(cq).getResultList(); return results.isEmpty() ? null : results.get(0); } - - @Override - public List getAnswers(TeacherPresentationConfig config) { - CriteriaBuilder cb = getCriteriaBuilder(); - CriteriaQuery cq = cb - .createQuery(TeacherPresentationReflectionAnswer.class); - Root root = cq - .from(TeacherPresentationReflectionAnswer.class); - cq.select(root).where(cb.equal(root.get("teacherPresentationConfig"), config)); - return entityManager.createQuery(cq).getResultList(); - } - - @Override - public TeacherPresentationReflectionAnswer getAnswer(TeacherPresentationConfig config, - String questionId) { - CriteriaBuilder cb = getCriteriaBuilder(); - CriteriaQuery cq = cb - .createQuery(TeacherPresentationReflectionAnswer.class); - Root root = cq - .from(TeacherPresentationReflectionAnswer.class); - cq.select(root).where(cb.equal(root.get("teacherPresentationConfig"), config), - cb.equal(root.get("questionId"), questionId)); - TypedQuery query = entityManager.createQuery(cq); - List results = query.getResultList(); - return results.isEmpty() ? null : results.get(0); - } - - @Override - public void saveAnswer(TeacherPresentationReflectionAnswer answer) { - if (answer.getId() == null) { - entityManager.persist(answer); - } else { - entityManager.merge(answer); - } - } } diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java index 30aba3822..8b47b9d00 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java @@ -25,7 +25,6 @@ import org.wise.portal.service.user.UserService; import org.wise.portal.service.workgroup.WorkgroupService; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; -import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; /** * REST API for TeacherPresentation: the teacher's selected student work, student name display @@ -55,17 +54,9 @@ public record ConfigRequest(Long runId, Long periodId, String nodeId, String com String componentType, List items, String studentNamesDisplay, String prompt) { } - public record AnswerRequest(Long runId, Long periodId, String nodeId, String componentId, - String componentType, String questionId, String questionText, String answerText) { - } - - public record AnswerResponse(String questionId, String questionText, String answerText, - Long answeredByWorkgroupId, Long updatedAt) { - } - public record ConfigResponse(Long id, Long runId, Long periodId, String nodeId, String componentId, String componentType, List items, String studentNamesDisplay, - String prompt, Long updatedByWorkgroupId, Long updatedAt, List answers) { + String prompt, Long updatedByWorkgroupId, Long updatedAt) { } @GetMapping @@ -82,7 +73,7 @@ ConfigResponse getConfig(Authentication auth, @RequestParam Long runId, nodeId, componentId); if (config == null) { return new ConfigResponse(null, runId, periodId, nodeId, componentId, null, new ArrayList<>(), - TeacherPresentationConfig.NAMES_HIDE, null, null, null, new ArrayList<>()); + TeacherPresentationConfig.NAMES_HIDE, null, null, null); } return toResponse(config); } @@ -112,26 +103,6 @@ ConfigResponse saveConfig(Authentication auth, @RequestBody ConfigRequest reques } } - @PutMapping("/answers") - AnswerResponse saveAnswer(Authentication auth, @RequestBody AnswerRequest request) - throws ObjectNotFoundException { - Run run = runService.retrieveById(request.runId()); - User user = userService.retrieveUserByUsername(auth.getName()); - if (!runService.isAllowedToGradeStudentWork(run, user)) { - throw new AccessDeniedException("Not permitted"); - } - Group period = getPeriod(run, request.periodId()); - try { - TeacherPresentationReflectionAnswer answer = teacherPresentationConfigService.saveAnswer(run, - period, request.nodeId(), request.componentId(), request.componentType(), - request.questionId(), request.questionText(), request.answerText(), - getTeacherWorkgroup(run, user)); - return toAnswerResponse(answer); - } catch (IllegalArgumentException e) { - throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage()); - } - } - private Group getPeriod(Run run, Long periodId) { for (Group period : run.getPeriods()) { if (period.getId().equals(periodId)) { @@ -151,22 +122,10 @@ private ConfigResponse toResponse(TeacherPresentationConfig config) { for (Integer id : teacherPresentationConfigService.getStudentWorkIds(config)) { items.add(new Item(id)); } - List answers = new ArrayList<>(); - for (TeacherPresentationReflectionAnswer answer : teacherPresentationConfigService - .getAnswers(config)) { - answers.add(toAnswerResponse(answer)); - } Workgroup updatedBy = config.getUpdatedByWorkgroup(); return new ConfigResponse(config.getId(), config.getRun().getId(), config.getPeriod().getId(), config.getNodeId(), config.getComponentId(), config.getComponentType(), items, config.getStudentNamesDisplay(), config.getPrompt(), - updatedBy == null ? null : updatedBy.getId(), config.getUpdatedAt().getTime(), answers); - } - - private AnswerResponse toAnswerResponse(TeacherPresentationReflectionAnswer answer) { - Workgroup answeredBy = answer.getAnsweredByWorkgroup(); - return new AnswerResponse(answer.getQuestionId(), answer.getQuestionText(), - answer.getAnswerText(), answeredBy == null ? null : answeredBy.getId(), - answer.getUpdatedAt().getTime()); + updatedBy == null ? null : updatedBy.getId(), config.getUpdatedAt().getTime()); } } diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java index 6eef6a227..d77adb36b 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java @@ -6,7 +6,6 @@ import org.wise.portal.domain.run.Run; import org.wise.portal.domain.workgroup.Workgroup; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; -import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; public interface TeacherPresentationConfigService { @@ -18,8 +17,6 @@ public interface TeacherPresentationConfigService { */ TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, String componentId); - List getAnswers(TeacherPresentationConfig config); - List getStudentWorkIds(TeacherPresentationConfig config); /** @@ -33,14 +30,4 @@ public interface TeacherPresentationConfigService { TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, String componentType, List studentWorkIds, String studentNamesDisplay, String prompt, Workgroup teacherWorkgroup); - - /** - * Creates or updates the answer to a reflection question. Creates the config if it does not - * exist yet. - * - * @throws IllegalArgumentException if the input is invalid - */ - TeacherPresentationReflectionAnswer saveAnswer(Run run, Group period, String nodeId, - String componentId, String componentType, String questionId, String questionText, - String answerText, Workgroup teacherWorkgroup); } diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java index 1c9bcf165..c1c976cc0 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java @@ -18,7 +18,6 @@ import org.wise.portal.domain.workgroup.Workgroup; import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; -import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; import org.wise.vle.domain.work.StudentWork; import com.fasterxml.jackson.core.JsonProcessingException; @@ -29,8 +28,6 @@ public class TeacherPresentationConfigServiceImpl implements TeacherPresentationConfigService { private static final int MAX_ID_LENGTH = 30; - private static final int MAX_QUESTION_TEXT_LENGTH = 5000; - private static final int MAX_ANSWER_TEXT_LENGTH = 20000; private static final Set NAMES_DISPLAY_OPTIONS = Set.of( TeacherPresentationConfig.NAMES_SHOW, TeacherPresentationConfig.NAMES_HIDE, TeacherPresentationConfig.NAMES_ANONYMIZE); @@ -50,12 +47,6 @@ public TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, return teacherPresentationConfigDao.getConfig(run, period, nodeId, componentId); } - @Override - @Transactional(readOnly = true) - public List getAnswers(TeacherPresentationConfig config) { - return teacherPresentationConfigDao.getAnswers(config); - } - @Override public List getStudentWorkIds(TeacherPresentationConfig config) { try { @@ -96,43 +87,6 @@ public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId return config; } - @Override - @Transactional - public TeacherPresentationReflectionAnswer saveAnswer(Run run, Group period, String nodeId, - String componentId, String componentType, String questionId, String questionText, - String answerText, Workgroup teacherWorkgroup) { - validateComponent(nodeId, componentId, componentType); - if (questionId == null || questionId.isBlank() || questionId.length() > MAX_ID_LENGTH) { - throw new IllegalArgumentException("Invalid questionId"); - } - if (questionText == null || questionText.length() > MAX_QUESTION_TEXT_LENGTH) { - throw new IllegalArgumentException("Invalid questionText"); - } - if (answerText != null && answerText.length() > MAX_ANSWER_TEXT_LENGTH) { - throw new IllegalArgumentException("answerText is too long"); - } - TeacherPresentationConfig config = getOrCreateConfig(run, period, nodeId, componentId, - componentType); - if (config.getId() == null) { - teacherPresentationConfigDao.save(config); - } - Timestamp now = new Timestamp(System.currentTimeMillis()); - TeacherPresentationReflectionAnswer answer = teacherPresentationConfigDao.getAnswer(config, - questionId); - if (answer == null) { - answer = new TeacherPresentationReflectionAnswer(); - answer.setTeacherPresentationConfig(config); - answer.setQuestionId(questionId); - answer.setCreatedAt(now); - } - answer.setQuestionText(questionText); - answer.setAnswerText(answerText); - answer.setAnsweredByWorkgroup(teacherWorkgroup); - answer.setUpdatedAt(now); - teacherPresentationConfigDao.saveAnswer(answer); - return answer; - } - private TeacherPresentationConfig getOrCreateConfig(Run run, Group period, String nodeId, String componentId, String componentType) { TeacherPresentationConfig config = teacherPresentationConfigDao.getConfig(run, period, nodeId, diff --git a/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationReflectionAnswer.java b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationReflectionAnswer.java deleted file mode 100644 index e44e54403..000000000 --- a/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationReflectionAnswer.java +++ /dev/null @@ -1,64 +0,0 @@ -package org.wise.vle.domain.teacherpresentation; - -import java.sql.Timestamp; - -import com.fasterxml.jackson.annotation.JsonIgnore; - -import jakarta.persistence.Column; -import jakarta.persistence.Entity; -import jakarta.persistence.FetchType; -import jakarta.persistence.GeneratedValue; -import jakarta.persistence.GenerationType; -import jakarta.persistence.Id; -import jakarta.persistence.JoinColumn; -import jakarta.persistence.ManyToOne; -import jakarta.persistence.Table; -import jakarta.persistence.UniqueConstraint; -import lombok.Getter; -import lombok.Setter; -import org.wise.portal.domain.workgroup.Workgroup; -import org.wise.portal.domain.workgroup.impl.WorkgroupImpl; - -/** - * A teacher's answer to one reflection question of a TeacherPresentation. Only the latest answer - * per question is kept. The question text is copied at answer time so research data survives later - * edits to the question. - */ -@Entity(name = "teacherPresentationReflectionAnswer") -@Table(name = "teacher_presentation_reflection_answers", - uniqueConstraints = @UniqueConstraint( - name = "teacherPresentationReflectionAnswersUniqueKey", - columnNames = { "teacherPresentationConfigId", "questionId" })) -@Getter -@Setter -public class TeacherPresentationReflectionAnswer { - - @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) - private Long id; - - @ManyToOne(fetch = FetchType.LAZY) - @JoinColumn(name = "teacherPresentationConfigId", nullable = false) - @JsonIgnore - private TeacherPresentationConfig teacherPresentationConfig; - - @Column(name = "questionId", length = 30, nullable = false) - private String questionId; - - @Column(name = "questionText", length = 65536, columnDefinition = "text", nullable = false) - private String questionText; - - @Column(name = "answerText", length = 65536, columnDefinition = "text") - private String answerText; - - @ManyToOne(targetEntity = WorkgroupImpl.class, fetch = FetchType.LAZY) - @JoinColumn(name = "answeredByWorkgroupId") - @JsonIgnore - private Workgroup answeredByWorkgroup; - - @Column(name = "createdAt", nullable = false) - private Timestamp createdAt; - - @Column(name = "updatedAt", nullable = false) - private Timestamp updatedAt; -} diff --git a/src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql b/src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql index 157005495..27b743728 100644 --- a/src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql +++ b/src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql @@ -18,18 +18,3 @@ create table teacher_presentation_config ( constraint teacherPresentationConfigPeriodIdFK foreign key (periodId) references `groups` (id), constraint teacherPresentationConfigUpdatedByWorkgroupIdFK foreign key (updatedByWorkgroupId) references workgroups (id) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; - -create table teacher_presentation_reflection_answers ( - id bigint not null auto_increment, - teacherPresentationConfigId bigint not null, - questionId varchar(30) not null, - questionText text not null, - answerText text, - answeredByWorkgroupId bigint, - createdAt datetime(3) not null, - updatedAt datetime(3) not null, - primary key (id), - unique key teacherPresentationReflectionAnswersUniqueKey (teacherPresentationConfigId, questionId), - constraint teacherPresentationReflectionAnswersConfigIdFK foreign key (teacherPresentationConfigId) references teacher_presentation_config (id) on delete cascade, - constraint teacherPresentationReflectionAnswersWorkgroupIdFK foreign key (answeredByWorkgroupId) references workgroups (id) -) ENGINE=InnoDB DEFAULT CHARSET=utf8; diff --git a/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java b/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java index 4cbf3a21c..98e24fcdd 100644 --- a/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java +++ b/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java @@ -3,12 +3,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import java.sql.Timestamp; import java.util.Calendar; import java.util.Date; -import java.util.List; import java.util.Set; import java.util.TreeSet; @@ -23,7 +21,6 @@ import org.wise.portal.domain.workgroup.Workgroup; import org.wise.portal.junit.AbstractTransactionalDbTests; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; -import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; @SpringBootTest public class HibernateTeacherPresentationConfigDaoTest extends AbstractTransactionalDbTests { @@ -83,21 +80,6 @@ private TeacherPresentationConfig createConfig(Run run, Group period, String nod return config; } - private TeacherPresentationReflectionAnswer createAnswer(TeacherPresentationConfig config, - String questionId, String questionText, String answerText) { - TeacherPresentationReflectionAnswer answer = new TeacherPresentationReflectionAnswer(); - answer.setTeacherPresentationConfig(config); - answer.setQuestionId(questionId); - answer.setQuestionText(questionText); - answer.setAnswerText(answerText); - answer.setAnsweredByWorkgroup(teacherWorkgroup); - Timestamp now = new Timestamp(System.currentTimeMillis()); - answer.setCreatedAt(now); - answer.setUpdatedAt(now); - teacherPresentationConfigDao.saveAnswer(answer); - return answer; - } - @Test public void getConfig_WhenNoneExists_ReturnsNull() { TeacherPresentationConfig config = teacherPresentationConfigDao.getConfig(run, period1, @@ -135,78 +117,6 @@ public void getConfig_WithDifferentParameters_ReturnsNull() { assertNull(teacherPresentationConfigDao.getConfig(run, period1, "node1", "otherComponent")); } - @Test - public void saveAnswer_NewAnswer_PersistsAndCanBeRetrievedByQuestionId() { - TeacherPresentationConfig config = createConfig(run, period1, "node1", "component1", - "Discussion"); - - TeacherPresentationReflectionAnswer answer = createAnswer(config, "q1", - "What patterns did you notice?", "Students struggled with step 2."); - assertNotNull(answer.getId()); - - TeacherPresentationReflectionAnswer retrieved = teacherPresentationConfigDao.getAnswer(config, - "q1"); - assertNotNull(retrieved); - assertEquals(answer.getId(), retrieved.getId()); - assertEquals("q1", retrieved.getQuestionId()); - assertEquals("What patterns did you notice?", retrieved.getQuestionText()); - assertEquals("Students struggled with step 2.", retrieved.getAnswerText()); - assertEquals(teacherWorkgroup.getId(), retrieved.getAnsweredByWorkgroup().getId()); - } - - @Test - public void saveAnswer_ExistingAnswer_UpdatesAnswer() { - TeacherPresentationConfig config = createConfig(run, period1, "node1", "component1", - "Discussion"); - TeacherPresentationReflectionAnswer answer = createAnswer(config, "q1", - "Initial question", "Initial answer"); - Long answerId = answer.getId(); - - answer.setAnswerText("Updated answer"); - answer.setUpdatedAt(new Timestamp(System.currentTimeMillis())); - teacherPresentationConfigDao.saveAnswer(answer); - flush(); - - TeacherPresentationReflectionAnswer updated = teacherPresentationConfigDao.getAnswer(config, - "q1"); - assertNotNull(updated); - assertEquals(answerId, updated.getId()); - assertEquals("Updated answer", updated.getAnswerText()); - } - - @Test - public void getAnswers_MultipleAnswersForConfig_ReturnsOnlyAnswersForThatConfig() { - TeacherPresentationConfig config1 = createConfig(run, period1, "node1", "component1", - "Discussion"); - TeacherPresentationConfig config2 = createConfig(run, period1, "node2", "component2", - "Discussion"); - - createAnswer(config1, "q1", "Q1 text", "Answer 1"); - createAnswer(config1, "q2", "Q2 text", "Answer 2"); - createAnswer(config2, "q1", "Q1 for config2", "Answer other"); - - List answersConfig1 = teacherPresentationConfigDao - .getAnswers(config1); - assertEquals(2, answersConfig1.size()); - assertTrue(answersConfig1.stream().anyMatch(a -> "q1".equals(a.getQuestionId()))); - assertTrue(answersConfig1.stream().anyMatch(a -> "q2".equals(a.getQuestionId()))); - - List answersConfig2 = teacherPresentationConfigDao - .getAnswers(config2); - assertEquals(1, answersConfig2.size()); - assertEquals("q1", answersConfig2.get(0).getQuestionId()); - assertEquals("Answer other", answersConfig2.get(0).getAnswerText()); - } - - @Test - public void getAnswer_NonExistentQuestionId_ReturnsNull() { - TeacherPresentationConfig config = createConfig(run, period1, "node1", "component1", - "Discussion"); - createAnswer(config, "q1", "Question 1", "Answer 1"); - - assertNull(teacherPresentationConfigDao.getAnswer(config, "nonExistentQuestion")); - } - @Test public void delete_Config_RemovesConfig() { TeacherPresentationConfig config = createConfig(run, period1, "node1", "component1", diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java index 4b5ddf5b5..90897b64d 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java @@ -26,14 +26,11 @@ import org.springframework.web.server.ResponseStatusException; import org.wise.portal.dao.ObjectNotFoundException; import org.wise.portal.presentation.web.controllers.APIControllerTest; -import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.AnswerRequest; -import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.AnswerResponse; import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.ConfigRequest; import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.ConfigResponse; import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.Item; import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; -import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; @ExtendWith(EasyMockExtension.class) public class TeacherPresentationConfigAPIControllerTest extends APIControllerTest { @@ -72,7 +69,6 @@ public void getConfig_NoConfigSaved_ReturnDefaults() throws Exception { assertTrue(response.items().isEmpty()); assertEquals("hide", response.studentNamesDisplay()); assertNull(response.prompt()); - assertTrue(response.answers().isEmpty()); verify(teacherPresentationConfigService); } @@ -132,8 +128,6 @@ public void saveConfig_ValidRequest_SaveWithTeacherWorkgroupAndReturnConfig() th eq("anonymize"), eq("My prompt"), eq(teacher1Run1Workgroup))).andReturn(saved); expect(teacherPresentationConfigService.getStudentWorkIds(saved)) .andReturn(Arrays.asList(7, 8)); - expect(teacherPresentationConfigService.getAnswers(saved)) - .andReturn(Arrays.asList()); replay(runService, userService, workgroupService, teacherPresentationConfigService); ConfigResponse response = controller.saveConfig(teacherAuth, request(run1Period1Id, Arrays.asList(new Item(7), new Item(8)), "anonymize", "My prompt")); @@ -145,63 +139,7 @@ public void saveConfig_ValidRequest_SaveWithTeacherWorkgroupAndReturnConfig() th } @Test - public void saveAnswer_ValidRequest_ReturnAnswer() throws Exception { - expectTeacherWithPermissions(true, true); - TeacherPresentationReflectionAnswer answer = new TeacherPresentationReflectionAnswer(); - answer.setQuestionId("q1"); - answer.setQuestionText("Why?"); - answer.setAnswerText("Because"); - answer.setAnsweredByWorkgroup(teacher1Run1Workgroup); - answer.setUpdatedAt(new Timestamp(2000)); - expect(teacherPresentationConfigService.saveAnswer(run1, run1Period1, run1Node1Id, - run1Component1Id, "Discussion", "q1", "Why?", "Because", teacher1Run1Workgroup)) - .andReturn(answer); - replay(runService, userService, workgroupService, teacherPresentationConfigService); - AnswerResponse response = controller.saveAnswer(teacherAuth, new AnswerRequest(runId1, - run1Period1Id, run1Node1Id, run1Component1Id, "Discussion", "q1", "Why?", "Because")); - assertEquals("q1", response.questionId()); - assertEquals("Because", response.answerText()); - verify(teacherPresentationConfigService); - } - - @Test - public void saveAnswer_ServiceRejectsInput_ThrowBadRequest() throws Exception { - expectTeacherWithPermissions(true, true); - expect(teacherPresentationConfigService.saveAnswer(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(""), anyObject(), - anyObject(), eq(teacher1Run1Workgroup))) - .andThrow(new IllegalArgumentException("Invalid questionId")); - replay(runService, userService, workgroupService, teacherPresentationConfigService); - - ResponseStatusException e = assertThrows(ResponseStatusException.class, () -> controller - .saveAnswer(teacherAuth, new AnswerRequest(runId1, run1Period1Id, run1Node1Id, - run1Component1Id, "Discussion", "", "Why?", "Because"))); - assertEquals(HttpStatus.BAD_REQUEST, e.getStatusCode()); - } - - @Test - public void saveAnswer_NoGradePermission_ThrowAccessDenied() throws Exception { - expectTeacherWithPermissions(true, false); - replay(runService, userService, workgroupService, teacherPresentationConfigService); - - assertThrows(AccessDeniedException.class, () -> controller.saveAnswer(teacherAuth, - new AnswerRequest(runId1, run1Period1Id, run1Node1Id, run1Component1Id, - "Discussion", "q1", "Why?", "Because"))); - } - - @Test - public void saveAnswer_PeriodNotInRun_ThrowBadRequest() throws Exception { - expectTeacherWithPermissions(true, true); - replay(runService, userService, workgroupService, teacherPresentationConfigService); - - ResponseStatusException e = assertThrows(ResponseStatusException.class, () -> controller - .saveAnswer(teacherAuth, new AnswerRequest(runId1, 99999L, run1Node1Id, - run1Component1Id, "Discussion", "q1", "Why?", "Because"))); - assertEquals(HttpStatus.BAD_REQUEST, e.getStatusCode()); - } - - @Test - public void getConfig_ExistingConfigWithAnswers_ReturnPopulatedResponse() throws Exception { + public void getConfig_ExistingConfig_ReturnPopulatedResponse() throws Exception { expectTeacherWithPermissions(true, true); TeacherPresentationConfig config = new TeacherPresentationConfig(); @@ -216,19 +154,10 @@ public void getConfig_ExistingConfigWithAnswers_ReturnPopulatedResponse() throws config.setUpdatedAt(new Timestamp(1000)); config.setUpdatedByWorkgroup(teacher1Run1Workgroup); - TeacherPresentationReflectionAnswer answer = new TeacherPresentationReflectionAnswer(); - answer.setQuestionId("q1"); - answer.setQuestionText("Reflection question"); - answer.setAnswerText("Reflection answer"); - answer.setUpdatedAt(new Timestamp(2000)); - answer.setAnsweredByWorkgroup(teacher1Run1Workgroup); - expect(teacherPresentationConfigService.getConfig(run1, run1Period1, run1Node1Id, run1Component1Id)).andReturn(config); expect(teacherPresentationConfigService.getStudentWorkIds(config)) .andReturn(Arrays.asList(101, 102)); - expect(teacherPresentationConfigService.getAnswers(config)) - .andReturn(Arrays.asList(answer)); replay(runService, userService, workgroupService, teacherPresentationConfigService); @@ -240,9 +169,6 @@ public void getConfig_ExistingConfigWithAnswers_ReturnPopulatedResponse() throws assertEquals(101, response.items().get(0).studentWorkId()); assertEquals("Prompt override", response.prompt()); assertEquals("show", response.studentNamesDisplay()); - assertEquals(1, response.answers().size()); - assertEquals("q1", response.answers().get(0).questionId()); - assertEquals("Reflection answer", response.answers().get(0).answerText()); verify(teacherPresentationConfigService); } @@ -263,8 +189,6 @@ public void saveConfig_NullItems_SaveWithEmptyList() throws Exception { eq("hide"), isNull(), eq(teacher1Run1Workgroup))).andReturn(saved); expect(teacherPresentationConfigService.getStudentWorkIds(saved)) .andReturn(new ArrayList<>()); - expect(teacherPresentationConfigService.getAnswers(saved)) - .andReturn(new ArrayList<>()); replay(runService, userService, workgroupService, teacherPresentationConfigService); ConfigResponse response = controller.saveConfig(teacherAuth, request(run1Period1Id, null, "hide", null)); @@ -299,8 +223,6 @@ public void saveConfig_TeacherWithNoWorkgroup_SaveWithNullWorkgroup() throws Exc eq("hide"), isNull(), isNull())).andReturn(saved); expect(teacherPresentationConfigService.getStudentWorkIds(saved)) .andReturn(new ArrayList<>()); - expect(teacherPresentationConfigService.getAnswers(saved)) - .andReturn(new ArrayList<>()); replay(runService, userService, workgroupService, teacherPresentationConfigService); ConfigResponse response = controller.saveConfig(teacherAuth, diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java index 7996aef45..75ba3332b 100644 --- a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java @@ -24,7 +24,6 @@ import org.wise.portal.domain.group.impl.PersistentGroup; import org.wise.portal.domain.run.impl.RunImpl; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; -import org.wise.vle.domain.teacherpresentation.TeacherPresentationReflectionAnswer; import org.wise.vle.domain.work.StudentWork; @ExtendWith(EasyMockExtension.class) @@ -136,30 +135,4 @@ public void saveConfig_ExistingConfig_UpdatePromptAndNames() throws Exception { assertEquals("My prompt", config.getPrompt()); assertEquals(0, service.getStudentWorkIds(config).size()); } - - @Test - public void saveAnswer_ExistingAnswer_UpdateInsteadOfCreatingHistory() { - TeacherPresentationConfig config = new TeacherPresentationConfig(); - config.setId(3L); - TeacherPresentationReflectionAnswer existing = new TeacherPresentationReflectionAnswer(); - existing.setId(9L); - existing.setQuestionId("q1"); - expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")).andReturn(config); - expect(teacherPresentationConfigDao.getAnswer(config, "q1")).andReturn(existing); - teacherPresentationConfigDao.saveAnswer(existing); - expectLastCall(); - replay(teacherPresentationConfigDao, studentWorkDao); - TeacherPresentationReflectionAnswer answer = service.saveAnswer(run, period, "node1", "comp1", - "Discussion", "q1", "Why?", "New answer", null); - assertEquals(9L, answer.getId()); - assertEquals("New answer", answer.getAnswerText()); - assertEquals("Why?", answer.getQuestionText()); - } - - @Test - public void saveAnswer_InvalidQuestionId_ThrowIllegalArgument() { - replay(teacherPresentationConfigDao, studentWorkDao); - assertThrows(IllegalArgumentException.class, () -> service.saveAnswer(run, period, "node1", - "comp1", "Discussion", " ", "Why?", "a", null)); - } } From 5d6853816d1806dd238593ab6673009c826463ab Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Wed, 14 Oct 2026 14:13:45 -0700 Subject: [PATCH 06/12] fix: rename teacher presentation migration script from V2 to V3 --- ...tion_tables.sql => V3__create_teacher_presentation_tables.sql} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/main/resources/db/migration/{V2__create_teacher_presentation_tables.sql => V3__create_teacher_presentation_tables.sql} (100%) diff --git a/src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql b/src/main/resources/db/migration/V3__create_teacher_presentation_tables.sql similarity index 100% rename from src/main/resources/db/migration/V2__create_teacher_presentation_tables.sql rename to src/main/resources/db/migration/V3__create_teacher_presentation_tables.sql From 1cdf647469018828e18b47fa40b3abc63428cb1c Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Fri, 16 Oct 2026 09:05:29 -0700 Subject: [PATCH 07/12] feat: add support for ideas item type and show component prompt in teacher presentation config --- ...eacherPresentationConfigAPIController.java | 65 +++++-- .../teacherpresentation/DiscussionItem.java | 4 + .../teacherpresentation/IdeasItem.java | 11 ++ .../teacherpresentation/IdeasResponse.java | 11 ++ .../TeacherPresentationConfigService.java | 32 +++- .../TeacherPresentationConfigServiceImpl.java | 177 ++++++++++++++++-- .../TeacherPresentationConfig.java | 9 + ...em_type_to_teacher_presentation_config.sql | 1 + ..._prompt_to_teacher_presentation_config.sql | 1 + ...rnateTeacherPresentationConfigDaoTest.java | 1 + ...erPresentationConfigAPIControllerTest.java | 102 ++++++++-- ...cherPresentationConfigServiceImplTest.java | 96 ++++++++++ 12 files changed, 454 insertions(+), 56 deletions(-) create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/DiscussionItem.java create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/IdeasItem.java create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/IdeasResponse.java create mode 100644 src/main/resources/db/migration/V4__add_item_type_to_teacher_presentation_config.sql create mode 100644 src/main/resources/db/migration/V5__add_show_component_prompt_to_teacher_presentation_config.sql diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java index 8b47b9d00..df1c39c45 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java @@ -21,6 +21,9 @@ import org.wise.portal.domain.user.User; import org.wise.portal.domain.workgroup.Workgroup; import org.wise.portal.service.run.RunService; +import org.wise.portal.service.teacherpresentation.DiscussionItem; +import org.wise.portal.service.teacherpresentation.IdeasItem; +import org.wise.portal.service.teacherpresentation.IdeasResponse; import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; import org.wise.portal.service.user.UserService; import org.wise.portal.service.workgroup.WorkgroupService; @@ -47,16 +50,24 @@ public class TeacherPresentationConfigAPIController { @Autowired private WorkgroupService workgroupService; - public record Item(Integer studentWorkId) { + public record Item(Integer studentWorkId, String ideaId, List responses) { + public Item(Integer studentWorkId) { + this(studentWorkId, null, null); + } + + public Item(String ideaId, List responses) { + this(null, ideaId, responses); + } } public record ConfigRequest(Long runId, Long periodId, String nodeId, String componentId, - String componentType, List items, String studentNamesDisplay, String prompt) { + String componentType, String itemType, List items, String studentNamesDisplay, + Boolean showComponentPrompt, String prompt) { } public record ConfigResponse(Long id, Long runId, Long periodId, String nodeId, - String componentId, String componentType, List items, String studentNamesDisplay, - String prompt, Long updatedByWorkgroupId, Long updatedAt) { + String componentId, String componentType, String itemType, List items, String studentNamesDisplay, + Boolean showComponentPrompt, String prompt, Long updatedByWorkgroupId, Long updatedAt) { } @GetMapping @@ -72,8 +83,9 @@ ConfigResponse getConfig(Authentication auth, @RequestParam Long runId, TeacherPresentationConfig config = teacherPresentationConfigService.getConfig(run, period, nodeId, componentId); if (config == null) { - return new ConfigResponse(null, runId, periodId, nodeId, componentId, null, new ArrayList<>(), - TeacherPresentationConfig.NAMES_HIDE, null, null, null); + return new ConfigResponse(null, runId, periodId, nodeId, componentId, null, + null, new ArrayList<>(), + TeacherPresentationConfig.NAMES_HIDE, true, null, null, null); } return toResponse(config); } @@ -87,16 +99,35 @@ ConfigResponse saveConfig(Authentication auth, @RequestBody ConfigRequest reques throw new AccessDeniedException("Not permitted"); } Group period = getPeriod(run, request.periodId()); - List studentWorkIds = new ArrayList<>(); - if (request.items() != null) { - for (Item item : request.items()) { - studentWorkIds.add(item == null ? null : item.studentWorkId()); + String itemType = request.itemType() != null ? request.itemType() + : TeacherPresentationConfig.ITEM_TYPE_DISCUSSION; + List itemsToSave; + if (TeacherPresentationConfig.ITEM_TYPE_IDEAS.equals(itemType)) { + List ideasItems = new ArrayList<>(); + if (request.items() != null) { + for (Item item : request.items()) { + if (item != null) { + ideasItems.add(new IdeasItem(item.ideaId(), item.responses())); + } + } } + itemsToSave = ideasItems; + } else { + List discussionItems = new ArrayList<>(); + if (request.items() != null) { + for (Item item : request.items()) { + if (item != null) { + discussionItems.add(new DiscussionItem(item.studentWorkId())); + } + } + } + itemsToSave = discussionItems; } try { TeacherPresentationConfig config = teacherPresentationConfigService.saveConfig(run, period, - request.nodeId(), request.componentId(), request.componentType(), studentWorkIds, - request.studentNamesDisplay(), request.prompt(), getTeacherWorkgroup(run, user)); + request.nodeId(), request.componentId(), request.componentType(), itemType, itemsToSave, + request.studentNamesDisplay(), request.showComponentPrompt(), request.prompt(), + getTeacherWorkgroup(run, user)); return toResponse(config); } catch (IllegalArgumentException e) { throw new ResponseStatusException(HttpStatus.BAD_REQUEST, e.getMessage()); @@ -118,14 +149,12 @@ private Workgroup getTeacherWorkgroup(Run run, User user) { } private ConfigResponse toResponse(TeacherPresentationConfig config) { - List items = new ArrayList<>(); - for (Integer id : teacherPresentationConfigService.getStudentWorkIds(config)) { - items.add(new Item(id)); - } + List items = teacherPresentationConfigService.getItems(config); Workgroup updatedBy = config.getUpdatedByWorkgroup(); return new ConfigResponse(config.getId(), config.getRun().getId(), config.getPeriod().getId(), - config.getNodeId(), config.getComponentId(), config.getComponentType(), items, - config.getStudentNamesDisplay(), config.getPrompt(), + config.getNodeId(), config.getComponentId(), config.getComponentType(), + config.getItemType(), items, + config.getStudentNamesDisplay(), config.isShowComponentPrompt(), config.getPrompt(), updatedBy == null ? null : updatedBy.getId(), config.getUpdatedAt().getTime()); } } diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/DiscussionItem.java b/src/main/java/org/wise/portal/service/teacherpresentation/DiscussionItem.java new file mode 100644 index 000000000..b982ead5f --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/DiscussionItem.java @@ -0,0 +1,4 @@ +package org.wise.portal.service.teacherpresentation; + +public record DiscussionItem(Integer studentWorkId) { +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/IdeasItem.java b/src/main/java/org/wise/portal/service/teacherpresentation/IdeasItem.java new file mode 100644 index 000000000..a04455a28 --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/IdeasItem.java @@ -0,0 +1,11 @@ +package org.wise.portal.service.teacherpresentation; + +import java.util.List; + +public record IdeasItem(String ideaId, List responses) { + public IdeasItem { + if (responses == null) { + responses = List.of(); + } + } +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/IdeasResponse.java b/src/main/java/org/wise/portal/service/teacherpresentation/IdeasResponse.java new file mode 100644 index 000000000..77ea0ee28 --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/IdeasResponse.java @@ -0,0 +1,11 @@ +package org.wise.portal.service.teacherpresentation; + +/** + * A student response selected for presentation under an idea category. Dialog Guidance work is + * additive (each save contains the whole chat), so a response is identified by workgroupId and + * responseTimestamp (the time of the chat message). responseTimestamp is null for single response + * work, which is identified by studentWorkId. text is a snapshot of the response. + */ +public record IdeasResponse(Integer studentWorkId, Long workgroupId, Long responseTimestamp, + String text) { +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java index d77adb36b..33300a50e 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java @@ -19,15 +19,43 @@ public interface TeacherPresentationConfigService { List getStudentWorkIds(TeacherPresentationConfig config); + List getItems(TeacherPresentationConfig config); + /** - * Creates or updates the config for the run, period, node and component. + * Creates or updates the config for the run, period, node and component with itemType. * + * @param itemType one of Discussion or Ideas. + * @param items list of DiscussionItem or IdeasItem. * @param prompt the teacher's prompt. null or blank resets to the component default. * @param studentNamesDisplay one of show, hide or anonymize. null keeps the current value (or * hide for a new config). * @throws IllegalArgumentException if the input is invalid */ TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, + String componentType, String itemType, List items, String studentNamesDisplay, + Boolean showComponentPrompt, String prompt, Workgroup teacherWorkgroup); + + default TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, + String componentType, String itemType, List items, String studentNamesDisplay, + String prompt, Workgroup teacherWorkgroup) { + return saveConfig(run, period, nodeId, componentId, componentType, itemType, items, + studentNamesDisplay, null, prompt, teacherWorkgroup); + } + + /** + * Backward-compatible saveConfig for Discussion with a list of studentWorkIds. + */ + default TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, String componentType, List studentWorkIds, String studentNamesDisplay, - String prompt, Workgroup teacherWorkgroup); + String prompt, Workgroup teacherWorkgroup) { + List items = new java.util.ArrayList<>(); + if (studentWorkIds != null) { + for (Integer id : studentWorkIds) { + items.add(new DiscussionItem(id)); + } + } + return saveConfig(run, period, nodeId, componentId, componentType, + TeacherPresentationConfig.ITEM_TYPE_DISCUSSION, items, studentNamesDisplay, null, prompt, + teacherWorkgroup); + } } diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java index c1c976cc0..526f564f3 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java @@ -2,6 +2,7 @@ import java.sql.Timestamp; import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -16,6 +17,9 @@ import org.wise.portal.domain.group.Group; import org.wise.portal.domain.run.Run; import org.wise.portal.domain.workgroup.Workgroup; +import org.wise.portal.service.teacherpresentation.DiscussionItem; +import org.wise.portal.service.teacherpresentation.IdeasItem; +import org.wise.portal.service.teacherpresentation.IdeasResponse; import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; import org.wise.vle.domain.work.StudentWork; @@ -28,9 +32,15 @@ public class TeacherPresentationConfigServiceImpl implements TeacherPresentationConfigService { private static final int MAX_ID_LENGTH = 30; + private static final int MAX_RESPONSE_TEXT_LENGTH = 2000; + // items column is a MySQL text column (65535 bytes) + private static final int MAX_SERIALIZED_ITEMS_LENGTH = 60000; private static final Set NAMES_DISPLAY_OPTIONS = Set.of( TeacherPresentationConfig.NAMES_SHOW, TeacherPresentationConfig.NAMES_HIDE, TeacherPresentationConfig.NAMES_ANONYMIZE); + private static final Set ITEM_TYPE_OPTIONS = Set.of( + TeacherPresentationConfig.ITEM_TYPE_DISCUSSION, + TeacherPresentationConfig.ITEM_TYPE_IDEAS); private final ObjectMapper objectMapper = new ObjectMapper(); @@ -48,38 +58,86 @@ public TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, } @Override - public List getStudentWorkIds(TeacherPresentationConfig config) { + public List getItems(TeacherPresentationConfig config) { + if (config == null || config.getItems() == null) { + return Collections.emptyList(); + } + String itemType = config.getItemType(); try { - List> items = objectMapper.readValue(config.getItems(), - new TypeReference>>() { - }); - List ids = new ArrayList<>(); - for (Map item : items) { - ids.add(item.get("studentWorkId")); + if (TeacherPresentationConfig.ITEM_TYPE_IDEAS.equals(itemType)) { + return objectMapper.readValue(config.getItems(), + new TypeReference>() { + }); + } else { + return objectMapper.readValue(config.getItems(), + new TypeReference>() { + }); } - return ids; } catch (JsonProcessingException e) { - return new ArrayList<>(); + return Collections.emptyList(); + } + } + + @Override + public List getStudentWorkIds(TeacherPresentationConfig config) { + List items = getItems(config); + List ids = new ArrayList<>(); + for (Object item : items) { + if (item instanceof DiscussionItem di) { + if (di.studentWorkId() != null) { + ids.add(di.studentWorkId()); + } + } else if (item instanceof IdeasItem ii) { + for (IdeasResponse response : ii.responses()) { + if (response.studentWorkId() != null) { + ids.add(response.studentWorkId()); + } + } + } } + return ids; } @Override @Transactional public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, - String componentId, String componentType, List studentWorkIds, - String studentNamesDisplay, String prompt, Workgroup teacherWorkgroup) { + String componentId, String componentType, String itemType, List items, + String studentNamesDisplay, Boolean showComponentPrompt, String prompt, + Workgroup teacherWorkgroup) { validateComponent(nodeId, componentId, componentType); + String resolvedItemType = itemType != null ? itemType : TeacherPresentationConfig.ITEM_TYPE_DISCUSSION; + if (!ITEM_TYPE_OPTIONS.contains(resolvedItemType)) { + throw new IllegalArgumentException("Invalid itemType: " + itemType); + } if (studentNamesDisplay != null && !NAMES_DISPLAY_OPTIONS.contains(studentNamesDisplay)) { throw new IllegalArgumentException("Invalid studentNamesDisplay: " + studentNamesDisplay); } String normalizedPrompt = normalizePrompt(prompt); - Set uniqueIds = validateStudentWorkIds(run, period, studentWorkIds); + + String serializedItems; + if (TeacherPresentationConfig.ITEM_TYPE_IDEAS.equals(resolvedItemType)) { + List ideasItems = validateAndNormalizeIdeasItems(run, period, items); + serializedItems = serializeIdeasItems(ideasItems); + if (serializedItems.getBytes(java.nio.charset.StandardCharsets.UTF_8).length + > MAX_SERIALIZED_ITEMS_LENGTH) { + throw new IllegalArgumentException("Too many selected responses"); + } + } else { + List discussionItems = validateAndNormalizeDiscussionItems(run, period, items); + serializedItems = serializeDiscussionItems(discussionItems); + } + TeacherPresentationConfig config = getOrCreateConfig(run, period, nodeId, componentId, - componentType); - config.setItems(serializeItems(uniqueIds)); + componentType, resolvedItemType); + config.setComponentType(componentType); + config.setItemType(resolvedItemType); + config.setItems(serializedItems); if (studentNamesDisplay != null) { config.setStudentNamesDisplay(studentNamesDisplay); } + if (showComponentPrompt != null) { + config.setShowComponentPrompt(showComponentPrompt); + } config.setPrompt(normalizedPrompt); config.setUpdatedByWorkgroup(teacherWorkgroup); config.setUpdatedAt(new Timestamp(System.currentTimeMillis())); @@ -88,7 +146,7 @@ public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId } private TeacherPresentationConfig getOrCreateConfig(Run run, Group period, String nodeId, - String componentId, String componentType) { + String componentId, String componentType, String itemType) { TeacherPresentationConfig config = teacherPresentationConfigDao.getConfig(run, period, nodeId, componentId); if (config == null) { @@ -99,6 +157,7 @@ private TeacherPresentationConfig getOrCreateConfig(Run run, Group period, Strin config.setNodeId(nodeId); config.setComponentId(componentId); config.setComponentType(componentType); + config.setItemType(itemType); config.setCreatedAt(now); config.setUpdatedAt(now); } @@ -128,6 +187,82 @@ private String normalizePrompt(String prompt) { return prompt; } + private List validateAndNormalizeDiscussionItems(Run run, Group period, List items) { + List ids = new ArrayList<>(); + if (items != null) { + for (Object obj : items) { + if (obj instanceof DiscussionItem di) { + ids.add(di.studentWorkId()); + } else if (obj instanceof Integer id) { + ids.add(id); + } else if (obj != null) { + throw new IllegalArgumentException("Invalid discussion item type: " + obj.getClass()); + } + } + } + Set uniqueIds = validateStudentWorkIds(run, period, ids); + List normalized = new ArrayList<>(); + for (Integer id : uniqueIds) { + normalized.add(new DiscussionItem(id)); + } + return normalized; + } + + private List validateAndNormalizeIdeasItems(Run run, Group period, List items) { + if (items == null) { + return Collections.emptyList(); + } + if (items.size() > MAX_ITEMS) { + throw new IllegalArgumentException("Too many items. The maximum is " + MAX_ITEMS); + } + List normalized = new ArrayList<>(); + List allStudentWorkIds = new ArrayList<>(); + int responseCount = 0; + for (Object obj : items) { + if (obj instanceof IdeasItem ii) { + String ideaId = ii.ideaId(); + if (ideaId != null) { + ideaId = ideaId.trim(); + if (ideaId.isEmpty() || ideaId.length() > 100) { + throw new IllegalArgumentException("Invalid ideaId: " + ideaId); + } + } + if (ideaId == null && ii.responses().isEmpty()) { + throw new IllegalArgumentException("Item must specify ideaId or responses"); + } + List responses = new ArrayList<>(); + for (IdeasResponse response : ii.responses()) { + responses.add(normalizeIdeasResponse(response)); + allStudentWorkIds.add(response.studentWorkId()); + } + responseCount += responses.size(); + normalized.add(new IdeasItem(ideaId, responses)); + } else if (obj != null) { + throw new IllegalArgumentException("Invalid ideas item type: " + obj.getClass()); + } + } + if (responseCount > MAX_ITEMS) { + throw new IllegalArgumentException("Too many responses. The maximum is " + MAX_ITEMS); + } + validateStudentWorkIds(run, period, allStudentWorkIds); + return normalized; + } + + private IdeasResponse normalizeIdeasResponse(IdeasResponse response) { + if (response == null || response.studentWorkId() == null) { + throw new IllegalArgumentException("Invalid studentWorkId"); + } + if (response.workgroupId() == null) { + throw new IllegalArgumentException("Invalid workgroupId"); + } + String text = response.text() != null ? response.text() : ""; + if (text.length() > MAX_RESPONSE_TEXT_LENGTH) { + text = text.substring(0, MAX_RESPONSE_TEXT_LENGTH); + } + return new IdeasResponse(response.studentWorkId(), response.workgroupId(), + response.responseTimestamp(), text); + } + /** * Each student work must belong to the run and period. It is not required to belong to the * component, because discussion posts can come from connected components. @@ -158,11 +293,15 @@ private Set validateStudentWorkIds(Run run, Group period, List return uniqueIds; } - private String serializeItems(Set studentWorkIds) { - List> items = new ArrayList<>(); - for (Integer id : studentWorkIds) { - items.add(Map.of("studentWorkId", id)); + private String serializeDiscussionItems(List items) { + try { + return objectMapper.writeValueAsString(items); + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); } + } + + private String serializeIdeasItems(List items) { try { return objectMapper.writeValueAsString(items); } catch (JsonProcessingException e) { diff --git a/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java index 34e07aa6e..49cb56ff7 100644 --- a/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java +++ b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java @@ -42,6 +42,9 @@ public class TeacherPresentationConfig { public static final String NAMES_HIDE = "hide"; public static final String NAMES_ANONYMIZE = "anonymize"; + public static final String ITEM_TYPE_DISCUSSION = "Discussion"; + public static final String ITEM_TYPE_IDEAS = "Ideas"; + @Id @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; @@ -65,6 +68,9 @@ public class TeacherPresentationConfig { @Column(name = "componentType", length = 30, nullable = false) private String componentType; + @Column(name = "itemType", length = 30, nullable = false) + private String itemType = ITEM_TYPE_DISCUSSION; + /** JSON array of the form [{"studentWorkId": 123}, ...] */ @Column(name = "items", length = 65536, columnDefinition = "text", nullable = false) private String items = "[]"; @@ -72,6 +78,9 @@ public class TeacherPresentationConfig { @Column(name = "studentNamesDisplay", length = 30, nullable = false) private String studentNamesDisplay = NAMES_HIDE; + @Column(name = "showComponentPrompt", nullable = false) + private boolean showComponentPrompt = true; + /** The teacher's edited prompt. null means use the component's default prompt. */ @Column(name = "prompt", length = 65536, columnDefinition = "text") private String prompt; diff --git a/src/main/resources/db/migration/V4__add_item_type_to_teacher_presentation_config.sql b/src/main/resources/db/migration/V4__add_item_type_to_teacher_presentation_config.sql new file mode 100644 index 000000000..0387c20c8 --- /dev/null +++ b/src/main/resources/db/migration/V4__add_item_type_to_teacher_presentation_config.sql @@ -0,0 +1 @@ +alter table teacher_presentation_config add column itemType varchar(30) not null after componentType; diff --git a/src/main/resources/db/migration/V5__add_show_component_prompt_to_teacher_presentation_config.sql b/src/main/resources/db/migration/V5__add_show_component_prompt_to_teacher_presentation_config.sql new file mode 100644 index 000000000..0b2a5ecba --- /dev/null +++ b/src/main/resources/db/migration/V5__add_show_component_prompt_to_teacher_presentation_config.sql @@ -0,0 +1 @@ +alter table teacher_presentation_config add column showComponentPrompt boolean not null default true after studentNamesDisplay; diff --git a/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java b/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java index 98e24fcdd..584e28d87 100644 --- a/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java +++ b/src/test/java/org/wise/portal/dao/teacherpresentation/impl/HibernateTeacherPresentationConfigDaoTest.java @@ -103,6 +103,7 @@ public void saveAndGetConfig_WhenExists_ReturnsMatchingConfig() { assertEquals("Discussion", retrieved.getComponentType()); assertEquals("[{\"studentWorkId\":101}]", retrieved.getItems()); assertEquals(TeacherPresentationConfig.NAMES_HIDE, retrieved.getStudentNamesDisplay()); + assertEquals(true, retrieved.isShowComponentPrompt()); assertEquals("Sample discussion prompt", retrieved.getPrompt()); assertEquals(teacherWorkgroup.getId(), retrieved.getUpdatedByWorkgroup().getId()); } diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java index 90897b64d..c41a8ae6f 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java @@ -14,6 +14,7 @@ import java.sql.Timestamp; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.easymock.EasyMockExtension; @@ -29,6 +30,9 @@ import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.ConfigRequest; import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.ConfigResponse; import org.wise.portal.presentation.web.controllers.teacher.TeacherPresentationConfigAPIController.Item; +import org.wise.portal.service.teacherpresentation.DiscussionItem; +import org.wise.portal.service.teacherpresentation.IdeasItem; +import org.wise.portal.service.teacherpresentation.IdeasResponse; import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; @@ -54,8 +58,14 @@ private void expectTeacherWithPermissions(boolean canView, boolean canGrade) private ConfigRequest request(Long periodId, List items, String namesDisplay, String prompt) { - return new ConfigRequest(runId1, periodId, run1Node1Id, run1Component1Id, "Discussion", items, - namesDisplay, prompt); + return new ConfigRequest(runId1, periodId, run1Node1Id, run1Component1Id, "Discussion", + TeacherPresentationConfig.ITEM_TYPE_DISCUSSION, items, namesDisplay, true, prompt); + } + + private ConfigRequest ideasRequest(Long periodId, List items, String namesDisplay, + String prompt) { + return new ConfigRequest(runId1, periodId, run1Node1Id, run1Component1Id, "OpenResponse", + TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, namesDisplay, true, prompt); } @Test @@ -67,7 +77,9 @@ public void getConfig_NoConfigSaved_ReturnDefaults() throws Exception { ConfigResponse response = controller.getConfig(teacherAuth, runId1, run1Period1Id, run1Node1Id, run1Component1Id); assertTrue(response.items().isEmpty()); + assertNull(response.itemType()); assertEquals("hide", response.studentNamesDisplay()); + assertTrue(response.showComponentPrompt()); assertNull(response.prompt()); verify(teacherPresentationConfigService); } @@ -101,8 +113,9 @@ public void saveConfig_NoGradePermission_ThrowAccessDenied() throws Exception { public void saveConfig_ServiceRejectsInput_ThrowBadRequest() throws Exception { expectTeacherWithPermissions(true, true); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), anyObject(), eq("bogus"), - isNull(), eq(teacher1Run1Workgroup))).andThrow(new IllegalArgumentException("Invalid")); + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.ITEM_TYPE_DISCUSSION), + anyObject(), eq("bogus"), eq(true), isNull(), eq(teacher1Run1Workgroup))) + .andThrow(new IllegalArgumentException("Invalid")); replay(runService, userService, workgroupService, teacherPresentationConfigService); ResponseStatusException e = assertThrows(ResponseStatusException.class, () -> controller .saveConfig(teacherAuth, request(run1Period1Id, Arrays.asList(new Item(1)), "bogus", null))); @@ -119,25 +132,77 @@ public void saveConfig_ValidRequest_SaveWithTeacherWorkgroupAndReturnConfig() th saved.setNodeId(run1Node1Id); saved.setComponentId(run1Component1Id); saved.setComponentType("Discussion"); + saved.setItemType("Discussion"); saved.setUpdatedAt(new Timestamp(1000)); saved.setPrompt("My prompt"); saved.setStudentNamesDisplay("anonymize"); + saved.setShowComponentPrompt(true); saved.setUpdatedByWorkgroup(teacher1Run1Workgroup); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(Arrays.asList(7, 8)), - eq("anonymize"), eq("My prompt"), eq(teacher1Run1Workgroup))).andReturn(saved); - expect(teacherPresentationConfigService.getStudentWorkIds(saved)) - .andReturn(Arrays.asList(7, 8)); + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.ITEM_TYPE_DISCUSSION), + eq(Arrays.asList(new DiscussionItem(7), new DiscussionItem(8))), + eq("anonymize"), eq(true), eq("My prompt"), eq(teacher1Run1Workgroup))).andReturn(saved); + expect(teacherPresentationConfigService.getItems(saved)) + .andReturn((List) Arrays.asList(new DiscussionItem(7), new DiscussionItem(8))); replay(runService, userService, workgroupService, teacherPresentationConfigService); ConfigResponse response = controller.saveConfig(teacherAuth, request(run1Period1Id, Arrays.asList(new Item(7), new Item(8)), "anonymize", "My prompt")); assertEquals(5L, response.id()); assertEquals(2, response.items().size()); assertEquals("My prompt", response.prompt()); + assertTrue(response.showComponentPrompt()); assertEquals(teacher1Run1Workgroup.getId(), response.updatedByWorkgroupId()); verify(teacherPresentationConfigService); } + @Test + public void saveConfig_ValidIdeasRequest_SaveAndReturnConfig() throws Exception { + expectTeacherWithPermissions(true, true); + TeacherPresentationConfig saved = new TeacherPresentationConfig(); + saved.setId(8L); + saved.setRun(run1); + saved.setPeriod(run1Period1); + saved.setNodeId(run1Node1Id); + saved.setComponentId(run1Component1Id); + saved.setComponentType("OpenResponse"); + saved.setItemType("Ideas"); + saved.setUpdatedAt(new Timestamp(1000)); + saved.setPrompt("Ideas prompt"); + saved.setStudentNamesDisplay("hide"); + saved.setShowComponentPrompt(true); + saved.setUpdatedByWorkgroup(teacher1Run1Workgroup); + + List responses1 = Arrays.asList(new IdeasResponse(101, 1L, 1000L, "First"), + new IdeasResponse(101, 1L, 2000L, "Second")); + List responses3 = Arrays.asList(new IdeasResponse(103, 2L, null, "Standalone")); + List expectedSavedItems = Arrays.asList( + new IdeasItem("idea_1", responses1), + new IdeasItem("idea_2", Collections.emptyList()), + new IdeasItem(null, responses3) + ); + expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), + eq(run1Node1Id), eq(run1Component1Id), eq("OpenResponse"), eq(TeacherPresentationConfig.ITEM_TYPE_IDEAS), + eq(expectedSavedItems), eq("hide"), eq(true), eq("Ideas prompt"), eq(teacher1Run1Workgroup))).andReturn(saved); + expect(teacherPresentationConfigService.getItems(saved)) + .andReturn((List) expectedSavedItems); + + replay(runService, userService, workgroupService, teacherPresentationConfigService); + + List requestItems = Arrays.asList( + new Item("idea_1", responses1), + new Item("idea_2", Collections.emptyList()), + new Item(null, responses3) + ); + ConfigResponse response = controller.saveConfig(teacherAuth, + ideasRequest(run1Period1Id, requestItems, "hide", "Ideas prompt")); + + assertEquals(8L, response.id()); + assertTrue(response.showComponentPrompt()); + assertEquals("Ideas", response.itemType()); + assertEquals(3, response.items().size()); + verify(teacherPresentationConfigService); + } + @Test public void getConfig_ExistingConfig_ReturnPopulatedResponse() throws Exception { expectTeacherWithPermissions(true, true); @@ -149,6 +214,7 @@ public void getConfig_ExistingConfig_ReturnPopulatedResponse() throws Exception config.setNodeId(run1Node1Id); config.setComponentId(run1Component1Id); config.setComponentType("Discussion"); + config.setItemType("Discussion"); config.setPrompt("Prompt override"); config.setStudentNamesDisplay("show"); config.setUpdatedAt(new Timestamp(1000)); @@ -156,8 +222,8 @@ public void getConfig_ExistingConfig_ReturnPopulatedResponse() throws Exception expect(teacherPresentationConfigService.getConfig(run1, run1Period1, run1Node1Id, run1Component1Id)).andReturn(config); - expect(teacherPresentationConfigService.getStudentWorkIds(config)) - .andReturn(Arrays.asList(101, 102)); + expect(teacherPresentationConfigService.getItems(config)) + .andReturn((List) Arrays.asList(new DiscussionItem(101), new DiscussionItem(102))); replay(runService, userService, workgroupService, teacherPresentationConfigService); @@ -166,7 +232,7 @@ public void getConfig_ExistingConfig_ReturnPopulatedResponse() throws Exception assertEquals(10L, response.id()); assertEquals(2, response.items().size()); - assertEquals(101, response.items().get(0).studentWorkId()); + assertEquals(101, ((DiscussionItem) response.items().get(0)).studentWorkId()); assertEquals("Prompt override", response.prompt()); assertEquals("show", response.studentNamesDisplay()); verify(teacherPresentationConfigService); @@ -182,12 +248,13 @@ public void saveConfig_NullItems_SaveWithEmptyList() throws Exception { saved.setNodeId(run1Node1Id); saved.setComponentId(run1Component1Id); saved.setComponentType("Discussion"); + saved.setItemType("Discussion"); saved.setUpdatedAt(new Timestamp(1000)); saved.setStudentNamesDisplay("hide"); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(new ArrayList<>()), - eq("hide"), isNull(), eq(teacher1Run1Workgroup))).andReturn(saved); - expect(teacherPresentationConfigService.getStudentWorkIds(saved)) + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.ITEM_TYPE_DISCUSSION), + eq(new ArrayList<>()), eq("hide"), eq(true), isNull(), eq(teacher1Run1Workgroup))).andReturn(saved); + expect(teacherPresentationConfigService.getItems(saved)) .andReturn(new ArrayList<>()); replay(runService, userService, workgroupService, teacherPresentationConfigService); ConfigResponse response = controller.saveConfig(teacherAuth, @@ -214,14 +281,15 @@ public void saveConfig_TeacherWithNoWorkgroup_SaveWithNullWorkgroup() throws Exc saved.setNodeId(run1Node1Id); saved.setComponentId(run1Component1Id); saved.setComponentType("Discussion"); + saved.setItemType("Discussion"); saved.setUpdatedAt(new Timestamp(1000)); saved.setStudentNamesDisplay("hide"); saved.setUpdatedByWorkgroup(null); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), anyObject(), - eq("hide"), isNull(), isNull())).andReturn(saved); - expect(teacherPresentationConfigService.getStudentWorkIds(saved)) + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.ITEM_TYPE_DISCUSSION), + anyObject(), eq("hide"), eq(true), isNull(), isNull())).andReturn(saved); + expect(teacherPresentationConfigService.getItems(saved)) .andReturn(new ArrayList<>()); replay(runService, userService, workgroupService, teacherPresentationConfigService); diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java index 75ba3332b..16b08c61c 100644 --- a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java @@ -5,13 +5,20 @@ import static org.easymock.EasyMock.expectLastCall; import static org.easymock.EasyMock.replay; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import org.wise.portal.service.teacherpresentation.DiscussionItem; +import org.wise.portal.service.teacherpresentation.IdeasItem; +import org.wise.portal.service.teacherpresentation.IdeasResponse; + import org.easymock.EasyMockExtension; import org.easymock.Mock; import org.easymock.TestSubject; @@ -116,6 +123,7 @@ public void saveConfig_NewConfig_CreateWithDefaultsAndDeduplicateItems() throws replay(teacherPresentationConfigDao, studentWorkDao); TeacherPresentationConfig config = save(Arrays.asList(5, 5), null, " "); assertEquals("hide", config.getStudentNamesDisplay()); + assertTrue(config.isShowComponentPrompt()); assertNull(config.getPrompt()); assertEquals(Arrays.asList(5), service.getStudentWorkIds(config)); assertEquals("Discussion", config.getComponentType()); @@ -135,4 +143,92 @@ public void saveConfig_ExistingConfig_UpdatePromptAndNames() throws Exception { assertEquals("My prompt", config.getPrompt()); assertEquals(0, service.getStudentWorkIds(config).size()); } + + @Test + public void saveConfig_InvalidItemType_ThrowIllegalArgument() { + replay(teacherPresentationConfigDao, studentWorkDao); + assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", + "OpenResponse", "InvalidType", new ArrayList<>(), "hide", null, null)); + } + + @Test + public void saveConfig_Ideas_CategoryOnly_SaveAndReturnConfig() throws Exception { + expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")).andReturn(null); + teacherPresentationConfigDao.save(anyObject(TeacherPresentationConfig.class)); + expectLastCall(); + replay(teacherPresentationConfigDao, studentWorkDao); + + List items = Arrays.asList(new IdeasItem("idea_sunlight", Collections.emptyList())); + TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", "OpenResponse", + TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, "hide", null, null); + + assertEquals(TeacherPresentationConfig.ITEM_TYPE_IDEAS, config.getItemType()); + List retrieved = service.getItems(config); + assertEquals(1, retrieved.size()); + IdeasItem item = (IdeasItem) retrieved.get(0); + assertEquals("idea_sunlight", item.ideaId()); + assertTrue(item.responses().isEmpty()); + assertEquals(0, service.getStudentWorkIds(config).size()); + } + + @Test + public void saveConfig_Ideas_CategoryAndResponsesAndNullIdeaId_SaveAndReturnConfig() throws Exception { + expect(studentWorkDao.getById(5)).andReturn(work(run, period)).anyTimes(); + expect(studentWorkDao.getById(7)).andReturn(work(run, period)); + expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")).andReturn(null); + teacherPresentationConfigDao.save(anyObject(TeacherPresentationConfig.class)); + expectLastCall(); + replay(teacherPresentationConfigDao, studentWorkDao); + + // two messages from the same chat (same student work id, different message timestamps) + List items = Arrays.asList( + new IdeasItem("idea_1", Arrays.asList(new IdeasResponse(5, 1L, 1000L, "First message"), + new IdeasResponse(5, 1L, 2000L, "Second message"))), + new IdeasItem(null, Arrays.asList(new IdeasResponse(7, 2L, null, "Standalone"))) + ); + TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", "OpenResponse", + TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, "show", "Custom prompt", null); + + assertEquals(TeacherPresentationConfig.ITEM_TYPE_IDEAS, config.getItemType()); + List retrieved = service.getItems(config); + assertEquals(2, retrieved.size()); + IdeasItem first = (IdeasItem) retrieved.get(0); + assertEquals("idea_1", first.ideaId()); + assertEquals(2, first.responses().size()); + assertEquals(Long.valueOf(1000L), first.responses().get(0).responseTimestamp()); + assertEquals("Second message", first.responses().get(1).text()); + IdeasItem second = (IdeasItem) retrieved.get(1); + assertNull(second.ideaId()); + assertNull(second.responses().get(0).responseTimestamp()); + assertEquals(Arrays.asList(5, 5, 7), service.getStudentWorkIds(config)); + } + + @Test + public void saveConfig_Ideas_NullIdeaIdAndEmptyResponses_ThrowIllegalArgument() { + replay(teacherPresentationConfigDao, studentWorkDao); + List items = Arrays.asList(new IdeasItem(null, Collections.emptyList())); + assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", + "OpenResponse", TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, "hide", null, null)); + } + + @Test + public void saveConfig_Ideas_ResponseWithoutWorkgroupId_ThrowIllegalArgument() { + replay(teacherPresentationConfigDao, studentWorkDao); + List items = Arrays.asList( + new IdeasItem("idea_1", Arrays.asList(new IdeasResponse(5, null, 1000L, "text")))); + assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", + "OpenResponse", TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, "hide", null, null)); + } + + @Test + public void saveConfig_ShowComponentPrompt_SaveAndPersist() throws Exception { + expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")).andReturn(null); + teacherPresentationConfigDao.save(anyObject(TeacherPresentationConfig.class)); + expectLastCall(); + replay(teacherPresentationConfigDao, studentWorkDao); + TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", + "Discussion", TeacherPresentationConfig.ITEM_TYPE_DISCUSSION, new ArrayList<>(), "hide", + false, null, null); + assertFalse(config.isShowComponentPrompt()); + } } From fae903b8a0d24f959cde1cc5ec46461e0cf7700b Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Mon, 19 Oct 2026 12:31:40 -0700 Subject: [PATCH 08/12] Rename itemType property to type in TeacherPresentationConfig --- ...eacherPresentationConfigAPIController.java | 14 +++++----- .../TeacherPresentationConfigService.java | 12 ++++---- .../TeacherPresentationConfigServiceImpl.java | 28 +++++++++---------- .../TeacherPresentationConfig.java | 8 +++--- ...to_type_in_teacher_presentation_config.sql | 1 + ...erPresentationConfigAPIControllerTest.java | 28 +++++++++---------- ...cherPresentationConfigServiceImplTest.java | 16 +++++------ 7 files changed, 54 insertions(+), 53 deletions(-) create mode 100644 src/main/resources/db/migration/V6__rename_item_type_to_type_in_teacher_presentation_config.sql diff --git a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java index df1c39c45..f09e3d405 100644 --- a/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java +++ b/src/main/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIController.java @@ -61,12 +61,12 @@ public Item(String ideaId, List responses) { } public record ConfigRequest(Long runId, Long periodId, String nodeId, String componentId, - String componentType, String itemType, List items, String studentNamesDisplay, + String componentType, String type, List items, String studentNamesDisplay, Boolean showComponentPrompt, String prompt) { } public record ConfigResponse(Long id, Long runId, Long periodId, String nodeId, - String componentId, String componentType, String itemType, List items, String studentNamesDisplay, + String componentId, String componentType, String type, List items, String studentNamesDisplay, Boolean showComponentPrompt, String prompt, Long updatedByWorkgroupId, Long updatedAt) { } @@ -99,10 +99,10 @@ ConfigResponse saveConfig(Authentication auth, @RequestBody ConfigRequest reques throw new AccessDeniedException("Not permitted"); } Group period = getPeriod(run, request.periodId()); - String itemType = request.itemType() != null ? request.itemType() - : TeacherPresentationConfig.ITEM_TYPE_DISCUSSION; + String type = request.type() != null ? request.type() + : TeacherPresentationConfig.TYPE_DISCUSSION; List itemsToSave; - if (TeacherPresentationConfig.ITEM_TYPE_IDEAS.equals(itemType)) { + if (TeacherPresentationConfig.TYPE_IDEAS.equals(type)) { List ideasItems = new ArrayList<>(); if (request.items() != null) { for (Item item : request.items()) { @@ -125,7 +125,7 @@ ConfigResponse saveConfig(Authentication auth, @RequestBody ConfigRequest reques } try { TeacherPresentationConfig config = teacherPresentationConfigService.saveConfig(run, period, - request.nodeId(), request.componentId(), request.componentType(), itemType, itemsToSave, + request.nodeId(), request.componentId(), request.componentType(), type, itemsToSave, request.studentNamesDisplay(), request.showComponentPrompt(), request.prompt(), getTeacherWorkgroup(run, user)); return toResponse(config); @@ -153,7 +153,7 @@ private ConfigResponse toResponse(TeacherPresentationConfig config) { Workgroup updatedBy = config.getUpdatedByWorkgroup(); return new ConfigResponse(config.getId(), config.getRun().getId(), config.getPeriod().getId(), config.getNodeId(), config.getComponentId(), config.getComponentType(), - config.getItemType(), items, + config.getType(), items, config.getStudentNamesDisplay(), config.isShowComponentPrompt(), config.getPrompt(), updatedBy == null ? null : updatedBy.getId(), config.getUpdatedAt().getTime()); } diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java index 33300a50e..920b87fbb 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java @@ -22,9 +22,9 @@ public interface TeacherPresentationConfigService { List getItems(TeacherPresentationConfig config); /** - * Creates or updates the config for the run, period, node and component with itemType. + * Creates or updates the config for the run, period, node and component with type. * - * @param itemType one of Discussion or Ideas. + * @param type one of Discussion or Ideas. * @param items list of DiscussionItem or IdeasItem. * @param prompt the teacher's prompt. null or blank resets to the component default. * @param studentNamesDisplay one of show, hide or anonymize. null keeps the current value (or @@ -32,13 +32,13 @@ public interface TeacherPresentationConfigService { * @throws IllegalArgumentException if the input is invalid */ TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, - String componentType, String itemType, List items, String studentNamesDisplay, + String componentType, String type, List items, String studentNamesDisplay, Boolean showComponentPrompt, String prompt, Workgroup teacherWorkgroup); default TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, - String componentType, String itemType, List items, String studentNamesDisplay, + String componentType, String type, List items, String studentNamesDisplay, String prompt, Workgroup teacherWorkgroup) { - return saveConfig(run, period, nodeId, componentId, componentType, itemType, items, + return saveConfig(run, period, nodeId, componentId, componentType, type, items, studentNamesDisplay, null, prompt, teacherWorkgroup); } @@ -55,7 +55,7 @@ default TeacherPresentationConfig saveConfig(Run run, Group period, String nodeI } } return saveConfig(run, period, nodeId, componentId, componentType, - TeacherPresentationConfig.ITEM_TYPE_DISCUSSION, items, studentNamesDisplay, null, prompt, + TeacherPresentationConfig.TYPE_DISCUSSION, items, studentNamesDisplay, null, prompt, teacherWorkgroup); } } diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java index 526f564f3..acc56d072 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java @@ -38,9 +38,9 @@ public class TeacherPresentationConfigServiceImpl implements TeacherPresentation private static final Set NAMES_DISPLAY_OPTIONS = Set.of( TeacherPresentationConfig.NAMES_SHOW, TeacherPresentationConfig.NAMES_HIDE, TeacherPresentationConfig.NAMES_ANONYMIZE); - private static final Set ITEM_TYPE_OPTIONS = Set.of( - TeacherPresentationConfig.ITEM_TYPE_DISCUSSION, - TeacherPresentationConfig.ITEM_TYPE_IDEAS); + private static final Set TYPE_OPTIONS = Set.of( + TeacherPresentationConfig.TYPE_DISCUSSION, + TeacherPresentationConfig.TYPE_IDEAS); private final ObjectMapper objectMapper = new ObjectMapper(); @@ -62,9 +62,9 @@ public List getItems(TeacherPresentationConfig config) { if (config == null || config.getItems() == null) { return Collections.emptyList(); } - String itemType = config.getItemType(); + String type = config.getType(); try { - if (TeacherPresentationConfig.ITEM_TYPE_IDEAS.equals(itemType)) { + if (TeacherPresentationConfig.TYPE_IDEAS.equals(type)) { return objectMapper.readValue(config.getItems(), new TypeReference>() { }); @@ -101,13 +101,13 @@ public List getStudentWorkIds(TeacherPresentationConfig config) { @Override @Transactional public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, - String componentId, String componentType, String itemType, List items, + String componentId, String componentType, String type, List items, String studentNamesDisplay, Boolean showComponentPrompt, String prompt, Workgroup teacherWorkgroup) { validateComponent(nodeId, componentId, componentType); - String resolvedItemType = itemType != null ? itemType : TeacherPresentationConfig.ITEM_TYPE_DISCUSSION; - if (!ITEM_TYPE_OPTIONS.contains(resolvedItemType)) { - throw new IllegalArgumentException("Invalid itemType: " + itemType); + String resolvedType = type != null ? type : TeacherPresentationConfig.TYPE_DISCUSSION; + if (!TYPE_OPTIONS.contains(resolvedType)) { + throw new IllegalArgumentException("Invalid type: " + type); } if (studentNamesDisplay != null && !NAMES_DISPLAY_OPTIONS.contains(studentNamesDisplay)) { throw new IllegalArgumentException("Invalid studentNamesDisplay: " + studentNamesDisplay); @@ -115,7 +115,7 @@ public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId String normalizedPrompt = normalizePrompt(prompt); String serializedItems; - if (TeacherPresentationConfig.ITEM_TYPE_IDEAS.equals(resolvedItemType)) { + if (TeacherPresentationConfig.TYPE_IDEAS.equals(resolvedType)) { List ideasItems = validateAndNormalizeIdeasItems(run, period, items); serializedItems = serializeIdeasItems(ideasItems); if (serializedItems.getBytes(java.nio.charset.StandardCharsets.UTF_8).length @@ -128,9 +128,9 @@ public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId } TeacherPresentationConfig config = getOrCreateConfig(run, period, nodeId, componentId, - componentType, resolvedItemType); + componentType, resolvedType); config.setComponentType(componentType); - config.setItemType(resolvedItemType); + config.setType(resolvedType); config.setItems(serializedItems); if (studentNamesDisplay != null) { config.setStudentNamesDisplay(studentNamesDisplay); @@ -146,7 +146,7 @@ public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId } private TeacherPresentationConfig getOrCreateConfig(Run run, Group period, String nodeId, - String componentId, String componentType, String itemType) { + String componentId, String componentType, String type) { TeacherPresentationConfig config = teacherPresentationConfigDao.getConfig(run, period, nodeId, componentId); if (config == null) { @@ -157,7 +157,7 @@ private TeacherPresentationConfig getOrCreateConfig(Run run, Group period, Strin config.setNodeId(nodeId); config.setComponentId(componentId); config.setComponentType(componentType); - config.setItemType(itemType); + config.setType(type); config.setCreatedAt(now); config.setUpdatedAt(now); } diff --git a/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java index 49cb56ff7..35aafd0b1 100644 --- a/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java +++ b/src/main/java/org/wise/vle/domain/teacherpresentation/TeacherPresentationConfig.java @@ -42,8 +42,8 @@ public class TeacherPresentationConfig { public static final String NAMES_HIDE = "hide"; public static final String NAMES_ANONYMIZE = "anonymize"; - public static final String ITEM_TYPE_DISCUSSION = "Discussion"; - public static final String ITEM_TYPE_IDEAS = "Ideas"; + public static final String TYPE_DISCUSSION = "Discussion"; + public static final String TYPE_IDEAS = "Ideas"; @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @@ -68,8 +68,8 @@ public class TeacherPresentationConfig { @Column(name = "componentType", length = 30, nullable = false) private String componentType; - @Column(name = "itemType", length = 30, nullable = false) - private String itemType = ITEM_TYPE_DISCUSSION; + @Column(name = "`type`", length = 30, nullable = false) + private String type = TYPE_DISCUSSION; /** JSON array of the form [{"studentWorkId": 123}, ...] */ @Column(name = "items", length = 65536, columnDefinition = "text", nullable = false) diff --git a/src/main/resources/db/migration/V6__rename_item_type_to_type_in_teacher_presentation_config.sql b/src/main/resources/db/migration/V6__rename_item_type_to_type_in_teacher_presentation_config.sql new file mode 100644 index 000000000..6bfd8fded --- /dev/null +++ b/src/main/resources/db/migration/V6__rename_item_type_to_type_in_teacher_presentation_config.sql @@ -0,0 +1 @@ +alter table teacher_presentation_config change column itemType `type` varchar(30) not null; diff --git a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java index c41a8ae6f..75f590c09 100644 --- a/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java +++ b/src/test/java/org/wise/portal/presentation/web/controllers/teacher/TeacherPresentationConfigAPIControllerTest.java @@ -59,13 +59,13 @@ private void expectTeacherWithPermissions(boolean canView, boolean canGrade) private ConfigRequest request(Long periodId, List items, String namesDisplay, String prompt) { return new ConfigRequest(runId1, periodId, run1Node1Id, run1Component1Id, "Discussion", - TeacherPresentationConfig.ITEM_TYPE_DISCUSSION, items, namesDisplay, true, prompt); + TeacherPresentationConfig.TYPE_DISCUSSION, items, namesDisplay, true, prompt); } private ConfigRequest ideasRequest(Long periodId, List items, String namesDisplay, String prompt) { return new ConfigRequest(runId1, periodId, run1Node1Id, run1Component1Id, "OpenResponse", - TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, namesDisplay, true, prompt); + TeacherPresentationConfig.TYPE_IDEAS, items, namesDisplay, true, prompt); } @Test @@ -77,7 +77,7 @@ public void getConfig_NoConfigSaved_ReturnDefaults() throws Exception { ConfigResponse response = controller.getConfig(teacherAuth, runId1, run1Period1Id, run1Node1Id, run1Component1Id); assertTrue(response.items().isEmpty()); - assertNull(response.itemType()); + assertNull(response.type()); assertEquals("hide", response.studentNamesDisplay()); assertTrue(response.showComponentPrompt()); assertNull(response.prompt()); @@ -113,7 +113,7 @@ public void saveConfig_NoGradePermission_ThrowAccessDenied() throws Exception { public void saveConfig_ServiceRejectsInput_ThrowBadRequest() throws Exception { expectTeacherWithPermissions(true, true); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.ITEM_TYPE_DISCUSSION), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.TYPE_DISCUSSION), anyObject(), eq("bogus"), eq(true), isNull(), eq(teacher1Run1Workgroup))) .andThrow(new IllegalArgumentException("Invalid")); replay(runService, userService, workgroupService, teacherPresentationConfigService); @@ -132,14 +132,14 @@ public void saveConfig_ValidRequest_SaveWithTeacherWorkgroupAndReturnConfig() th saved.setNodeId(run1Node1Id); saved.setComponentId(run1Component1Id); saved.setComponentType("Discussion"); - saved.setItemType("Discussion"); + saved.setType("Discussion"); saved.setUpdatedAt(new Timestamp(1000)); saved.setPrompt("My prompt"); saved.setStudentNamesDisplay("anonymize"); saved.setShowComponentPrompt(true); saved.setUpdatedByWorkgroup(teacher1Run1Workgroup); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.ITEM_TYPE_DISCUSSION), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.TYPE_DISCUSSION), eq(Arrays.asList(new DiscussionItem(7), new DiscussionItem(8))), eq("anonymize"), eq(true), eq("My prompt"), eq(teacher1Run1Workgroup))).andReturn(saved); expect(teacherPresentationConfigService.getItems(saved)) @@ -165,7 +165,7 @@ public void saveConfig_ValidIdeasRequest_SaveAndReturnConfig() throws Exception saved.setNodeId(run1Node1Id); saved.setComponentId(run1Component1Id); saved.setComponentType("OpenResponse"); - saved.setItemType("Ideas"); + saved.setType("Ideas"); saved.setUpdatedAt(new Timestamp(1000)); saved.setPrompt("Ideas prompt"); saved.setStudentNamesDisplay("hide"); @@ -181,7 +181,7 @@ public void saveConfig_ValidIdeasRequest_SaveAndReturnConfig() throws Exception new IdeasItem(null, responses3) ); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("OpenResponse"), eq(TeacherPresentationConfig.ITEM_TYPE_IDEAS), + eq(run1Node1Id), eq(run1Component1Id), eq("OpenResponse"), eq(TeacherPresentationConfig.TYPE_IDEAS), eq(expectedSavedItems), eq("hide"), eq(true), eq("Ideas prompt"), eq(teacher1Run1Workgroup))).andReturn(saved); expect(teacherPresentationConfigService.getItems(saved)) .andReturn((List) expectedSavedItems); @@ -198,7 +198,7 @@ public void saveConfig_ValidIdeasRequest_SaveAndReturnConfig() throws Exception assertEquals(8L, response.id()); assertTrue(response.showComponentPrompt()); - assertEquals("Ideas", response.itemType()); + assertEquals("Ideas", response.type()); assertEquals(3, response.items().size()); verify(teacherPresentationConfigService); } @@ -214,7 +214,7 @@ public void getConfig_ExistingConfig_ReturnPopulatedResponse() throws Exception config.setNodeId(run1Node1Id); config.setComponentId(run1Component1Id); config.setComponentType("Discussion"); - config.setItemType("Discussion"); + config.setType("Discussion"); config.setPrompt("Prompt override"); config.setStudentNamesDisplay("show"); config.setUpdatedAt(new Timestamp(1000)); @@ -248,11 +248,11 @@ public void saveConfig_NullItems_SaveWithEmptyList() throws Exception { saved.setNodeId(run1Node1Id); saved.setComponentId(run1Component1Id); saved.setComponentType("Discussion"); - saved.setItemType("Discussion"); + saved.setType("Discussion"); saved.setUpdatedAt(new Timestamp(1000)); saved.setStudentNamesDisplay("hide"); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.ITEM_TYPE_DISCUSSION), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.TYPE_DISCUSSION), eq(new ArrayList<>()), eq("hide"), eq(true), isNull(), eq(teacher1Run1Workgroup))).andReturn(saved); expect(teacherPresentationConfigService.getItems(saved)) .andReturn(new ArrayList<>()); @@ -281,13 +281,13 @@ public void saveConfig_TeacherWithNoWorkgroup_SaveWithNullWorkgroup() throws Exc saved.setNodeId(run1Node1Id); saved.setComponentId(run1Component1Id); saved.setComponentType("Discussion"); - saved.setItemType("Discussion"); + saved.setType("Discussion"); saved.setUpdatedAt(new Timestamp(1000)); saved.setStudentNamesDisplay("hide"); saved.setUpdatedByWorkgroup(null); expect(teacherPresentationConfigService.saveConfig(eq(run1), eq(run1Period1), - eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.ITEM_TYPE_DISCUSSION), + eq(run1Node1Id), eq(run1Component1Id), eq("Discussion"), eq(TeacherPresentationConfig.TYPE_DISCUSSION), anyObject(), eq("hide"), eq(true), isNull(), isNull())).andReturn(saved); expect(teacherPresentationConfigService.getItems(saved)) .andReturn(new ArrayList<>()); diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java index 16b08c61c..d9116aa7b 100644 --- a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java @@ -145,7 +145,7 @@ public void saveConfig_ExistingConfig_UpdatePromptAndNames() throws Exception { } @Test - public void saveConfig_InvalidItemType_ThrowIllegalArgument() { + public void saveConfig_InvalidType_ThrowIllegalArgument() { replay(teacherPresentationConfigDao, studentWorkDao); assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", "OpenResponse", "InvalidType", new ArrayList<>(), "hide", null, null)); @@ -160,9 +160,9 @@ public void saveConfig_Ideas_CategoryOnly_SaveAndReturnConfig() throws Exception List items = Arrays.asList(new IdeasItem("idea_sunlight", Collections.emptyList())); TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", "OpenResponse", - TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, "hide", null, null); + TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null); - assertEquals(TeacherPresentationConfig.ITEM_TYPE_IDEAS, config.getItemType()); + assertEquals(TeacherPresentationConfig.TYPE_IDEAS, config.getType()); List retrieved = service.getItems(config); assertEquals(1, retrieved.size()); IdeasItem item = (IdeasItem) retrieved.get(0); @@ -187,9 +187,9 @@ public void saveConfig_Ideas_CategoryAndResponsesAndNullIdeaId_SaveAndReturnConf new IdeasItem(null, Arrays.asList(new IdeasResponse(7, 2L, null, "Standalone"))) ); TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", "OpenResponse", - TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, "show", "Custom prompt", null); + TeacherPresentationConfig.TYPE_IDEAS, items, "show", "Custom prompt", null); - assertEquals(TeacherPresentationConfig.ITEM_TYPE_IDEAS, config.getItemType()); + assertEquals(TeacherPresentationConfig.TYPE_IDEAS, config.getType()); List retrieved = service.getItems(config); assertEquals(2, retrieved.size()); IdeasItem first = (IdeasItem) retrieved.get(0); @@ -208,7 +208,7 @@ public void saveConfig_Ideas_NullIdeaIdAndEmptyResponses_ThrowIllegalArgument() replay(teacherPresentationConfigDao, studentWorkDao); List items = Arrays.asList(new IdeasItem(null, Collections.emptyList())); assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", - "OpenResponse", TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, "hide", null, null)); + "OpenResponse", TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null)); } @Test @@ -217,7 +217,7 @@ public void saveConfig_Ideas_ResponseWithoutWorkgroupId_ThrowIllegalArgument() { List items = Arrays.asList( new IdeasItem("idea_1", Arrays.asList(new IdeasResponse(5, null, 1000L, "text")))); assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", - "OpenResponse", TeacherPresentationConfig.ITEM_TYPE_IDEAS, items, "hide", null, null)); + "OpenResponse", TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null)); } @Test @@ -227,7 +227,7 @@ public void saveConfig_ShowComponentPrompt_SaveAndPersist() throws Exception { expectLastCall(); replay(teacherPresentationConfigDao, studentWorkDao); TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", - "Discussion", TeacherPresentationConfig.ITEM_TYPE_DISCUSSION, new ArrayList<>(), "hide", + "Discussion", TeacherPresentationConfig.TYPE_DISCUSSION, new ArrayList<>(), "hide", false, null, null); assertFalse(config.isShowComponentPrompt()); } From 11b8c3ceda433a222b6d28351c9b101dcca944b0 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Mon, 19 Oct 2026 16:32:01 -0700 Subject: [PATCH 09/12] refactor: replace single student work lookups with batch retrieval by IDs in DAO and service layer --- .../wise/portal/dao/work/StudentWorkDao.java | 2 + .../work/impl/HibernateStudentWorkDao.java | 13 +++++ .../TeacherPresentationConfigServiceImpl.java | 19 ++++-- .../impl/HibernateStudentWorkDaoTest.java | 19 ++++++ ...cherPresentationConfigServiceImplTest.java | 58 ++++++++++--------- 5 files changed, 79 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/wise/portal/dao/work/StudentWorkDao.java b/src/main/java/org/wise/portal/dao/work/StudentWorkDao.java index a162cddfa..699ec1a40 100644 --- a/src/main/java/org/wise/portal/dao/work/StudentWorkDao.java +++ b/src/main/java/org/wise/portal/dao/work/StudentWorkDao.java @@ -58,4 +58,6 @@ List getWorkForComponentByPeriod(Run run, Group period, String node List getStudentWork(PeerGroup peerGroup, String nodeId, String componentId); List getStudentWork(Set workgroups, String nodeId, String componentId); + + List getStudentWorkListByIds(List ids); } diff --git a/src/main/java/org/wise/portal/dao/work/impl/HibernateStudentWorkDao.java b/src/main/java/org/wise/portal/dao/work/impl/HibernateStudentWorkDao.java index 73362f002..0220653ab 100644 --- a/src/main/java/org/wise/portal/dao/work/impl/HibernateStudentWorkDao.java +++ b/src/main/java/org/wise/portal/dao/work/impl/HibernateStudentWorkDao.java @@ -212,4 +212,17 @@ public List getStudentWork(Run run, Group period, String nodeId, TypedQuery query = entityManager.createQuery(cq); return (List) query.getResultList(); } + + @Override + public List getStudentWorkListByIds(List ids) { + if (ids == null || ids.isEmpty()) { + return new ArrayList<>(); + } + CriteriaBuilder cb = getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(StudentWork.class); + Root studentWorkRoot = cq.from(StudentWork.class); + cq.select(studentWorkRoot).where(studentWorkRoot.get("id").in(ids)); + TypedQuery query = entityManager.createQuery(cq); + return query.getResultList(); + } } diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java index acc56d072..90804e83a 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java @@ -3,6 +3,7 @@ import java.sql.Timestamp; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -11,7 +12,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import org.wise.portal.dao.ObjectNotFoundException; import org.wise.portal.dao.teacherpresentation.TeacherPresentationConfigDao; import org.wise.portal.dao.work.StudentWorkDao; import org.wise.portal.domain.group.Group; @@ -279,12 +279,19 @@ private Set validateStudentWorkIds(Run run, Group period, List if (id == null) { throw new IllegalArgumentException("Invalid studentWorkId"); } - StudentWork work; - try { - work = studentWorkDao.getById(id); - } catch (ObjectNotFoundException e) { - throw new IllegalArgumentException("Unknown studentWorkId: " + id); + } + if (uniqueIds.isEmpty()) { + return uniqueIds; + } + List works = studentWorkDao.getStudentWorkListByIds(new ArrayList<>(uniqueIds)); + Map workMap = new HashMap<>(); + for (StudentWork work : works) { + if (work != null && work.getId() != null) { + workMap.put(work.getId(), work); } + } + for (Integer id : uniqueIds) { + StudentWork work = workMap.get(id); if (work == null || !work.getRun().getId().equals(run.getId()) || !work.getPeriod().getId().equals(period.getId())) { throw new IllegalArgumentException("Unknown studentWorkId: " + id); diff --git a/src/test/java/org/wise/portal/dao/work/impl/HibernateStudentWorkDaoTest.java b/src/test/java/org/wise/portal/dao/work/impl/HibernateStudentWorkDaoTest.java index 5d9a30eb1..de97142a1 100644 --- a/src/test/java/org/wise/portal/dao/work/impl/HibernateStudentWorkDaoTest.java +++ b/src/test/java/org/wise/portal/dao/work/impl/HibernateStudentWorkDaoTest.java @@ -26,7 +26,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import java.sql.Timestamp; +import java.util.Arrays; import java.util.Calendar; +import java.util.Collections; import java.util.List; import org.junit.jupiter.api.BeforeEach; @@ -173,6 +175,23 @@ public void getStudentWork_FromAllPeriods_ShouldReturnWork() { assertEquals(DUMMY_STUDENT_WORK3, getStudentData(studentWorkList, 1)); } + @Test + public void getStudentWorkListByIds_ValidIds_ShouldReturnStudentWork() { + StudentWork sw1 = createStudentWork(workgroup1, peerGroup1, "nodeX", "compX", "Work X"); + StudentWork sw2 = createStudentWork(workgroup1, peerGroup1, "nodeY", "compY", "Work Y"); + + List results = + studentWorkDao.getStudentWorkListByIds(Arrays.asList(sw1.getId(), sw2.getId())); + assertEquals(2, results.size()); + + List emptyResults = + studentWorkDao.getStudentWorkListByIds(Collections.emptyList()); + assertEquals(0, emptyResults.size()); + + List nullResults = studentWorkDao.getStudentWorkListByIds(null); + assertEquals(0, nullResults.size()); + } + private StudentWork createStudentWork(Workgroup workgroup, PeerGroup peerGroup, String nodeId, String componentId, String studentData) { StudentWork studentWork = new StudentWork(); diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java index d9116aa7b..d5a2de761 100644 --- a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java @@ -58,8 +58,9 @@ public void setUp() { otherPeriod.setId(11L); } - private StudentWork work(RunImpl workRun, PersistentGroup workPeriod) { + private StudentWork work(RunImpl workRun, PersistentGroup workPeriod, Integer id) { StudentWork work = new StudentWork(); + work.setId(id); work.setRun(workRun); work.setPeriod(workPeriod); return work; @@ -95,28 +96,32 @@ public void saveConfig_PromptTooLong_ThrowIllegalArgument() { @Test public void saveConfig_StudentWorkFromOtherPeriod_ThrowIllegalArgument() throws Exception { - expect(studentWorkDao.getById(5)).andReturn(work(run, otherPeriod)); + expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(5))) + .andReturn(Arrays.asList(work(run, otherPeriod, 5))); replay(teacherPresentationConfigDao, studentWorkDao); assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); } @Test public void saveConfig_StudentWorkFromOtherRun_ThrowIllegalArgument() throws Exception { - expect(studentWorkDao.getById(5)).andReturn(work(otherRun, period)); + expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(5))) + .andReturn(Arrays.asList(work(otherRun, period, 5))); replay(teacherPresentationConfigDao, studentWorkDao); assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); } @Test public void saveConfig_UnknownStudentWork_ThrowIllegalArgument() throws Exception { - expect(studentWorkDao.getById(5)).andThrow(new ObjectNotFoundException(5L, StudentWork.class)); + expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(5))) + .andReturn(Collections.emptyList()); replay(teacherPresentationConfigDao, studentWorkDao); assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); } @Test public void saveConfig_NewConfig_CreateWithDefaultsAndDeduplicateItems() throws Exception { - expect(studentWorkDao.getById(5)).andReturn(work(run, period)); + expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(5))) + .andReturn(Arrays.asList(work(run, period, 5))); expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")).andReturn(null); teacherPresentationConfigDao.save(anyObject(TeacherPresentationConfig.class)); expectLastCall(); @@ -147,8 +152,8 @@ public void saveConfig_ExistingConfig_UpdatePromptAndNames() throws Exception { @Test public void saveConfig_InvalidType_ThrowIllegalArgument() { replay(teacherPresentationConfigDao, studentWorkDao); - assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", - "OpenResponse", "InvalidType", new ArrayList<>(), "hide", null, null)); + assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", + "comp1", "OpenResponse", "InvalidType", new ArrayList<>(), "hide", null, null)); } @Test @@ -159,8 +164,8 @@ public void saveConfig_Ideas_CategoryOnly_SaveAndReturnConfig() throws Exception replay(teacherPresentationConfigDao, studentWorkDao); List items = Arrays.asList(new IdeasItem("idea_sunlight", Collections.emptyList())); - TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", "OpenResponse", - TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null); + TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", + "OpenResponse", TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null); assertEquals(TeacherPresentationConfig.TYPE_IDEAS, config.getType()); List retrieved = service.getItems(config); @@ -172,9 +177,10 @@ public void saveConfig_Ideas_CategoryOnly_SaveAndReturnConfig() throws Exception } @Test - public void saveConfig_Ideas_CategoryAndResponsesAndNullIdeaId_SaveAndReturnConfig() throws Exception { - expect(studentWorkDao.getById(5)).andReturn(work(run, period)).anyTimes(); - expect(studentWorkDao.getById(7)).andReturn(work(run, period)); + public void saveConfig_Ideas_CategoryAndResponsesAndNullIdeaId_SaveAndReturnConfig() + throws Exception { + expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(5, 7))) + .andReturn(Arrays.asList(work(run, period, 5), work(run, period, 7))); expect(teacherPresentationConfigDao.getConfig(run, period, "node1", "comp1")).andReturn(null); teacherPresentationConfigDao.save(anyObject(TeacherPresentationConfig.class)); expectLastCall(); @@ -182,12 +188,12 @@ public void saveConfig_Ideas_CategoryAndResponsesAndNullIdeaId_SaveAndReturnConf // two messages from the same chat (same student work id, different message timestamps) List items = Arrays.asList( - new IdeasItem("idea_1", Arrays.asList(new IdeasResponse(5, 1L, 1000L, "First message"), - new IdeasResponse(5, 1L, 2000L, "Second message"))), - new IdeasItem(null, Arrays.asList(new IdeasResponse(7, 2L, null, "Standalone"))) - ); - TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", "OpenResponse", - TeacherPresentationConfig.TYPE_IDEAS, items, "show", "Custom prompt", null); + new IdeasItem("idea_1", + Arrays.asList(new IdeasResponse(5, 1L, 1000L, "First message"), + new IdeasResponse(5, 1L, 2000L, "Second message"))), + new IdeasItem(null, Arrays.asList(new IdeasResponse(7, 2L, null, "Standalone")))); + TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", + "OpenResponse", TeacherPresentationConfig.TYPE_IDEAS, items, "show", "Custom prompt", null); assertEquals(TeacherPresentationConfig.TYPE_IDEAS, config.getType()); List retrieved = service.getItems(config); @@ -207,17 +213,17 @@ public void saveConfig_Ideas_CategoryAndResponsesAndNullIdeaId_SaveAndReturnConf public void saveConfig_Ideas_NullIdeaIdAndEmptyResponses_ThrowIllegalArgument() { replay(teacherPresentationConfigDao, studentWorkDao); List items = Arrays.asList(new IdeasItem(null, Collections.emptyList())); - assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", - "OpenResponse", TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null)); + assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", + "comp1", "OpenResponse", TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null)); } @Test public void saveConfig_Ideas_ResponseWithoutWorkgroupId_ThrowIllegalArgument() { replay(teacherPresentationConfigDao, studentWorkDao); - List items = Arrays.asList( - new IdeasItem("idea_1", Arrays.asList(new IdeasResponse(5, null, 1000L, "text")))); - assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", "comp1", - "OpenResponse", TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null)); + List items = Arrays + .asList(new IdeasItem("idea_1", Arrays.asList(new IdeasResponse(5, null, 1000L, "text")))); + assertThrows(IllegalArgumentException.class, () -> service.saveConfig(run, period, "node1", + "comp1", "OpenResponse", TeacherPresentationConfig.TYPE_IDEAS, items, "hide", null, null)); } @Test @@ -227,8 +233,8 @@ public void saveConfig_ShowComponentPrompt_SaveAndPersist() throws Exception { expectLastCall(); replay(teacherPresentationConfigDao, studentWorkDao); TeacherPresentationConfig config = service.saveConfig(run, period, "node1", "comp1", - "Discussion", TeacherPresentationConfig.TYPE_DISCUSSION, new ArrayList<>(), "hide", - false, null, null); + "Discussion", TeacherPresentationConfig.TYPE_DISCUSSION, new ArrayList<>(), "hide", false, + null, null); assertFalse(config.isShowComponentPrompt()); } } From e267fb4d5414b2d25be909d0fbd7a8047ad1dbc9 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Mon, 19 Oct 2026 16:36:48 -0700 Subject: [PATCH 10/12] refactor: remove deprecated saveConfig method and update tests to use DiscussionItem objects --- .../TeacherPresentationConfigService.java | 23 +++--------------- ...cherPresentationConfigServiceImplTest.java | 24 +++++++++++-------- 2 files changed, 17 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java index 920b87fbb..5f3ccd46e 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationConfigService.java @@ -35,27 +35,10 @@ TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, Strin String componentType, String type, List items, String studentNamesDisplay, Boolean showComponentPrompt, String prompt, Workgroup teacherWorkgroup); - default TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, - String componentType, String type, List items, String studentNamesDisplay, - String prompt, Workgroup teacherWorkgroup) { + default TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, + String componentId, String componentType, String type, List items, + String studentNamesDisplay, String prompt, Workgroup teacherWorkgroup) { return saveConfig(run, period, nodeId, componentId, componentType, type, items, studentNamesDisplay, null, prompt, teacherWorkgroup); } - - /** - * Backward-compatible saveConfig for Discussion with a list of studentWorkIds. - */ - default TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId, String componentId, - String componentType, List studentWorkIds, String studentNamesDisplay, - String prompt, Workgroup teacherWorkgroup) { - List items = new java.util.ArrayList<>(); - if (studentWorkIds != null) { - for (Integer id : studentWorkIds) { - items.add(new DiscussionItem(id)); - } - } - return saveConfig(run, period, nodeId, componentId, componentType, - TeacherPresentationConfig.TYPE_DISCUSSION, items, studentNamesDisplay, null, prompt, - teacherWorkgroup); - } } diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java index d5a2de761..bfdbcfaae 100644 --- a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java @@ -66,9 +66,9 @@ private StudentWork work(RunImpl workRun, PersistentGroup workPeriod, Integer id return work; } - private TeacherPresentationConfig save(List ids, String namesDisplay, String prompt) { - return service.saveConfig(run, period, "node1", "comp1", "Discussion", ids, namesDisplay, - prompt, null); + private TeacherPresentationConfig save(List items, String namesDisplay, String prompt) { + return service.saveConfig(run, period, "node1", "comp1", "Discussion", + TeacherPresentationConfig.TYPE_DISCUSSION, items, namesDisplay, prompt, null); } @Test @@ -79,12 +79,12 @@ public void saveConfig_InvalidNamesDisplay_ThrowIllegalArgument() { @Test public void saveConfig_MoreThan500Items_ThrowIllegalArgument() { - List ids = new ArrayList<>(); + List items = new ArrayList<>(); for (int i = 0; i < 501; i++) { - ids.add(i); + items.add(new DiscussionItem(i)); } replay(teacherPresentationConfigDao, studentWorkDao); - assertThrows(IllegalArgumentException.class, () -> save(ids, "hide", null)); + assertThrows(IllegalArgumentException.class, () -> save(items, "hide", null)); } @Test @@ -99,7 +99,8 @@ public void saveConfig_StudentWorkFromOtherPeriod_ThrowIllegalArgument() throws expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(5))) .andReturn(Arrays.asList(work(run, otherPeriod, 5))); replay(teacherPresentationConfigDao, studentWorkDao); - assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); + assertThrows(IllegalArgumentException.class, + () -> save(Arrays.asList(new DiscussionItem(5)), "hide", null)); } @Test @@ -107,7 +108,8 @@ public void saveConfig_StudentWorkFromOtherRun_ThrowIllegalArgument() throws Exc expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(5))) .andReturn(Arrays.asList(work(otherRun, period, 5))); replay(teacherPresentationConfigDao, studentWorkDao); - assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); + assertThrows(IllegalArgumentException.class, + () -> save(Arrays.asList(new DiscussionItem(5)), "hide", null)); } @Test @@ -115,7 +117,8 @@ public void saveConfig_UnknownStudentWork_ThrowIllegalArgument() throws Exceptio expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(5))) .andReturn(Collections.emptyList()); replay(teacherPresentationConfigDao, studentWorkDao); - assertThrows(IllegalArgumentException.class, () -> save(Arrays.asList(5), "hide", null)); + assertThrows(IllegalArgumentException.class, + () -> save(Arrays.asList(new DiscussionItem(5)), "hide", null)); } @Test @@ -126,7 +129,8 @@ public void saveConfig_NewConfig_CreateWithDefaultsAndDeduplicateItems() throws teacherPresentationConfigDao.save(anyObject(TeacherPresentationConfig.class)); expectLastCall(); replay(teacherPresentationConfigDao, studentWorkDao); - TeacherPresentationConfig config = save(Arrays.asList(5, 5), null, " "); + TeacherPresentationConfig config = + save(Arrays.asList(new DiscussionItem(5), new DiscussionItem(5)), null, " "); assertEquals("hide", config.getStudentNamesDisplay()); assertTrue(config.isShowComponentPrompt()); assertNull(config.getPrompt()); From 77e250525662870c14809f15309bfdec3d41c27a Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Mon, 19 Oct 2026 20:18:17 -0700 Subject: [PATCH 11/12] refactor: implement strategy pattern for teacher presentation item types and validation --- .../TeacherPresentationItemStrategy.java | 60 +++++ ...stractTeacherPresentationItemStrategy.java | 74 ++++++ .../DiscussionPresentationItemStrategy.java | 91 +++++++ .../impl/IdeasPresentationItemStrategy.java | 141 ++++++++++ .../TeacherPresentationConfigServiceImpl.java | 240 +++++------------- ...iscussionPresentationItemStrategyTest.java | 110 ++++++++ .../IdeasPresentationItemStrategyTest.java | 142 +++++++++++ ...cherPresentationConfigServiceImplTest.java | 3 + 8 files changed, 678 insertions(+), 183 deletions(-) create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationItemStrategy.java create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/impl/AbstractTeacherPresentationItemStrategy.java create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/impl/DiscussionPresentationItemStrategy.java create mode 100644 src/main/java/org/wise/portal/service/teacherpresentation/impl/IdeasPresentationItemStrategy.java create mode 100644 src/test/java/org/wise/portal/service/teacherpresentation/impl/DiscussionPresentationItemStrategyTest.java create mode 100644 src/test/java/org/wise/portal/service/teacherpresentation/impl/IdeasPresentationItemStrategyTest.java diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationItemStrategy.java b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationItemStrategy.java new file mode 100644 index 000000000..979f3e3c8 --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/TeacherPresentationItemStrategy.java @@ -0,0 +1,60 @@ +package org.wise.portal.service.teacherpresentation; + +import java.util.List; + +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; + +import com.fasterxml.jackson.databind.ObjectMapper; + +/** + * Strategy interface defining serialization, deserialization, validation, and ID extraction + * for different teacher presentation item types (e.g. Discussion, Ideas). + * + * @param the typed item domain class + */ +public interface TeacherPresentationItemStrategy { + + /** + * Returns the unique presentation config type string handled by this strategy + * (e.g. "Discussion", "Ideas"). + */ + String getType(); + + /** + * Deserializes the JSON items string into a typed list of items. + * + * @param json the serialized JSON items string + * @param objectMapper the Jackson object mapper + * @return list of parsed items, or empty list on null/empty/invalid input + */ + List parseItems(String json, ObjectMapper objectMapper); + + /** + * Serializes a list of items into a JSON string. + * + * @param items the list of typed items to serialize + * @param objectMapper the Jackson object mapper + * @return serialized JSON string + */ + String serializeItems(List items, ObjectMapper objectMapper); + + /** + * Validates and normalizes raw input items (e.g. from service or controller). + * + * @param run the active run + * @param period the active period group + * @param rawItems the raw input items list + * @return list of normalized typed items + * @throws IllegalArgumentException if items fail validation rules + */ + List validateAndNormalize(Run run, Group period, List rawItems); + + /** + * Extracts student work IDs referenced by the given items. + * + * @param items the list of typed items + * @return list of student work IDs + */ + List extractStudentWorkIds(List items); +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/AbstractTeacherPresentationItemStrategy.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/AbstractTeacherPresentationItemStrategy.java new file mode 100644 index 000000000..f635ab491 --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/AbstractTeacherPresentationItemStrategy.java @@ -0,0 +1,74 @@ +package org.wise.portal.service.teacherpresentation.impl; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.wise.portal.dao.work.StudentWorkDao; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; +import org.wise.portal.service.teacherpresentation.TeacherPresentationItemStrategy; +import org.wise.vle.domain.work.StudentWork; + +/** + * Base abstract strategy providing shared student work validation logic across presentation types. + * + * @param the typed item domain class + */ +public abstract class AbstractTeacherPresentationItemStrategy + implements TeacherPresentationItemStrategy { + + protected StudentWorkDao studentWorkDao; + + public AbstractTeacherPresentationItemStrategy(StudentWorkDao studentWorkDao) { + this.studentWorkDao = studentWorkDao; + } + + /** + * Validates student work IDs against run and period ownership using batch fetching. + * + * @param run the active run + * @param period the active period group + * @param ids the list of student work IDs to validate + * @return set of unique validated student work IDs preserving insertion order + * @throws IllegalArgumentException if an ID is null, unknown, belongs to another run/period, + * or exceeds MAX_ITEMS limit + */ + protected Set validateStudentWorkIds(Run run, Group period, List ids) { + Set uniqueIds = new LinkedHashSet<>(); + if (ids != null) { + uniqueIds.addAll(ids); + } + if (uniqueIds.size() > TeacherPresentationConfigService.MAX_ITEMS) { + throw new IllegalArgumentException("Too many items. The maximum is " + + TeacherPresentationConfigService.MAX_ITEMS); + } + for (Integer id : uniqueIds) { + if (id == null) { + throw new IllegalArgumentException("Invalid studentWorkId"); + } + } + if (uniqueIds.isEmpty()) { + return uniqueIds; + } + List works = studentWorkDao.getStudentWorkListByIds(new ArrayList<>(uniqueIds)); + Map workMap = new HashMap<>(); + for (StudentWork work : works) { + if (work != null && work.getId() != null) { + workMap.put(work.getId(), work); + } + } + for (Integer id : uniqueIds) { + StudentWork work = workMap.get(id); + if (work == null || !work.getRun().getId().equals(run.getId()) + || !work.getPeriod().getId().equals(period.getId())) { + throw new IllegalArgumentException("Unknown studentWorkId: " + id); + } + } + return uniqueIds; + } +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/DiscussionPresentationItemStrategy.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/DiscussionPresentationItemStrategy.java new file mode 100644 index 000000000..ebc5e07e2 --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/DiscussionPresentationItemStrategy.java @@ -0,0 +1,91 @@ +package org.wise.portal.service.teacherpresentation.impl; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; +import org.wise.portal.dao.work.StudentWorkDao; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.portal.service.teacherpresentation.DiscussionItem; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.work.StudentWork; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; + +@Component +public class DiscussionPresentationItemStrategy + extends AbstractTeacherPresentationItemStrategy { + + @Autowired + public DiscussionPresentationItemStrategy(StudentWorkDao studentWorkDao) { + super(studentWorkDao); + } + + @Override + public String getType() { + return TeacherPresentationConfig.TYPE_DISCUSSION; + } + + @Override + public List parseItems(String json, ObjectMapper objectMapper) { + if (json == null || json.isBlank()) { + return Collections.emptyList(); + } + try { + return objectMapper.readValue(json, new TypeReference>() { + }); + } catch (JsonProcessingException e) { + return Collections.emptyList(); + } + } + + @Override + public String serializeItems(List items, ObjectMapper objectMapper) { + try { + return objectMapper.writeValueAsString(items); + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); + } + } + + @Override + public List validateAndNormalize(Run run, Group period, List rawItems) { + List ids = new ArrayList<>(); + if (rawItems != null) { + for (Object obj : rawItems) { + if (obj instanceof DiscussionItem di) { + ids.add(di.studentWorkId()); + } else if (obj instanceof Integer id) { + ids.add(id); + } else if (obj != null) { + throw new IllegalArgumentException("Invalid discussion item type: " + obj.getClass()); + } + } + } + Set uniqueIds = validateStudentWorkIds(run, period, ids); + List normalized = new ArrayList<>(); + for (Integer id : uniqueIds) { + normalized.add(new DiscussionItem(id)); + } + return normalized; + } + + @Override + public List extractStudentWorkIds(List items) { + List ids = new ArrayList<>(); + if (items != null) { + for (DiscussionItem item : items) { + if (item != null && item.studentWorkId() != null) { + ids.add(item.studentWorkId()); + } + } + } + return ids; + } +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/IdeasPresentationItemStrategy.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/IdeasPresentationItemStrategy.java new file mode 100644 index 000000000..7fa6e07f4 --- /dev/null +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/IdeasPresentationItemStrategy.java @@ -0,0 +1,141 @@ +package org.wise.portal.service.teacherpresentation.impl; + +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; +import org.wise.portal.dao.work.StudentWorkDao; +import org.wise.portal.domain.group.Group; +import org.wise.portal.domain.run.Run; +import org.wise.portal.service.teacherpresentation.IdeasItem; +import org.wise.portal.service.teacherpresentation.IdeasResponse; +import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.work.StudentWork; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; + +@Component +public class IdeasPresentationItemStrategy + extends AbstractTeacherPresentationItemStrategy { + + private static final int MAX_RESPONSE_TEXT_LENGTH = 2000; + // items column is a MySQL text column (65535 bytes) + private static final int MAX_SERIALIZED_ITEMS_LENGTH = 60000; + + @Autowired + public IdeasPresentationItemStrategy(StudentWorkDao studentWorkDao) { + super(studentWorkDao); + } + + @Override + public String getType() { + return TeacherPresentationConfig.TYPE_IDEAS; + } + + @Override + public List parseItems(String json, ObjectMapper objectMapper) { + if (json == null || json.isBlank()) { + return Collections.emptyList(); + } + try { + return objectMapper.readValue(json, new TypeReference>() { + }); + } catch (JsonProcessingException e) { + return Collections.emptyList(); + } + } + + @Override + public String serializeItems(List items, ObjectMapper objectMapper) { + try { + String serialized = objectMapper.writeValueAsString(items); + if (serialized.getBytes(StandardCharsets.UTF_8).length > MAX_SERIALIZED_ITEMS_LENGTH) { + throw new IllegalArgumentException("Too many selected responses"); + } + return serialized; + } catch (JsonProcessingException e) { + throw new IllegalStateException(e); + } + } + + @Override + public List validateAndNormalize(Run run, Group period, List rawItems) { + if (rawItems == null) { + return Collections.emptyList(); + } + if (rawItems.size() > TeacherPresentationConfigService.MAX_ITEMS) { + throw new IllegalArgumentException("Too many items. The maximum is " + + TeacherPresentationConfigService.MAX_ITEMS); + } + List normalized = new ArrayList<>(); + List allStudentWorkIds = new ArrayList<>(); + int responseCount = 0; + for (Object obj : rawItems) { + if (obj instanceof IdeasItem ii) { + String ideaId = ii.ideaId(); + if (ideaId != null) { + ideaId = ideaId.trim(); + if (ideaId.isEmpty() || ideaId.length() > 100) { + throw new IllegalArgumentException("Invalid ideaId: " + ideaId); + } + } + if (ideaId == null && ii.responses().isEmpty()) { + throw new IllegalArgumentException("Item must specify ideaId or responses"); + } + List responses = new ArrayList<>(); + for (IdeasResponse response : ii.responses()) { + responses.add(normalizeIdeasResponse(response)); + allStudentWorkIds.add(response.studentWorkId()); + } + responseCount += responses.size(); + normalized.add(new IdeasItem(ideaId, responses)); + } else if (obj != null) { + throw new IllegalArgumentException("Invalid ideas item type: " + obj.getClass()); + } + } + if (responseCount > TeacherPresentationConfigService.MAX_ITEMS) { + throw new IllegalArgumentException("Too many responses. The maximum is " + + TeacherPresentationConfigService.MAX_ITEMS); + } + validateStudentWorkIds(run, period, allStudentWorkIds); + return normalized; + } + + @Override + public List extractStudentWorkIds(List items) { + List ids = new ArrayList<>(); + if (items != null) { + for (IdeasItem ii : items) { + if (ii != null && ii.responses() != null) { + for (IdeasResponse response : ii.responses()) { + if (response != null && response.studentWorkId() != null) { + ids.add(response.studentWorkId()); + } + } + } + } + } + return ids; + } + + private IdeasResponse normalizeIdeasResponse(IdeasResponse response) { + if (response == null || response.studentWorkId() == null) { + throw new IllegalArgumentException("Invalid studentWorkId"); + } + if (response.workgroupId() == null) { + throw new IllegalArgumentException("Invalid workgroupId"); + } + String text = response.text() != null ? response.text() : ""; + if (text.length() > MAX_RESPONSE_TEXT_LENGTH) { + text = text.substring(0, MAX_RESPONSE_TEXT_LENGTH); + } + return new IdeasResponse(response.studentWorkId(), response.workgroupId(), + response.responseTimestamp(), text); + } +} diff --git a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java index 90804e83a..085e20ab9 100644 --- a/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java +++ b/src/main/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImpl.java @@ -1,10 +1,8 @@ package org.wise.portal.service.teacherpresentation.impl; import java.sql.Timestamp; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -17,30 +15,20 @@ import org.wise.portal.domain.group.Group; import org.wise.portal.domain.run.Run; import org.wise.portal.domain.workgroup.Workgroup; -import org.wise.portal.service.teacherpresentation.DiscussionItem; -import org.wise.portal.service.teacherpresentation.IdeasItem; -import org.wise.portal.service.teacherpresentation.IdeasResponse; import org.wise.portal.service.teacherpresentation.TeacherPresentationConfigService; +import org.wise.portal.service.teacherpresentation.TeacherPresentationItemStrategy; import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; import org.wise.vle.domain.work.StudentWork; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; @Service public class TeacherPresentationConfigServiceImpl implements TeacherPresentationConfigService { private static final int MAX_ID_LENGTH = 30; - private static final int MAX_RESPONSE_TEXT_LENGTH = 2000; - // items column is a MySQL text column (65535 bytes) - private static final int MAX_SERIALIZED_ITEMS_LENGTH = 60000; private static final Set NAMES_DISPLAY_OPTIONS = Set.of( TeacherPresentationConfig.NAMES_SHOW, TeacherPresentationConfig.NAMES_HIDE, TeacherPresentationConfig.NAMES_ANONYMIZE); - private static final Set TYPE_OPTIONS = Set.of( - TeacherPresentationConfig.TYPE_DISCUSSION, - TeacherPresentationConfig.TYPE_IDEAS); private final ObjectMapper objectMapper = new ObjectMapper(); @@ -50,6 +38,18 @@ public class TeacherPresentationConfigServiceImpl implements TeacherPresentation @Autowired private StudentWorkDao studentWorkDao; + private final Map> strategies = new HashMap<>(); + + @Autowired + public void setStrategies(List> strategyList) { + this.strategies.clear(); + if (strategyList != null) { + for (TeacherPresentationItemStrategy strategy : strategyList) { + this.strategies.put(strategy.getType(), strategy); + } + } + } + @Override @Transactional(readOnly = true) public TeacherPresentationConfig getConfig(Run run, Group period, String nodeId, @@ -62,40 +62,25 @@ public List getItems(TeacherPresentationConfig config) { if (config == null || config.getItems() == null) { return Collections.emptyList(); } - String type = config.getType(); - try { - if (TeacherPresentationConfig.TYPE_IDEAS.equals(type)) { - return objectMapper.readValue(config.getItems(), - new TypeReference>() { - }); - } else { - return objectMapper.readValue(config.getItems(), - new TypeReference>() { - }); - } - } catch (JsonProcessingException e) { + String resolvedType = resolveType(config.getType()); + TeacherPresentationItemStrategy strategy = getStrategy(resolvedType); + if (strategy == null) { return Collections.emptyList(); } + return strategy.parseItems(config.getItems(), objectMapper); } @Override public List getStudentWorkIds(TeacherPresentationConfig config) { - List items = getItems(config); - List ids = new ArrayList<>(); - for (Object item : items) { - if (item instanceof DiscussionItem di) { - if (di.studentWorkId() != null) { - ids.add(di.studentWorkId()); - } - } else if (item instanceof IdeasItem ii) { - for (IdeasResponse response : ii.responses()) { - if (response.studentWorkId() != null) { - ids.add(response.studentWorkId()); - } - } - } + if (config == null || config.getItems() == null) { + return Collections.emptyList(); + } + String resolvedType = resolveType(config.getType()); + TeacherPresentationItemStrategy strategy = getStrategy(resolvedType); + if (strategy == null) { + return Collections.emptyList(); } - return ids; + return parseAndExtractStudentWorkIds(strategy, config.getItems()); } @Override @@ -105,8 +90,9 @@ public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId String studentNamesDisplay, Boolean showComponentPrompt, String prompt, Workgroup teacherWorkgroup) { validateComponent(nodeId, componentId, componentType); - String resolvedType = type != null ? type : TeacherPresentationConfig.TYPE_DISCUSSION; - if (!TYPE_OPTIONS.contains(resolvedType)) { + String resolvedType = resolveType(type); + TeacherPresentationItemStrategy strategy = getStrategy(resolvedType); + if (strategy == null) { throw new IllegalArgumentException("Invalid type: " + type); } if (studentNamesDisplay != null && !NAMES_DISPLAY_OPTIONS.contains(studentNamesDisplay)) { @@ -114,18 +100,7 @@ public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId } String normalizedPrompt = normalizePrompt(prompt); - String serializedItems; - if (TeacherPresentationConfig.TYPE_IDEAS.equals(resolvedType)) { - List ideasItems = validateAndNormalizeIdeasItems(run, period, items); - serializedItems = serializeIdeasItems(ideasItems); - if (serializedItems.getBytes(java.nio.charset.StandardCharsets.UTF_8).length - > MAX_SERIALIZED_ITEMS_LENGTH) { - throw new IllegalArgumentException("Too many selected responses"); - } - } else { - List discussionItems = validateAndNormalizeDiscussionItems(run, period, items); - serializedItems = serializeDiscussionItems(discussionItems); - } + String serializedItems = validateAndSerialize(strategy, run, period, items); TeacherPresentationConfig config = getOrCreateConfig(run, period, nodeId, componentId, componentType, resolvedType); @@ -145,6 +120,34 @@ public TeacherPresentationConfig saveConfig(Run run, Group period, String nodeId return config; } + private TeacherPresentationItemStrategy getStrategy(String type) { + TeacherPresentationItemStrategy strategy = strategies.get(type); + if (strategy == null) { + if (TeacherPresentationConfig.TYPE_DISCUSSION.equals(type)) { + return new DiscussionPresentationItemStrategy(studentWorkDao); + } else if (TeacherPresentationConfig.TYPE_IDEAS.equals(type)) { + return new IdeasPresentationItemStrategy(studentWorkDao); + } + } + return strategy; + } + + private String validateAndSerialize(TeacherPresentationItemStrategy strategy, Run run, + Group period, List items) { + List normalized = strategy.validateAndNormalize(run, period, items); + return strategy.serializeItems(normalized, objectMapper); + } + + private List parseAndExtractStudentWorkIds( + TeacherPresentationItemStrategy strategy, String itemsJson) { + List items = strategy.parseItems(itemsJson, objectMapper); + return strategy.extractStudentWorkIds(items); + } + + private String resolveType(String type) { + return type != null ? type : TeacherPresentationConfig.TYPE_DISCUSSION; + } + private TeacherPresentationConfig getOrCreateConfig(Run run, Group period, String nodeId, String componentId, String componentType, String type) { TeacherPresentationConfig config = teacherPresentationConfigDao.getConfig(run, period, nodeId, @@ -186,133 +189,4 @@ private String normalizePrompt(String prompt) { } return prompt; } - - private List validateAndNormalizeDiscussionItems(Run run, Group period, List items) { - List ids = new ArrayList<>(); - if (items != null) { - for (Object obj : items) { - if (obj instanceof DiscussionItem di) { - ids.add(di.studentWorkId()); - } else if (obj instanceof Integer id) { - ids.add(id); - } else if (obj != null) { - throw new IllegalArgumentException("Invalid discussion item type: " + obj.getClass()); - } - } - } - Set uniqueIds = validateStudentWorkIds(run, period, ids); - List normalized = new ArrayList<>(); - for (Integer id : uniqueIds) { - normalized.add(new DiscussionItem(id)); - } - return normalized; - } - - private List validateAndNormalizeIdeasItems(Run run, Group period, List items) { - if (items == null) { - return Collections.emptyList(); - } - if (items.size() > MAX_ITEMS) { - throw new IllegalArgumentException("Too many items. The maximum is " + MAX_ITEMS); - } - List normalized = new ArrayList<>(); - List allStudentWorkIds = new ArrayList<>(); - int responseCount = 0; - for (Object obj : items) { - if (obj instanceof IdeasItem ii) { - String ideaId = ii.ideaId(); - if (ideaId != null) { - ideaId = ideaId.trim(); - if (ideaId.isEmpty() || ideaId.length() > 100) { - throw new IllegalArgumentException("Invalid ideaId: " + ideaId); - } - } - if (ideaId == null && ii.responses().isEmpty()) { - throw new IllegalArgumentException("Item must specify ideaId or responses"); - } - List responses = new ArrayList<>(); - for (IdeasResponse response : ii.responses()) { - responses.add(normalizeIdeasResponse(response)); - allStudentWorkIds.add(response.studentWorkId()); - } - responseCount += responses.size(); - normalized.add(new IdeasItem(ideaId, responses)); - } else if (obj != null) { - throw new IllegalArgumentException("Invalid ideas item type: " + obj.getClass()); - } - } - if (responseCount > MAX_ITEMS) { - throw new IllegalArgumentException("Too many responses. The maximum is " + MAX_ITEMS); - } - validateStudentWorkIds(run, period, allStudentWorkIds); - return normalized; - } - - private IdeasResponse normalizeIdeasResponse(IdeasResponse response) { - if (response == null || response.studentWorkId() == null) { - throw new IllegalArgumentException("Invalid studentWorkId"); - } - if (response.workgroupId() == null) { - throw new IllegalArgumentException("Invalid workgroupId"); - } - String text = response.text() != null ? response.text() : ""; - if (text.length() > MAX_RESPONSE_TEXT_LENGTH) { - text = text.substring(0, MAX_RESPONSE_TEXT_LENGTH); - } - return new IdeasResponse(response.studentWorkId(), response.workgroupId(), - response.responseTimestamp(), text); - } - - /** - * Each student work must belong to the run and period. It is not required to belong to the - * component, because discussion posts can come from connected components. - */ - private Set validateStudentWorkIds(Run run, Group period, List ids) { - Set uniqueIds = new LinkedHashSet<>(); - if (ids != null) { - uniqueIds.addAll(ids); - } - if (uniqueIds.size() > MAX_ITEMS) { - throw new IllegalArgumentException("Too many items. The maximum is " + MAX_ITEMS); - } - for (Integer id : uniqueIds) { - if (id == null) { - throw new IllegalArgumentException("Invalid studentWorkId"); - } - } - if (uniqueIds.isEmpty()) { - return uniqueIds; - } - List works = studentWorkDao.getStudentWorkListByIds(new ArrayList<>(uniqueIds)); - Map workMap = new HashMap<>(); - for (StudentWork work : works) { - if (work != null && work.getId() != null) { - workMap.put(work.getId(), work); - } - } - for (Integer id : uniqueIds) { - StudentWork work = workMap.get(id); - if (work == null || !work.getRun().getId().equals(run.getId()) - || !work.getPeriod().getId().equals(period.getId())) { - throw new IllegalArgumentException("Unknown studentWorkId: " + id); - } - } - return uniqueIds; - } - - private String serializeDiscussionItems(List items) { - try { - return objectMapper.writeValueAsString(items); - } catch (JsonProcessingException e) { - throw new IllegalStateException(e); - } - } - - private String serializeIdeasItems(List items) { - try { - return objectMapper.writeValueAsString(items); - } catch (JsonProcessingException e) { - throw new IllegalStateException(e); - } - } } diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/DiscussionPresentationItemStrategyTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/DiscussionPresentationItemStrategyTest.java new file mode 100644 index 000000000..d04e3a045 --- /dev/null +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/DiscussionPresentationItemStrategyTest.java @@ -0,0 +1,110 @@ +package org.wise.portal.service.teacherpresentation.impl; + +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Arrays; +import java.util.List; + +import org.easymock.EasyMockExtension; +import org.easymock.Mock; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.wise.portal.dao.work.StudentWorkDao; +import org.wise.portal.domain.group.impl.PersistentGroup; +import org.wise.portal.domain.run.impl.RunImpl; +import org.wise.portal.service.teacherpresentation.DiscussionItem; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.work.StudentWork; + +import com.fasterxml.jackson.databind.ObjectMapper; + +@ExtendWith(EasyMockExtension.class) +public class DiscussionPresentationItemStrategyTest { + + private DiscussionPresentationItemStrategy strategy; + + @Mock + private StudentWorkDao studentWorkDao; + + private final ObjectMapper objectMapper = new ObjectMapper(); + private RunImpl run = new RunImpl(); + private PersistentGroup period = new PersistentGroup(); + + @BeforeEach + public void setUp() { + run.setId(1L); + period.setId(10L); + strategy = new DiscussionPresentationItemStrategy(studentWorkDao); + } + + private StudentWork work(Integer id) { + StudentWork w = new StudentWork(); + w.setId(id); + w.setRun(run); + w.setPeriod(period); + return w; + } + + @Test + public void getType_ReturnsDiscussion() { + assertEquals(TeacherPresentationConfig.TYPE_DISCUSSION, strategy.getType()); + } + + @Test + public void parseItems_ValidJson_ReturnsList() { + String json = "[{\"studentWorkId\":101},{\"studentWorkId\":102}]"; + List items = strategy.parseItems(json, objectMapper); + assertEquals(2, items.size()); + assertEquals(101, items.get(0).studentWorkId()); + assertEquals(102, items.get(1).studentWorkId()); + } + + @Test + public void parseItems_NullOrEmptyOrInvalid_ReturnsEmptyList() { + assertTrue(strategy.parseItems(null, objectMapper).isEmpty()); + assertTrue(strategy.parseItems("", objectMapper).isEmpty()); + assertTrue(strategy.parseItems("invalid-json", objectMapper).isEmpty()); + } + + @Test + public void serializeItems_ReturnsJson() { + List items = Arrays.asList(new DiscussionItem(101)); + String json = strategy.serializeItems(items, objectMapper); + assertEquals("[{\"studentWorkId\":101}]", json); + } + + @Test + public void validateAndNormalize_ValidItemsAndIntegerIds_Deduplicates() { + expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(101, 102))) + .andReturn(Arrays.asList(work(101), work(102))); + replay(studentWorkDao); + + List rawItems = Arrays.asList(new DiscussionItem(101), 102, new DiscussionItem(101)); + List normalized = strategy.validateAndNormalize(run, period, rawItems); + assertEquals(2, normalized.size()); + assertEquals(101, normalized.get(0).studentWorkId()); + assertEquals(102, normalized.get(1).studentWorkId()); + verify(studentWorkDao); + } + + @Test + public void validateAndNormalize_InvalidItemType_ThrowsIllegalArgument() { + replay(studentWorkDao); + assertThrows(IllegalArgumentException.class, + () -> strategy.validateAndNormalize(run, period, Arrays.asList("string-item"))); + } + + @Test + public void extractStudentWorkIds_ReturnsIds() { + List items = Arrays.asList(new DiscussionItem(101), new DiscussionItem(102), + new DiscussionItem(null)); + List ids = strategy.extractStudentWorkIds(items); + assertEquals(Arrays.asList(101, 102), ids); + } +} diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/IdeasPresentationItemStrategyTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/IdeasPresentationItemStrategyTest.java new file mode 100644 index 000000000..1c0a388d0 --- /dev/null +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/IdeasPresentationItemStrategyTest.java @@ -0,0 +1,142 @@ +package org.wise.portal.service.teacherpresentation.impl; + +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import org.easymock.EasyMockExtension; +import org.easymock.Mock; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.wise.portal.dao.work.StudentWorkDao; +import org.wise.portal.domain.group.impl.PersistentGroup; +import org.wise.portal.domain.run.impl.RunImpl; +import org.wise.portal.service.teacherpresentation.IdeasItem; +import org.wise.portal.service.teacherpresentation.IdeasResponse; +import org.wise.vle.domain.teacherpresentation.TeacherPresentationConfig; +import org.wise.vle.domain.work.StudentWork; + +import com.fasterxml.jackson.databind.ObjectMapper; + +@ExtendWith(EasyMockExtension.class) +public class IdeasPresentationItemStrategyTest { + + private IdeasPresentationItemStrategy strategy; + + @Mock + private StudentWorkDao studentWorkDao; + + private final ObjectMapper objectMapper = new ObjectMapper(); + private RunImpl run = new RunImpl(); + private PersistentGroup period = new PersistentGroup(); + + @BeforeEach + public void setUp() { + run.setId(1L); + period.setId(10L); + strategy = new IdeasPresentationItemStrategy(studentWorkDao); + } + + private StudentWork work(Integer id) { + StudentWork w = new StudentWork(); + w.setId(id); + w.setRun(run); + w.setPeriod(period); + return w; + } + + @Test + public void getType_ReturnsIdeas() { + assertEquals(TeacherPresentationConfig.TYPE_IDEAS, strategy.getType()); + } + + @Test + public void parseItems_ValidJson_ReturnsList() { + String json = "[{\"ideaId\":\"idea1\",\"responses\":[{\"studentWorkId\":101,\"workgroupId\":1,\"responseTimestamp\":1000,\"text\":\"test\"}]}]"; + List items = strategy.parseItems(json, objectMapper); + assertEquals(1, items.size()); + assertEquals("idea1", items.get(0).ideaId()); + assertEquals(1, items.get(0).responses().size()); + assertEquals(101, items.get(0).responses().get(0).studentWorkId()); + } + + @Test + public void parseItems_NullOrEmptyOrInvalid_ReturnsEmptyList() { + assertTrue(strategy.parseItems(null, objectMapper).isEmpty()); + assertTrue(strategy.parseItems("", objectMapper).isEmpty()); + assertTrue(strategy.parseItems("invalid-json", objectMapper).isEmpty()); + } + + @Test + public void serializeItems_ExceedsLimit_ThrowsIllegalArgument() { + // 60,000 bytes limit: generate items exceeding 60,000 bytes + List responses = new ArrayList<>(); + for (int i = 0; i < 40; i++) { + responses.add(new IdeasResponse(100 + i, 1L, (long) i, "A".repeat(1800))); + } + List items = Arrays.asList(new IdeasItem("idea1", responses)); + assertThrows(IllegalArgumentException.class, + () -> strategy.serializeItems(items, objectMapper)); + } + + @Test + public void validateAndNormalize_TruncatesLongText() { + expect(studentWorkDao.getStudentWorkListByIds(Arrays.asList(101))) + .andReturn(Arrays.asList(work(101))); + replay(studentWorkDao); + + String longText = "x".repeat(2500); + List rawItems = Arrays + .asList(new IdeasItem("idea1", Arrays.asList(new IdeasResponse(101, 1L, 1000L, longText)))); + List normalized = strategy.validateAndNormalize(run, period, rawItems); + assertEquals(1, normalized.size()); + assertEquals(2000, normalized.get(0).responses().get(0).text().length()); + verify(studentWorkDao); + } + + @Test + public void validateAndNormalize_InvalidIdeaId_ThrowsIllegalArgument() { + replay(studentWorkDao); + List rawItems = Arrays.asList(new IdeasItem(" ", Collections.emptyList())); + assertThrows(IllegalArgumentException.class, + () -> strategy.validateAndNormalize(run, period, rawItems)); + } + + @Test + public void validateAndNormalize_NullIdeaIdAndEmptyResponses_ThrowsIllegalArgument() { + replay(studentWorkDao); + List rawItems = Arrays.asList(new IdeasItem(null, Collections.emptyList())); + assertThrows(IllegalArgumentException.class, + () -> strategy.validateAndNormalize(run, period, rawItems)); + } + + @Test + public void validateAndNormalize_MoreThan500Responses_ThrowsIllegalArgument() { + replay(studentWorkDao); + List responses = new ArrayList<>(); + for (int i = 0; i < 501; i++) { + responses.add(new IdeasResponse(i, 1L, (long) i, "msg")); + } + List rawItems = Arrays.asList(new IdeasItem("idea1", responses)); + assertThrows(IllegalArgumentException.class, + () -> strategy.validateAndNormalize(run, period, rawItems)); + } + + @Test + public void extractStudentWorkIds_ReturnsIds() { + List items = Arrays.asList( + new IdeasItem("idea1", Arrays.asList(new IdeasResponse(101, 1L, null, "a"))), + new IdeasItem(null, Arrays.asList(new IdeasResponse(102, 2L, null, "b")))); + List ids = strategy.extractStudentWorkIds(items); + assertEquals(Arrays.asList(101, 102), ids); + } +} diff --git a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java index bfdbcfaae..4bb8747ba 100644 --- a/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java +++ b/src/test/java/org/wise/portal/service/teacherpresentation/impl/TeacherPresentationConfigServiceImplTest.java @@ -56,6 +56,9 @@ public void setUp() { otherRun.setId(2L); period.setId(10L); otherPeriod.setId(11L); + service.setStrategies(Arrays.asList( + new DiscussionPresentationItemStrategy(studentWorkDao), + new IdeasPresentationItemStrategy(studentWorkDao))); } private StudentWork work(RunImpl workRun, PersistentGroup workPeriod, Integer id) { From 4e0714f6f9e55fa88de6c1846a3144d9ae653195 Mon Sep 17 00:00:00 2001 From: Hiroki Terashima Date: Tue, 20 Oct 2026 11:04:22 -0700 Subject: [PATCH 12/12] refactor: squash teacher presentation migration scripts (V3-V6) into initial table creation schema (V3) --- .../db/migration/V3__create_teacher_presentation_tables.sql | 2 ++ .../V4__add_item_type_to_teacher_presentation_config.sql | 1 - ...add_show_component_prompt_to_teacher_presentation_config.sql | 1 - ..._rename_item_type_to_type_in_teacher_presentation_config.sql | 1 - 4 files changed, 2 insertions(+), 3 deletions(-) delete mode 100644 src/main/resources/db/migration/V4__add_item_type_to_teacher_presentation_config.sql delete mode 100644 src/main/resources/db/migration/V5__add_show_component_prompt_to_teacher_presentation_config.sql delete mode 100644 src/main/resources/db/migration/V6__rename_item_type_to_type_in_teacher_presentation_config.sql diff --git a/src/main/resources/db/migration/V3__create_teacher_presentation_tables.sql b/src/main/resources/db/migration/V3__create_teacher_presentation_tables.sql index 27b743728..23efd8953 100644 --- a/src/main/resources/db/migration/V3__create_teacher_presentation_tables.sql +++ b/src/main/resources/db/migration/V3__create_teacher_presentation_tables.sql @@ -5,8 +5,10 @@ create table teacher_presentation_config ( nodeId varchar(30) not null, componentId varchar(30) not null, componentType varchar(30) not null, + `type` varchar(30) not null, items text not null, studentNamesDisplay varchar(30) not null default 'hide', + showComponentPrompt boolean not null default true, prompt text, updatedByWorkgroupId bigint, createdAt datetime(3) not null, diff --git a/src/main/resources/db/migration/V4__add_item_type_to_teacher_presentation_config.sql b/src/main/resources/db/migration/V4__add_item_type_to_teacher_presentation_config.sql deleted file mode 100644 index 0387c20c8..000000000 --- a/src/main/resources/db/migration/V4__add_item_type_to_teacher_presentation_config.sql +++ /dev/null @@ -1 +0,0 @@ -alter table teacher_presentation_config add column itemType varchar(30) not null after componentType; diff --git a/src/main/resources/db/migration/V5__add_show_component_prompt_to_teacher_presentation_config.sql b/src/main/resources/db/migration/V5__add_show_component_prompt_to_teacher_presentation_config.sql deleted file mode 100644 index 0b2a5ecba..000000000 --- a/src/main/resources/db/migration/V5__add_show_component_prompt_to_teacher_presentation_config.sql +++ /dev/null @@ -1 +0,0 @@ -alter table teacher_presentation_config add column showComponentPrompt boolean not null default true after studentNamesDisplay; diff --git a/src/main/resources/db/migration/V6__rename_item_type_to_type_in_teacher_presentation_config.sql b/src/main/resources/db/migration/V6__rename_item_type_to_type_in_teacher_presentation_config.sql deleted file mode 100644 index 6bfd8fded..000000000 --- a/src/main/resources/db/migration/V6__rename_item_type_to_type_in_teacher_presentation_config.sql +++ /dev/null @@ -1 +0,0 @@ -alter table teacher_presentation_config change column itemType `type` varchar(30) not null;