Merge pull request #482 from softwareconstruction240/469-config-penal…

…ty-magic-numbers

Add configuration of penalty magic numbers

src/main/java/edu/byu/cs/autograder/score/LateDayCalculator.java

@@ -3,6 +3,7 @@
 import edu.byu.cs.autograder.GradingException;
 import edu.byu.cs.canvas.CanvasException;
 import edu.byu.cs.canvas.CanvasService;
+import edu.byu.cs.dataAccess.ConfigurationDao;
 import edu.byu.cs.dataAccess.DaoService;
 import edu.byu.cs.dataAccess.DataAccessException;
 import edu.byu.cs.model.Phase;
@@ -30,10 +31,6 @@ public class LateDayCalculator {
     private static final Logger LOGGER = Logger.getLogger(LateDayCalculator.class.getName());
 
 
-    /**
-     * The max number of days that the late penalty should be applied for.
-     */
-    private static final int MAX_LATE_DAYS_TO_PENALIZE = 5;
     private Set<LocalDate> publicHolidays;
 
     public LateDayCalculator() {
@@ -54,7 +51,8 @@ public int calculateLateDays(Phase phase, String netId) throws GradingException,
         }
 
         ZonedDateTime handInDate = ScorerHelper.getHandInDateZoned(netId);
-        return Math.min(getNumDaysLate(handInDate, dueDate), MAX_LATE_DAYS_TO_PENALIZE);
+        int maxLateDaysToPenalize = DaoService.getConfigurationDao().getConfiguration(ConfigurationDao.Configuration.MAX_LATE_DAYS_TO_PENALIZE, Integer.class);
+        return Math.min(getNumDaysLate(handInDate, dueDate), maxLateDaysToPenalize);
     }
 
     /**

src/main/java/edu/byu/cs/autograder/score/Scorer.java

@@ -9,6 +9,7 @@
 import edu.byu.cs.canvas.model.CanvasRubricAssessment;
 import edu.byu.cs.canvas.model.CanvasRubricItem;
 import edu.byu.cs.canvas.model.CanvasSubmission;
+import edu.byu.cs.dataAccess.ConfigurationDao;
 import edu.byu.cs.dataAccess.DaoService;
 import edu.byu.cs.dataAccess.DataAccessException;
 import edu.byu.cs.dataAccess.UserDao;
@@ -33,11 +34,18 @@ public class Scorer {
      * The penalty to be applied per day to a late submission.
      * This is out of 1. So putting 0.1 would be a 10% deduction per day
      */
