Merge pull request #5 from jheinzel/quality-of-life

jPlag parser logs, plagiarism folder moved, ChooseFeedbackCommand refactored

README.md

@@ -16,12 +16,11 @@ https://github.com/jheinzel/tutorbot/releases
 
 Tutorbot comes with a range of different features, it can support you by:
 
+* downloading all reviews for a certain exercise
 * downloading (and extracting) all submissions for a certain exercise
 * checking submissions for plagiarism
-* downloading all reviews for a certain exercise
+* choosing reviews to give feedback on based on the amount of feedbacks a student has received
 * sending feedback emails to students
-* choosing reviews to give feedback to
-* saving amount of feedbacks each student has received
 
 ## Configuration
 
@@ -137,6 +136,7 @@ under `/build/libs/tutorbot.jar`. Please note that a JDK with version 11 or high
 tool, this limitation comes from JPlag. 
 
 ## Releasing a new version
+First the version in the `build.gradle.kts` has to be updated. Then the following steps have to be performed:
 Before releasing, check if your changes are merged in the main branch.  
 To release a new version, create and push a `tag` on the main branch with a name like `v*.*`. 
 This can be done with the following commands:

build.gradle.kts

@@ -5,7 +5,7 @@ plugins {
 }
 
 group = "at.fhooe.hagenberg"
