From b39fdb053783ff5e0aa022e68c8ae85a59e95af8 Mon Sep 17 00:00:00 2001
From: KrishaDarji <dkrisha080@gmail.com>
Date: Sun, 19 Apr 2026 18:15:06 -0400
Subject: [PATCH 1/6] secure password workflow and class method lookup with STI
 conversion

---
 app/controllers/users_controller.rb |   2 -
 app/models/user.rb                  |  26 +++--
 db/schema.rb                        |   5 +-
 spec/models/user_spec.rb            | 153 ++++++++++++++++++++++++++++
 4 files changed, 172 insertions(+), 14 deletions(-)
 create mode 100644 spec/models/user_spec.rb

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 83d7352fd..1581901b9 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -15,8 +15,6 @@ def show
 
   # POST /users
   def create
-    # Add default password for a user if the password is not provided
-    params[:user][:password] ||= 'password'
     user = User.new(user_params)
     if user.save
       render json: user, status: :created
diff --git a/app/models/user.rb b/app/models/user.rb
index 0e77e25dc..f0b23c278 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -8,7 +8,8 @@ class User < ApplicationRecord
   validates :name, presence: true, uniqueness: true, allow_blank: false
                    # format: { with: /\A[a-z]+\z/, message: 'must be in lowercase' }
   validates :email, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP }
-  validates :password, length: { minimum: 6 }, presence: true, allow_nil: true
+  validates :password, presence: true, if: :password_required?
+  validates :password, length: { minimum: 6 }, allow_nil: true
   validates :full_name, presence: true, length: { maximum: 50 }
 
   belongs_to :role
@@ -34,17 +35,18 @@ class User < ApplicationRecord
   delegate :super_administrator?, to: :role
 
   def self.instantiate(record)
-    case record.role
-    when Role::TEACHING_ASSISTANT
+    case record.role.name
+    when /Super Administrator/
+      record.becomes(SuperAdministrator)
+    when /Teaching Assistant/
       record.becomes(Ta)
-    when Role::INSTRUCTOR
+    when /Instructor/
       record.becomes(Instructor)
-    when Role::ADMINISTRATOR
+    when /Administrator/
       record.becomes(Administrator)
-    when Role::SUPER_ADMINISTRATOR
-      record.becomes(SuperAdministrator)
     else
-      super
+      # Student or other roles remain as User
+      record
     end
   end
 
@@ -65,8 +67,12 @@ def self.login_user(login)
   # Reset the password for the user
   def reset_password
     random_password = SecureRandom.alphanumeric(10)
-    user.password_digest = BCrypt::Password.create(random_password)
-    user.save
+    self.password = random_password
+    save
+  end
+
+  def password_required?
+    password_digest.blank? || !password.nil?
   end
 
   # Get instructor_id of the user, if the user is TA,
diff --git a/db/schema.rb b/db/schema.rb
index cddbe12c6..e9103fc77 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.0].define(version: 2026_03_13_064334) do
+ActiveRecord::Schema[8.0].define(version: 2026_03_14_000000) do
   create_table "account_requests", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
     t.string "username"
     t.string "full_name"
@@ -408,6 +408,7 @@
     t.bigint "participant_id", null: false
     t.integer "user_id", null: false
     t.index ["participant_id"], name: "index_teams_participants_on_participant_id"
+    t.index ["participant_id"], name: "index_teams_participants_on_participant_id_unique", unique: true
     t.index ["team_id"], name: "index_teams_participants_on_team_id"
     t.index ["user_id"], name: "index_teams_participants_on_user_id"
   end
@@ -456,9 +457,9 @@
   add_foreign_key "assignments_duties", "duties"
   add_foreign_key "courses", "institutions"
   add_foreign_key "courses", "users", column: "instructor_id"
+  add_foreign_key "duties", "users", column: "instructor_id"
   add_foreign_key "invitations", "participants", column: "from_id"
   add_foreign_key "invitations", "participants", column: "to_id"
-  add_foreign_key "duties", "users", column: "instructor_id"
   add_foreign_key "items", "questionnaires"
   add_foreign_key "participants", "duties"
   add_foreign_key "participants", "join_team_requests"
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
new file mode 100644
index 000000000..033d56e24
--- /dev/null
+++ b/spec/models/user_spec.rb
@@ -0,0 +1,153 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe User, type: :model do
+  describe 'password validations' do
+    it 'requires a password when password_digest is blank' do
+      user = User.new(name: 'testuser', email: 'test@example.com', full_name: 'Test User', role: create(:role))
+
+      expect(user).not_to be_valid
+      expect(user.errors[:password]).to include("can't be blank")
+    end
+
+    it 'allows updating a user without providing password' do
+      user = create(:user, password: 'password', password_confirmation: 'password')
+      user.name = 'updated_name'
+
+      expect(user).to be_valid
+    end
+
+    it 'enforces a minimum password length when password is provided' do
+      user = User.new(name: 'shortpass', email: 'short@example.com', full_name: 'Short Pass', role: create(:role), password: '123', password_confirmation: '123')
+
+      expect(user).not_to be_valid
+      expect(user.errors[:password]).to include('is too short (minimum is 6 characters)')
+    end
+
+    it 'generates a password_digest when password is set' do
+      user = create(:user, password: 'securepass', password_confirmation: 'securepass')
+
+      expect(user.password_digest).to be_present
+      expect(user.authenticate('securepass')).to eq(user)
+    end
+  end
+
+  describe '.login_user' do
+    let(:student_role) { create(:role, :student) }
+
+    it 'finds user by email' do
+      user = create(:user, email: 'john@example.com', role: student_role)
+      found_user = User.login_user('john@example.com')
+
+      expect(found_user).to eq(user)
+    end
+
+    it 'finds user by username when email is not found' do
+      user = create(:user, role: student_role)
+      found_user = User.login_user(user.name)
+
+      expect(found_user).to eq(user)
+    end
+
+    it 'extracts username from email-like input when looking up by name' do
+      user = create(:user, role: student_role)
+      # Simulate email-like input by using part of username
+      found_user = User.login_user(user.name)
+
+      expect(found_user).to eq(user)
+    end
+
+    it 'returns nil when user is not found' do
+      found_user = User.login_user('nonexistent@example.com')
+
+      expect(found_user).to be_nil
+    end
+  end
+
+  describe '.from_params' do
+    let(:student_role) { create(:role, :student) }
+    let(:user) { create(:user, role: student_role) }
+
+    it 'finds user by user_id when provided' do
+      params = { user_id: user.id }
+      found_user = User.from_params(params)
+
+      expect(found_user).to eq(user)
+    end
+
+    it 'finds user by name when user_id is not provided' do
+      params = { user: { name: user.name } }
+      found_user = User.from_params(params)
+
+      expect(found_user).to eq(user)
+    end
+
+    it 'raises ActiveRecord::RecordNotFound when user_id does not exist' do
+      params = { user_id: 999_999 }
+
+      expect { User.from_params(params) }.to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    it 'raises error when user is not found by name' do
+      params = { user: { name: 'nonexistent_user' } }
+
+      expect { User.from_params(params) }.to raise_error("User nonexistent_user not found")
+    end
+  end
+
+  describe '.instantiate (STI conversion)' do
+    let(:institution) { create(:institution) }
+
+    it 'converts user to Ta when role is teaching assistant' do
+      ta_role = create(:role, :ta)
+      user_record = create(:user, role: ta_role, institution: institution)
+
+      result = User.instantiate(user_record)
+
+      expect(result).to be_a(Ta)
+      expect(result.id).to eq(user_record.id)
+    end
+
+    it 'converts user to Instructor when role is instructor' do
+      instructor_role = create(:role, :instructor)
+      user_record = create(:user, role: instructor_role, institution: institution)
+
+      result = User.instantiate(user_record)
+
+      expect(result).to be_a(Instructor)
+      expect(result.id).to eq(user_record.id)
+    end
+
+    it 'converts user to Administrator when role is administrator' do
+      admin_role = create(:role, :administrator)
+      user_record = create(:user, role: admin_role, institution: institution)
+
+      result = User.instantiate(user_record)
+
+      expect(result).to be_a(Administrator)
+      expect(result.id).to eq(user_record.id)
+    end
+
+    it 'converts user to SuperAdministrator when role is super administrator' do
+      super_admin_role = create(:role, :super_administrator)
+      user_record = create(:user, role: super_admin_role, institution: institution)
+
+      result = User.instantiate(user_record)
+
+      expect(result).to be_a(SuperAdministrator)
+      expect(result.id).to eq(user_record.id)
+    end
+
+    it 'returns User when role is student (no STI conversion)' do
+      student_role = create(:role, :student)
+      user_record = create(:user, role: student_role, institution: institution)
+
+      result = User.instantiate(user_record)
+
+      expect(result).to be_a(User)
+      expect(result).not_to be_a(Ta)
+      expect(result.id).to eq(user_record.id)
+    end
+  end
+end

From 40f5f04782404830f8db477b85750023d75f22be Mon Sep 17 00:00:00 2001
From: KrishaDarji <dkrisha080@gmail.com>
Date: Sun, 19 Apr 2026 18:25:43 -0400
Subject: [PATCH 2/6] comprehensive instance method and regression tests

---
 app/models/user.rb       |   6 +-
 spec/models/user_spec.rb | 255 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 225 insertions(+), 36 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index f0b23c278..40c1dbd44 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -78,10 +78,10 @@ def password_required?
   # Get instructor_id of the user, if the user is TA,
   # return the id of the instructor else return the id of the user for superior roles
   def instructor_id
-    case role
-    when Role::INSTRUCTOR, Role::ADMINISTRATOR, Role::SUPER_ADMINISTRATOR
+    case role.name
+    when /Instructor/, /Administrator/, /Super Administrator/
       id
-    when Role::TEACHING_ASSISTANT
+    when /Teaching Assistant/
       my_instructor
     else
       raise NotImplementedError, "Unknown role: #{role.name}"
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 033d56e24..5ce64b213 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -96,58 +96,247 @@
     end
   end
 
-  describe '.instantiate (STI conversion)' do
+  describe 'instance methods - role authorization' do
     let(:institution) { create(:institution) }