-    private static final float PER_DAY_LATE_PENALTY = 0.1F;
+    private final float PER_DAY_LATE_PENALTY;
     private final GradingContext gradingContext;
 
     public Scorer(GradingContext gradingContext) {
         this.gradingContext = gradingContext;
+        try {
+            ConfigurationDao dao = DaoService.getConfigurationDao();
+            PER_DAY_LATE_PENALTY = dao.getConfiguration(ConfigurationDao.Configuration.PER_DAY_LATE_PENALTY, Float.class);
+        } catch (DataAccessException e) {
+            LOGGER.error("Error while getting Per Day Late Penalty for Scorer.");
+            throw new RuntimeException(e);
+        }
     }
 
     /**
@@ -394,8 +402,13 @@ public Submission generateSubmissionObject(Rubric rubric, CommitVerificationResu
         String headHash = commitVerificationResult.headHash();
         String netId = gradingContext.netId();
 
-        if (numDaysLate > 0)
-            notes += " " + numDaysLate + " days late. -" + (int)(numDaysLate * PER_DAY_LATE_PENALTY * 100) + "%";
+        Integer maxLateDays = DaoService.getConfigurationDao().getConfiguration(ConfigurationDao.Configuration.MAX_LATE_DAYS_TO_PENALIZE, Integer.class);
+
+        if (numDaysLate >= maxLateDays)
+            notes += " Late penalty maxed out at " + numDaysLate + " days late: -";
+        else if (numDaysLate > 0)
+            notes += " " + numDaysLate + " days late: -";
+        notes += (int)(numDaysLate * PER_DAY_LATE_PENALTY * 100) + "% ";
 
         ZonedDateTime handInDate = ScorerHelper.getHandInDateZoned(netId);
         Submission.VerifiedStatus verifiedStatus;

src/main/java/edu/byu/cs/controller/ConfigController.java

@@ -1,9 +1,9 @@
 package edu.byu.cs.controller;
 
-import com.google.gson.Gson;
 import com.google.gson.JsonObject;
 import edu.byu.cs.dataAccess.DataAccessException;
 import edu.byu.cs.model.*;
+import edu.byu.cs.model.request.ConfigPenaltyUpdateRequest;
 import edu.byu.cs.service.ConfigService;
 import edu.byu.cs.util.Serializer;
 import spark.Route;
@@ -111,4 +111,19 @@ public class ConfigController {
         res.status(200);
         return "";
     };
+
+    public static final Route updatePenalties = (req, res) -> {
+        User user = req.session().attribute("user");
+
+        ConfigPenaltyUpdateRequest request = Serializer.deserialize(req.body(), ConfigPenaltyUpdateRequest.class);
+
+        try {
+            ConfigService.processPenaltyUpdates(user, request);
+        } catch (DataAccessException e) {
+            res.status(500);
+            res.body(e.getMessage());
+        }
+
+        return "";
+    };
 }

src/main/java/edu/byu/cs/dataAccess/ConfigurationDao.java

@@ -19,6 +19,11 @@ enum Configuration {
         PHASE4_ASSIGNMENT_NUMBER,
         PHASE5_ASSIGNMENT_NUMBER,
         PHASE6_ASSIGNMENT_NUMBER,
-        COURSE_NUMBER
+        COURSE_NUMBER,
+        MAX_LATE_DAYS_TO_PENALIZE,
+        PER_DAY_LATE_PENALTY,
+        GIT_COMMIT_PENALTY,
+        LINES_PER_COMMIT_REQUIRED,
+        CLOCK_FORGIVENESS_MINUTES
     }
 }

src/main/java/edu/byu/cs/dataAccess/DaoService.java

@@ -66,6 +66,17 @@ public static void initializeMemoryDAOs() {
         DaoService.setSubmissionDao(new SubmissionMemoryDao());
         DaoService.setConfigurationDao(new ConfigurationMemoryDao());
         DaoService.setRepoUpdateDao(new RepoUpdateMemoryDao());
+
+        /* Initialize crucial default values in Config for testing purposes */
+        try {
+            configurationDao.setConfiguration(ConfigurationDao.Configuration.GIT_COMMIT_PENALTY, 0.1f, Float.class);
+            configurationDao.setConfiguration(ConfigurationDao.Configuration.MAX_LATE_DAYS_TO_PENALIZE, 5, Integer.class);
+            configurationDao.setConfiguration(ConfigurationDao.Configuration.PER_DAY_LATE_PENALTY, 0.1f, Float.class);
+            configurationDao.setConfiguration(ConfigurationDao.Configuration.LINES_PER_COMMIT_REQUIRED, 5, Integer.class);
+            configurationDao.setConfiguration(ConfigurationDao.Configuration.CLOCK_FORGIVENESS_MINUTES, 3, Integer.class);
+        } catch (DataAccessException e) {
+            throw new RuntimeException(e);
+        }
     }
 
     public static void initializeSqlDAOs() throws DataAccessException {

src/main/java/edu/byu/cs/dataAccess/sql/ConfigurationSqlDao.java

@@ -50,17 +50,17 @@ public <T> T getConfiguration(Configuration key, Class<T> type) throws DataAcces
             statement.setString(1, key.toString());
             var rs = statement.executeQuery();
             if (rs.next()) {
-                return getValue(rs.getString("value"), type);
+                return getValue(key, rs.getString("value"), type);
             }
             throw new DataAccessException("Configuration not found: " + key);
         } catch (Exception e) {
             throw new DataAccessException("Error getting configuration", e);
         }
     }
 
