Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:
- name: Install system dependencies
run: |
sudo apt-get update
sudo apt-get install -y netcat
sudo apt-get install -y netcat-traditional

- name: Install Ruby dependencies
run: |
Expand Down Expand Up @@ -73,4 +73,4 @@ jobs:
with:
context: .
push: false
tags: expertiza-backend:latest
tags: expertiza-backend:latest
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ group :development, :test do
gem 'rswag-specs'
gem 'rubocop'
gem 'simplecov', require: false, group: :test
gem 'database_cleaner-active_record'
end

group :development do
Expand Down
5 changes: 0 additions & 5 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ GEM
builder (3.2.4)
concurrent-ruby (1.2.2)
crass (1.0.6)
database_cleaner-active_record (2.2.0)
activerecord (>= 5.a)
database_cleaner-core (~> 2.0.0)
database_cleaner-core (2.0.1)
date (3.3.3)
debug (1.8.0)
irb (>= 1.5.0)
Expand Down Expand Up @@ -256,7 +252,6 @@ PLATFORMS
DEPENDENCIES
bcrypt (~> 3.1.7)
bootsnap
database_cleaner-active_record
debug
factory_bot_rails
faker
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/api/v1/account_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,11 @@ def account_request_params

# Create a new user if account request is approved
def create_approved_user
@new_user = User.new(name: @account_request.username, role_id: @account_request.role_id, institution_id: @account_request.institution_id, fullname: @account_request.full_name, email: @account_request.email, password: 'password')
if User.exists?(email: @account_request.email)
render json: { error: 'A user with this email already exists. Cannot approve the account request.' }, status: :unprocessable_entity
return
end
@new_user = User.new(name: @account_request.username, role_id: @account_request.role_id, institution_id: @account_request.institution_id, full_name: @account_request.full_name, email: @account_request.email, password: 'password')
if @new_user.save
render json: { success: 'Account Request Approved and User successfully created.', user: @new_user}, status: :ok
else
Expand Down
5 changes: 5 additions & 0 deletions app/controllers/api/v1/assignments_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class Api::V1::AssignmentsController < ApplicationController
rescue_from ActiveRecord::RecordNotFound, with: :not_found

# GET /api/v1/assignments
def index
Expand Down Expand Up @@ -32,6 +33,10 @@ def update
end
end

def not_found
render json: { error: "Assignment not found" }, status: :not_found
end

# DELETE /api/v1/assignments/:id
def destroy
assignment = Assignment.find_by(id: params[:id])
Expand Down
11 changes: 10 additions & 1 deletion app/controllers/api/v1/bookmarks_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
class Api::V1::BookmarksController < ApplicationController

rescue_from ActiveRecord::RecordNotFound, with: :not_found

def action_allowed?
has_privileges_of?('Student')
end
# Index method returns the list of JSON objects of the bookmark
# GET on /bookmarks
def index
Expand Down Expand Up @@ -43,6 +47,11 @@ def update
end
end

# Handle the case when an invalid bookmark id is being passed
def not_found
render json: { error: "Couldn't find Bookmark" }, status: :not_found
end

# Destroy method deletes the bookmark object with id- {:id}
# DELETE on /bookmarks/:id
def destroy
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/courses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Api::V1::CoursesController < ApplicationController
rescue_from ActionController::ParameterMissing, with: :parameter_missing

def action_allowed?
has_required_role?('Instructor')
has_privileges_of?('Instructor')
end

# GET /courses
Expand Down
3 changes: 3 additions & 0 deletions app/controllers/api/v1/institutions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
class Api::V1::InstitutionsController < ApplicationController
rescue_from ActiveRecord::RecordNotFound, with: :institution_not_found
def action_allowed?
has_role?('Instructor')
end
# GET /institutions
def index
@institutions = Institution.all
Expand Down
4 changes: 3 additions & 1 deletion app/controllers/api/v1/questions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
class Api::V1::QuestionsController < ApplicationController

def action_allowed?
has_role?('Instructor')
end
# Index method returns the list of questions JSON object
# GET on /questions
def index
Expand Down
4 changes: 4 additions & 0 deletions app/controllers/api/v1/roles_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ class Api::V1::RolesController < ApplicationController
# rescue_from ActiveRecord::RecordNotFound, with: :role_not_found
rescue_from ActionController::ParameterMissing, with: :parameter_missing

def action_allowed?
has_privileges_of?('Administrator')
end