+    let(:student_role) { create(:role, :student) }
+    let(:ta_role) { create(:role, :ta) }
+    let(:instructor_role) { create(:role, :instructor) }
+    let(:admin_role) { create(:role, :administrator) }
+    let(:super_admin_role) { create(:role, :super_administrator) }
+
+    describe '#instructor_id' do
+      it 'returns own id for instructor' do
+        instructor = create(:user, role: instructor_role, institution: institution)
+        expect(instructor.instructor_id).to eq(instructor.id)
+      end
+
+      it 'returns own id for administrator' do
+        admin = create(:user, role: admin_role, institution: institution)
+        expect(admin.instructor_id).to eq(admin.id)
+      end
+
+      it 'returns own id for super administrator' do
+        super_admin = create(:user, role: super_admin_role, institution: institution)
+        expect(super_admin.instructor_id).to eq(super_admin.id)
+      end
+
+      it 'returns instructor id for teaching assistant' do
+        instructor = create(:user, role: instructor_role, institution: institution)
+        ta = create(:user, role: ta_role, institution: institution, parent: instructor)
+        expect(ta.instructor_id).to eq(instructor.id)
+      end
+
+      it 'raises NotImplementedError for student role' do
+        student = create(:user, role: student_role, institution: institution)
+        expect { student.instructor_id }.to raise_error(NotImplementedError, /Unknown role/)
+      end
+    end
 
-    it 'converts user to Ta when role is teaching assistant' do
-      ta_role = create(:role, :ta)
-      user_record = create(:user, role: ta_role, institution: institution)
+    describe '#can_impersonate?' do
+      let(:student1) { create(:user, role: student_role, institution: institution) }
+      let(:student2) { create(:user, role: student_role, institution: institution) }
+      let(:instructor) { create(:user, role: instructor_role, institution: institution) }
+      let(:super_admin) { create(:user, role: super_admin_role, institution: institution) }
+
+      it 'super admin can impersonate anyone' do
+        expect(super_admin.can_impersonate?(student1)).to be true
+        expect(super_admin.can_impersonate?(instructor)).to be true
+        expect(super_admin.can_impersonate?(super_admin)).to be true
+      end
+
+      it 'parent can impersonate direct child' do
+        child = create(:user, role: student_role, institution: institution, parent: instructor)
+        expect(instructor.can_impersonate?(child)).to be true
+      end
+
+      it 'parent cannot impersonate unrelated user' do
+        expect(instructor.can_impersonate?(student1)).to be false
+      end
+
+      it 'regular user cannot impersonate anyone' do
+        expect(student1.can_impersonate?(student2)).to be false
+      end
+    end
 
-      result = User.instantiate(user_record)
+    describe '#recursively_parent_of' do
+      let(:grandparent) { create(:user, role: instructor_role, institution: institution) }
+      let(:parent) { create(:user, role: instructor_role, institution: institution, parent: grandparent) }
+      let(:child) { create(:user, role: student_role, institution: institution, parent: parent) }
 
-      expect(result).to be_a(Ta)
-      expect(result.id).to eq(user_record.id)
-    end
+      it 'identifies direct parent' do
+        expect(parent.recursively_parent_of(child)).to be true
+      end
 
-    it 'converts user to Instructor when role is instructor' do
-      instructor_role = create(:role, :instructor)
-      user_record = create(:user, role: instructor_role, institution: institution)
+      it 'identifies grandparent in hierarchy' do
+        expect(grandparent.recursively_parent_of(child)).to be true
+      end
 
-      result = User.instantiate(user_record)
+      it 'returns false for non-parent' do
+        other_user = create(:user, role: student_role, institution: institution)
+        expect(other_user.recursively_parent_of(child)).to be false
+      end
 
-      expect(result).to be_a(Instructor)
-      expect(result.id).to eq(user_record.id)
+      it 'returns false when user has no parent' do
+        orphan = create(:user, role: student_role, institution: institution)
+        expect(grandparent.recursively_parent_of(orphan)).to be false
+      end
     end
 
-    it 'converts user to Administrator when role is administrator' do
-      admin_role = create(:role, :administrator)
-      user_record = create(:user, role: admin_role, institution: institution)
+    describe '#teaching_assistant?' do
+      it 'returns true for teaching assistant role' do
+        ta = create(:user, role: ta_role, institution: institution)
+        expect(ta.teaching_assistant?).to be true
+      end
+
+      it 'returns false for non-ta roles' do
+        student = create(:user, role: student_role, institution: institution)
+        instructor = create(:user, role: instructor_role, institution: institution)
+        
+        expect(student.teaching_assistant?).to be false
+        expect(instructor.teaching_assistant?).to be false
+      end
+    end
 
-      result = User.instantiate(user_record)
+    describe '#as_json' do
+      let(:user) { create(:user, role: student_role, institution: institution) }
+
+      it 'includes only permitted user fields' do
+        json = user.as_json
+        
+        expect(json.keys).to include('id', 'name', 'email', 'full_name')
+        expect(json.keys).to include('email_on_review', 'email_on_submission', 'email_on_review_of_review')
+      end
+
+      it 'excludes sensitive fields' do
+        json = user.as_json
+        
+        expect(json.keys).not_to include('password_digest', 'password', 'updated_at', 'created_at')
+      end
+
+      it 'includes role with only id and name' do
+        json = user.as_json
+        
+        expect(json['role']).to be_a(Hash)
+        expect(json['role'].keys).to contain_exactly('id', 'name')
+      end
+
+      it 'includes parent with default nil values when none exists' do
+        json = user.as_json
+        
+        expect(json['parent']).to eq({ 'id' => nil, 'name' => nil })
+      end
+
+      it 'includes institution with data when assigned' do
+        json = user.as_json
+        
+        expect(json['institution']).to be_a(Hash)
+        expect(json['institution']['id']).to eq(institution.id)
+        expect(json['institution']['name']).to eq(institution.name)
+      end
+
+      it 'includes parent data when user has parent' do
+        parent_user = create(:user, role: instructor_role, institution: institution)
+        child_user = create(:user, role: student_role, institution: institution, parent: parent_user)
+        json = child_user.as_json
+        
+        expect(json['parent']['id']).to eq(parent_user.id)
+        expect(json['parent']['name']).to eq(parent_user.name)
+      end
+    end
 
-      expect(result).to be_a(Administrator)
-      expect(result.id).to eq(user_record.id)
+    describe '#generate_jwt' do
+      let(:user) { create(:user, role: student_role, institution: institution) }
+
+      it 'generates a valid JWT token' do
+        token = user.generate_jwt
+        
+        expect(token).to be_a(String)
+        expect(token.split('.').length).to eq(3)  # JWT has 3 parts
+      end
+
+      it 'encodes user id in token' do
+        token = user.generate_jwt
+        decoded = JWT.decode(token, Rails.application.credentials.secret_key_base)
+        
+        expect(decoded[0]['id']).to eq(user.id)
+      end
+
+      it 'sets expiration to 60 days from now' do
+        token = user.generate_jwt
+        decoded = JWT.decode(token, Rails.application.credentials.secret_key_base)
+        
+        exp_time = Time.at(decoded[0]['exp'])
+        expect(exp_time).to be_between(59.days.from_now, 61.days.from_now)
+      end
+
+      it 'generates different tokens on each call' do
+        token1 = user.generate_jwt
+        token2 = user.generate_jwt
+        
+        expect(token1).not_to eq(token2)  # exp time changes
+      end
     end
+  end
 
-    it 'converts user to SuperAdministrator when role is super administrator' do
-      super_admin_role = create(:role, :super_administrator)
-      user_record = create(:user, role: super_admin_role, institution: institution)
+  describe 'regression checks - edge cases and fragile areas' do
+    let(:institution) { create(:institution) }
+    let(:student_role) { create(:role, :student) }
+    let(:instructor_role) { create(:role, :instructor) }
+    let(:super_admin_role) { create(:role, :super_administrator) }
+
+    it 'handles user with nil parent gracefully' do
+      user = create(:user, role: student_role, institution: institution)
+      
+      expect(user.parent).to be_nil
+      expect { user.recursively_parent_of(user) }.not_to raise_error
+    end
 
-      result = User.instantiate(user_record)
+    it 'prevents infinite recursion in parent hierarchy' do
+      user1 = create(:user, role: instructor_role, institution: institution)
+      user2 = create(:user, role: student_role, institution: institution, parent: user1)
+      
+      # user1 is parent of user2, so this should return false (not traverse infinitely)
+      expect(user2.recursively_parent_of(user1)).to be false
+    end
 
-      expect(result).to be_a(SuperAdministrator)
-      expect(result.id).to eq(user_record.id)
+    it 'handles as_json with missing institution' do
+      user = create(:user, role: student_role, institution_id: nil)
+      json = user.as_json
+      
+      expect(json['institution']).to eq({ 'id' => nil, 'name' => nil })
     end
 
-    it 'returns User when role is student (no STI conversion)' do
-      student_role = create(:role, :student)
-      user_record = create(:user, role: student_role, institution: institution)
+    it 'generates distinct JWTs for different users' do
+      user1 = create(:user, role: student_role, institution: institution)
+      user2 = create(:user, role: student_role, institution: institution)
+      
+      token1 = user1.generate_jwt
+      token2 = user2.generate_jwt
+      
+      decoded1 = JWT.decode(token1, Rails.application.credentials.secret_key_base)
+      decoded2 = JWT.decode(token2, Rails.application.credentials.secret_key_base)
+      
+      expect(decoded1[0]['id']).to eq(user1.id)
+      expect(decoded2[0]['id']).to eq(user2.id)
+    end
 
-      result = User.instantiate(user_record)
+    it 'instructor_id works correctly after role changes' do
+      user = create(:user, role: student_role, institution: institution)
+      
+      expect { user.instructor_id }.to raise_error(NotImplementedError)
+      
+      user.update(role: instructor_role)
+      expect(user.instructor_id).to eq(user.id)
+    end
 
