Skip to content
Open
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
2 changes: 1 addition & 1 deletion app/controllers/account_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def account_request_params
elsif !['Approved', 'Rejected'].include?(params[:account_request][:status])
raise StandardError, 'Status can only be Approved or Rejected'
end
params.require(:account_request).permit(:username, :full_name, :email, :status, :introduction, :role_id, :institution_id)
params.require(:account_request).permit(:username, :full_name, :email, :status, :introduction, :role_id, :institution_id, :date_format_pref)
end

# Create a new user if account request is approved
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/authentication_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def login
user = User.find_by(name: params[:user_name]) || User.find_by(email: params[:user_name])
if user&.authenticate(params[:password])
payload = { id: user.id, name: user.name, full_name: user.full_name, role: user.role.name,
institution_id: user.institution.id }
institution_id: user.institution.id, date_format_pref: user.date_format_pref }
token = JsonWebToken.encode(payload, 24.hours.from_now)
render json: { token: }, status: :ok
else
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/institutions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class InstitutionsController < ApplicationController
rescue_from ActiveRecord::RecordNotFound, with: :institution_not_found
def action_allowed?
has_role?('Instructor')
has_role?('Instructor') || has_role?('Administrator')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using the disjunction in a lot of action_allowed methods, I believe there is a has_instructor_privileges? method that implements the disjunction.

end
# GET /institutions
def index
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def role_users
def user_params
params.require(:user).permit(:id, :name, :role_id, :full_name, :email, :parent_id, :institution_id,
:email_on_review, :email_on_submission, :email_on_review_of_review,
:handle, :copy_of_emails, :password, :password_confirmation)
:handle, :copy_of_emails, :password, :password_confirmation, :date_format_pref)
end

def user_not_found
Expand Down
12 changes: 6 additions & 6 deletions app/models/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ class Role < ApplicationRecord
belongs_to :parent, class_name: 'Role', optional: true
has_many :users, dependent: :nullify

# Role IDs
STUDENT_ID = 1
TEACHING_ASSISTANT_ID = 2
# Role IDs (must match db/seeds.rb)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a db table with these correspondences?

SUPER_ADMINISTRATOR_ID = 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this just be SUPER_ADMINISTRATOR, etc.? All files would need to be refactored to effect this change, but it would enhance readability. But, would it lead to namespace conflicts? If it doesn't cause name conflicts, it would be worth doing.

ADMINISTRATOR_ID = 2
INSTRUCTOR_ID = 3
ADMINISTRATOR_ID = 4
SUPER_ADMINISTRATOR_ID = 5
TEACHING_ASSISTANT_ID = 4
STUDENT_ID = 5

def super_administrator?
name['Super Administrator']
Expand Down Expand Up @@ -67,4 +67,4 @@ def as_json(options = nil)
options = options || {} # Ensure options is a hash
super(options.merge({ only: %i[id name parent_id] }))
end
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an EOL

30 changes: 15 additions & 15 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ class User < ApplicationRecord
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 }
scope :students, -> { where role_id: Role::STUDENT_ID }
scope :tas, -> { where role_id: Role::TEACHING_ASSISTANT_ID }
scope :instructors, -> { where role_id: Role::INSTRUCTOR_ID }
scope :administrators, -> { where role_id: Role::ADMINISTRATOR_ID }
scope :superadministrators, -> { where role_id: Role::SUPER_ADMINISTRATOR_ID }

delegate :student?, to: :role
delegate :ta?, to: :role
Expand All @@ -34,17 +34,17 @@ class User < ApplicationRecord
delegate :super_administrator?, to: :role

def self.instantiate(record)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refactor, according to the recommendations in https://chatgpt.com/share/694abed0-f590-800d-ac2a-4d87911c5de3.

case record.role
when Role::TEACHING_ASSISTANT
case record.role_id
when Role::TEACHING_ASSISTANT_ID
record.becomes(Ta)
when Role::INSTRUCTOR
when Role::INSTRUCTOR_ID
record.becomes(Instructor)
when Role::ADMINISTRATOR
when Role::ADMINISTRATOR_ID
record.becomes(Administrator)
when Role::SUPER_ADMINISTRATOR
when Role::SUPER_ADMINISTRATOR_ID
record.becomes(SuperAdministrator)
else
super
record
end
end

Expand Down Expand Up @@ -72,10 +72,10 @@ def reset_password
# 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_id
when Role::INSTRUCTOR_ID, Role::ADMINISTRATOR_ID, Role::SUPER_ADMINISTRATOR_ID
id
when Role::TEACHING_ASSISTANT
when Role::TEACHING_ASSISTANT_ID
my_instructor
else
raise NotImplementedError, "Unknown role: #{role.name}"
Expand Down Expand Up @@ -153,4 +153,4 @@ def generate_jwt
JWT.encode({ id: id, exp: 60.days.from_now.to_i }, Rails.application.credentials.secret_key_base)
end

end
end
5 changes: 5 additions & 0 deletions db/migrate/20251201034750_add_date_format_pref_to_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddDateFormatPrefToUsers < ActiveRecord::Migration[8.0]
def change
add_column :users, :date_format_pref, :string, default: "mm/dd/yyyy"
end
end
3 changes: 2 additions & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading