Alphanumeric registration section (#2069)

* registration section is now alphanumeric * registration section order is corrected * create migration for sections NOTE: undo-ing this PR, rolling back the migration for sections (from alphabetic to integer will fail if non numeric sections exist.
Submitty · Jun 12, 2018 · 3423da9 · 3423da9
1 parent 33e60f5
commit 3423da9
Show file tree

Hide file tree

Showing 11 changed files with 152 additions and 49 deletions.
diff --git a/.setup/bin/setup_sample_courses.py b/.setup/bin/setup_sample_courses.py
@@ -647,7 +647,7 @@ def create(self):
         print("(tables loaded)...")
         for section in range(1, self.registration_sections+1):
             print("Create section {}".format(section))
-            conn.execute(table.insert(), sections_registration_id=section)
+            conn.execute(table.insert(), sections_registration_id=str(section))
 
         print("Creating rotating sections ", end="")
         table = Table("sections_rotating", metadata, autoload=True)
@@ -671,6 +671,8 @@ def create(self):
             rot_section = user.get_detail(self.code, "rotating_section")
             if rot_section is not None and rot_section > self.rotating_sections:
                 rot_section = None
+            if reg_section is not None:
+                reg_section=str(reg_section)    
             # We already have a row in submitty.users for this user,
             # just need to add a row in courses_users which will put a
             # a row in the course specific DB, and off we go.
@@ -696,7 +698,7 @@ def create(self):
                 for grading_registration_section in grading_registration_sections:
                     conn.execute(reg_table.insert(),
                                  user_id=user.get_detail(self.code, "id"),
-                                 sections_registration_id=grading_registration_section)
+                                 sections_registration_id=str(grading_registration_section))
 
             if user.unix_groups is None:
                 if user.get_detail(self.code, "group") <= 1:
@@ -765,7 +767,7 @@ def create(self):
                     print("Adding team for " + unique_team_id + " in gradeable " + gradeable.id)
                     #adding json data for team history                     
                     teams_registration = select([gradeable_teams_table]).where(
-                        (gradeable_teams_table.c['registration_section'] == reg_section) &
+                        (gradeable_teams_table.c['registration_section'] == str(reg_section)) &
                         (gradeable_teams_table.c['g_id'] == gradeable.id))
                     res = conn.execute(teams_registration)
                     added = False
@@ -790,7 +792,7 @@ def create(self):
                         conn.execute(gradeable_teams_table.insert(),
                                      team_id=unique_team_id,
                                      g_id=gradeable.id,
-                                     registration_section=reg_section,
+                                     registration_section=str(reg_section),
                                      rotation_section=None)
                         conn.execute(teams_table.insert(),
                                      team_id=unique_team_id, 

diff --git a/.setup/update_database.py b/.setup/update_database.py
@@ -36,6 +36,7 @@
 # Remove developer group
 os.system("""PGPASSWORD='{}' psql --host={} --username={} --dbname={} -c 'ALTER TABLE users ADD CONSTRAINT users_user_group_check CHECK ((user_group >= 1) AND (user_group <= 4))'""".format(*variables))
 
+
 # Run migrator tool initial
 create_table = """CREATE TABLE migrations_{} (
       id VARCHAR(100) PRIMARY KEY NOT NULL,
@@ -135,6 +136,12 @@
         os.system("""PGPASSWORD='{}' psql --host={} --username={} --dbname={} -c 'ALTER TABLE ONLY peer_assign DROP CONSTRAINT peer_assign_g_id_fkey'""".format(*variables))
         os.system("""PGPASSWORD='{}' psql --host={} --username={} --dbname={} -c 'ALTER TABLE ONLY peer_assign ADD CONSTRAINT peer_assign_g_id_fkey FOREIGN KEY (g_id) REFERENCES gradeable(g_id) ON UPDATE CASCADE ON DELETE CASCADE'""".format(*variables))
 
+
+        # add a couple missing foreign keys
+        os.system("""PGPASSWORD='{}' psql --host={} --username={} --dbname={} -c 'ALTER TABLE ONLY gradeable_teams ADD CONSTRAINT gradeable_teams_registration_section_fkey FOREIGN KEY (registration_section) REFERENCES sections_registration(sections_registration_id)'""".format(*variables))
+        os.system("""PGPASSWORD='{}' psql --host={} --username={} --dbname={} -c 'ALTER TABLE ONLY gradeable_teams ADD CONSTRAINT gradeable_teams_rotating_section_fkey FOREIGN KEY (rotating_section) REFERENCES sections_rotating(sections_rotating_id)'""".format(*variables))
+
+
         # add migrations table
         os.system("""PGPASSWORD='{1}' psql --host={2} --username={3} --dbname={4} -c '{0}'""".format(create_table.format('course'), *variables))
         # add user/database

diff --git a/migration/data/course_tables.sql b/migration/data/course_tables.sql
@@ -336,7 +336,7 @@ ALTER SEQUENCE gradeable_data_gd_id_seq OWNED BY gradeable_data.gd_id;
 --
 
 CREATE TABLE grading_registration (
-    sections_registration_id integer NOT NULL,
+    sections_registration_id character varying(255) NOT NULL,
     user_id character varying NOT NULL
 );
 
@@ -406,7 +406,7 @@ CREATE TABLE migrations_course (
 --
 
 CREATE TABLE sections_registration (
-    sections_registration_id integer NOT NULL
+    sections_registration_id character varying(255) NOT NULL
 );
 
 
@@ -443,7 +443,7 @@ CREATE TABLE users (
     user_lastname character varying NOT NULL,
     user_email character varying NOT NULL,
     user_group integer NOT NULL,
-    registration_section integer,
+    registration_section character varying(255),
     rotating_section integer,
     manual_registration boolean DEFAULT false,
     last_updated timestamp(6) with time zone,
@@ -458,7 +458,7 @@ CREATE TABLE users (
 CREATE TABLE gradeable_teams (
     team_id character varying(255) NOT NULL,
     g_id character varying(255) NOT NULL,
-    registration_section integer,
+    registration_section character varying(255),
     rotating_section integer
 );
 
@@ -967,13 +967,21 @@ ALTER TABLE ONLY users
     ADD CONSTRAINT users_registration_section_fkey FOREIGN KEY (registration_section) REFERENCES sections_registration(sections_registration_id);
 
 
+ALTER TABLE ONLY gradeable_teams
+    ADD CONSTRAINT gradeable_teams_registration_section_fkey FOREIGN KEY (registration_section) REFERENCES sections_registration(sections_registration_id);
+
+
+
 --
 -- Name: users_rotating_section_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
 --
 
 ALTER TABLE ONLY users
     ADD CONSTRAINT users_rotating_section_fkey FOREIGN KEY (rotating_section) REFERENCES sections_rotating(sections_rotating_id);
 
+ALTER TABLE ONLY gradeable_teams
+    ADD CONSTRAINT gradeable_teams_rotating_section_fkey FOREIGN KEY (rotating_section) REFERENCES sections_rotating(sections_rotating_id);
+
 --
 -- Name: gradeable_teams_g_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: -
 --

diff --git a/migration/data/submitty_db.sql b/migration/data/submitty_db.sql
@@ -51,9 +51,9 @@ CREATE TABLE courses (
 CREATE TABLE mapped_courses (
     semester character varying(255) NOT NULL,
     course character varying(255) NOT NULL,
-    registration_section integer NOT NULL,
+    registration_section character varying(255) NOT NULL,
     mapped_course character varying(255) NOT NULL,
-    mapped_section integer NOT NULL
+    mapped_section character varying(255) NOT NULL
 );
 
 
@@ -81,7 +81,7 @@ CREATE TABLE courses_users (
     course character varying(255) NOT NULL,
     user_id character varying NOT NULL,
     user_group integer NOT NULL,
-    registration_section integer,
+    registration_section character varying(255),
     manual_registration boolean DEFAULT false,
     CONSTRAINT users_user_group_check CHECK ((user_group >= 1) AND (user_group <= 4))
 );

diff --git a/migration/migrations/course/20180611125006_alphanumeric_sections.py b/migration/migrations/course/20180611125006_alphanumeric_sections.py
@@ -0,0 +1,40 @@
+def up(conn):
+    with conn.cursor() as cursor:
+        # drop foreign key contraints as the foreign keys also store type of columns in their definitions
+        cursor.execute("ALTER TABLE ONLY grading_registration DROP CONSTRAINT grading_registration_sections_registration_id_fkey")
+        cursor.execute("ALTER TABLE ONLY users DROP CONSTRAINT users_registration_section_fkey")
+
+        # typecast from integer to character varying
+        cursor.execute("ALTER TABLE ONLY sections_registration ALTER COLUMN sections_registration_id SET DATA TYPE character varying(255) USING sections_registration_id::varchar(255)")
+        cursor.execute("ALTER TABLE ONLY grading_registration ALTER COLUMN sections_registration_id SET DATA TYPE character varying(255) USING sections_registration_id::varchar(255)")
+        cursor.execute("ALTER TABLE ONLY users ALTER COLUMN registration_section SET DATA TYPE character varying(255) USING registration_section::varchar(255)")
+        cursor.execute("ALTER TABLE ONLY gradeable_teams ALTER COLUMN registration_section SET DATA TYPE character varying(255) USING registration_section::varchar(255)")
+
+        # renable the foreign key contraints as we're done typecasting
+        cursor.execute("ALTER TABLE ONLY grading_registration ADD CONSTRAINT grading_registration_sections_registration_id_fkey FOREIGN KEY (sections_registration_id) REFERENCES sections_registration(sections_registration_id)")
+        cursor.execute("ALTER TABLE ONLY users ADD CONSTRAINT users_registration_section_fkey FOREIGN KEY (registration_section) REFERENCES sections_registration(sections_registration_id)")
+
+        # add missing foreign key contraints
+        cursor.execute("ALTER TABLE ONLY gradeable_teams ADD CONSTRAINT gradeable_teams_registration_section_fkey FOREIGN KEY (registration_section) REFERENCES sections_registration(sections_registration_id)")
+        cursor.execute("ALTER TABLE ONLY gradeable_teams ADD CONSTRAINT gradeable_teams_rotating_section_fkey FOREIGN KEY (rotating_section) REFERENCES sections_rotating(sections_rotating_id)")
+
+
+def down(conn):
+    with conn.cursor() as cursor:
+        # drop foreign key contraints while we typecast the columns
+        cursor.execute("ALTER TABLE ONLY gradeable_teams DROP CONSTRAINT gradeable_teams_registration_section_fkey")
+        cursor.execute("ALTER TABLE ONLY gradeable_teams DROP CONSTRAINT gradeable_teams_rotating_section_fkey")
+
+        # disable foreign key contraints while we typecast the columns
+        cursor.execute("ALTER TABLE ONLY grading_registration DROP CONSTRAINT grading_registration_sections_registration_id_fkey")
+        cursor.execute("ALTER TABLE ONLY users DROP CONSTRAINT users_registration_section_fkey")
+
+        # typecast from character varying to integer
+        cursor.execute("ALTER TABLE ONLY sections_registration ALTER COLUMN sections_registration_id SET DATA TYPE integer USING sections_registration_id::integer")
+        cursor.execute("ALTER TABLE ONLY grading_registration ALTER COLUMN sections_registration_id SET DATA TYPE integer USING sections_registration_id::integer")
+        cursor.execute("ALTER TABLE ONLY users ALTER COLUMN registration_section SET DATA TYPE integer USING registration_section::integer")
+        cursor.execute("ALTER TABLE ONLY gradeable_teams ALTER COLUMN registration_section SET DATA TYPE integer USING registration_section::integer")
+
+        # renable the foreign key contraints as we're done typecasting
+        cursor.execute("ALTER TABLE ONLY grading_registration ADD CONSTRAINT grading_registration_sections_registration_id_fkey FOREIGN KEY (sections_registration_id) REFERENCES sections_registration(sections_registration_id)")
+        cursor.execute("ALTER TABLE ONLY users ADD CONSTRAINT users_registration_section_fkey FOREIGN KEY (registration_section) REFERENCES sections_registration(sections_registration_id)")
diff --git a/migration/migrations/master/20180611125006_alphanumeric_sections.py b/migration/migrations/master/20180611125006_alphanumeric_sections.py
@@ -0,0 +1,12 @@
+def up(conn):
+    with conn.cursor() as cursor:
+        cursor.execute('ALTER TABLE ONLY mapped_courses ALTER COLUMN registration_section SET DATA TYPE character varying(255) USING registration_section::varchar(255)')
+        cursor.execute('ALTER TABLE ONLY mapped_courses ALTER COLUMN mapped_section SET DATA TYPE character varying(255) USING mapped_section::varchar(255)')
+        cursor.execute('ALTER TABLE ONLY courses_users ALTER COLUMN registration_section SET DATA TYPE character varying(255) USING registration_section::varchar(255)')
+
+
+def down(conn):
+    with conn.cursor() as cursor:
+        cursor.execute('ALTER TABLE ONLY mapped_courses ALTER COLUMN registration_section SET DATA TYPE integer USING registration_section::integer')
+        cursor.execute('ALTER TABLE ONLY mapped_courses ALTER COLUMN mapped_section SET DATA TYPE integer USING mapped_section::integer')
+        cursor.execute('ALTER TABLE ONLY courses_users ALTER COLUMN registration_section SET DATA TYPE integer USING registration_section::integer')
diff --git a/site/app/controllers/admin/UsersController.php b/site/app/controllers/admin/UsersController.php
@@ -165,7 +165,7 @@ public function updateUser($action='students') {
             $user->setRegistrationSection(null);
         }
         else {
-            $user->setRegistrationSection(intval($_POST['registered_section']));
+            $user->setRegistrationSection($_POST['registered_section']);
         }
 
         if ($_POST['rotating_section'] == "null") {
@@ -180,7 +180,7 @@ public function updateUser($action='students') {
         $user->setInstructorUpdated(true);
         $user->setManualRegistration(isset($_POST['manual_registration']));
         if (isset($_POST['grading_registration_section'])) {
-            $user->setGradingRegistrationSections(array_map("intval", $_POST['grading_registration_section']));
+            $user->setGradingRegistrationSections($_POST['grading_registration_section']);
         }
         else {
             $user->setGradingRegistrationSections(array());
@@ -226,64 +226,74 @@ public function updateRegistrationSections() {
         $students = $this->core->getQueries()->getAllUsers();
         $graders = $this->core->getQueries()->getAllGraders();
         if (isset($_POST['add_reg_section']) && $_POST['add_reg_section'] != "") {
-            $flag=0;
+            $reg_section_exists=false;
             foreach ($reg_sections as $section) {
                 $section = $section['sections_registration_id'];
-                if ($section == intval($_POST['add_reg_section'])){
-                    $flag = 1;
+                if ($section == ($_POST['add_reg_section'])){
+                    $reg_section_exists = true;
                     break;
                 }
             }
-            if ($flag == 1) {
+            if ($reg_section_exists == true) {
                 $this->core->addErrorMessage("Registration Section already present");
                 $_SESSION['request'] = $_POST;
                 $this->core->redirect($return_url);       
             }
             else {
-                $this->core->getQueries()->insertNewRegistrationSection(intval($_POST['add_reg_section']));
+                #validation for registration section
+                if(preg_match("~^[A-Za-z0-9_\-]+$~", $_POST['add_reg_section']) === 1) {
+                    $this->core->getQueries()->insertNewRegistrationSection($_POST['add_reg_section']);
+                    $this->core->addSuccessMessage("Registration section {$_POST['add_reg_section']} added");
+                }
+                else {
+                    $this->core->addErrorMessage("Registration Section entered do not follow specified format");
+                    $_SESSION['request'] = $_POST;
+                    $this->core->redirect($return_url);       
+                }
             }
         }
 
         if (isset($_POST['delete_reg_section']) && $_POST['delete_reg_section'] != "") {
-            $valid_reg_section_flag=0;
+            $valid_reg_section_flag=false;
             foreach ($reg_sections as $section) {
                 $section = $section['sections_registration_id'];
-                if ($section == intval($_POST['delete_reg_section'])){
-                    $valid_reg_section_flag = 1;
+                if ($section == $_POST['delete_reg_section']){
+                    $valid_reg_section_flag = true;
                     break;
                 }
             }
-            if ($valid_reg_section_flag == 0) {
+            if ($valid_reg_section_flag == false) {
                 $this->core->addErrorMessage("Not a valid Registration Section");
                 $_SESSION['request'] = $_POST;
                 $this->core->redirect($return_url); 
             }
             else {
-                $no_user_flag=1;
-                $no_grader_flag=1;
+                $no_user_flag=true;
+                $no_grader_flag=true;
                 foreach ($students as $student) {
                     $registration = ($student->getRegistrationSection() === null) ? "NULL" : $student->getRegistrationSection();
-                    if ($registration == intval($_POST['delete_reg_section'])) {
-                        $no_user_flag=0;
+                    if ($registration == $_POST['delete_reg_section']) {
+                        $no_user_flag=false;
                         break;
                     }    
                 }
 
                 foreach ($graders as $grader) {
                     if(($grader->getGroup() == 2) || ($grader->getGroup() == 3)) {
-                        if(in_array(intval($_POST['delete_reg_section']), $grader->getGradingRegistrationSections() )) {
-                            $no_grader_flag=0;
+                        if(in_array(($_POST['delete_reg_section']), $grader->getGradingRegistrationSections() )) {
+                            $no_grader_flag=false;
                             break;
                         }
                     }
                 }    
-                if (($no_user_flag != 1) || ($no_grader_flag != 1)) {
+                if (($no_user_flag != true) || ($no_grader_flag != true)) {
                     $this->core->addErrorMessage("Cannot delete registration section that has users and/or graders assigned to it");
                     $_SESSION['request'] = $_POST;
                     $this->core->redirect($return_url);      
                 }
                 else {
-                    $this->core->getQueries()->deleteRegistrationSection(intval($_POST['delete_reg_section']));
+                    $this->core->getQueries()->deleteRegistrationSection($_POST['delete_reg_section']);
+                    $this->core->addSuccessMessage("Registration section {$_POST['delete_reg_section']} deleted");
                 }    
             }
         }
@@ -611,10 +621,7 @@ public function uploadClassList() {
             $vals = array_map('trim', $vals);
 
             if (isset($vals[4])) {
-                if (is_numeric($vals[4])) {
-                    $vals[4] = intval($vals[4]);
-                }
-                else if (strtolower($vals[4]) === "null") {
+                if (strtolower($vals[4]) === "null") {
                     $vals[4] = null;
                 }
             }
@@ -629,8 +636,8 @@ public function uploadClassList() {
             //Check email address for appropriate format. e.g. "student@university.edu", "student@cs.university.edu", etc.
             $error_message .= User::validateUserData('user_email', $vals[3]) ? "" : "ERROR on row {$row_num}, email \"".strip_tags($vals[3])."\"<br>";
 
-            //Student section must be greater than zero (intval($str) returns zero when $str is not integer)
-            $error_message .= (($vals[4] > 0 && $vals[4] <= $num_reg_sections) || $vals[4] === null) ? "" : "ERROR on row {$row_num}, Registration Section \"".strip_tags($vals[4])."\"<br>";
+            //Check registration for appropriate format. Allowed characters - A-Z,a-z,_,-
+            $error_message .= User::validateUserData('registration_section', $vals[4]) ? "" : "ERROR on row {$row_num}, Registration Section \"".strip_tags($vals[4])."\"<br>";
 
             //Preferred first name must be alpha characters, white-space, or certain punctuation.
             if (isset($vals[$pref_name_idx]) && ($vals[$pref_name_idx] != "")) {