-      expect(result).to be_a(User)
-      expect(result).not_to be_a(Ta)
-      expect(result.id).to eq(user_record.id)
+    it 'can_impersonate? respects role hierarchy' do
+      instructor = create(:user, role: instructor_role, institution: institution)
+      super_admin = create(:user, role: super_admin_role, institution: institution)
+      student = create(:user, role: student_role, institution: institution)
+      
+      expect(instructor.can_impersonate?(student)).to be false
+      expect(super_admin.can_impersonate?(instructor)).to be true
+      expect(super_admin.can_impersonate?(student)).to be true
     end
   end
 end

From 9c92120c25199be3e06c6a6280504e12ea1a3e44 Mon Sep 17 00:00:00 2001
From: KrishaDarji <dkrisha080@gmail.com>
Date: Mon, 20 Apr 2026 14:59:23 -0400
Subject: [PATCH 3/6] comprehensive RSpec tests for User model and
 UsersController

---
 app/models/ta.rb                              |   9 +-
 app/models/user.rb                            |  18 +-
 spec/models/user_spec.rb                      |  88 ++++++-
 spec/requests/api/v1/users_controller_spec.rb |  67 ++++++
 spec/requests/api/v1/users_spec.rb            | 222 ++++++++++++++++++
 5 files changed, 379 insertions(+), 25 deletions(-)
 create mode 100644 spec/requests/api/v1/users_controller_spec.rb
 create mode 100644 spec/requests/api/v1/users_spec.rb

diff --git a/app/models/ta.rb b/app/models/ta.rb
index 3855d9de6..f56ee46e7 100644
--- a/app/models/ta.rb
+++ b/app/models/ta.rb
@@ -8,16 +8,11 @@ def managed_users
   end
 
   def my_instructor
-    # code here
+    parent_id
   end
 
   def courses_assisted_with
-    courses = TaMapping.where(ta_id: id)
-    courses.map { |c| Course.find(c.course_id) }
-  end
-
-  def courses_assisted_with
-    courses = TaMapping.where(ta_id: id)
+    courses = TaMapping.where(user_id: id)
     courses.map { |c| Course.find(c.course_id) }
   end
 end
\ No newline at end of file
diff --git a/app/models/user.rb b/app/models/user.rb
index 40c1dbd44..07493a4ac 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -82,14 +82,14 @@ def instructor_id
     when /Instructor/, /Administrator/, /Super Administrator/
       id
     when /Teaching Assistant/
-      my_instructor
+      Ta.find(id).my_instructor
     else
       raise NotImplementedError, "Unknown role: #{role.name}"
     end
   end
 
   def can_impersonate?(user)
-    return true if role.super_admin?
+    return true if role.super_administrator?
     return true if teaching_assistant_for?(user)
     return true if recursively_parent_of(user)
 
@@ -100,7 +100,7 @@ def recursively_parent_of(user)
     p = user.parent
     return false if p.nil?
     return true if p == self
-    return false if p.role.super_admin?
+    return false if p.role.super_administrator?
 
     recursively_parent_of(p)
   end
@@ -109,16 +109,14 @@ def teaching_assistant_for?(student)
     return false unless teaching_assistant?
     return false unless student.role.name == 'Student'
 
-    # We have to use the Ta object instead of User object
-    # because single table inheritance is not currently functioning
     ta = Ta.find(id)
-    return true if ta.courses_assisted_with.any? do |c|
+    ta.courses_assisted_with.any? do |c|
       c.assignments.map(&:participants).flatten.map(&:user_id).include? student.id
     end
   end
 
   def teaching_assistant?
-    true if role.ta?
+    role.ta? ? true : false
   end
 
   def self.from_params(params)
@@ -141,8 +139,8 @@ def as_json(options = {})
                             institution: { only: %i[id name] }
                           }
                         })).tap do |hash|
-      hash['parent'] ||= { id: nil, name: nil }
-      hash['institution'] ||= { id: nil, name: nil }
+      hash['parent'] ||= { 'id' => nil, 'name' => nil }
+      hash['institution'] ||= { 'id' => nil, 'name' => nil }
     end
   end
 
@@ -156,7 +154,7 @@ def set_defaults
   end
 
   def generate_jwt
-    JWT.encode({ id: id, exp: 60.days.from_now.to_i }, Rails.application.credentials.secret_key_base)
+    JWT.encode({ id: id, exp: 60.days.from_now.to_i }, Rails.application.credentials.secret_key_base, 'HS256')
   end
 
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 5ce64b213..12e61462e 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -65,6 +65,41 @@
     end
   end
 
+  describe '.instantiate' do
+    let(:institution) { create(:institution) }
+    let(:instructor_role) { create(:role, :instructor) }
+    let(:ta_role) { create(:role, :ta) }
+    let(:admin_role) { create(:role, :administrator) }
+    let(:super_admin_role) { create(:role, :super_administrator) }
+    let(:student_role) { create(:role, :student) }
+
+    it 'returns Instructor for instructor role' do
+      user = create(:user, role: instructor_role, institution: institution)
+      expect(User.instantiate(user)).to be_a(Instructor)
+    end
+
+    it 'returns Ta for teaching assistant role' do
+      user = create(:user, role: ta_role, institution: institution)
+      expect(User.instantiate(user)).to be_a(Ta)
+    end
+
+    it 'returns Administrator for administrator role' do
+      user = create(:user, role: admin_role, institution: institution)
+      expect(User.instantiate(user)).to be_a(Administrator)
+    end
+
+    it 'returns SuperAdministrator for super administrator role' do
+      user = create(:user, role: super_admin_role, institution: institution)
+      expect(User.instantiate(user)).to be_a(SuperAdministrator)
+    end
+
+    it 'returns User as-is for student role' do
+      user = create(:user, role: student_role, institution: institution)
+      expect(User.instantiate(user)).to be_a(User)
+      expect(User.instantiate(user)).not_to be_a(Instructor)
+    end
+  end
+
   describe '.from_params' do
     let(:student_role) { create(:role, :student) }
     let(:user) { create(:user, role: student_role) }
@@ -182,6 +217,41 @@
       end
     end
 
+    describe '#teaching_assistant_for?' do
+      let(:ta_role)       { Role.find_or_create_by!(name: 'Teaching Assistant') }
+      let(:student_role)  { Role.find_or_create_by!(name: 'Student') }
+      let(:course) { Course.create!(name: 'Test Course', directory_path: 'test_course', institution: institution, instructor_id: create(:user, role: instructor_role, institution: institution).id) }
+      let(:ta_user) { create(:user, role: ta_role, institution: institution) }
+      let(:student) { create(:user, role: student_role, institution: institution) }
+      let(:assignment) { Assignment.create!(name: 'Test Assignment', instructor_id: create(:user, role: instructor_role, institution: institution).id, course_id: course.id) }
+
+      it 'returns false when user is not a teaching assistant' do
+        instructor = create(:user, role: instructor_role, institution: institution)
+        expect(instructor.teaching_assistant_for?(student)).to be false
+      end
+
+      it 'returns false when target user is not a student' do
+        other_instructor = create(:user, role: instructor_role, institution: institution)
+        expect(ta_user.teaching_assistant_for?(other_instructor)).to be false
+      end
+
+      it 'returns true when TA assists a course that has the student as participant' do
+        TaMapping.create!(user_id: ta_user.id, course_id: course.id)
+        AssignmentParticipant.create!(user_id: student.id, parent_id: assignment.id, handle: student.name)
+        expect(ta_user.teaching_assistant_for?(student)).to be true
+      end
+
+      it 'returns false when TA has no course mappings' do
+        expect(ta_user.teaching_assistant_for?(student)).to be false
+      end
+
+      it 'returns false when student is not a participant in any TA course' do
+        other_course = Course.create!(name: 'Other Course', directory_path: 'other_course', institution: institution, instructor_id: create(:user, role: instructor_role, institution: institution).id)
+        TaMapping.create!(user_id: ta_user.id, course_id: other_course.id)
+        expect(ta_user.teaching_assistant_for?(student)).to be false
+      end
+    end
+
     describe '#teaching_assistant?' do
       it 'returns true for teaching assistant role' do
         ta = create(:user, role: ta_role, institution: institution)
@@ -256,24 +326,26 @@
 
       it 'encodes user id in token' do
         token = user.generate_jwt
-        decoded = JWT.decode(token, Rails.application.credentials.secret_key_base)
-        
+        decoded = JWT.decode(token, nil, false)
         expect(decoded[0]['id']).to eq(user.id)
       end
 
       it 'sets expiration to 60 days from now' do
         token = user.generate_jwt
-        decoded = JWT.decode(token, Rails.application.credentials.secret_key_base)
-        
+        decoded = JWT.decode(token, nil, false)
         exp_time = Time.at(decoded[0]['exp'])
         expect(exp_time).to be_between(59.days.from_now, 61.days.from_now)
       end
 
       it 'generates different tokens on each call' do
         token1 = user.generate_jwt
+        decoded1 = JWT.decode(token1, nil, false)
         token2 = user.generate_jwt
-        
-        expect(token1).not_to eq(token2)  # exp time changes
+        decoded2 = JWT.decode(token2, nil, false)
+        # Both encode the same user id; tokens may be identical within same second — just verify structure
+        expect(token1).to be_a(String)
+        expect(decoded1[0]['id']).to eq(user.id)
+        expect(decoded2[0]['id']).to eq(user.id)
       end
     end
   end
@@ -313,8 +385,8 @@
       token1 = user1.generate_jwt
       token2 = user2.generate_jwt
       
-      decoded1 = JWT.decode(token1, Rails.application.credentials.secret_key_base)
-      decoded2 = JWT.decode(token2, Rails.application.credentials.secret_key_base)
+      decoded1 = JWT.decode(token1, nil, false)
+      decoded2 = JWT.decode(token2, nil, false)
       
       expect(decoded1[0]['id']).to eq(user1.id)
       expect(decoded2[0]['id']).to eq(user2.id)
diff --git a/spec/requests/api/v1/users_controller_spec.rb b/spec/requests/api/v1/users_controller_spec.rb
new file mode 100644
index 000000000..18487a998
--- /dev/null
+++ b/spec/requests/api/v1/users_controller_spec.rb
@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require 'swagger_helper'
+require 'json_web_token'
+
+RSpec.describe 'Users API', type: :request do
+  before(:each) do
+    @roles = create_roles_hierarchy
+  end
+
+  let!(:user) do
+    User.create!(
+      name: 'testuser',
+      password: 'password',
+      password_confirmation: 'password',
+      role_id: @roles[:student].id,
+      full_name: 'Test User',
+      email: 'testuser@example.com'
+    )
+  end
+
+  let(:token) { JsonWebToken.encode({ id: user.id }) }
+  let(:Authorization) { "Bearer #{token}" }
+
+  path '/users' do
+    get 'List all users' do
+      tags 'Users'
+      produces 'application/json'
+      parameter name: 'Authorization', in: :header, type: :string, required: true
+
+      response '200', 'Returns user list' do
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data).to be_an(Array)
+          expect(data.map { |u| u['id'] }).to include(user.id)
+        end
+      end
+    end
+  end
+
+  path '/users/{id}' do
+    get 'Get a user by id' do
+      tags 'Users'
+      produces 'application/json'
+      parameter name: :id, in: :path, type: :integer
+      parameter name: 'Authorization', in: :header, type: :string, required: true
+
+      response '200', 'Returns the user' do
+        let(:id) { user.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          expect(data['id']).to eq(user.id)
+          expect(data.keys).to include('id', 'email')
+        end
+      end
+
+      response '404', 'User not found' do
+        let(:id) { 0 }
+
+        run_test! do |response|
+          expect(JSON.parse(response.body)['error']).to include('not found')
+        end
+      end
+    end
+  end
+end
diff --git a/spec/requests/api/v1/users_spec.rb b/spec/requests/api/v1/users_spec.rb
new file mode 100644
index 000000000..d61e9a3b7
--- /dev/null
+++ b/spec/requests/api/v1/users_spec.rb
@@ -0,0 +1,222 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+require 'json_web_token'
+
+RSpec.describe 'Users API', type: :request do
+  before(:all) do
+    @roles = create_roles_hierarchy
+    @institution = Institution.first || Institution.create!(name: "Test Institution")
+  end
+
+  let(:student_role) { @roles[:student] }
+  let(:instructor_role) { @roles[:instructor] }
+
+  let(:instructor) do
+    User.create!(
+      name: 'instructor_test',
+      password: 'password123',
+      role_id: instructor_role.id,
+      full_name: 'Instructor Test',
+      email: 'instructor@example.com',
+      institution: @institution
+    )
+  end
+
+  let(:student) do
+    User.create!(
+      name: 'student_test',
+      password: 'password123',
+      role_id: student_role.id,
+      full_name: 'Student Test',
+      email: 'student@example.com',
+      institution: @institution
+    )
+  end
+
+  let(:token) { JsonWebToken.encode({ id: instructor.id }) }
+  let(:headers) { { 'Authorization' => "Bearer #{token}" } }
+
+  describe 'GET /users' do
+    it 'returns 200 with list of users when authenticated' do
+      get '/users', headers: headers
+
+      expect(response.status).to eq(200)
+      json_response = JSON.parse(response.body)
+
+      expect(json_response).to be_an(Array)
+
+      # Verify instructor is in list
+      instructor_data = json_response.find { |u| u['id'] == instructor.id }
+      expect(instructor_data).to be_present
+      expect(instructor_data['name']).to eq(instructor.name)
+      expect(instructor_data['email']).to eq(instructor.email)
+      expect(instructor_data['full_name']).to eq(instructor.full_name)
+      expect(instructor_data['role']).to be_a(Hash)
+      expect(instructor_data['role']['id']).to eq(instructor_role.id)
+
+      # Verify sensitive data is not included
+      expect(instructor_data).not_to have_key('password_digest')
+      expect(instructor_data).not_to have_key('password')
+    end
+
+    it 'returns 401 when not authenticated' do
+      get '/users'
+
+      expect(response.status).to eq(401)
+    end
+
+    it 'returns payload with all users' do
+      # Create multiple users
+      user1 = instructor
+      user2 = student
+
+      get '/users', headers: headers
+
+      json_response = JSON.parse(response.body)
+      user_ids = json_response.map { |u| u['id'] }
+
+      expect(user_ids).to include(user1.id, user2.id)
+    end
+  end
+
+  describe 'GET /users/:id' do
+    it 'returns 200 with user data for existing user' do
+      get "/users/#{student.id}", headers: headers
+
+      expect(response.status).to eq(200)
+      json_response = JSON.parse(response.body)
+
+      expect(json_response['id']).to eq(student.id)
+      expect(json_response['name']).to eq(student.name)
+      expect(json_response['email']).to eq(student.email)
+      expect(json_response['full_name']).to eq(student.full_name)
+      expect(json_response['role']).to be_a(Hash)
+      expect(json_response['role']['id']).to eq(student_role.id)
+
+      # Verify sensitive data is not included
+      expect(json_response).not_to have_key('password_digest')
+      expect(json_response).not_to have_key('password')
+    end
+
+    it 'returns 404 for non-existent user' do
+      get '/users/999999', headers: headers
+
+      expect(response.status).to eq(404)
+      json_response = JSON.parse(response.body)
+      expect(json_response['error']).to be_present
+    end
+
+    it 'returns 401 when not authenticated' do
+      get "/users/#{student.id}"
+
+      expect(response.status).to eq(401)
+    end
+
+    it 'includes institution data in response' do
+      get "/users/#{instructor.id}", headers: headers
+
+      json_response = JSON.parse(response.body)
+      expect(json_response['institution']).to be_a(Hash)
+      expect(json_response['institution']['id']).to eq(@institution.id)
+      expect(json_response['institution']['name']).to eq(@institution.name)
+    end
+
+    it 'includes parent field with nil when no parent' do
+      get "/users/#{student.id}", headers: headers
+
+      json_response = JSON.parse(response.body)
+      expect(json_response['parent']).to eq({ 'id' => nil, 'name' => nil })
+    end
+  end
+
+  describe 'POST /users' do
+    it 'creates a user with valid parameters' do
+      user_params = {
+        user: {
+          name: 'new_user',
+          email: 'newuser@example.com',
+          full_name: 'New User',
+          password: 'securepass123',
+          password_confirmation: 'securepass123',
+          role_id: student_role.id,
+          institution_id: @institution.id
+        }
+      }
+
+      post '/users', params: user_params, headers: headers
+
+      expect(response.status).to eq(201)
+      json_response = JSON.parse(response.body)
+
+      expect(json_response['name']).to eq('new_user')
+      expect(json_response['email']).to eq('newuser@example.com')
+      expect(json_response['full_name']).to eq('New User')
+      expect(json_response).not_to have_key('password_digest')
+    end
+
+    it 'returns 422 when password is too short' do
+      user_params = {
+        user: {
+          name: 'bad_user',
+          email: 'bad@example.com',
+          full_name: 'Bad User',
+          password: '123',
+          password_confirmation: '123',
+          role_id: student_role.id
+        }
+      }
+
+      post '/users', params: user_params, headers: headers
+
+      expect(response.status).to eq(422)
+    end
+  end
+
+  describe 'PATCH /users/:id' do
+    it 'updates user successfully' do
+      update_params = {
+        user: { full_name: 'Updated Name' }
+      }
+
+      patch "/users/#{student.id}", params: update_params, headers: headers
+
+      expect(response.status).to eq(200)
+      json_response = JSON.parse(response.body)
+      expect(json_response['full_name']).to eq('Updated Name')
+    end
+
+    it 'returns 404 for non-existent user' do
+      update_params = {
+        user: { full_name: 'Does Not Matter' }
+      }
+
+      patch '/users/999999', params: update_params, headers: headers
+
+      expect(response.status).to eq(404)
+    end
+  end
+
+  describe 'DELETE /users/:id' do
+    it 'deletes user successfully' do
+      user_to_delete = User.create!(
+        name: 'user_to_delete',
+        password: 'password123',
+        role_id: student_role.id,
+        full_name: 'To Delete',
+        email: 'delete@example.com'
+      )
+
+      delete "/users/#{user_to_delete.id}", headers: headers
+
+      expect(response.status).to eq(204)
+      expect(User.find_by(id: user_to_delete.id)).to be_nil
+    end
+
+    it 'returns 404 for non-existent user' do
+      delete '/users/999999', headers: headers
+
+      expect(response.status).to eq(404)
+    end
+  end
+end

From 5c5f8db9b3d38bf476592c3f435f61a164db8170 Mon Sep 17 00:00:00 2001
From: AtharvaW195 <atharvawaingankar195@gmail.com>
Date: Fri, 24 Apr 2026 17:35:35 -0400
Subject: [PATCH 4/6] Added test Cases User Controllers Spec

---
 spec/requests/api/v1/users_controller_spec.rb | 620 +++++++++++++++++-
 spec/requests/api/v1/users_spec.rb            | 222 -------
 2 files changed, 615 insertions(+), 227 deletions(-)
 delete mode 100644 spec/requests/api/v1/users_spec.rb

diff --git a/spec/requests/api/v1/users_controller_spec.rb b/spec/requests/api/v1/users_controller_spec.rb
index 18487a998..3e8e1d3b2 100644
--- a/spec/requests/api/v1/users_controller_spec.rb
+++ b/spec/requests/api/v1/users_controller_spec.rb
@@ -8,7 +8,7 @@
     @roles = create_roles_hierarchy
   end
 
-  let!(:user) do
+  let!(:auth_user) do
     User.create!(
       name: 'testuser',
       password: 'password',
@@ -19,7 +19,7 @@
     )
   end
 
-  let(:token) { JsonWebToken.encode({ id: user.id }) }
+  let(:token) { JsonWebToken.encode({ id: auth_user.id }) }
   let(:Authorization) { "Bearer #{token}" }
 
   path '/users' do
@@ -32,7 +32,16 @@
         run_test! do |response|
           data = JSON.parse(response.body)
           expect(data).to be_an(Array)
-          expect(data.map { |u| u['id'] }).to include(user.id)
+          expect(data.map { |u| u['id'] }).to include(auth_user.id)
+        end
+      end
+      # Scenario: invalid token should be rejected before hitting controller action
+      response '401', 'Unauthorized when token is invalid' do
+        let(:Authorization) { 'Bearer invalid_token' }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to eq('Not Authorized')
         end
       end
     end
@@ -46,11 +55,11 @@
       parameter name: 'Authorization', in: :header, type: :string, required: true
 
       response '200', 'Returns the user' do
-        let(:id) { user.id }
+        let(:id) { auth_user.id }
 
         run_test! do |response|
           data = JSON.parse(response.body)
-          expect(data['id']).to eq(user.id)
+          expect(data['id']).to eq(auth_user.id)
           expect(data.keys).to include('id', 'email')
         end
       end
@@ -64,4 +73,605 @@
       end
     end
   end
+  path '/users' do
+    post 'Create a user' do
+      tags 'Users'
+      consumes 'application/json'
+      produces 'application/json'
+      parameter name: :payload, in: :body, schema: {
+        type: :object,
+        properties: {
+          user: {
+            type: :object,
+            properties: {
+              name: { type: :string },
+              role_id: { type: :integer },
+              full_name: { type: :string },
+              email: { type: :string },
+              parent_id: { type: :integer },
+              institution_id: { type: :integer },
+              password: { type: :string },
+              password_confirmation: { type: :string }
+            },
+            required: %w[name role_id full_name email password password_confirmation]
+          }
+        },
+        required: ['user']
+      }
+
+      # Scenario: valid payload creates user
+      response '201', 'Creates a user with valid payload' do
+        let(:payload) do
+          {
+            user: {
+              name: 'newstudentuser',
+              role_id: @roles[:student].id,
+              full_name: 'New Student User',
+              email: 'newstudentuser@example.com',
+              password: 'password',
+              password_confirmation: 'password'
+            }
+          }
+        end
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['username']).to eq('newstudentuser')
+          expect(body['email']).to eq('newstudentuser@example.com')
+        end
+      end
+
+      # Scenario: invalid payload returns validation errors
+      response '422', 'Rejects invalid payload' do
+        let(:payload) do
+          {
+            user: {
+              name: 'bademailuser',
+              role_id: @roles[:student].id,
+              full_name: 'Bad Email User',
+              email: 'invalid_email',
+              password: 'password',
+              password_confirmation: 'password'
+            }
+          }
+        end
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body).to have_key('email')
+        end
+      end
+      # Scenario: duplicate username should fail uniqueness validation
+      response '422', 'Rejects duplicate username' do
+        let!(:existing_user) do
+          User.create!(
+            name: 'duplicateuser',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Duplicate User One',
+            email: 'duplicateuser1@example.com'
+          )
+        end
+
+         let(:payload) do
+          {
+            user: {
+              name: 'duplicateuser',
+              role_id: @roles[:student].id,
+              full_name: 'Duplicate User Two',
+              email: 'duplicateuser2@example.com',
+              password: 'password',
+              password_confirmation: 'password'
+            }
+          }
+        end
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body).to have_key('name')
+        end
+      end
+      # Scenario: password confirmation mismatch should be rejected
+      response '422', 'Rejects mismatched password confirmation' do
+        let(:payload) do
+          {
+            user: {
+              name: 'pw_mismatch_user',
+              role_id: @roles[:student].id,
+              full_name: 'Password Mismatch User',
+              email: 'pw_mismatch_user@example.com',
+              password: 'password',
+              password_confirmation: 'different_password'
+            }
+          }
+        end
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body).to have_key('password_confirmation')
+        end
+      end
+
+      # Scenario: missing user param triggers ParameterMissing handler
+      response '422', 'Rejects request when user param is missing' do
+        let(:payload) { {} }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to eq('Parameter missing')
+        end
+      end
+    end
+  end
+
+  path '/users/{id}' do
+    parameter name: :id, in: :path, type: :integer
+    parameter name: 'Authorization', in: :header, type: :string, required: true
+
+    put 'Update a user' do
+      tags 'Users'
+      consumes 'application/json'
+      produces 'application/json'
+      parameter name: :payload, in: :body, schema: {
+        type: :object,
+        properties: {
+          user: {
+            type: :object,
+            properties: {
+              full_name: { type: :string },
+              email: { type: :string }
+            }
+          }
+        },
+        required: ['user']
+      }
+
+
+      # Scenario: valid update returns 200
+      response '200', 'Updates user with valid payload' do
+        let(:id) { auth_user.id }
+        let(:payload) { { user: { full_name: 'Updated Full Name' } } }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['fullName']).to eq('Updated Full Name')
+        end
+      end
+
+      # Scenario: invalid update returns 422
+      response '422', 'Rejects invalid update payload' do
+        let(:id) { auth_user.id }
+        let(:payload) { { user: { email: 'not_an_email' } } }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body).to have_key('email')
+        end
+      end
+
+      # Scenario: missing user param should trigger ParameterMissing handler
+      response '422', 'Rejects update when user param is missing' do
+        let(:id) { auth_user.id }
+        let(:payload) { {} }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to eq('Parameter missing')
+        end
+      end
+
+      # Scenario: missing id returns 404
+      response '404', 'Returns not found for missing user id' do
+        let(:id) { 0 }
+        let(:payload) { { user: { full_name: 'No User' } } }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to include('not found')
+        end
+      end
+    end
+
+    patch 'Partially update a user' do
+      tags 'Users'
+      consumes 'application/json'
+      produces 'application/json'
+      parameter name: :payload, in: :body, schema: {
+        type: :object,
+        properties: {
+          user: {
+            type: :object,
+            properties: {
+              full_name: { type: :string },
+              email: { type: :string }
+            }
+          }
+        },
+        required: ['user']
+      }
+
+      # Scenario: valid partial update returns 200
+      response '200', 'Partially updates user with valid payload' do
+        let(:id) { auth_user.id }
+        let(:payload) { { user: { full_name: 'Patched Full Name' } } }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['fullName']).to eq('Patched Full Name')
+        end
+      end
+
+      # Scenario: invalid partial update returns 422
+      response '422', 'Rejects invalid partial update payload' do
+        let(:id) { auth_user.id }
+        let(:payload) { { user: { email: 'still_invalid_email' } } }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body).to have_key('email')
+        end
+      end
+      # Scenario: missing user param should trigger ParameterMissing handler on patch
+      response '422', 'Rejects partial update when user param is missing' do
+        let(:id) { auth_user.id }
+        let(:payload) { {} }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to eq('Parameter missing')
+        end
+      end
+
+      # Scenario: missing id returns 404
+      response '404', 'Returns not found for missing user id on patch' do
+        let(:id) { 0 }
+        let(:payload) { { user: { full_name: 'No Patch User' } } }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to include('not found')
+        end
+      end
+    end
+
+    delete 'Delete a user' do
+      tags 'Users'
+      produces 'application/json'
+
+      # Scenario: existing id delete path
+      response '204', 'Deletes existing user' do
+        let!(:deletable_user) do
+          User.create!(
+            name: 'deletableuser',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Deletable User',
+            email: 'deletableuser@example.com'
+          )
+        end
+        let(:id) { deletable_user.id }
+
+        run_test! do |_response|
+          expect(User.exists?(id)).to eq(false)
+        end
+      end
+
+      # Scenario: missing id returns 404
+      response '404', 'Returns not found when deleting missing user' do
+        let(:id) { 0 }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to include('not found')
+        end
+      end
+    end
+  end
+
+  path '/users/institution/{id}' do
+    parameter name: :id, in: :path, type: :integer
+    parameter name: 'Authorization', in: :header, type: :string, required: true
+
+    get 'Get users by institution' do
+      tags 'Users'
+      produces 'application/json'
+
+      let!(:institution_a) { Institution.create!(name: 'Institution A') }
+      let!(:institution_b) { Institution.create!(name: 'Institution B') }
+
+      let!(:inst_user_1) do
+        User.create!(
+          name: 'instuserone',
+          password: 'password',
+          password_confirmation: 'password',
+          role_id: @roles[:student].id,
+          full_name: 'Institution User One',
+          email: 'instuserone@example.com',
+          institution_id: institution_a.id
+        )
+      end
+
+      let!(:inst_user_2) do
+        User.create!(
+          name: 'instusertwo',
+          password: 'password',
+          password_confirmation: 'password',
+          role_id: @roles[:ta].id,
+          full_name: 'Institution User Two',
+          email: 'instusertwo@example.com',
+          institution_id: institution_a.id
+        )
+      end
+
+      let!(:other_inst_user) do
+        User.create!(
+          name: 'otherinstuser',
+          password: 'password',
+          password_confirmation: 'password',
+          role_id: @roles[:student].id,
+          full_name: 'Other Institution User',
+          email: 'otherinstuser@example.com',
+          institution_id: institution_b.id
+        )
+      end
+
+      # Scenario: valid institution id returns users for that institution
+      response '200', 'Returns institution-scoped users' do
+        let(:id) { institution_a.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          ids = data.map { |u| u['id'] }
+          expect(ids).to include(inst_user_1.id, inst_user_2.id)
+          expect(ids).not_to include(other_inst_user.id)
+        end
+      end
+
+      # Scenario: missing institution id returns 404
+      response '404', 'Returns not found for missing institution' do
+        let(:id) { 0 }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to include("Couldn't find Institution")
+        end
+      end
+    end
+  end
+
+  path '/users/{id}/managed' do
+    parameter name: :id, in: :path, type: :integer
+    parameter name: 'Authorization', in: :header, type: :string, required: true
+
+    get 'Get managed users' do
+      tags 'Users'
+      produces 'application/json'
+
+      # Scenario: student parent cannot manage users
+      response '422', 'Returns error for student parent' do
+        let!(:student_parent) do
+          User.create!(
+            name: 'studentparent',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Student Parent',
+            email: 'studentparent@example.com'
+          )
+        end
+        let(:id) { student_parent.id }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to eq('Students do not manage any users')
+        end
+      end
+
+      # Scenario: instructor parent returns managed users
+      response '200', 'Returns managed users for instructor parent' do
+        let!(:instructor_parent) do
+          User.create!(
+            name: 'instructorparent',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:instructor].id,
+            full_name: 'Instructor Parent',
+            email: 'instructorparent@example.com'
+          )
+        end
+
+        let!(:managed_user) do
+          User.create!(
+            name: 'managedchild',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Managed Child',
+            email: 'managedchild@example.com',
+            parent_id: instructor_parent.id
+          )
+        end
+
+        let!(:unmanaged_user) do
+          User.create!(
+            name: 'unmanagedchild',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Unmanaged Child',
+            email: 'unmanagedchild@example.com'
+          )
+        end
+
+        let(:id) { instructor_parent.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          ids = data.map { |u| u['id'] }
+          expect(ids).to include(managed_user.id)
+          expect(ids).not_to include(unmanaged_user.id)
+        end
+      end
+            # Scenario: administrator should manage users from same institution except self
+      response '200', 'Returns institution users for administrator parent' do
+        let!(:institution_admin) { Institution.create!(name: 'Admin Institution') }
+        let!(:other_institution) { Institution.create!(name: 'Other Institution') }
+
+        let!(:admin_parent) do
+          User.create!(
+            name: 'adminparent',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:admin].id,
+            full_name: 'Administrator Parent',
+            email: 'adminparent@example.com',
+            institution_id: institution_admin.id
+          )
+        end
+
+        let!(:same_institution_user) do
+          User.create!(
+            name: 'sameinstuser',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Same Institution User',
+            email: 'sameinstuser@example.com',
+            institution_id: institution_admin.id
+          )
+        end
+
+        let!(:other_institution_user) do
+          User.create!(
+            name: 'otherinstmanaged',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Other Institution User',
+            email: 'otherinstmanaged@example.com',
+            institution_id: other_institution.id
+          )
+        end
+
+        let(:id) { admin_parent.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          ids = data.map { |u| u['id'] }
+          expect(ids).to include(same_institution_user.id)
+          expect(ids).not_to include(admin_parent.id)
+          expect(ids).not_to include(other_institution_user.id)
+        end
+      end
+
+      # Scenario: super administrator should manage all users
+      response '200', 'Returns all users for super administrator parent' do
+        let!(:super_admin_parent) do
+          User.create!(
+            name: 'superadminparent',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:super_admin].id,
+            full_name: 'Super Administrator Parent',
+            email: 'superadminparent@example.com'
+          )
+        end
+
+        let!(:another_user) do
+          User.create!(
+            name: 'anothermanageduser',
+            password: 'password',
+            password_confirmation: 'password',
+            role_id: @roles[:student].id,
+            full_name: 'Another Managed User',
+            email: 'anothermanageduser@example.com'
+          )
+        end
+
+        let(:id) { super_admin_parent.id }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          ids = data.map { |u| u['id'] }
+          expect(ids).to include(super_admin_parent.id, another_user.id, auth_user.id)
+        end
+      end
+    end
+  end
+
+  path '/users/role/{name}' do
+    parameter name: :name, in: :path, type: :string
+    parameter name: 'Authorization', in: :header, type: :string, required: true
+
+    get 'Get users by role name' do
+      tags 'Users'
+      produces 'application/json'
+
+      let!(:ta_user_1) do
+        User.create!(
+          name: 'tauserone',
+          password: 'password',
+          password_confirmation: 'password',
+          role_id: @roles[:ta].id,
+          full_name: 'TA User One',
+          email: 'tauserone@example.com'
+        )
+      end
+
+      let!(:ta_user_2) do
+        User.create!(
+          name: 'tausertwo',
+          password: 'password',
+          password_confirmation: 'password',
+          role_id: @roles[:ta].id,
+          full_name: 'TA User Two',
+          email: 'tausertwo@example.com'
+        )
+      end
+
+      let!(:student_user_for_filter) do
+        User.create!(
+          name: 'studentforfilter',
+          password: 'password',
+          password_confirmation: 'password',
+          role_id: @roles[:student].id,
+          full_name: 'Student For Filter',
+          email: 'studentforfilter@example.com'
+        )
+      end
+
+      # Scenario: valid role name returns role-scoped users
+      response '200', 'Returns users for a valid role name' do
+        let(:name) { 'teaching_assistant' }
+
+        run_test! do |response|
+          data = JSON.parse(response.body)
+          ids = data.map { |u| u['id'] }
+          expect(ids).to include(ta_user_1.id, ta_user_2.id)
+          expect(ids).not_to include(student_user_for_filter.id)
+        end
+      end
+
+      # Scenario: invalid token should return 401 for role-based listing
+      response '401', 'Unauthorized when token is invalid' do
+        let(:name) { 'student' }
+        let(:Authorization) { 'Bearer invalid_token' }
+
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to eq('Not Authorized')
+        end
+      end
+
+      # Scenario: invalid role name currently triggers server error in controller
+      # (graceful handling would be 404 after controller fix)
+      response '500', 'Returns server error for invalid role name (current behavior)' do
+        let(:name) { 'role_that_does_not_exist' }
+
+        run_test!
+      end
+    end
+  end
 end
diff --git a/spec/requests/api/v1/users_spec.rb b/spec/requests/api/v1/users_spec.rb
deleted file mode 100644
index d61e9a3b7..000000000
--- a/spec/requests/api/v1/users_spec.rb
+++ /dev/null
@@ -1,222 +0,0 @@
-# frozen_string_literal: true
-
-require 'rails_helper'
-require 'json_web_token'
-
-RSpec.describe 'Users API', type: :request do
-  before(:all) do
-    @roles = create_roles_hierarchy
-    @institution = Institution.first || Institution.create!(name: "Test Institution")
-  end
-
-  let(:student_role) { @roles[:student] }
-  let(:instructor_role) { @roles[:instructor] }
-
-  let(:instructor) do
-    User.create!(
-      name: 'instructor_test',
-      password: 'password123',
-      role_id: instructor_role.id,
-      full_name: 'Instructor Test',
-      email: 'instructor@example.com',
-      institution: @institution
-    )
-  end
-
-  let(:student) do
-    User.create!(
-      name: 'student_test',
-      password: 'password123',
-      role_id: student_role.id,
-      full_name: 'Student Test',
-      email: 'student@example.com',
-      institution: @institution
-    )
-  end
-
-  let(:token) { JsonWebToken.encode({ id: instructor.id }) }
-  let(:headers) { { 'Authorization' => "Bearer #{token}" } }
-
-  describe 'GET /users' do
-    it 'returns 200 with list of users when authenticated' do
-      get '/users', headers: headers
-
-      expect(response.status).to eq(200)
-      json_response = JSON.parse(response.body)
-
-      expect(json_response).to be_an(Array)
-
-      # Verify instructor is in list
-      instructor_data = json_response.find { |u| u['id'] == instructor.id }
-      expect(instructor_data).to be_present
-      expect(instructor_data['name']).to eq(instructor.name)
-      expect(instructor_data['email']).to eq(instructor.email)
-      expect(instructor_data['full_name']).to eq(instructor.full_name)
-      expect(instructor_data['role']).to be_a(Hash)
-      expect(instructor_data['role']['id']).to eq(instructor_role.id)
-
-      # Verify sensitive data is not included
-      expect(instructor_data).not_to have_key('password_digest')
-      expect(instructor_data).not_to have_key('password')
-    end
-
-    it 'returns 401 when not authenticated' do
-      get '/users'
-
-      expect(response.status).to eq(401)
-    end
-
-    it 'returns payload with all users' do
-      # Create multiple users
-      user1 = instructor
-      user2 = student
-
-      get '/users', headers: headers
-
-      json_response = JSON.parse(response.body)
-      user_ids = json_response.map { |u| u['id'] }
-
-      expect(user_ids).to include(user1.id, user2.id)
-    end
-  end
-
-  describe 'GET /users/:id' do
-    it 'returns 200 with user data for existing user' do
-      get "/users/#{student.id}", headers: headers
-
-      expect(response.status).to eq(200)
-      json_response = JSON.parse(response.body)
-
-      expect(json_response['id']).to eq(student.id)
-      expect(json_response['name']).to eq(student.name)
-      expect(json_response['email']).to eq(student.email)
-      expect(json_response['full_name']).to eq(student.full_name)
-      expect(json_response['role']).to be_a(Hash)
-      expect(json_response['role']['id']).to eq(student_role.id)
-
-      # Verify sensitive data is not included
-      expect(json_response).not_to have_key('password_digest')
-      expect(json_response).not_to have_key('password')
-    end
-
-    it 'returns 404 for non-existent user' do
-      get '/users/999999', headers: headers
-
-      expect(response.status).to eq(404)
-      json_response = JSON.parse(response.body)
-      expect(json_response['error']).to be_present
-    end
-
-    it 'returns 401 when not authenticated' do
-      get "/users/#{student.id}"
-
-      expect(response.status).to eq(401)
-    end
-
-    it 'includes institution data in response' do
-      get "/users/#{instructor.id}", headers: headers
-
-      json_response = JSON.parse(response.body)
-      expect(json_response['institution']).to be_a(Hash)
-      expect(json_response['institution']['id']).to eq(@institution.id)
-      expect(json_response['institution']['name']).to eq(@institution.name)
-    end
-
-    it 'includes parent field with nil when no parent' do
-      get "/users/#{student.id}", headers: headers
-
-      json_response = JSON.parse(response.body)
-      expect(json_response['parent']).to eq({ 'id' => nil, 'name' => nil })
-    end
-  end
-
-  describe 'POST /users' do
-    it 'creates a user with valid parameters' do
-      user_params = {
-        user: {
-          name: 'new_user',
-          email: 'newuser@example.com',
-          full_name: 'New User',
-          password: 'securepass123',
-          password_confirmation: 'securepass123',
-          role_id: student_role.id,
-          institution_id: @institution.id
-        }
-      }
-
-      post '/users', params: user_params, headers: headers
-
-      expect(response.status).to eq(201)
-      json_response = JSON.parse(response.body)
-
-      expect(json_response['name']).to eq('new_user')
-      expect(json_response['email']).to eq('newuser@example.com')
-      expect(json_response['full_name']).to eq('New User')
-      expect(json_response).not_to have_key('password_digest')
-    end
-
-    it 'returns 422 when password is too short' do
-      user_params = {
-        user: {
-          name: 'bad_user',
-          email: 'bad@example.com',
-          full_name: 'Bad User',
-          password: '123',
-          password_confirmation: '123',
-          role_id: student_role.id
-        }
-      }
-
-      post '/users', params: user_params, headers: headers
-
-      expect(response.status).to eq(422)
-    end
-  end
-
-  describe 'PATCH /users/:id' do
-    it 'updates user successfully' do
-      update_params = {
-        user: { full_name: 'Updated Name' }
-      }
-
-      patch "/users/#{student.id}", params: update_params, headers: headers
-
-      expect(response.status).to eq(200)
-      json_response = JSON.parse(response.body)
-      expect(json_response['full_name']).to eq('Updated Name')
-    end
-
-    it 'returns 404 for non-existent user' do
-      update_params = {
-        user: { full_name: 'Does Not Matter' }
-      }
-
-      patch '/users/999999', params: update_params, headers: headers
-
-      expect(response.status).to eq(404)
-    end
-  end
-
-  describe 'DELETE /users/:id' do
-    it 'deletes user successfully' do
-      user_to_delete = User.create!(
-        name: 'user_to_delete',
-        password: 'password123',
-        role_id: student_role.id,
-        full_name: 'To Delete',
-        email: 'delete@example.com'
-      )
-
-      delete "/users/#{user_to_delete.id}", headers: headers
-
-      expect(response.status).to eq(204)
-      expect(User.find_by(id: user_to_delete.id)).to be_nil
-    end
-
-    it 'returns 404 for non-existent user' do
-      delete '/users/999999', headers: headers
-
-      expect(response.status).to eq(404)
-    end
-  end
-end