-    private <T> T getValue(String value, Class<T> type) {
+    private <T> T getValue(Configuration key, String value, Class<T> type) {
         if (value.equals(DEFAULT_VALUE)) {
-            LOGGER.warn("Using default configuration value for key: {}", type);
+            LOGGER.warn("Using default configuration value for key: {} of type {}", key, type);
 
             if (type == String.class) {
                 return type.cast("");
@@ -70,6 +70,8 @@ private <T> T getValue(String value, Class<T> type) {
                 return type.cast(false);
             } else if (type == Instant.class) {
                 return type.cast(Instant.MAX);
+            } else if (type == Float.class) {
+                return type.cast(0f);
             } else {
                 throw new IllegalArgumentException("Unsupported configuration type: " + type);
             }
@@ -83,6 +85,8 @@ private <T> T getValue(String value, Class<T> type) {
             return type.cast(Boolean.parseBoolean(value));
         } else if (type == Instant.class) {
             return type.cast(Instant.parse(value));
+        } else if (type == Float.class) {
+            return type.cast(Float.parseFloat(value));
         } else {
             throw new IllegalArgumentException("Unsupported configuration type: " + type);
         }

src/main/java/edu/byu/cs/model/request/ConfigPenaltyUpdateRequest.java

@@ -0,0 +1,9 @@
+package edu.byu.cs.model.request;
+
+public record ConfigPenaltyUpdateRequest(
+        float perDayLatePenalty,
+        int maxLateDaysPenalized,
+        float gitCommitPenalty,
+        int linesChangedPerCommit,
+        int clockForgivenessMinutes
+) {}

src/main/java/edu/byu/cs/server/Server.java

@@ -117,6 +117,8 @@ public static int setupEndpoints(int port) {
 
                     post("/courseIds", updateCourseIdsPost);
                     get("/courseIds", updateCourseIdsUsingCanvasGet);
+
+                    post("/penalties", updatePenalties);
                 });
             });
         });

src/main/java/edu/byu/cs/service/ConfigService.java

@@ -5,9 +5,11 @@
 import edu.byu.cs.canvas.CanvasIntegrationImpl;
 import edu.byu.cs.canvas.model.CanvasAssignment;
 import edu.byu.cs.dataAccess.ConfigurationDao;
+import edu.byu.cs.dataAccess.ConfigurationDao.Configuration;
 import edu.byu.cs.dataAccess.DaoService;
 import edu.byu.cs.dataAccess.DataAccessException;
 import edu.byu.cs.model.*;
+import edu.byu.cs.model.request.ConfigPenaltyUpdateRequest;
 import edu.byu.cs.util.PhaseUtils;
 import edu.byu.cs.util.Serializer;
 import org.slf4j.Logger;
@@ -64,11 +66,19 @@ private static void addBannerConfig(JsonObject response) throws DataAccessExcept
         response.addProperty("bannerColor", dao.getConfiguration(ConfigurationDao.Configuration.BANNER_COLOR, String.class));
     }
 
+    private static void addPenaltyConfig(JsonObject response) throws DataAccessException {
+        response.addProperty("perDayLatePenalty", dao.getConfiguration(Configuration.PER_DAY_LATE_PENALTY, Float.class));
+        response.addProperty("gitCommitPenalty", dao.getConfiguration(Configuration.GIT_COMMIT_PENALTY, Float.class));
+        response.addProperty("maxLateDaysPenalized", dao.getConfiguration(Configuration.MAX_LATE_DAYS_TO_PENALIZE, Integer.class));
+        response.addProperty("linesChangedPerCommit", dao.getConfiguration(Configuration.LINES_PER_COMMIT_REQUIRED, Integer.class));
+        response.addProperty("clockForgivenessMinutes", dao.getConfiguration(Configuration.CLOCK_FORGIVENESS_MINUTES, Integer.class));
+    }
+
     private static void clearBannerConfig() throws DataAccessException {
-        dao.setConfiguration(ConfigurationDao.Configuration.BANNER_MESSAGE, "", String.class);
-        dao.setConfiguration(ConfigurationDao.Configuration.BANNER_LINK, "", String.class);
-        dao.setConfiguration(ConfigurationDao.Configuration.BANNER_COLOR, "", String.class);
-        dao.setConfiguration(ConfigurationDao.Configuration.BANNER_EXPIRATION, Instant.MAX, Instant.class);
+        dao.setConfiguration(Configuration.BANNER_MESSAGE, "", String.class);
+        dao.setConfiguration(Configuration.BANNER_LINK, "", String.class);
+        dao.setConfiguration(Configuration.BANNER_COLOR, "", String.class);
+        dao.setConfiguration(Configuration.BANNER_EXPIRATION, Instant.MAX, Instant.class);
 
         logAutomaticConfigChange("Banner message has expired");
     }
