Index: lams_common/src/java/org/lamsfoundation/lams/qb/QbOption.java =================================================================== diff -u -rfd46695dcabc301f28799564c23314aa53d72dc6 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_common/src/java/org/lamsfoundation/lams/qb/QbOption.java (.../QbOption.java) (revision fd46695dcabc301f28799564c23314aa53d72dc6) +++ lams_common/src/java/org/lamsfoundation/lams/qb/QbOption.java (.../QbOption.java) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -11,6 +11,9 @@ import javax.persistence.ManyToOne; import javax.persistence.Table; +import org.apache.commons.lang.builder.EqualsBuilder; +import org.apache.commons.lang.builder.HashCodeBuilder; + /** * One of possible answers for a question in Question Bank. * @@ -24,7 +27,7 @@ @Id @Column @GeneratedValue(strategy = GenerationType.IDENTITY) - private Long uid; + Long uid; @Column private String name; @@ -36,6 +39,30 @@ @JoinColumn(name = "qb_question_uid") private QbQuestion question; + @Override + public QbOption clone() { + QbOption clone = null; + try { + clone = (QbOption) super.clone(); + clone.question = null; + } catch (CloneNotSupportedException e) { + // it should never happen + e.printStackTrace(); + } + return clone; + } + + @Override + public boolean equals(Object o) { + QbOption other = (QbOption) o; + return new EqualsBuilder().append(this.name, other.name).append(this.correct, other.correct).isEquals(); + } + + @Override + public int hashCode() { + return new HashCodeBuilder().append(this.name).append(this.correct).toHashCode(); + } + public String getName() { return name; } Index: lams_common/src/java/org/lamsfoundation/lams/qb/QbQuestion.java =================================================================== diff -u -rfd46695dcabc301f28799564c23314aa53d72dc6 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_common/src/java/org/lamsfoundation/lams/qb/QbQuestion.java (.../QbQuestion.java) (revision fd46695dcabc301f28799564c23314aa53d72dc6) +++ lams_common/src/java/org/lamsfoundation/lams/qb/QbQuestion.java (.../QbQuestion.java) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -12,11 +12,11 @@ import javax.persistence.GenerationType; import javax.persistence.Id; import javax.persistence.OneToMany; -import javax.persistence.PostLoad; import javax.persistence.Table; -import javax.persistence.Transient; import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.builder.EqualsBuilder; +import org.apache.commons.lang.builder.HashCodeBuilder; /** * A question in Question Bank. @@ -67,45 +67,52 @@ @Column private String feedback; - @OneToMany(mappedBy = "question", fetch = FetchType.EAGER, cascade = CascadeType.ALL) + @OneToMany(mappedBy = "question", fetch = FetchType.EAGER, cascade = CascadeType.ALL, orphanRemoval = true) private List options = new ArrayList<>(); - // question state when it was loaded from DB - @Transient - private QbQuestion initialContent; - - // runs when the question gets loaded from DB - @PostLoad - private void setInitialContent() throws CloneNotSupportedException { - initialContent = this.clone(); - } - // compares if current question state and the state when it was loaded from DB // it detects if question is the same or should another question/version be created - public boolean isModified() { - return !equals(initialContent); + public boolean isModified(QbQuestion modifiedQuestion) { + return !equals(modifiedQuestion); } // checks if important parts of another question are the same as current question's @Override public boolean equals(Object o) { QbQuestion other = (QbQuestion) o; - return other != null && StringUtils.equals(other.name, name) && StringUtils.equals(other.feedback, feedback) - && (mark == null ? other.mark == null : mark.equals(other.mark)); + return new EqualsBuilder().append(name, other.name).append(feedback, other.feedback).append(mark, other.mark) + .append(options.toArray(), other.getOptions().toArray()).isEquals(); } @Override + public int hashCode() { + return new HashCodeBuilder().append(name).append(feedback).append(mark).toHashCode(); + } + + @Override public QbQuestion clone() { QbQuestion clone = null; try { clone = (QbQuestion) super.clone(); - clone.uid = null; - } catch (Exception e) { - // this will never happen + } catch (CloneNotSupportedException e) { + // it should never happen e.printStackTrace(); } + List optionsClone = new ArrayList<>(options.size()); + clone.setOptions(optionsClone); + for (QbOption option : options) { + QbOption optionClone = option.clone(); + optionClone.setQuestion(clone); + optionsClone.add(optionClone); + } return clone; + } + public void clearID() { + this.uid = null; + for (QbOption option : options) { + option.uid = null; + } } public Long getUid() { Index: lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/dao/IMcQueContentDAO.java =================================================================== diff -u -rd9b3433f41c2b25011809144d241d33237d9b9d8 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/dao/IMcQueContentDAO.java (.../IMcQueContentDAO.java) (revision d9b3433f41c2b25011809144d241d33237d9b9d8) +++ lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/dao/IMcQueContentDAO.java (.../IMcQueContentDAO.java) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -93,7 +93,11 @@ void releaseQuestionFromCache(McQueContent question); - // finds next question ID for Question Ban question + // finds next question ID for Question Bank question // it will be moved to a common service in the future public int getMaxQbQuestionId(); + + // finds next version for given question ID for Question Bank question + // it will be moved to a common service in the future + public int getMaxQbQuestionVersion(Integer qbQuestionId); } Index: lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/dao/hibernate/McQueContentDAO.java =================================================================== diff -u -rd9b3433f41c2b25011809144d241d33237d9b9d8 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/dao/hibernate/McQueContentDAO.java (.../McQueContentDAO.java) (revision d9b3433f41c2b25011809144d241d33237d9b9d8) +++ lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/dao/hibernate/McQueContentDAO.java (.../McQueContentDAO.java) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -121,4 +121,13 @@ Integer max = (Integer) result; return max == null ? 1 : max + 1; } + + @Override + public int getMaxQbQuestionVersion(Integer qbQuestionId) { + Object result = this.getSession() + .createQuery("SELECT MAX(version) FROM QbQuestion AS q WHERE q.questionId = :questionId") + .setParameter("questionId", qbQuestionId).uniqueResult(); + Integer max = (Integer) result; + return max == null ? 1 : max + 1; + } } \ No newline at end of file Index: lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/model/McOptsContent.java =================================================================== diff -u -rfd46695dcabc301f28799564c23314aa53d72dc6 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/model/McOptsContent.java (.../McOptsContent.java) (revision fd46695dcabc301f28799564c23314aa53d72dc6) +++ lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/model/McOptsContent.java (.../McOptsContent.java) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -61,7 +61,7 @@ @ManyToOne(optional = false, fetch = FetchType.EAGER, cascade = { CascadeType.DETACH, CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH }) @JoinColumn(name = "qb_option_uid") - private QbOption qbOption = new QbOption(); + private QbOption qbOption; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "mc_que_content_id") Index: lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/model/McQueContent.java =================================================================== diff -u -rd9b3433f41c2b25011809144d241d33237d9b9d8 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/model/McQueContent.java (.../McQueContent.java) (revision d9b3433f41c2b25011809144d241d33237d9b9d8) +++ lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/model/McQueContent.java (.../McQueContent.java) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -67,7 +67,7 @@ @ManyToOne(optional = false, fetch = FetchType.EAGER, cascade = { CascadeType.DETACH, CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REFRESH }) @JoinColumn(name = "qb_question_uid") - private QbQuestion qbQuestion = new QbQuestion(); + private QbQuestion qbQuestion; /** * It stores sha1(question) value that allows us to search for the McQueContentc with the same question Index: lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/service/McService.java =================================================================== diff -u -rfd46695dcabc301f28799564c23314aa53d72dc6 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/service/McService.java (.../McService.java) (revision fd46695dcabc301f28799564c23314aa53d72dc6) +++ lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/service/McService.java (.../McService.java) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -287,27 +287,50 @@ // if it does not exist, create a new one qbQuestion = new QbQuestion(); qbQuestion.setType(QbQuestion.TYPE_MULTIPLE_CHOICE_SINGLE_ANSWER); + } + + QbQuestion qbQuestionClone = qbQuestion.clone(); + + // set question's data to current values + qbQuestionClone.setName(currentQuestionText); + qbQuestionClone.setMark(Integer.valueOf(currentMark)); + qbQuestionClone.setFeedback(currentFeedback); + // if it is a new question, it is always considered modified + // if it is an existing question, it gets checked whether data changed + + List optionDTOs = questionDTO.getOptionDtos(); + boolean isModified = qbQuestion.isModified(qbQuestionClone) + || optionDTOs.size() != qbQuestionClone.getOptions().size(); + if (!isModified) { + for (McOptionDTO optionDTO : optionDTOs) { + String optionText = optionDTO.getCandidateAnswer(); + boolean isCorrectOption = "Correct".equals(optionDTO.getCorrect()); + + //find persisted option if it exists + for (QbOption qbOption : qbQuestionClone.getOptions()) { + if (optionDTO.getQbOptionUid().equals(qbOption.getUid())) { + qbOption.setCorrect(isCorrectOption); + qbOption.setName(optionText); + break; + } + } + } + isModified = qbQuestion.isModified(qbQuestionClone); + } + + if (isModified) { + qbQuestion = qbQuestionClone; + // if vital data changed, create a new question version instead of modifying existing one + int newVersion = mcQueContentDAO.getMaxQbQuestionVersion(qbQuestion.getQuestionId()); + qbQuestion.setVersion(newVersion); } else { // if it exists, detach it so we do not change data on the existing entity, // otherwise it gets saved on transaction end with modified data - mcQueContentDAO.releaseFromCache(qbQuestion); - for (QbOption option : qbQuestion.getOptions()) { + mcQueContentDAO.releaseFromCache(qbQuestionClone); + for (QbOption option : qbQuestionClone.getOptions()) { mcQueContentDAO.releaseFromCache(option); } } - // set question's data to current values - qbQuestion.setName(currentQuestionText); - qbQuestion.setMark(Integer.valueOf(currentMark)); - qbQuestion.setFeedback(currentFeedback); - // if it is a new question, it is always considered modified - // if it is an existing question, it gets checked whether data changed - if (qbQuestion.isModified()) { - // the cloned entity will be attached to session, unlike the previously detached qbQuestion - qbQuestion = qbQuestion.clone(); - // if vital data changed, create a new question instead of modifying existing one - qbQuestion.setQuestionId(mcQueContentDAO.getMaxQbQuestionId()); - qbQuestion.setVersion(1); - } // in case question doesn't exist if (question == null) { @@ -324,10 +347,10 @@ } // persist candidate answers - List optionDTOs = questionDTO.getOptionDtos(); Set oldOptions = question.getMcOptionsContents(); Set newOptions = new HashSet<>(); int displayOrderOption = 1; + Set qbOptionsToRemove = new HashSet<>(qbQuestion.getOptions()); for (McOptionDTO optionDTO : optionDTOs) { Long optionUid = optionDTO.getUid(); @@ -337,17 +360,21 @@ //find persisted option if it exists McOptsContent option = new McOptsContent(); for (McOptsContent oldOption : oldOptions) { - mcQueContentDAO.releaseFromCache(oldOption.getQbOption()); - mcQueContentDAO.releaseFromCache(oldOption.getQbOption().getQuestion()); if (oldOption.getUid().equals(optionUid)) { option = oldOption; + break; } } - for (QbOption qbOption : question.getQbQuestion().getOptions()) { - if (qbOption.getUid().equals(optionDTO.getQbOptionUid())) { - option.setQbOption(qbOption); - break; + if (optionDTO.getQbOptionUid() == null) { + option.setQbOption(new QbOption()); + } else { + for (QbOption qbOption : qbQuestion.getOptions()) { + if (qbOption.getUid().equals(optionDTO.getQbOptionUid())) { + option.setQbOption(qbOption); + qbOptionsToRemove.remove(qbOption); + break; + } } } @@ -361,6 +388,12 @@ } question.setMcOptionsContents(newOptions); + qbQuestion.getOptions().removeAll(qbOptionsToRemove); + qbOptionsToRemove.clear(); + if (isModified) { + qbQuestion.clearID(); + } + // updating the existing question content saveOrUpdateMcQueContent(question); Index: lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/web/controller/McController.java =================================================================== diff -u -rd9b3433f41c2b25011809144d241d33237d9b9d8 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/web/controller/McController.java (.../McController.java) (revision d9b3433f41c2b25011809144d241d33237d9b9d8) +++ lams_tool_lamc/src/java/org/lamsfoundation/lams/tool/mc/web/controller/McController.java (.../McController.java) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -798,6 +798,7 @@ List optionDtos = new LinkedList<>(); for (int i = 0; i < McAppConstants.MAX_OPTION_COUNT; i++) { String optionText = request.getParameter("ca" + i); + Long qbOptionUid = WebUtil.readLongParam(request, "qbOptionUid" + i, true); Long optionUid = WebUtil.readLongParam(request, "caUid" + i, true); String isCorrect = "Incorrect"; @@ -811,6 +812,7 @@ optionDTO.setUid(optionUid); optionDTO.setCandidateAnswer(optionText); optionDTO.setCorrect(isCorrect); + optionDTO.setQbOptionUid(qbOptionUid); optionDtos.add(optionDTO); } } Index: lams_tool_lamc/web/authoring/candidateAnswersList.jsp =================================================================== diff -u -rfd46695dcabc301f28799564c23314aa53d72dc6 -r4b2b48600119f8594fd3c82b58783c28b63324de --- lams_tool_lamc/web/authoring/candidateAnswersList.jsp (.../candidateAnswersList.jsp) (revision fd46695dcabc301f28799564c23314aa53d72dc6) +++ lams_tool_lamc/web/authoring/candidateAnswersList.jsp (.../candidateAnswersList.jsp) (revision 4b2b48600119f8594fd3c82b58783c28b63324de) @@ -39,7 +39,7 @@ - +