From 3031f7610f143b9de0eb7e91281dd7c8111d22c7 Mon Sep 17 00:00:00 2001
From: Saladin Al-Bataineh <saalbata_ncstate@users.noreply.github.com>
Date: Mon, 27 Apr 2026 21:54:11 -0400
Subject: [PATCH 5/6] Add User model specs: validations, associations, scopes,
 role checks, defaults

- Add presence, uniqueness, format, and length validation tests for
  name, email, and full_name fields
- Add association tests for all belongs_to and has_many declarations
  using shoulda-matchers
- Fix has_many :assignments to use through: :participants
- Add has_many :instructed_assignments with correct foreign key
- Remove incorrect has_many :invitations (no valid foreign key path)
- Fix scopes to use joins(:role).where(roles: { name: }) instead of
  hardcoded Role::*_ID constants that do not exist
- Add scope tests with two-sided inclusion/exclusion assertions
- Add delegated role predicate tests for student?, ta?, instructor?,
  administrator?, super_administrator?
- Add defaults tests verifying after_initialize sets all flags correctly
---
 app/models/user.rb       |  26 +++--
 spec/models/user_spec.rb | 238 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 255 insertions(+), 9 deletions(-)

diff --git a/app/models/user.rb b/app/models/user.rb
index 07493a4ac..038c7dacd 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -16,17 +16,25 @@ class User < ApplicationRecord
   belongs_to :institution, optional: true
   belongs_to :parent, class_name: 'User', optional: true
   has_many :users, foreign_key: 'parent_id', dependent: :nullify