@@ -79,7 +89,7 @@ public static JsonObject getPublicConfig() throws DataAccessException {
         JsonObject response = new JsonObject();
 
         addBannerConfig(response);
-        response.addProperty("phases", dao.getConfiguration(ConfigurationDao.Configuration.STUDENT_SUBMISSIONS_ENABLED, String.class));
+        response.addProperty("phases", dao.getConfiguration(Configuration.STUDENT_SUBMISSIONS_ENABLED, String.class));
 
         Instant shutdownTimestamp = dao.getConfiguration(ConfigurationDao.Configuration.GRADER_SHUTDOWN_DATE, Instant.class);
         if (shutdownTimestamp.isBefore(Instant.now())) { //shutdown time has passed
@@ -120,8 +130,10 @@ public static JsonObject getPrivateConfig() throws DataAccessException {
         }
 
         JsonObject response = getPublicConfig();
+        addPenaltyConfig(response);
+
         int courseNumber = dao.getConfiguration(
-                ConfigurationDao.Configuration.COURSE_NUMBER,
+                Configuration.COURSE_NUMBER,
                 Integer.class
         );
         response.addProperty("courseNumber", courseNumber);
@@ -131,7 +143,7 @@ public static JsonObject getPrivateConfig() throws DataAccessException {
     }
 
     public static void updateLivePhases(ArrayList phasesArray, User user) throws DataAccessException {
-        dao.setConfiguration(ConfigurationDao.Configuration.STUDENT_SUBMISSIONS_ENABLED, phasesArray, ArrayList.class);
+        dao.setConfiguration(Configuration.STUDENT_SUBMISSIONS_ENABLED, phasesArray, ArrayList.class);
 
         logConfigChange("set the following phases as live: %s".formatted(phasesArray), user.netId());
     }
@@ -228,10 +240,10 @@ public static void updateBannerMessage(User user, String expirationString, Strin
             throw new IllegalArgumentException("Invalid hex color code. Must provide a hex code starting with a # symbol, followed by 6 hex digits");
         }
 
-        dao.setConfiguration(ConfigurationDao.Configuration.BANNER_MESSAGE, message, String.class);
-        dao.setConfiguration(ConfigurationDao.Configuration.BANNER_LINK, link, String.class);
-        dao.setConfiguration(ConfigurationDao.Configuration.BANNER_COLOR, color, String.class);
-        dao.setConfiguration(ConfigurationDao.Configuration.BANNER_EXPIRATION, expirationTimestamp, Instant.class);
+        dao.setConfiguration(Configuration.BANNER_MESSAGE, message, String.class);
+        dao.setConfiguration(Configuration.BANNER_LINK, link, String.class);
+        dao.setConfiguration(Configuration.BANNER_COLOR, color, String.class);
+        dao.setConfiguration(Configuration.BANNER_EXPIRATION, expirationTimestamp, Instant.class);
 
         if (message.isEmpty()) {
             logConfigChange("cleared the banner message", user.netId());
@@ -256,7 +268,7 @@ public static void updateCourseIds(User user, SetCourseIdsRequest setCourseIdsRe
 
         // Course Number
         dao.setConfiguration(
-                ConfigurationDao.Configuration.COURSE_NUMBER,
+                Configuration.COURSE_NUMBER,
                 setCourseIdsRequest.courseNumber(),
                 Integer.class
         );
@@ -300,4 +312,50 @@ public static void updateCourseIdsUsingCanvas(User user) throws CanvasException,
                 user.netId()
         );
     }
+
+    public static void processPenaltyUpdates(User user, ConfigPenaltyUpdateRequest request) throws DataAccessException {
+        validateValidPercentFloat(request.gitCommitPenalty(), "Git Commit Penalty");
+        validateValidPercentFloat(request.perDayLatePenalty(), "Per Day Late Penalty");
+        validateNonNegativeInt(request.clockForgivenessMinutes(), "Clock Forgiveness Minutes");
+        validateNonNegativeInt(request.maxLateDaysPenalized(), "Max Late Days Penalized");
+        validateNonNegativeInt(request.linesChangedPerCommit(), "Lines Changed Per Commit");
+
+        setConfigItem(user, Configuration.GIT_COMMIT_PENALTY, request.gitCommitPenalty(), Float.class);
+        setConfigItem(user, Configuration.PER_DAY_LATE_PENALTY, request.perDayLatePenalty(), Float.class);
+        setConfigItem(user, Configuration.CLOCK_FORGIVENESS_MINUTES, request.clockForgivenessMinutes(), Integer.class);
+        setConfigItem(user, Configuration.MAX_LATE_DAYS_TO_PENALIZE, request.maxLateDaysPenalized(), Integer.class);
+        setConfigItem(user, Configuration.LINES_PER_COMMIT_REQUIRED, request.linesChangedPerCommit(), Integer.class);
+    }
+
+    /**
+     * throws IllegalArgumentException if percent is not valid
+     * @param percent a float that should be between 0 and 1 (inclusive)
+     * @param name the name of the value, only used when throwing the exception
+     */
+    private static void validateValidPercentFloat(float percent, String name) {
+        if ((percent < 0) || (percent > 1)) {
+            throw new IllegalArgumentException(name + " must be 0-1");
+        }
+    }
+
+    /**
+     * throws IllegalArgumentException if value is negative
+     * @param value number we're checking is >= 0
+     * @param name name of the value, used when throwing the exception
+     */
+    private static void validateNonNegativeInt(int value, String name) {
+        if (value < 0) {
+            throw new IllegalArgumentException(name + " must be non-negative");
+        }
+    }
+
+    private static <T> void setConfigItem(User admin, Configuration configKey, T value, Class<T> type) throws DataAccessException {
+        ConfigurationDao dao = DaoService.getConfigurationDao();
+
+        T current = dao.getConfiguration(configKey, type);
+        if (current.equals(value)) return;
+
+        dao.setConfiguration(configKey, value, type);
+        logConfigChange("changed %s to %s".formatted(configKey.name(), value.toString()), admin.netId());
+    }
 }

src/main/java/edu/byu/cs/service/SubmissionService.java

@@ -173,8 +173,7 @@ public static Collection<Submission> getSubmissionsForUser(String netId) throws
     public static void approveSubmission(String adminNetId, ApprovalRequest request) throws GradingException, DataAccessException {
         int penalty = 0;
         if (request.penalize()) {
-            //TODO: Put somewhere better/more configurable
-            penalty = 10;
+            penalty = Math.round((DaoService.getConfigurationDao().getConfiguration(ConfigurationDao.Configuration.GIT_COMMIT_PENALTY, Float.class) * 100));
         }
 
         SubmissionUtils.approveSubmission(request.netId(), request.phase(), adminNetId, penalty);

src/main/java/edu/byu/cs/util/PhaseUtils.java

@@ -188,9 +188,18 @@ public static float extraCreditValue(Phase phase) {
     }
 
     public static CommitVerificationConfig verificationConfig(Phase phase) throws GradingException {
-        int minimumLinesChanged = 5;
-        int penaltyPct = 10;
-        int forgivenessMinutesHead = 3;
+        ConfigurationDao dao = DaoService.getConfigurationDao();
+        int minimumLinesChanged;
+        int penaltyPct;
+        int forgivenessMinutesHead;
+        try {
+            penaltyPct = Math.round(dao.getConfiguration(ConfigurationDao.Configuration.GIT_COMMIT_PENALTY, Float.class) * 100);
+            minimumLinesChanged = dao.getConfiguration(ConfigurationDao.Configuration.LINES_PER_COMMIT_REQUIRED, Integer.class);
+            forgivenessMinutesHead = dao.getConfiguration(ConfigurationDao.Configuration.CLOCK_FORGIVENESS_MINUTES, Integer.class);
+        } catch (DataAccessException e) {
+            throw new GradingException("Error getting git commit config", e);
+        }
+
         return switch (phase) {
             case Phase0, Phase1 -> new CommitVerificationConfig(8, 2, minimumLinesChanged, penaltyPct, forgivenessMinutesHead);
             case Phase3, Phase4, Phase5, Phase6 -> new CommitVerificationConfig(12, 3, minimumLinesChanged, penaltyPct, forgivenessMinutesHead);

src/main/resources/frontend/src/components/config/BannerConfigEditor.vue

@@ -32,10 +32,11 @@ const submitBanner = async () => {
   }
   try {
     await setBanner(bannerMessageToSubmit.value, bannerLinkToSubmit.value, bannerColorToSubmit.value, combinedDateTime)
+    closeEditor()
   } catch (e) {
+    appConfigStore.updateConfig()
     alert("There was a problem in saving the updated banner message:\n" + e)
   }
-  closeEditor()
 }
 </script>
 
@@ -66,8 +67,8 @@ const submitBanner = async () => {
   </div>
 
   <div>
-    <button class="small" @click="submitBanner" :disabled="bannerWillExpire && (bannerExpirationDate.length == 0)">Save</button>
-    <button class="small" @click="clearBannerMessage">Clear</button>
+    <button @click="submitBanner" :disabled="bannerWillExpire && (bannerExpirationDate.length == 0)">Submit</button>
+    <button class="small" @click="clearBannerMessage">Clear Editor</button>
   </div>
 </template>