# GET /roles
def index
roles = Role.order(:id)
Expand Down
3 changes: 3 additions & 0 deletions app/controllers/api/v1/student_tasks_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
class Api::V1::StudentTasksController < ApplicationController

# List retrieves all student tasks associated with the current logged-in user.
def action_allowed?
has_privileges_of?('Student')
end
def list
# Retrieves all tasks that belong to the current user.
@student_tasks = StudentTask.from_user(current_user)
Expand Down
4 changes: 3 additions & 1 deletion app/controllers/concerns/authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module Authorization

# Authorize all actions
def authorize
Rails.logger.info "Authorization Header: #{request.headers['Authorization']}"
Rails.logger.info "Current User: #{current_user&.inspect}"
unless all_actions_allowed?
render json: {
error: "You are not authorized to #{params[:action]} this #{params[:controller]}"
Expand All @@ -13,7 +15,7 @@ def authorize

# Check if all actions are allowed
def all_actions_allowed?
return true if has_required_role?('Super Administrator')
return true if has_privileges_of?('Super Administrator')
action_allowed?
end

Expand Down
16 changes: 8 additions & 8 deletions app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Course < ApplicationRecord
validates :name, presence: true
validates :directory_path, presence: true
has_many :ta_mappings, dependent: :destroy
has_many :tas, through: :ta_mappings
has_many :tas, through: :ta_mappings, source: :ta

# Returns the submission directory for the course
def path
Expand All @@ -16,25 +16,25 @@ def path
def add_ta(user)
if user.nil?
return { success: false, message: "The user with id #{user.id} does not exist" }
elsif TaMapping.exists?(ta_id: user.id, course_id: id)
elsif TaMapping.exists?(user_id: user.id, course_id: id)
return { success: false, message: "The user with id #{user.id} is already a TA for this course." }
else
ta_mapping = TaMapping.create(ta_id: user.id, course_id: id)
ta_mapping = TaMapping.create(user_id: user.id, course_id: id)
user.update(role: Role::TEACHING_ASSISTANT)
if ta_mapping.save
return { success: true, data: ta_mapping.slice(:course_id, :ta_id) }
return { success: true, data: ta_mapping.slice(:course_id, :user_id) }
else
return { success: false, message: ta_mapping.errors }
end
end
end

# Removes Teaching Assistant from the course
def remove_ta(ta_id)
ta_mapping = ta_mappings.find_by(ta_id: ta_id, course_id: :id)
def remove_ta(user_id)
ta_mapping = ta_mappings.find_by(user_id: user_id, course_id: :id)
return { success: false, message: "No TA mapping found for the specified course and TA" } if ta_mapping.nil?
ta = User.find(ta_mapping.ta_id)
ta_count = TaMapping.where(ta_id: ta_id).size - 1
ta = User.find(ta_mapping.user_id)
ta_count = TaMapping.where(user_id: user_id).size - 1
if ta_count.zero?
ta.update(role: Role::STUDENT)
end
Expand Down
1 change: 1 addition & 0 deletions app/models/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class Role < ApplicationRecord
SUPER_ADMINISTRATOR = find_by_name('Super Administrator')
end


def super_administrator?
name['Super Administrator']
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/ta_mapping.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
class TaMapping < ApplicationRecord
belongs_to :course
belongs_to :ta
belongs_to :ta, class_name: 'User', foreign_key: 'user_id'

#Returns course ids of the TA
def self.get_course_ids(user_id)
TaMapping.find_by(ta_id: user_id).course_id
TaMapping.find_by(user_id: user_id).course_id
end

#Returns courses of the TA
Expand Down
32 changes: 16 additions & 16 deletions spec/controllers/concerns/authorization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
end

##########################################
# Tests for has_required_role? method
# Tests for has_privileges_of? method
##########################################
describe '#has_required_role?' do
describe '#has_privileges_of?' do
describe 'role validation' do
context 'when required_role is a string' do
let(:admin_role) { instance_double('Role') }
Expand All @@ -28,7 +28,7 @@

it 'finds the role and checks privileges' do
expect(role).to receive(:all_privileges_of?).with(admin_role).and_return(true)
expect(controller.has_required_role?('Administrator')).to be true
expect(controller.has_privileges_of?('Administrator')).to be true
end
end

Expand All @@ -37,7 +37,7 @@

it 'directly checks privileges' do
expect(role).to receive(:all_privileges_of?).with(instructor_role).and_return(false)
expect(controller.has_required_role?(instructor_role)).to be false
expect(controller.has_privileges_of?(instructor_role)).to be false
end
end
end
Expand All @@ -49,7 +49,7 @@
end

it 'returns false' do
expect(controller.has_required_role?('Administrator')).to be false
expect(controller.has_privileges_of?('Administrator')).to be false
end
end

Expand All @@ -59,28 +59,28 @@
end

it 'returns false' do
expect(controller.has_required_role?('Administrator')).to be false
expect(controller.has_privileges_of?('Administrator')).to be false
end
end
end
end

##########################################
# Tests for is_role? method
# Tests for has_role? method
##########################################
describe '#is_role?' do
describe '#has_role?' do
describe 'role matching' do
context 'when role_name is a string' do
before do
allow(role).to receive(:name).and_return('Student')
end

it 'returns true when roles match' do
expect(controller.is_role?('Student')).to be true
expect(controller.has_role?('Student')).to be true
end

it 'returns false when roles do not match' do
expect(controller.is_role?('Instructor')).to be false
expect(controller.has_role?('Instructor')).to be false
end
end

Expand All @@ -94,7 +94,7 @@
end

it 'compares using the role name' do
expect(controller.is_role?(role_object)).to be true
expect(controller.has_role?(role_object)).to be true
end
end
end
Expand All @@ -106,7 +106,7 @@
end

it 'returns false' do
expect(controller.is_role?('Student')).to be false
expect(controller.has_role?('Student')).to be false
end
end

Expand All @@ -116,7 +116,7 @@
end

it 'returns false' do
expect(controller.is_role?('Student')).to be false
expect(controller.has_role?('Student')).to be false
end
end
end
Expand All @@ -128,7 +128,7 @@
describe '#all_actions_allowed?' do
context 'when the user has the Super Administrator role' do
before do
allow(controller).to receive(:has_required_role?).with('Super Administrator').and_return(true)
allow(controller).to receive(:has_privileges_of?).with('Super Administrator').and_return(true)
end

it 'returns true' do
Expand All @@ -138,7 +138,7 @@

context 'when the user does not have the Super Administrator role' do
before do
allow(controller).to receive(:has_required_role?).with('Super Administrator').and_return(false)
allow(controller).to receive(:has_privileges_of?).with('Super Administrator').and_return(false)
allow(controller).to receive(:action_allowed?).and_return(false)
end

Expand All @@ -149,7 +149,7 @@

context 'when action_allowed? returns true' do
before do
allow(controller).to receive(:has_required_role?).with('Super Administrator').and_return(false)
allow(controller).to receive(:has_privileges_of?).with('Super Administrator').and_return(false)
allow(controller).to receive(:action_allowed?).and_return(true)
end

Expand Down
2 changes: 1 addition & 1 deletion spec/factories/courses.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
factory :course do
sequence(:name) { |n| "Course #{n}" }
sequence(:directory_path) { |n| "course_#{n}" }
association :instructor, factory: [:user, :instructor]
association :instructor, factory: [:role, :instructor]
association :institution
end
end
13 changes: 13 additions & 0 deletions spec/factories/roles.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
factory :role do
sequence(:name) { |n| "Role #{n}" }

initialize_with { Role.find_or_create_by(id: id) }

trait :student do
id { Role::STUDENT }
name { 'Student' }
Expand All @@ -27,6 +29,17 @@
id { Role::SUPER_ADMINISTRATOR }
name { 'Super Administrator' }
end

# Add a trait to create roles with a parent
trait :with_parent do
transient do
parent { nil }
end

after(:create) do |role, evaluator|
role.update(parent_id: evaluator.parent.id) if evaluator.parent
end
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/models/course_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
let(:instructor) { Instructor.create(name: 'testinstructor', email: '[email protected]', full_name: 'Test Instructor', password: '123456', role: role) }
let(:institution) { create(:institution, id: 1) }
let(:course) { create(:course, id: 1, name: 'ECE517', instructor: instructor, institution: institution) }
let(:user1) { create(:user, name: 'abcdef', full_name:'abc bbc', email: '[email protected]', password: '123456789', password_confirmation: '123456789') }
let(:user1) { create(:user, name: 'abcdef', full_name:'abc bbc', email: '[email protected]', password: '123456789', password_confirmation: '123456789', role: role) }

describe 'validations' do
it 'validates presence of name' do
Expand Down
Loading