-  has_many :invitations
-  has_many :assignments
+  #Looking at invitation.rb, invitations relate to users only through participants 
+  #(from_id and to_id both point to AssignmentParticipant, not User). 
+  #There is no user foreign key on the invitations table at all.
+ # has_many :invitations
+  has_many :participants
+ # A user participates in assignments via the participants join table
+  has_many :assignments, through: :participants 
+# A user can also be the instructor of many assignments directly 
+# via instructor_id on the assignments table
+  has_many :instructed_assignments, class_name: 'Assignment', foreign_key: 'instructor_id'
   has_many :teams_users, dependent: :destroy
   has_many :teams, through: :teams_users
-  has_many :participants
-
-  scope :students, -> { where role_id: Role::STUDENT }
-  scope :tas, -> { where role_id: Role::TEACHING_ASSISTANT }
-  scope :instructors, -> { where role_id: Role::INSTRUCTOR }
-  scope :administrators, -> { where role_id: Role::ADMINISTRATOR }
-  scope :superadministrators, -> { where role_id: Role::SUPER_ADMINISTRATOR }
+  
+  #join on role name instead of hardcoded IDs, matching create_roles_heirarchy
+  scope :students,           -> { joins(:role).where(roles: { name: 'Student' }) }
+  scope :tas,                -> { joins(:role).where(roles: { name: 'Teaching Assistant' }) }
+  scope :instructors,        -> { joins(:role).where(roles: { name: 'Instructor' }) }
+  scope :administrators,     -> { joins(:role).where(roles: { name: 'Administrator' }) }
+  scope :superadministrators,-> { joins(:role).where(roles: { name: 'Super Administrator' }) }
 
   delegate :student?, to: :role
   delegate :ta?, to: :role
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 12e61462e..c743b3d90 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -3,6 +3,244 @@
 require 'rails_helper'
 
 RSpec.describe User, type: :model do
+
+
+  describe 'validations' do
+    let(:role) { create(:role, :student) }
+
+    describe 'name' do
+      it 'is invalid without a name' do
+        user = User.new(email: 'test@example.com', full_name: 'Test User', role: role, password: 'password')
+        expect(user).not_to be_valid
+        expect(user.errors[:name]).to include("can't be blank")
+      end
+
+      it 'is invalid when name is not unique' do
+        create(:user, name: 'duplicate', role: role)
+        user = User.new(name: 'duplicate', email: 'other@example.com', full_name: 'Other User', role: role, password: 'password')
+        expect(user).not_to be_valid
+        expect(user.errors[:name]).to include('has already been taken')
+      end
+
+      it 'is valid with a unique name' do
+        user = User.new(name: 'uniqueuser', email: 'unique@example.com', full_name: 'Unique User', role: role, password: 'password')
+        expect(user).to be_valid
+      end
+    end 
+
+    describe 'email' do
+      it 'is invalid without an email' do
+        user = User.new(name: 'testuser', full_name: 'Test User', role: role, password: 'password')
+        expect(user).not_to be_valid
+        expect(user.errors[:email]).to include("can't be blank")
+      end
+
+      it 'is invalid with a malformed email' do
+        user = User.new(name: 'testuser', email: 'not-an-email', full_name: 'Test User', role: role, password: 'password')
+        expect(user).not_to be_valid
+        expect(user.errors[:email]).to be_present
+      end
+
+      it 'is valid with a properly formatted email' do
+        user = User.new(name: 'testuser', email: 'valid@example.com', full_name: 'Test User', role: role, password: 'password')
+        expect(user).to be_valid
+      end
+    end
+
+    describe 'full_name' do
+      it 'is invalid without a full_name' do
+        user = User.new(name: 'testuser', email: 'test@example.com', role: role, password: 'password')
+        expect(user).not_to be_valid
+        expect(user.errors[:full_name]).to include("can't be blank")
+      end
+
+      it 'is invalid when full_name exceeds 50 characters' do
+        user = User.new(name: 'testuser', email: 'test@example.com', full_name: 'A' * 51, role: role, password: 'password')
+        expect(user).not_to be_valid
+        expect(user.errors[:full_name]).to include('is too long (maximum is 50 characters)')
+      end
+
+      it 'is valid with a full_name at exactly 50 characters' do
+        user = User.new(name: 'testuser', email: 'test@example.com', full_name: 'A' * 50, role: role, password: 'password')
+        expect(user).to be_valid
+      end
+    end
+  end
+
+  # Use shoulda-matchers to assert the association is declared correctly on the model
+  # inspects the model's reflection metadata rather than hitting the database
+  describe 'associations' do
+    it { is_expected.to belong_to(:role) }
+    it { is_expected.to belong_to(:institution).optional }
+    it { is_expected.to belong_to(:parent).class_name('User').optional }
+    it { is_expected.to have_many(:users).with_foreign_key('parent_id').dependent(:nullify) }
+    it { is_expected.to have_many(:assignments).through(:participants) }
+    it { is_expected.to have_many(:instructed_assignments).class_name('Assignment').with_foreign_key('instructor_id') }
+    it { is_expected.to have_many(:teams_users).dependent(:destroy) }
+    it { is_expected.to have_many(:teams).through(:teams_users) }
+    it { is_expected.to have_many(:participants) }
+  end 
+
+  describe 'scopes' do
+    let(:student_role)    { create(:role, name: 'Student') }
+    let(:ta_role)         { create(:role, name: 'Teaching Assistant') }
+    let(:instructor_role) { create(:role, name: 'Instructor') }
+    let(:admin_role)      { create(:role, name: 'Administrator') }
+    let(:super_admin_role){ create(:role, name: 'Super Administrator') }
+
+    describe '.students' do
+      it 'includes users with student role and excludes others' do
+        student = create(:user, role: student_role)
+        instructor = create(:user, role: instructor_role)
+
+        expect(User.students).to include(student)
+        expect(User.students).not_to include(instructor)
+      end
+    end
+
+    describe '.tas' do
+      it 'includes users with teaching assistant role and excludes others' do
+        ta = create(:user, role: ta_role)
+        student = create(:user, role: student_role)
+
+        expect(User.tas).to include(ta)
+        expect(User.tas).not_to include(student)
+      end
+    end
+
+    describe '.instructors' do
+      it 'includes users with instructor role and excludes others' do
+        instructor = create(:user, role: instructor_role)
+        student = create(:user, role: student_role)
+
+        expect(User.instructors).to include(instructor)
+        expect(User.instructors).not_to include(student)
+      end
+    end
+
+    describe '.administrators' do
+      it 'includes users with administrator role and excludes others' do
+        admin = create(:user, role: admin_role)
+        student = create(:user, role: student_role)
+
+        expect(User.administrators).to include(admin)
+        expect(User.administrators).not_to include(student)
+      end
+    end
+
+    describe '.superadministrators' do
+      it 'includes users with super administrator role and excludes others' do
+        super_admin = create(:user, role: super_admin_role)
+        student = create(:user, role: student_role)
+
+        expect(User.superadministrators).to include(super_admin)
+        expect(User.superadministrators).not_to include(student)
+      end
+    end
+  end
+
+  describe 'delegated role checks' do
+    let(:student_role)     { create(:role, name: 'Student') }
+    let(:ta_role)          { create(:role, name: 'Teaching Assistant') }
+    let(:instructor_role)  { create(:role, name: 'Instructor') }
+    let(:admin_role)       { create(:role, name: 'Administrator') }
+    let(:super_admin_role) { create(:role, name: 'Super Administrator') }
+
+    describe '#student?' do
+      it 'returns true for a student role' do
+        user = create(:user, role: student_role)
+        expect(user.student?).to be_truthy 
+      end
+
+      it 'returns false for a non-student role' do
+        user = create(:user, role: instructor_role)
+        expect(user.student?).to be_falsy
+      end
+    end
+
+    describe '#ta?' do
+      it 'returns true for a teaching assistant role' do
+        user = create(:user, role: ta_role)
+        expect(user.ta?).to be_truthy
+      end
+
+      it 'returns false for a non-ta role' do
+        user = create(:user, role: student_role)
+        expect(user.ta?).to be_falsy
+      end
+    end
+
+    describe '#instructor?' do
+      it 'returns true for an instructor role' do
+        user = create(:user, role: instructor_role)
+        expect(user.instructor?).to be_truthy
+      end
+
+      it 'returns false for a non-instructor role' do
+        user = create(:user, role: student_role)
+        expect(user.instructor?).to be_falsy
+      end
+    end
+
+    describe '#administrator?' do
+      it 'returns true for an administrator role' do
+        user = create(:user, role: admin_role)
+        expect(user.administrator?).to be_truthy
+      end
+
+      it 'returns false for a non-administrator role' do
+        user = create(:user, role: student_role)
+        expect(user.administrator?).to be_falsy
+      end
+    end
+
+    describe '#super_administrator?' do
+      it 'returns true for a super administrator role' do
+        user = create(:user, role: super_admin_role)
+        expect(user.super_administrator?).to be_truthy
+      end
+
+      it 'returns false for a plain administrator role' do
+        user = create(:user, role: admin_role)
+        expect(user.super_administrator?).to be_falsy
+      end
+
+      it 'returns false for a non-admin role' do
+        user = create(:user, role: student_role)
+        expect(user.super_administrator?).to be_falsy
+      end
+    end
+  end
+
+  describe 'defaults' do
+    subject(:user) { User.new }
+
+    it 'sets is_new_user to true' do
+      expect(user.is_new_user).to be true
+    end
+
+    it 'sets copy_of_emails to false' do
+      expect(user.copy_of_emails).to be false
+    end
+
+    it 'sets email_on_review to false' do
+      expect(user.email_on_review).to be false
+    end
+
+    it 'sets email_on_submission to false' do
+      expect(user.email_on_submission).to be false
+    end
+
+    it 'sets email_on_review_of_review to false' do
+      expect(user.email_on_review_of_review).to be false
+    end
+
+    it 'sets etc_icons_on_homepage to true' do
+      expect(user.etc_icons_on_homepage).to be true
+    end
+  end
+
+
   describe 'password validations' do
     it 'requires a password when password_digest is blank' do
       user = User.new(name: 'testuser', email: 'test@example.com', full_name: 'Test User', role: create(:role))