-version = "1.4.0"
+version = "1.4.1"
 
 repositories {
     mavenCentral()

src/main/kotlin/at/fhooe/hagenberg/tutorbot/commands/ChooseFeedbackCommand.kt

@@ -1,126 +1,36 @@
 package at.fhooe.hagenberg.tutorbot.commands
 
 import at.fhooe.hagenberg.tutorbot.components.ConfigHandler
-import at.fhooe.hagenberg.tutorbot.components.FeedbackHelper
-import at.fhooe.hagenberg.tutorbot.components.FeedbackHelper.FeedbackCount
-import at.fhooe.hagenberg.tutorbot.components.FeedbackHelper.Review
+import at.fhooe.hagenberg.tutorbot.components.FeedbackChooseLogic
+import at.fhooe.hagenberg.tutorbot.components.FeedbackFileHelper
+import at.fhooe.hagenberg.tutorbot.domain.FeedbackCount
+import at.fhooe.hagenberg.tutorbot.domain.Review
 import at.fhooe.hagenberg.tutorbot.util.exitWithError
 import at.fhooe.hagenberg.tutorbot.util.printlnCyan
 import at.fhooe.hagenberg.tutorbot.util.printlnGreen
 import at.fhooe.hagenberg.tutorbot.util.promptBooleanInput
 import picocli.CommandLine.Command
-import java.io.File
 import java.io.FileNotFoundException
 import java.nio.file.Path
 import javax.inject.Inject
-import kotlin.random.Random
 
 @Command(
     name = "choose-feedback",
     description = ["Choose reviews to give feedback on. Students who have gotten less feedbacks on submissions or reviews have a greater chance of being picked."]
 )
 class ChooseFeedbackCommand @Inject constructor(
     private val configHandler: ConfigHandler,
-    private val feedbackHelper: FeedbackHelper,
-    private val random: Random
+    private val feedbackFileHelper: FeedbackFileHelper,
+    private val feedbackChooseLogic: FeedbackChooseLogic,
+    private val saveFeedbackCommand: SaveFeedbackCommand
 ) : BaseCommand() {
-    /**
-     * Adds the review to the chosenReviews if both participating students are not in the chosenReviews already.
-     * @return true if added
-     */
-    private fun addIfAllowed(review: Review, chosenReviews: MutableCollection<Review>): Boolean =
-        if (chosenReviews.none { r ->
-                r.subStudentNr == review.revStudentNr || r.subStudentNr == review.subStudentNr
-                        || r.revStudentNr == review.revStudentNr || r.revStudentNr == review.subStudentNr
-            }) chosenReviews.add(review) else false
-
-    /**
-     * Tries to pop a Review from the reviews, which either has the student as submitter or reviewer.
-     * @return Review if present in reviews
-     */
-    private fun tryPopReviewForStudent(
-        student: String,
-        reviews: MutableCollection<Review>,
-        asSubmitter: Boolean
-    ): Review? =
-        when (asSubmitter) {
-            true -> reviews.firstOrNull { r -> r.subStudentNr == student }
-            false -> reviews.firstOrNull { r -> r.revStudentNr == student }
-        }?.also { reviews.remove(it) }
-
-    /**
-     * Chooses reviews to be selected for feedback. Students who have gotten the least feedbacks on submissions and reviews are preferred.
-     * Random reviews regardless of their received feedback amount may also be chosen if defined (useful to avoid predictability).
-     */
-    private fun pickReviewsToFeedback(
-        reviews: MutableSet<Review>,
-        feedbackCsv: File,
-        feedbackCount: Int,
-        randomCount: Int
-    ): Set<Review> {
-        val feedbackCountMap = try {
-            feedbackHelper.readFeedbackCountFromCsv(feedbackCsv)
-        } catch (e: FileNotFoundException) {
-            mapOf<String, FeedbackCount>() // If file does not exist, just return empty map
-        } catch (e: Exception) {
-            exitWithError(e.message ?: "Parsing feedback CSV failed.")
-        }
-        val chosenReviews = mutableSetOf<Review>()
-        val canStillPickReviews = { reviews.isNotEmpty() && chosenReviews.size < feedbackCount }
-
-        // 1) Chose random reviews first
-        while (canStillPickReviews() && chosenReviews.size < randomCount) {
-            val review = reviews.random(random).also { reviews.remove(it) }
-            addIfAllowed(review, chosenReviews)
-        }
-
-        if (!canStillPickReviews()) return chosenReviews
-
-        // 2) Choose students who have not gotten any feedback, does not matter if feedback on submission or review
-        val studentsWithNoFeedback = reviews
-            .flatMap { r -> listOf(r.subStudentNr, r.revStudentNr) }
-            .distinct()
-            .filter { s -> s !in feedbackCountMap }
-            .shuffled(random)
-            .toMutableList()
-        while (canStillPickReviews() && studentsWithNoFeedback.isNotEmpty()) {
-            val student = studentsWithNoFeedback.first()
-            studentsWithNoFeedback.remove(student)
-            tryPopReviewForStudent(student, reviews, true)?.also {
-                addIfAllowed(it, chosenReviews)
-            }
-        }
-
-        if (!canStillPickReviews()) return chosenReviews
-
-        // 3) Choose students by ordering amount of feedback received
-        val feedbackOrdering = feedbackCountMap
-            .toList()
-            .filter { pair -> pair.first in reviews.flatMap { r -> listOf(r.subStudentNr, r.revStudentNr) } }
-            .shuffled(random)
-            .sortedBy { pair -> pair.second }
-            .toMutableList()
-        while (canStillPickReviews() && feedbackOrdering.isNotEmpty()) {
-            val picked = feedbackOrdering.first()
-            feedbackOrdering.remove(picked)
-
-            // Try to keep amount of feedbacks on submissions and reviews the same
-            val shouldPickSubmission = picked.second.submission <= picked.second.review
-            tryPopReviewForStudent(picked.first, reviews, shouldPickSubmission)?.also {
-                addIfAllowed(it, chosenReviews)
-            }
-        }
-
-        return chosenReviews
-    }
-
     override fun execute() {
         // Current ex. reviews path
         val baseDir = configHandler.getBaseDir()
         val exerciseSubDir = configHandler.getExerciseSubDir()
         val reviewsDir = configHandler.getReviewsSubDir()
         val sourceDirectory = Path.of(baseDir, exerciseSubDir, reviewsDir)
-        val reviews = feedbackHelper.readAllReviewsFromDir(sourceDirectory.toFile())
+        val reviews = feedbackFileHelper.readAllReviewsFromDir(sourceDirectory.toFile())
         if (reviews.isEmpty()) exitWithError("Reviews folder does not contain any valid files!")
 
         // Get CSV with count of previous feedbacks
@@ -134,15 +44,36 @@ class ChooseFeedbackCommand @Inject constructor(
         if (randomCount < 0 || randomCount > feedbackCount) exitWithError("Random feedback count must be >= 0 and <= feedback count.")
 
         // Pick reviews
+        val feedbackCountMap = try {
+            feedbackFileHelper.readFeedbackCountFromCsv(feedbackCsv)
+        } catch (e: FileNotFoundException) {
+            mapOf<String, FeedbackCount>() // If file does not exist, just return empty map
+        } catch (e: Exception) {
+            exitWithError(e.message ?: "Parsing feedback CSV failed.")
+        }
         val reviewsToFeedback =
-            pickReviewsToFeedback(reviews.toMutableSet(), feedbackCsv, feedbackCount, randomCount)
+            feedbackChooseLogic.pickReviewsToFeedback(feedbackCountMap, reviews, feedbackCount, randomCount)
         val reviewsToMove = reviews - reviewsToFeedback
 
         if (reviewsToFeedback.size != feedbackCount)
             printlnCyan("Could only pick ${reviewsToFeedback.size}/$feedbackCount reviews.")
         else
             printlnGreen("Picked $feedbackCount reviews.")
 
+        moveReviews(sourceDirectory, reviewsToMove)
+        printlnGreen("Finished selecting reviews to feedback.")
+
+        if (promptBooleanInput("Save current feedback selection?")) {
+            saveFeedbackCommand.execute()
+        } else {
+            printlnCyan("Feedback selection was not saved. Please save your selection with save-feedback when you are done.")
+        }
+    }
+
+    private fun moveReviews(
+        sourceDirectory: Path,
+        reviewsToMove: Set<Review>
+    ) {
         val targetDirectory = sourceDirectory.resolve(NOT_SELECTED_DIR)
         if (!targetDirectory.toFile().mkdir()) {
             if (!promptBooleanInput("Target location $targetDirectory already exists, should its contents be overwritten?")) {
@@ -161,8 +92,6 @@ class ChooseFeedbackCommand @Inject constructor(
                 exitWithError(ex.message ?: "Moving ${rev.fileName} failed.")
             }
         }
-
-        printlnGreen("Finished selecting reviews to feedback.")
     }
 
     companion object {

src/main/kotlin/at/fhooe/hagenberg/tutorbot/commands/InstructionsCommand.kt

@@ -5,20 +5,21 @@ import javax.inject.Inject
 
 @Command(
     name = "instructions",
-    description = ["Prints out general instructions on which steps need to be done by the tutor"]
+    description = ["Prints out general instructions on which steps need to be done by the tutor."]
 )
 class InstructionsCommand @Inject constructor() : BaseCommand() {
 
     override fun execute() {
         println("To review a homework, the following steps need to be taken:")
-        println("- Download all the reviews (use the reviews command). May also download submissions in this step if needed.")
+        println("- Download all the reviews (use the reviews command). May also download submissions to skip the next step.")
         println("- Download all the submissions and check for plagiarism (use the download submissions command).")
+        println("- Check the plagiarism report (index.html) and if there are errors parser.log will have more information.")
         println("- Select reviews randomly by using the choose-feedback command or manually making sure everybody gets chosen fairly.")
+        println("- If you have not done it already in the choose-feedback command, save the feedback count using the save-feedback command. This data will be used for choosing reviews next time.")
+        println("- Enter which students you are going to review in the excel sheet.")
         println("- Add your feedback to the selected reviews.")
         println("- Collect general feedback and common mistakes for this homework and write it in a markdown file.")
-        println("- Send emails to students with the reviewed PDFs (use the mail command). May also save the feedback counts in this step.")
-        println("- (Do this only if it was not already done in the mail command!) Save the feedback amount using the save-feedback command. This data will be used for choosing reviews next time.")
-        println("- Upload all reviewed PDfs as well as the markdown file to the file share.")
-        println("- Enter which students you reviewed in the excel sheet.")
+        println("- Send emails to students with the reviewed PDFs (use the mail command).")
+        println("- Upload all reviewed PDFs as well as the markdown file to the file share.")
     }
 }

src/main/kotlin/at/fhooe/hagenberg/tutorbot/commands/MailCommand.kt

@@ -12,11 +12,10 @@ import javax.mail.AuthenticationFailedException
 
 @Command(
     name = "mail",
-    description = ["Sends PDFs containing the feedback via email"]
+    description = ["Sends PDFs containing the feedback via email."]
 )
 class MailCommand @Inject constructor(
     private val mailClient: MailClient,
-    private val saveFeedbackCommand: SaveFeedbackCommand,
     private val credentialStore: CredentialStore,
     private val configHandler: ConfigHandler
 ) : BaseCommand() {
@@ -72,12 +71,6 @@ class MailCommand @Inject constructor(
                 }
             }
         }
-
-        // Prompt for feedback saving also
-        if (promptBooleanInput("Do you also want to save the feedbacks count to CSV?"))
-        {
-            saveFeedbackCommand.execute()
-        }
     }
 
     private fun String.promptTemplateArguments(name: String): String {

src/main/kotlin/at/fhooe/hagenberg/tutorbot/commands/PlagiarismCommand.kt

@@ -1,30 +1,36 @@
 package at.fhooe.hagenberg.tutorbot.commands
 
+import at.fhooe.hagenberg.tutorbot.components.ConfigHandler
 import at.fhooe.hagenberg.tutorbot.components.PlagiarismChecker
 import at.fhooe.hagenberg.tutorbot.util.exitWithError
-import at.fhooe.hagenberg.tutorbot.util.promptTextInput
 import picocli.CommandLine.Command
 import java.io.File
+import java.nio.file.Path
 import javax.inject.Inject
 
 @Command(
     name = "plagiarism",
-    description = ["Checks downloaded submissions for plagiarism"]
+    description = ["Checks downloaded submissions for plagiarism."]
 )
 class PlagiarismCommand @Inject constructor(
-    private val plagiarismChecker: PlagiarismChecker
+    private val plagiarismChecker: PlagiarismChecker,
+    private val configHandler: ConfigHandler
 ) : BaseCommand() {
 
     override fun execute() {
-        val submissionsPath = promptTextInput("Enter submissions location (leave empty for current directory):")
-        val submissionsDirectory = File(submissionsPath)
+        val baseDir = configHandler.getBaseDir()
+        val exerciseSubDir = configHandler.getExerciseSubDir()
+        val submissionsDir = configHandler.getSubmissionsSubDir()
+        // Target is exercise dir not submissions dir so it's easier to find the folder
+        val targetDirectory = File(baseDir, exerciseSubDir)
+        val submissionsDirectory = Path.of(baseDir, exerciseSubDir, submissionsDir).toFile()
 
         // Make sure the submissions directory exists
         if (!submissionsDirectory.isDirectory) {
-            exitWithError("Submissions directory $submissionsPath does not point to a valid directory.")
+            exitWithError("Submissions directory '${submissionsDirectory.absolutePath}' does not point to a valid directory.")
         }
 
         // Check the results for plagiarism
-        plagiarismChecker.generatePlagiarismReport(submissionsDirectory)
+        plagiarismChecker.generatePlagiarismReport(submissionsDirectory, targetDirectory)
     }
 }

src/main/kotlin/at/fhooe/hagenberg/tutorbot/commands/ReviewsCommand.kt

@@ -14,7 +14,7 @@ import javax.inject.Inject
 
 @Command(
     name = "reviews",
-    description = ["Downloads all reviews for a certain exercise"]
+    description = ["Downloads all reviews for a certain exercise. Optionally also downloads the submissions and performs a plagiarism check."]
 )
 class ReviewsCommand @Inject constructor(
     private val moodleClient: MoodleClient,

src/main/kotlin/at/fhooe/hagenberg/tutorbot/commands/SaveFeedbackCommand.kt

@@ -1,8 +1,8 @@
 package at.fhooe.hagenberg.tutorbot.commands
 
 import at.fhooe.hagenberg.tutorbot.components.ConfigHandler
-import at.fhooe.hagenberg.tutorbot.components.FeedbackHelper
-import at.fhooe.hagenberg.tutorbot.components.FeedbackHelper.FeedbackCount
+import at.fhooe.hagenberg.tutorbot.components.FeedbackFileHelper
+import at.fhooe.hagenberg.tutorbot.domain.FeedbackCount
 import at.fhooe.hagenberg.tutorbot.util.exitWithError
 import at.fhooe.hagenberg.tutorbot.util.printlnGreen
 import picocli.CommandLine.Command
@@ -12,26 +12,26 @@ import javax.inject.Inject
 
 @Command(
     name = "save-feedback",
-    description = ["Counts the feedbacks of an exercise and appends the data to the feedback CSV file"]
+    description = ["Should only be run if the feedback count was not saved with choose-feedback. Counts the feedbacks in an exercise folder and updates the feedback CSV file."]
 )
 class SaveFeedbackCommand @Inject constructor(
     private val configHandler: ConfigHandler,
-    private val feedbackHelper: FeedbackHelper
+    private val feedbackFileHelper: FeedbackFileHelper
 ) : BaseCommand() {
     override fun execute() {
         // Current ex. reviews path
         val baseDir = configHandler.getBaseDir()
         val exerciseSubDir = configHandler.getExerciseSubDir()
         val reviewsDir = configHandler.getReviewsSubDir()
         val sourceDirectory = Path.of(baseDir, exerciseSubDir, reviewsDir)
-        val feedbackCount = feedbackHelper.readFeedbackCountFromReviews(sourceDirectory.toFile())
+        val feedbackCount = feedbackFileHelper.readFeedbackCountFromReviews(sourceDirectory.toFile())
         if (feedbackCount.isEmpty()) exitWithError("Reviews folder does not contain any valid files!")
 
         // Get CSV with count of previous feedbacks
         val feedbackDirPath = configHandler.getFeedbackCsv()
         val feedbackCsv = Path.of(feedbackDirPath).toFile()
         val existingFeedbackCount = try {
-            feedbackHelper.readFeedbackCountFromCsv(feedbackCsv)
+            feedbackFileHelper.readFeedbackCountFromCsv(feedbackCsv)
         } catch (e: FileNotFoundException) {
             mapOf<String, FeedbackCount>() // If file does not exist, just return empty map
         } catch (e: Exception) {
@@ -50,7 +50,7 @@ class SaveFeedbackCommand @Inject constructor(
             }
 
         try {
-            feedbackHelper.writeFeedbackCountToCsv(feedbackCsv, mergedCount)
+            feedbackFileHelper.writeFeedbackCountToCsv(feedbackCsv, mergedCount)
             printlnGreen("Successfully wrote new feedback count to ${feedbackCsv.absolutePath}")
         } catch (e: Exception){
             exitWithError(e.message ?: "Could not write to ${feedbackCsv.absolutePath}")