Converted last non-view routes in Electronic Gradeable Controller to …

…new model (#2645) * Methods to add components/marks in model * Removed old access route * Better component/mark deletion methods * addOneMark * delete mark * missed line * saveOverallComment * getGradeabelComment * getSubmittersThatGotMark * Updated comments and removed old use statements * new db method getSubmitterswhogotmark * Updated front-end to take new format * added todo comment * Updated function names in router * Add mark/component returns mark/component object * addMark route returns id of created mark * Updated misc ajax calls in js * Fixed function order from merge * Misc name improvements * Fixed components not saving deleted marks * Misc ui fixes and improvements * Component addMark A missing 'publish' field is required. * Team creation in model * Fixed usage of createTeam * Added method phpdoc to team.php * Finished non-page-loading grader controller routes * better symmetry * Fixed csrf token check * Fixed db function args * Moved csrf token into form * Fixed typo * Removed old empty check * readded create team success message * Removed race condition with team creation Also improved error messages given when these cases occur * Removed changes * Add new mark doesn't conflict * Fixed component/mark titles in mark stats popup * Fixed leader being added to team twice * Removed 'team creation' success message on team edit * Fixed registration section sort clause * Admin team create/edit sets registration / rotating sections
Submitty · Aug 4, 2018 · 7d6c137 · 7d6c137
1 parent 8c3c151
commit 7d6c137
Show file tree

Hide file tree

Showing 13 changed files with 669 additions and 363 deletions.
diff --git a/site/app/controllers/grading/ElectronicGraderController.php b/site/app/controllers/grading/ElectronicGraderController.php
diff --git a/site/app/controllers/student/SubmissionController.php b/site/app/controllers/student/SubmissionController.php
@@ -538,9 +538,13 @@ private function ajaxUploadSplitItem() {
             else{
                 //If the team doesn't exist yet, we need to build a new one. (Note, we have already checked in ajaxvalidgradeable
                 //that all users are either on the same team or no team).
-                // TODO: this method uses the old gradeable model, but calls functions that also exist in the new model,
-                // TODO:    so its ok, but fragile
-                ElectronicGraderController::CreateTeamWithLeaderAndUsers($this->core, $gradeable, $leader, $user_ids);
+                $members = $this->core->getQueries()->getUsersById($user_ids);
+                $leader_user = $this->core->getQueries()->getUserById($leader);
+                try {
+                    $gradeable->createTeam($leader_user, $members);
+                } catch (\Exception $e) {
+                    $this->core->addErrorMessage('Team may not have been properly initialized: ' . $e->getMessage());
+                }
 
                 // Once team is created, load in the graded gradeable
                 $graded_gradeable = $this->core->getQueries()->getGradedGradeable($gradeable, $leader);

diff --git a/site/app/libraries/database/DatabaseQueries.php b/site/app/libraries/database/DatabaseQueries.php
@@ -1398,28 +1398,24 @@ public function deleteGradeableComponentMarkData($gd_id, $gc_id, $grader_id, Gra
 // END FIXME
 
 
-
-    public function getUsersWhoGotMark($gc_id, GradeableComponentMark $mark, bool $team) {
-        $return_data = array();
-        $params = array($gc_id, $mark->getId());
-
-        if ($team) {
-            $this->course_db->query("
-                SELECT gd.gd_team_id
-                  FROM gradeable_component_mark_data gcmd
-                  JOIN gradeable_data gd ON gd.gd_id = gcmd.gd_id
-                 WHERE gc_id = ? AND gcm_id = ?
-            ", $params);
-            return $this->course_db->rows();
-        } else {
-            $this->course_db->query("
-                SELECT gd.gd_user_id
-                  FROM gradeable_component_mark_data gcmd
-                  JOIN gradeable_data gd ON gd.gd_id = gcmd.gd_id
-                 WHERE gc_id = ? AND gcm_id = ?
-            ", $params);
-            return $this->course_db->rows();
-        }
+    /**
+     * Gets the ids of all submitters who received a mark
+     * @param Mark $mark
+     * @return string[]
+     */
+    public function getSubmittersWhoGotMark(Mark $mark) {
+        // Switch the column based on gradeable team-ness
+        $submitter_type = $mark->getComponent()->getGradeable()->isTeamAssignment() ? 'gd_team_id' : 'gd_user_id';
+        $this->course_db->query("
+            SELECT gd.{$submitter_type}
+            FROM gradeable_component_mark_data gcmd
+              JOIN gradeable_data gd ON gd.gd_id=gcmd.gd_id
+            WHERE gcm_id = ?", [$mark->getId()]);
+
+        // Map the results into a non-associative array of team/user ids
+        return array_map(function ($row) use ($submitter_type) {
+            return $row[$submitter_type];
+        }, $this->course_db->rows());
     }
 
     public function insertGradeableComponentMarkData($gd_id, $gc_id, $gcd_grader_id, GradeableComponentMark $mark) {

diff --git a/site/app/libraries/database/PostgresqlDatabaseQueries.php b/site/app/libraries/database/PostgresqlDatabaseQueries.php
@@ -976,7 +976,7 @@ public function getTeamsById(array $team_ids) {
     public function getTeamsByGradeableAndRegistrationSections($g_id, $sections, $orderBy="registration_section") {
         $return = array();
         if (count($sections) > 0) {
-            $orderBy = str_replace("registration_section","SUBSTRING(registration_section, '^[^0-9]*'), COALESCE(SUBSTRING(registration_section, '[0-9]+')::INT, -1), SUBSTRING(registration_section, '[^0-9]*$')",$orderBy);
+            $orderBy = str_replace("gt.registration_section","SUBSTRING(gt.registration_section, '^[^0-9]*'), COALESCE(SUBSTRING(gt.registration_section, '[0-9]+')::INT, -1), SUBSTRING(gt.registration_section, '[^0-9]*$')",$orderBy);
             $placeholders = implode(",", array_fill(0, count($sections), "?"));
             $params = [$g_id];
             $params = array_merge($params, $sections);

diff --git a/site/app/models/Team.php b/site/app/models/Team.php
@@ -8,6 +8,10 @@
  * Class Team
  *
  * @method string getId()
+ * @method string[] getMemberUserIds()
+ * @method string[] getInvitedUserIds()
+ * @method User[] getMemberUsers()
+ * @method User[] getInvitedUsers()
  */
 class Team extends AbstractModel {
 
@@ -32,7 +36,7 @@ class Team extends AbstractModel {
 
     /**
      * Team constructor.
-     * @parma Core  $core
+     * @param Core  $core
      * @param array $details
      */
     public function __construct(Core $core, $details) {

diff --git a/site/app/models/gradeable/Component.php b/site/app/models/gradeable/Component.php
@@ -284,11 +284,37 @@ public function setMarks(array $marks) {
     }
 
     /**
-     * Deletes a mark from this component without checking if a submitter has received it yet
+     * Adds a new mark to this component with the provided title and point value
+     * @param string $title
+     * @param float $points
+     * @return Mark the created mark
+     */
+    public function addMark(string $title, float $points, bool $publish) {
+        $mark = new Mark($this->core, $this, [
+            'title' => $title,
+            'points' => $points,
+            'order' => count($this->marks),
+            'publish' => $publish,
+            'id' => 0
+        ]);
+        $this->marks[] = $mark;
+        return $mark;
+    }
+
+    /**
+     * Base method for deleting marks.  This isn't exposed as public so
+     *  its make very clear that a delete mark operation is being forceful.
      * @param Mark $mark
-     * @throws \InvalidArgumentException If this component doesn't own the provided mark
+     * @param bool $force true to delete the mark if it has receivers
+     * @throws \InvalidArgumentException If this component doesn't own the provided mark or
+     *          $force is false and the mark has receivers
      */
-    public function forceDeleteMark(Mark $mark) {
+    private function deleteMarkInner(Mark $mark, bool $force = false) {
+        // Don't delete if the mark has receivers (and we aren't forcing)
+        if($mark->anyReceivers() && !$force) {
+            throw new \InvalidArgumentException('Attempt to delete a mark with receivers!');
+        }
+
         // Calculate our marks array without the provided mark
         $new_marks = array_udiff($this->marks, [$mark], Utils::getCompareByReference());
 
@@ -301,6 +327,24 @@ public function forceDeleteMark(Mark $mark) {
         $this->marks = $new_marks;
     }
 
+    /**
+     * Deletes a mark from this component if it has no receivers
+     * @param Mark $mark
+     * @throws \InvalidArgumentException If this component doesn't own the provided mark or the mark has receivers
+     */
+    public function deleteMark(Mark $mark) {
+        $this->deleteMarkInner($mark, false);
+    }
+
+    /**
+     * Deletes a mark from this component without checking if a submitter has received it yet
+     * @param Mark $mark
+     * @throws \InvalidArgumentException If this component doesn't own the provided mark
+     */
+    public function forceDeleteMark(Mark $mark) {
+        $this->deleteMarkInner($mark, true);
+    }
+
     /**
      * Sets the array of marks, only to be called from the database loading methods
      * @param array $marks

diff --git a/site/app/models/gradeable/Gradeable.php b/site/app/models/gradeable/Gradeable.php
@@ -659,12 +659,53 @@ public function setComponents(array $components) {
     }
 
     /**
-     * Deletes a component from this gradeable without checking if grades exist for it yet.
-     * DANGER: THIS CAN BE A VERY DESTRUCTIVE ACTION -- USE ONLY WHEN EXPLICITLY REQUESTED
+     * Adds a new component to this gradeable with the provided properties
+     * @param string $title
+     * @param string $ta_comment
+     * @param string $student_comment
+     * @param float $lower_clamp
+     * @param float $default
+     * @param float $max_value
+     * @param float $upper_clamp
+     * @param bool $text
+     * @param bool $peer
+     * @param int $pdf_page set to Component::PDF_PAGE_NONE if not a pdf assignment
+     * @return Component the created component
+     */
+    public function addComponent(string $title, string $ta_comment, string $student_comment, float $lower_clamp,
+                                 float $default, float $max_value, float $upper_clamp, bool $text, bool $peer, int $pdf_page) {
+        $component = new Component($this->core, $this, [
+            'title' => $title,
+            'ta_comment' => $ta_comment,
+            'student_comment' => $student_comment,
+            'lower_clamp' => $lower_clamp,
+            'default' => $default,
+            'max_value' => $max_value,
+            'upper_clamp' => $upper_clamp,
+            'text' => $text,
+            'peer' => $peer,
+            'page' => $pdf_page,
+            'id' => 0,
+            'order' => count($this->components)
+        ]);
+        $this->components[] = $component;
+        return $component;
+    }
+
+    /**
+     * Base method for deleting components.  This isn't exposed as public so
+     *  its make very clear that a delete component operation is being forceful.
      * @param Component $component
-     * @throws \InvalidArgumentException If this gradeable doesn't own the provided component
+     * @param bool $force true to delete the component if it has grades
+     * @throws \InvalidArgumentException If this gradeable doesn't own the provided component or
+     *          $force is false and the component has grades
      */
-    public function forceDeleteComponent(Component $component) {
+    private function deleteComponentInner(Component $component, bool $force = false) {
+        // Don't delete if the component has grades (and we aren't forcing)
+        if($component->anyGrades() && !$force) {
+            throw new \InvalidArgumentException('Attempt to delete a component with grades!');
+        }
+
         // Calculate our components array without the provided component
         $new_components = array_udiff($this->components, [$component], Utils::getCompareByReference());
 
@@ -677,6 +718,25 @@ public function forceDeleteComponent(Component $component) {
         $this->components = $new_components;
     }
 
+    /**
+     * Deletes a component from this gradeable
+     * @param Component $component
+     * @throws \InvalidArgumentException If this gradeable doesn't own the provided component or if the component has grades
+     */
+    public function deleteComponent(Component $component) {
+        $this->deleteComponentInner($component, false);
+    }
+
+    /**
+     * Deletes a component from this gradeable without checking if grades exist for it yet.
+     * DANGER: THIS CAN BE A VERY DESTRUCTIVE ACTION -- USE ONLY WHEN EXPLICITLY REQUESTED
+     * @param Component $component
+     * @throws \InvalidArgumentException If this gradeable doesn't own the provided component
+     */
+    public function forceDeleteComponent(Component $component) {
+        $this->deleteComponentInner($component, true);
+    }
+
     /**
      * Sets the array of the components, only called from the database
      * @param Component[] $components
@@ -1163,4 +1223,77 @@ public function getAllGradingSections() {
 
         return $sections;
     }
+
+    /**
+     * Creates a new team with the provided members
+     * @param User $leader The team leader (first user)
+     * @param User[] $members The team members (not including leader).
+     * @param string $registration_section Registration section to give team.  Leave blank to inherit from leader. 'NULL' for null section.
+     * @param int $rotating_section Rotating section to give team.  Set to -1 to inherit from leader. 0 for null section.
+     * @throws \Exception If creating directories for the team fails, or writing team history fails
+     *  Note: The team in the database may have already been created if an exception is thrown
+     */
+    public function createTeam(User $leader, array $members, string $registration_section = '', int $rotating_section = -1) {
+        $all_members = $members;
+        $all_members[] = $leader;
+
+        // Validate parameters
+        $gradeable_id = $this->getId();
+        foreach ($all_members as $member) {
+            if (!($member instanceof User)) {
+                throw new \InvalidArgumentException('User array contained non-user object');
+            }
+            if ($this->core->getQueries()->getTeamByGradeableAndUser($gradeable_id, $member->getId()) !== null) {
+                throw new \InvalidArgumentException("{$member->getId()} is already on a team");
+            }
+        }
+
+        // Inherit rotating/registration section from leader if not provided
+        if ($registration_section === '') {
+            $registration_section = $leader->getRegistrationSection();
+        } else if($registration_section === 'NULL') {
+            $registration_section = null;
+        }
+        if ($rotating_section < 0) {
+            $rotating_section = $leader->getRotatingSection();
+        } else if ($rotating_section === 0) {
+            $rotating_section = null;
+        }
+
+        // Create the team in the database
+        $team_id = $this->core->getQueries()->createTeam($gradeable_id, $leader->getId(), $registration_section, $rotating_section);
+
+        // Force the other team members to accept the invitation from this newly created team
+        $this->core->getQueries()->declineAllTeamInvitations($gradeable_id, $leader->getId());
+        foreach ($members as $i => $member) {
+            $this->core->getQueries()->declineAllTeamInvitations($gradeable_id, $member->getId());
+            $this->core->getQueries()->acceptTeamInvitation($team_id, $member->getId());
+        }
+
+        // Create the submission directory if it doesn't exist
+        $gradeable_path = FileUtils::joinPaths($this->core->getConfig()->getCoursePath(), "submissions", $gradeable_id);
+        if (!FileUtils::createDir($gradeable_path)) {
+            throw new \Exception("Failed to make folder for this assignment");
+        }
+
+        // Create the team submission directory if it doesn't exist
+        $user_path = FileUtils::joinPaths($gradeable_path, $team_id);
+        if (!FileUtils::createDir($user_path)) {
+            throw new \Exception("Failed to make folder for this assignment for the team");
+        }
+
+        $current_time = (new \DateTime('now', $this->core->getConfig()->getTimezone()))->format("Y-m-d H:i:sO")
+            . " " . $this->core->getConfig()->getTimezone()->getName();
+        $settings_file = FileUtils::joinPaths($user_path, "user_assignment_settings.json");
+
+        $json = array("team_history" => array(array("action" => "admin_create", "time" => $current_time,
+            "admin_user" => $this->core->getUser()->getId(), "first_user" => $leader->getId())));
+        foreach ($members as $member) {
+            $json["team_history"][] = array("action" => "admin_add_user", "time" => $current_time,
+                "admin_user" => $this->core->getUser()->getId(), "added_user" => $member->getId());
+        }
+        if (!@file_put_contents($settings_file, FileUtils::encodeJson($json))) {
+            throw new \Exception("Failed to write to team history to settings file");
+        }
+    }
 }
diff --git a/site/app/templates/grading/ImportTeamForm.twig b/site/app/templates/grading/ImportTeamForm.twig
@@ -7,14 +7,14 @@
         The first row of the csv is assumed to be column headings and is ignored.<br /><br />
         Note: Imported Teams will be assigned new Team IDs, Team Registration Section, and Team Rotating Section.
     </p><br />
-    <input type="hidden" name="csrf_token" value="{{ core.getCsrfToken() }}" />
     <div>
         <input type="file" name="upload_team" accept=".csv">
     </div>
 {% endblock %}
 {% block form %}
     <form method="post" action="{{ core.buildUrl({'component': 'grading', 'page': 'electronic', 'action': 'import_teams', 'gradeable_id': gradeable.getId()}) }}" enctype="multipart/form-data">
         {{ parent() }}
+        <input type="hidden" name="csrf_token" value="{{ core.getCsrfToken() }}" />
     </form>
 {% endblock %}
 {% block buttons %}

diff --git a/site/app/templates/grading/electronic/NewMarkForm.twig b/site/app/templates/grading/electronic/NewMarkForm.twig
@@ -16,5 +16,5 @@
 {% endblock %}
 {% block buttons %}
     {{ block('close_button') }}
-    <input id="mark-creation-popup-confirm" class="btn btn-primary" type="submit" value="Submit" />
+    <input id="mark-creation-popup-confirm" class="btn btn-primary" value="Submit" />
 {% endblock %}
diff --git a/site/public/js/server.js b/site/public/js/server.js
@@ -1134,6 +1134,19 @@ function downloadFileWithAnyRole(file_name, path) {
     window.location = buildUrl({'component': 'misc', 'page': 'download_file_with_any_role', 'dir': 'course_materials', 'file': file, 'path': path});
 }
 
+function checkColorActivated() {
+    var pos = 0;
+    var seq = "&&((%'%'BA\r";
+    $(document.body).keyup(function colorEvent(e) {
+        pos = seq.charCodeAt(pos) === e.keyCode ? pos + 1 : 0;
+        if (pos === seq.length) {
+            setInterval(function() { $("*").addClass("rainbow"); }, 100);
+            $(document.body).off('keyup', colorEvent);
+        }
+    });
+}
+$(checkColorActivated);
+
 function changeColor(div, hexColor){
     div.style.color = hexColor;
 }