From f7c37299d967a1539759a035f60f9e7500aa75ad Mon Sep 17 00:00:00 2001
From: AtharvaW195 <atharvawaingankar195@gmail.com>
Date: Tue, 28 Apr 2026 12:44:12 -0400
Subject: [PATCH 6/6] Suggested Fixes Added

---
 app/controllers/users_controller.rb           |  4 ++++
 app/models/ta.rb                              |  7 ++++---
 app/models/user.rb                            | 21 +++++++++++--------
 ..._update_teams_participants_unique_index.rb |  9 ++++++++
 db/schema.rb                                  |  4 ++--
 spec/models/user_spec.rb                      |  6 ++++++
 spec/requests/api/v1/users_controller_spec.rb | 12 ++++++-----
 7 files changed, 44 insertions(+), 19 deletions(-)
 create mode 100644 db/migrate/20260428000000_update_teams_participants_unique_index.rb

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 1581901b9..0a5b287bd 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -68,6 +68,10 @@ def managed_users
   def role_users
     name = params[:name].split('_').map(&:capitalize).join(' ')
     role = Role.find_by(name:)
+    unless role
+      render json: { error: "Role '#{name}' not found" }, status: :not_found
+      return
+    end
     users = role.users
     render json: users, status: :ok
   rescue ActiveRecord::RecordNotFound => e
diff --git a/app/models/ta.rb b/app/models/ta.rb
index f56ee46e7..aa4162c0d 100644
--- a/app/models/ta.rb
+++ b/app/models/ta.rb
@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class Ta < User
+  validates :parent_id, presence: true
+
   # Get all users whose parent is the TA
   # @return [Array<User>] all users that belongs to courses that is mapped to the TA
   def managed_users
@@ -12,7 +14,6 @@ def my_instructor
   end
 
   def courses_assisted_with
-    courses = TaMapping.where(user_id: id)
-    courses.map { |c| Course.find(c.course_id) }
+    Course.joins(:ta_mappings).where(ta_mappings: { user_id: id }).distinct.to_a
   end
-end
\ No newline at end of file
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index 038c7dacd..1f507a6ac 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -16,15 +16,18 @@ class User < ApplicationRecord
   belongs_to :institution, optional: true
   belongs_to :parent, class_name: 'User', optional: true
   has_many :users, foreign_key: 'parent_id', dependent: :nullify
-  #Looking at invitation.rb, invitations relate to users only through participants 
-  #(from_id and to_id both point to AssignmentParticipant, not User). 
-  #There is no user foreign key on the invitations table at all.
- # has_many :invitations
+
+  # Looking at invitation.rb, invitations relate to users only through participants.
+  # (from_id and to_id both point to AssignmentParticipant, not User).
+  # There is no user foreign key on the invitations table at all.
+  # has_many :invitations
   has_many :participants
- # A user participates in assignments via the participants join table
-  has_many :assignments, through: :participants 
-# A user can also be the instructor of many assignments directly 
-# via instructor_id on the assignments table
+
+  # A user participates in assignments via the participants join table
+  has_many :assignments, through: :participants
+
+  # A user can also be the instructor of many assignments directly
+  # via instructor_id on the assignments table
   has_many :instructed_assignments, class_name: 'Assignment', foreign_key: 'instructor_id'
   has_many :teams_users, dependent: :destroy
   has_many :teams, through: :teams_users
@@ -124,7 +127,7 @@ def teaching_assistant_for?(student)
   end
 
   def teaching_assistant?
-    role.ta? ? true : false
+    !!role.ta?
   end
 
   def self.from_params(params)
diff --git a/db/migrate/20260428000000_update_teams_participants_unique_index.rb b/db/migrate/20260428000000_update_teams_participants_unique_index.rb
new file mode 100644
index 000000000..0f4ec474a
--- /dev/null
+++ b/db/migrate/20260428000000_update_teams_participants_unique_index.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class UpdateTeamsParticipantsUniqueIndex < ActiveRecord::Migration[8.0]
+  def change
+    remove_index :teams_participants, name: :index_teams_participants_on_participant_id_unique
+    add_index :teams_participants, %i[team_id participant_id], unique: true,
+                                                              name: :index_teams_participants_on_team_participant_unique
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index e9103fc77..15528e5f4 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.0].define(version: 2026_03_14_000000) do
+ActiveRecord::Schema[8.0].define(version: 2026_04_28_000000) do
   create_table "account_requests", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
     t.string "username"
     t.string "full_name"
@@ -408,7 +408,7 @@
     t.bigint "participant_id", null: false
     t.integer "user_id", null: false
     t.index ["participant_id"], name: "index_teams_participants_on_participant_id"
-    t.index ["participant_id"], name: "index_teams_participants_on_participant_id_unique", unique: true
+    t.index ["team_id", "participant_id"], name: "index_teams_participants_on_team_participant_unique", unique: true
     t.index ["team_id"], name: "index_teams_participants_on_team_id"
     t.index ["user_id"], name: "index_teams_participants_on_user_id"
   end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index c743b3d90..a0f9fb9d3 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -399,6 +399,12 @@
         expect(ta.instructor_id).to eq(instructor.id)
       end
 
+      it 'returns nil instructor id for a TA without a parent record' do
+        ta = create(:user, role: ta_role, institution: institution, parent: nil)
+
+        expect(ta.instructor_id).to be_nil
+      end
+
       it 'raises NotImplementedError for student role' do
         student = create(:user, role: student_role, institution: institution)
         expect { student.instructor_id }.to raise_error(NotImplementedError, /Unknown role/)
diff --git a/spec/requests/api/v1/users_controller_spec.rb b/spec/requests/api/v1/users_controller_spec.rb
index 3e8e1d3b2..887d8e56f 100644
--- a/spec/requests/api/v1/users_controller_spec.rb
+++ b/spec/requests/api/v1/users_controller_spec.rb
@@ -154,7 +154,7 @@
           )
         end
 
-         let(:payload) do
+        let(:payload) do
           {
             user: {
               name: 'duplicateuser',
@@ -665,12 +665,14 @@
         end
       end
 
-      # Scenario: invalid role name currently triggers server error in controller
-      # (graceful handling would be 404 after controller fix)
-      response '500', 'Returns server error for invalid role name (current behavior)' do
+      # Scenario: invalid role name returns 404 from controller
+      response '404', 'Returns not found for invalid role name' do
         let(:name) { 'role_that_does_not_exist' }
 
-        run_test!
+        run_test! do |response|
+          body = JSON.parse(response.body)
+          expect(body['error']).to eq("Role 'Role That Does Not Exist' not found")
+        end
       end
     end
   end