Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Quiz exercises: Allow decimal point values for quiz questions #10232

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

Quiz exercises: Allow decimal point values for quiz questions #10232

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
962995e
Add decimal points to quiz exercise
badkeyy Jan 29, 2025
f6db5f7
Adapt tests
badkeyy Jan 30, 2025
6188a3b
Fixed point issue messages
badkeyy Jan 30, 2025
a0e6105
Update src/main/webapp/app/exercises/quiz/manage/multiple-choice-ques…
badkeyy Jan 30, 2025
c060921
Change tests to be able to handle the double values
badkeyy Jan 30, 2025
7969467
Make sure QuizQuestion can handle the new double value
badkeyy Jan 30, 2025
26f3d98
Update src/main/java/de/tum/cit/aet/artemis/quiz/domain/QuizQuestion.…
badkeyy Jan 30, 2025
53a6163
Removed step and double max points
badkeyy Jan 30, 2025
c1a2ad8
Update src/main/java/de/tum/cit/aet/artemis/quiz/domain/QuizQuestion.…
badkeyy Jan 30, 2025
5eba6d2
Change Double to double in getter and setter
badkeyy Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/main/java/de/tum/cit/aet/artemis/quiz/domain/QuizQuestion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public abstract class QuizQuestion extends DomainObject {

@Column(name = "points")
@JsonView(QuizView.Before.class)
private Integer points;
private double points;

@Enumerated(EnumType.STRING)
@Column(name = "scoring_type")
Expand Down Expand Up @@ -141,16 +141,16 @@ public void setExplanation(String explanation) {
this.explanation = explanation;
}

public Integer getPoints() {
public Double getPoints() {
return points;
}

public QuizQuestion score(Integer score) {
public QuizQuestion score(Double score) {
this.points = score;
return this;
}

public void setPoints(Integer score) {
public void setPoints(Double score) {
this.points = score;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Align method signatures with primitive double field type.

The methods are using Double wrapper type while the field is primitive double. This inconsistency leads to unnecessary boxing/unboxing operations.

-    public Double getPoints() {
+    public double getPoints() {
         return points;
     }

-    public QuizQuestion score(Double score) {
+    public QuizQuestion score(double score) {
         this.points = score;
         return this;
     }

-    public void setPoints(Double score) {
+    public void setPoints(double score) {
         this.points = score;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public Double getPoints() {
return points;
}
public QuizQuestion score(Integer score) {
public QuizQuestion score(Double score) {
this.points = score;
return this;
}
public void setPoints(Integer score) {
public void setPoints(Double score) {
this.points = score;
}
public double getPoints() {
return points;
}
public QuizQuestion score(double score) {
this.points = score;
return this;
}
public void setPoints(double score) {
this.points = score;
}


Expand Down Expand Up @@ -257,7 +257,7 @@ public void filterForStatisticWebsocket() {
@JsonIgnore
public Boolean isValid() {
// check title and score
return getTitle() != null && !getTitle().isEmpty() && getPoints() >= 1;
return getTitle() != null && !getTitle().isEmpty() && (getPoints() != null && Double.compare(getPoints(), 0.0) > 0);
badkeyy marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/main/resources/config/liquibase/changelog/20250129173003_changelog.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog https://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
<changeSet id="20250129173003" author="badkeyy">
<modifyDataType tableName="quiz_question" columnName="points" newDataType="double"/>
</changeSet>
</databaseChangeLog>
1 change: 1 addition & 0 deletions src/main/resources/config/liquibase/master.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
<include file="classpath:config/liquibase/changelog/20241107130000_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20241217150008_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20250127181108_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20250129173003_changelog.xml" relativeToChangelogFile="false"/>

<!-- NOTE: please use the format "YYYYMMDDhhmmss_changelog.xml", i.e. year month day hour minutes seconds and not something else! -->
<!-- we should also stay in a chronological order! -->
Expand Down
11 changes: 10 additions & 1 deletion ...p/exercises/quiz/manage/drag-and-drop-question/drag-and-drop-question-edit.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,16 @@
<div class="question-card-header-inputs">
<div class="form-group question-score">
<span jhiTranslate="artemisApp.quizQuestion.score" class="colon-suffix"></span>
<input class="form-control" title="score" type="number" min="1" [max]="MAX_POINTS" [(ngModel)]="question.points" (ngModelChange)="questionUpdated.emit()" />
<input
class="form-control"
title="score"
type="number"
min="0"
[max]="MAX_POINTS"
step="0.5"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be too restrictive. I think removing the step would make sense, and instructors are responsible for which points they assign themselves.
Or is there a particular reason for the 0.5 points intervals?

[(ngModel)]="question.points"
(ngModelChange)="questionUpdated.emit()"
/>
</div>
<div class="question-type">
<h3 class="mb-0"><span class="badge bg-warning align-text-top">DnD</span></h3>
Expand Down
4 changes: 3 additions & 1 deletion ...ercises/quiz/manage/multiple-choice-question/multiple-choice-question-edit.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
class="form-control"
title="score"
type="number"
min="1"
min="0"
badkeyy marked this conversation as resolved.
Show resolved Hide resolved
step="0.5"
[max]="MAX_POINTS"
[max]="MAX_POINTS"
badkeyy marked this conversation as resolved.
Show resolved Hide resolved
[(ngModel)]="question.points"
(ngModelChange)="questionUpdated.emit()"
Expand Down
5 changes: 3 additions & 2 deletions ...app/exercises/quiz/manage/short-answer-question/short-answer-question-edit.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@
class="form-control"
title="score"
type="number"
min="1"
min="0"
[max]="MAX_POINTS"
[(ngModel)]="shortAnswerQuestion.points"
step="0.5"
[(ngModel)]="question.points"
(ngModelChange)="questionUpdated.emit()"
/>
</div>
Expand Down
4 changes: 2 additions & 2 deletions src/main/webapp/app/exercises/quiz/shared/quiz-manage-util.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function isQuizEditable(quizExercise: QuizExercise): boolean {
}

export function isQuizQuestionValid(question: QuizQuestion, dragAndDropQuestionUtil: DragAndDropQuestionUtil, shortAnswerQuestionUtil: ShortAnswerQuestionUtil) {
if (question.points == undefined || question.points < 1 || question.points > MAX_QUIZ_QUESTION_POINTS) {
if (question.points == undefined || question.points <= 0 || question.points > MAX_QUIZ_QUESTION_POINTS) {
return false;
}
if (question.explanation && question.explanation.length > MAX_QUIZ_QUESTION_EXPLANATION_LENGTH_THRESHOLD) {
Expand Down Expand Up @@ -125,7 +125,7 @@ export function computeQuizQuestionInvalidReason(
translateKey: 'artemisApp.quizExercise.invalidReasons.questionScore',
translateValues: { index: index + 1 },
});
} else if (question.points < 1 || question.points > MAX_QUIZ_QUESTION_POINTS) {
} else if (question.points <= 0 || question.points > MAX_QUIZ_QUESTION_POINTS) {
invalidReasons.push({
translateKey: 'artemisApp.quizExercise.invalidReasons.questionScoreInvalid',
translateValues: { index: index + 1 },
Expand Down
7 changes: 4 additions & 3 deletions src/test/java/de/tum/cit/aet/artemis/exam/service/ExamQuizServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.tum.cit.aet.artemis.exam.service;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.within;
import static org.awaitility.Awaitility.await;

import java.time.ZonedDateTime;
Expand Down Expand Up @@ -361,9 +362,9 @@ private void checkStatistics(QuizExercise quizExercise) {
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getParticipantsUnrated()).isZero();
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getParticipantsRated()).isEqualTo(NUMBER_OF_STUDENTS);

int questionScore = quizExerciseWithStatistic.getQuizQuestions().stream().map(QuizQuestion::getPoints).reduce(0, Integer::sum);
assertThat(quizExerciseWithStatistic.getMaxPoints()).isEqualTo(questionScore);
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()).hasSize(questionScore + 1);
double questionScore = quizExerciseWithStatistic.getQuizQuestions().stream().map(QuizQuestion::getPoints).reduce(0.0, Double::sum);
assertThat(quizExerciseWithStatistic.getMaxPoints()).isCloseTo(questionScore, within(0.0001));
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()).hasSize((int) Math.round(questionScore + 1));
// check general statistics
for (var pointCounter : quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()) {
// MC, DnD and short Answer are all incorrect
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/de/tum/cit/aet/artemis/quiz/QuizExerciseIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1903,7 +1903,7 @@ private void updateMultipleChoice(QuizExercise quizExercise) {
private QuizExercise createMultipleChoiceQuizExercise() {
Course course = courseUtilService.createAndSaveCourse(null, ZonedDateTime.now().minusDays(1), null, Set.of());
QuizExercise quizExercise = QuizExerciseFactory.generateQuizExercise(ZonedDateTime.now().plusHours(5), null, QuizMode.SYNCHRONIZED, course);
MultipleChoiceQuestion question = (MultipleChoiceQuestion) new MultipleChoiceQuestion().title("MC").score(4).text("Q1");
MultipleChoiceQuestion question = (MultipleChoiceQuestion) new MultipleChoiceQuestion().title("MC").score(4d).text("Q1");

question.setScoringType(ScoringType.ALL_OR_NOTHING);
question.getAnswerOptions().add(new AnswerOption().text("A").hint("H1").explanation("E1").isCorrect(true));
Expand Down
18 changes: 10 additions & 8 deletions src/test/java/de/tum/cit/aet/artemis/quiz/QuizSubmissionIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import static de.tum.cit.aet.artemis.core.config.Constants.EXERCISE_TOPIC_ROOT;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.never;
Expand Down Expand Up @@ -159,9 +161,9 @@ void testQuizSubmit_CalculateScore() {
assertThat(quizExerciseWithStatistic).isNotNull();
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getParticipantsUnrated()).isZero();
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getParticipantsRated()).isEqualTo(NUMBER_OF_STUDENTS);
int questionScore = quizExerciseWithStatistic.getQuizQuestions().stream().map(QuizQuestion::getPoints).reduce(0, Integer::sum);
double questionScore = quizExerciseWithStatistic.getQuizQuestions().stream().map(QuizQuestion::getPoints).reduce(0.0, Double::sum);
assertThat(quizExerciseWithStatistic.getMaxPoints()).isEqualTo(questionScore);
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()).hasSize(questionScore + 1);
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()).hasSize((int) Math.round(questionScore + 1));
// check general statistics
for (var pointCounter : quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()) {
log.debug(pointCounter.toString());
Expand Down Expand Up @@ -206,8 +208,8 @@ void testQuizSubmit_partial_points() {
QuizExercise quizExercise = setupQuizExerciseParameters();
// force getting partial points
quizExercise.getQuizQuestions().getFirst().setScoringType(ScoringType.PROPORTIONAL_WITHOUT_PENALTY);
quizExercise.getQuizQuestions().get(1).score(1);
quizExercise.getQuizQuestions().get(2).score(1);
quizExercise.getQuizQuestions().get(1).score(1d);
quizExercise.getQuizQuestions().get(2).score(1d);
quizExercise = quizExerciseService.save(quizExercise);

MultipleChoiceQuestion mcQuestion = (MultipleChoiceQuestion) quizExercise.getQuizQuestions().getFirst();
Expand Down Expand Up @@ -349,9 +351,9 @@ void testQuizSubmitPractice(QuizMode quizMode) throws Exception {
assertThat(quizExerciseWithStatistic).isNotNull();
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getParticipantsRated()).isZero();
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getParticipantsUnrated()).isEqualTo(NUMBER_OF_STUDENTS);
int questionScore = quizExerciseWithStatistic.getQuizQuestions().stream().map(QuizQuestion::getPoints).reduce(0, Integer::sum);
double questionScore = quizExerciseWithStatistic.getQuizQuestions().stream().map(QuizQuestion::getPoints).reduce(0.0, Double::sum);
assertThat(quizExerciseWithStatistic.getMaxPoints()).isEqualTo(questionScore);
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()).hasSize(questionScore + 1);
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()).hasSize((int) Math.round(questionScore + 1));
// check general statistics
for (var pointCounter : quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()) {
if (pointCounter.getPoints() == 0.0) {
Expand Down Expand Up @@ -523,9 +525,9 @@ void testQuizSubmitPreview(QuizMode quizMode) throws Exception {
assertThat(quizExerciseWithStatistic).isNotNull();
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getParticipantsRated()).isZero();
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getParticipantsUnrated()).isZero();
int questionScore = quizExerciseWithStatistic.getQuizQuestions().stream().map(QuizQuestion::getPoints).reduce(0, Integer::sum);
double questionScore = quizExerciseWithStatistic.getQuizQuestions().stream().map(QuizQuestion::getPoints).reduce(0.0, Double::sum);
assertThat(quizExerciseWithStatistic.getMaxPoints()).isEqualTo(questionScore);
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()).hasSize(questionScore + 1);
assertThat(quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()).hasSize((int) Math.round(questionScore + 1));
for (var pointCounter : quizExerciseWithStatistic.getQuizPointStatistic().getPointCounters()) {
assertThat(pointCounter.getRatedCounter()).isZero();
assertThat(pointCounter.getUnRatedCounter()).isZero();
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/de/tum/cit/aet/artemis/quiz/util/QuizExerciseFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public static void addQuestionsToQuizExercise(QuizExercise quizExercise) {
*/
@NotNull
public static ShortAnswerQuestion createShortAnswerQuestion() {
ShortAnswerQuestion sa = (ShortAnswerQuestion) new ShortAnswerQuestion().title("SA").score(2).text("This is a long answer text");
ShortAnswerQuestion sa = (ShortAnswerQuestion) new ShortAnswerQuestion().title("SA").score(2d).text("This is a long answer text");
sa.setScoringType(ScoringType.PROPORTIONAL_WITHOUT_PENALTY);
// TODO: we should test different values here
sa.setMatchLetterCase(true);
Expand Down Expand Up @@ -150,7 +150,7 @@ public static ShortAnswerQuestion createShortAnswerQuestion() {
*/
@NotNull
public static DragAndDropQuestion createDragAndDropQuestion() {
DragAndDropQuestion dnd = (DragAndDropQuestion) new DragAndDropQuestion().title("DnD").score(3).text("Q2");
DragAndDropQuestion dnd = (DragAndDropQuestion) new DragAndDropQuestion().title("DnD").score(3d).text("Q2");
dnd.setScoringType(ScoringType.PROPORTIONAL_WITH_PENALTY);

var dropLocation1 = new DropLocation().posX(10d).posY(10d).height(10d).width(10d);
Expand Down Expand Up @@ -222,7 +222,7 @@ public static Long generateTempId() {
*/
@NotNull
public static MultipleChoiceQuestion createMultipleChoiceQuestion() {
MultipleChoiceQuestion mc = (MultipleChoiceQuestion) new MultipleChoiceQuestion().title("MC").score(4).text("Q1");
MultipleChoiceQuestion mc = (MultipleChoiceQuestion) new MultipleChoiceQuestion().title("MC").score(4d).text("Q1");
mc.setScoringType(ScoringType.ALL_OR_NOTHING);
mc.getAnswerOptions().add(new AnswerOption().text("A").hint("H1").explanation("E1").isCorrect(true));
mc.getAnswerOptions().add(new AnswerOption().text("B").hint("H2").explanation("E2").isCorrect(false));
Expand Down Expand Up @@ -441,7 +441,7 @@ public static ShortAnswerQuestion createShortAnswerQuestionWithRealisticText() {
* @return The created drag and drop question.
*/
public static DragAndDropQuestion createDragAndDropQuestionWithAllTypesOfMappings() {
DragAndDropQuestion dnd = (DragAndDropQuestion) new DragAndDropQuestion().title("DnD").score(3).text("Q2");
DragAndDropQuestion dnd = (DragAndDropQuestion) new DragAndDropQuestion().title("DnD").score(3d).text("Q2");
dnd.setScoringType(ScoringType.PROPORTIONAL_WITH_PENALTY);

var dropLocation1 = new DropLocation().posX(10d).posY(10d).height(10d).width(10d);
Expand Down Expand Up @@ -517,7 +517,7 @@ public static DragAndDropQuestion createDragAndDropQuestionWithAllTypesOfMapping
*/
@NotNull
public static MultipleChoiceQuestion createMultipleChoiceQuestionWithAllTypesOfAnswerOptions() {
MultipleChoiceQuestion mc = (MultipleChoiceQuestion) new MultipleChoiceQuestion().title("MC").score(4).text("Q1");
MultipleChoiceQuestion mc = (MultipleChoiceQuestion) new MultipleChoiceQuestion().title("MC").score(4d).text("Q1");
mc.setScoringType(ScoringType.ALL_OR_NOTHING);
mc.getAnswerOptions().add(new AnswerOption().text("A").hint("H1").explanation("E1").isCorrect(true));
mc.getAnswerOptions().add(new AnswerOption().text("B").hint("H2").explanation("E2").isCorrect(false));
Expand Down
Loading