CSC/ECE 517 Fall 2017/E1746 Refactor user.rb

E1746 [Test First Development] Refactor user.rb

This page provides a description of the Expertiza based OSS project.

About Expertiza

Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of topics the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

Problem Statement

The following tasks were accomplished in this project:

• Complete the pending tests in user_spec.rb
• Refactor get_user_list method
• Refactor self.search_users method
• Rename method and change all other places it is used
• Use find_by instead of dynamic method

Current Implementation

Missions and Solutions

• Mission 1:Complete the pending tests in user_spec.rb

There are many pending tests in the file 'user.rb'. What we have done is finishing these pending tests and makes them pass.The user_spec.rb file on github is below.

include Rails.application.routes.url_helpers

describe User do

 let(:user) do
   User.new name: 'abc', fullname: 'abc xyz', email: 'abcxyz@gmail.com', password: '12345678', password_confirmation: '12345678',
            email_on_submission: 1, email_on_review: 1, email_on_review_of_review: 0, copy_of_emails: 1, handle: 'handle'
 end
 let(:user1) { User.new name: 'abc', fullname: 'abc bbc', email: 'abcbbc@gmail.com', password: '123456789', password_confirmation: '123456789' }
 let(:user2) { User.new name: 'abc', fullname: 'abc bbc', email: 'abcbbe@gmail.com', password: '123456789', password_confirmation: '123456789' }

 describe '#name' do
   it 'returns the name of the user' do
     expect(user.name).to eq('abc')
   end
   it 'Validate presence of name which cannot be blank' do
     expect(user).to be_valid
     user.name = '  '
     expect(user).not_to be_valid
   end
   it 'Validate that name is always unique' do
     expect(user1).to validate_uniqueness_of(:name)
   end
 end

 describe '#fullname' do
   it 'returns the full name of the user' do
     expect(user.fullname).to eq('abc xyz')
   end
 end

 describe '#email' do
   it 'returns the email of the user' do
     expect(user.email).to eq('abcxyz@gmail.com')
   end

   it 'Validate presence of email which cannot be blank' do
     user.email = '  '
     expect(user).not_to be_valid
   end

   it 'Validate the email format' do
     user.email = 'a@x'
     expect(user).not_to be_valid
   end

   it 'Validate the email format' do
     user.email = 'ax.com'
     expect(user).not_to be_valid
   end

   it 'Validate the email format' do
     user.email = 'axc'
     expect(user).not_to be_valid
   end

   it 'Validate the email format' do
     user.email = '123'
     expect(user).not_to be_valid
   end

   it 'Validate the email format correctness' do
     user.email = 'a@x.com'
     expect(user).to be_valid
   end
 end

 describe '#salt_first?' do
   it 'will always return true' do
     expect(user.salt_first?).to be true
   end
 end

 describe '#get_available_users' do
   before(:each) do
     role = Role.new
   end
   it 'returns the first 10 visible users' do
     all_users = double
     user_mock1 = double(:role=>'Instructor')
     user_mock2 = double(:role=>'Administrator')
     user_mock3 = double(:role=>'Student')
     allow(@role).to receive(:get_parents).and_return(['Instructor','Administrator'])
     allow(User).to receive(:all).and_return([user_mock1,user_mock2,user_mock3])
     allow(all_users).to receive(:select).and_yield(user_mock2).and_yield(user_mock2).and_yield(user_mock3)
     expect(user.get_available_users("abc")).to eq ([user_mock1,user_mock2])
   end
 end

 describe '#can_impersonate?' do
   it 'can impersonate target user if current user is super admin' do
     allow(user1).to receive_message_chain("role.super_admin?"){true}
     expect(user1.can_impersonate?(user)).to be true
   end
   it 'can impersonate target user if current user is the TA of target user'do
     allow(user1).to receive_message_chain("role.super_admin?"){false}
     allow(user1).to receive(:is_teaching_assistant_for?).and_return(user)
     expect(user1.can_impersonate?(user)).to be true

   end
   it 'can impersonate target user if current user is the recursively parent of target user'do
     allow(user1).to receive_message_chain("role.super_admin?"){true}
     allow(user1).to receive(:is_recursively_parent_of).and_return(user)
     expect(user1.can_impersonate?(user)).to be true
   end
   it 'cannot impersonate target user if current user does not satisfy all requirements'do
     allow(user1).to receive_message_chain("role.super_admin?"){false}
     allow(user1).to receive_message_chain("role.ta?"){false}
     expect(user1.can_impersonate?(user)).to be false
   end
 end

 describe '#is_recursively_parent_of' do
   context 'when the parent of target user (user) is nil' do
     it 'returns false' do
       allow(user).to receive(:parent).and_return(nil)
       expect(user1.is_recursively_parent_of(user)).to eq false
     end
   end

   context 'when the parent of target user (user) is current user (user1)' do
     it 'returns true' do
       allow(user).to receive(:parent).and_return(user1)
       expect(user1.is_recursively_parent_of(user)).to eq true
     end
   end

   context 'when the parent of target user (user) is not current user (user1), but super admin (user2)' do
     it 'returns false' do
       allow(user).to receive(:parent).and_return(user2)
       allow(user2).to receive_message_chain("role.super_admin?") { true }
       expect(user1.is_recursively_parent_of(user)).to eq false
     end
   end
 end

 describe '#get_user_list' do
   context 'when current user is super admin' do
     it 'fetches all users' do
       user_list=double.as_null_object
       allow(user).to receive_message_chain("role.super_admin?"){ true }
       allow(User).to receive_message_chain("all.find_each").and_yield(user1).and_yield(user2)
       allow(user).to receive_message_chain("role.instructor?"){ false }
       allow(user).to receive_message_chain("role.ta?"){false}
       User.all.find_each do |user|
         user_list<<user
       end
       user.get_user_list
       end
     end

   context 'when current user is an instructor' do
     before(:each) do
       course = Course.new
       assignment = Assignment.new
     end
     it 'fetches all users in his/her course/assignment' do
       user_list = double
       course = double
       assignment = double
       allow(user).to receive_message_chain("role.super_admin?"){ false }
       allow(user).to receive_message_chain("role.instructor?"){ true }
       allow(Course).to receive_message_chain(:where,:find_each).and_yield(course)
       allow(course).to receive(:get_participants).and_return(user1)
       allow(Assignment).to receive_message_chain(:where,:find_each).and_yield(assignment)
       allow(assignment).to receive(:participants).and_return(user2)
       allow_any_instance_of(User).to receive(:empty?).and_return(false)
       allow_any_instance_of(User).to receive(:each).and_yield(user)
       allow_any_instance_of(User).to receive(:user).and_return(user)
       allow_any_instance_of(User).to receive_message_chain(:role,:hasAllPrivilegesOf).and_return(true)
       allow(user).to receive_message_chain("role.ta?"){false}
       expect(user.get_user_list).to eq ([user])
     end
   end

   context 'when current user is a TA' do
     before(:each) do
       course = Course.new
       end
     it 'fetches all users in his/her courses'do
       courses=double
       course=double
       allow(user).to receive_message_chain("role.super_admin?"){ false }
       allow(user).to receive_message_chain("role.ta?"){ true }
       allow(user).to receive_message_chain("role.instructor?"){ false }
       allow(Ta).to receive(:get_mapped_courses).and_return(courses)
       allow(courses).to receive(:each).and_yield(course)
       allow(Course).to receive(:find).and_return(course)
       allow(course).to receive(:get_participants).and_return(user1)
       allow_any_instance_of(User).to receive(:empty?).and_return(false)
       allow_any_instance_of(User).to receive(:each).and_yield(user)
       allow_any_instance_of(User).to receive(:user).and_return(user)
       allow_any_instance_of(User).to receive_message_chain(:role,:hasAllPrivilegesOf).and_return(true)
       expect(user.get_user_list).to eq ([user])
   end
   end
 end

 describe '#super_admin?' do
   it 'returns ture if role name is Super-Administrator' do
     allow(user).to receive_message_chain("role.name"){'Super-Administrator'}
     expect(user.super_admin?).to be_truthy
   end

   it 'returns false if role name is not Super-Administrator' do
     allow(user).to receive_message_chain("role.name"){'CAt'}
     expect(user.super_admin?).to be_falsey
   end
 end

 describe '#is_creator_of?' do
   it 'returns true of current user (user) is the creator of target user (user1)' do
     allow(user1).to receive(:creator).and_return(user)
     expect(user.is_creator_of?(user1)).to be true
   end

   it 'returns false of current user (user) is not the creator of target user (user1)' do
     allow(user1).to receive(:creator).and_return(user2)
     expect(user.is_creator_of?(user1)).to be false
     expect(user2.is_creator_of?(user1)).to be true

   end
 end

 describe '.import' do
   it 'raises error if import column does not equal to 3' do
     row = ["abc","abc xyz"]
     _row_header = double
     seesion = {:user=>user}
     _id = double
     expect { User.import(row, _row_header,seesion,_id) }.to raise_error("Not enough items: expect 3 columns: your login name, your full name (first and last name, not seperated with the delimiter), and your email.")
   end
   it 'updates an existing user with info from impor file' do
     row = ["abc","abc xyz","abcxyz@gamil.com"]
     _row_header = double
     seesion = {:user=>user}
     _id = double
     allow(User).to receive(:find_by_name).and_return(user)
     allow_any_instance_of(User).to receive(:nil?).and_return(false)
     allow_any_instance_of(User).to receive(:id).and_return(1)
     expect_any_instance_of(User).to receive(:save)
     User.import(row,_row_header,seesion,_id)
   end

 end

 describe '.yesorno' do
   it 'returns yes when input is true' do
     expect(User.yesorno(true)).to eq "yes"
   end
   it 'returns no when input is false' do
     expect(User.yesorno(false)).to eq "no"
   end
   it 'returns empty string when input is other content' do
     content = "TEXT"
     expect(User.yesorno(content)).to eq ""
   end
 end

 describe '.find_by_login' do
   context 'when user\'s email is stored in DB' do
     it 'finds user by email' do
       email = 'abcxyz@gmail.com'
       allow(User).to receive(:find_by_email).with(email).and_return(user)
       expect(User.find_by_login(email)).to eq user
     end
   end

   context 'when user\'s email is not stored in DB' do
     it 'finds user by email if the local part of email is the same as username' do
       allow(User).to receive(:find_by_email).and_return(nil)
       allow(User).to receive(:where).and_return([{name: 'abc', fullname: 'abc bbc'}])
       expect(User.find_by_login('abcxyz@gmail.com')).to eq ({:name=>"abc", :fullname=>"abc bbc"})
     end
   end
 end

 describe '#get_instructor' do
   it 'gets the instructor id' do
     allow(user).to receive(:id).and_return(123)
     expect(user.get_instructor).to eq(123)
     end
 end

 describe '#instructor_id' do
   it 'returns id when role of current user is a super admin' do
     allow(user).to receive_message_chain(:role,:name).and_return('Super-Administrator')
     allow(user).to receive(:id).and_return(1)
     expect(user.instructor_id).to eq(1)
   end

   it 'returns id when role of current user is an Administrator' do
     allow(user).to receive_message_chain(:role,:name).and_return('Administrator')
     allow(user).to receive(:id).and_return(2)
     expect(user.instructor_id).to eq(2)
   end

   it 'returns id when role of current user is an Instructor' do
     allow(user).to receive_message_chain(:role,:name).and_return('Instructor')
     allow(user).to receive(:id).and_return(3)
     expect(user.instructor_id).to eq(3)
   end

   it 'returns instructor_id when role of current user is a TA' do
     allow(user).to receive_message_chain(:role,:name).and_return('Teaching Assistant')
     allow(Ta).to receive(:get_my_instructor).and_return(4)
     expect(user.instructor_id).to eq(4)
   end

   it 'raise an error when role of current user is other type' do
     allow(user).to receive_message_chain(:role,:name).and_return('abc')
     expect{user.instructor_id}.to raise_error(NotImplementedError,"for role abc")
   end

 end

 describe '.export' do
   before(:each) do
     allow(user).to receive_message_chain(:role,:name).and_return('abc')
     allow(user).to receive_message_chain(:parent,:name).and_return('abc')
     allow(User).to receive(:all).and_return([user])
     allow_any_instance_of(User).to receive(:each).and_yield(user)
   end

   it 'exports all information setting in options' do
     options={"personal_details"=>"true", "role"=>"true","parent"=>"true","email_options"=>"true","handle"=>"true"}
     csv=[]
     User.export(csv,0 , options)
     expect(csv).to eq([[user.name,user.fullname,user.email,
                                                user.role.name,user.parent.name,user.email_on_submission, user.email_on_review,
                                                user.email_on_review_of_review, user.copy_of_emails,user.handle]])
   end

   it 'exports only personal_details'do
     options={"personal_details"=>"true", "role"=>"false","parent"=>"false","email_options"=>"false","handle"=>"false"}
     csv=[]
     User.export(csv,0 , options)
     expect(csv).to eq(user.name,user.fullname,user.email)
   end

   it 'exports only current role and parent' do
     options={"personal_details"=>"false", "role"=>"true","parent"=>"true","email_options"=>"false","handle"=>"false"}
     csv=[]
     User.export(csv,0 , options)
     expect(csv).to eq(user.role.name,user.parent.name)
   end

   it 'exports only email_options' do
     options={"personal_details"=>"false", "role"=>"false","parent"=>"false","email_options"=>"true","handle"=>"false"}
     csv=[]
     User.export(csv,0 , options)
     expect(csv).to eq(user.email_on_submission, user.email_on_review,user.email_on_review_of_review, user.copy_of_emails)
   end

   it 'exports only handle' do
     options={"personal_details"=>"false", "role"=>"false","parent"=>"false","email_options"=>"false","handle"=>"true"}
     csv=[]
     User.export(csv,0 , options)
     expect(csv).to eq(user.handle)
   end
 end

 describe '.export_fields' do
   it 'exports all information setting in options' do
     options={"personal_details"=>"true","role"=>"true","parent"=>"true","email_options"=>"true","handle"=>"true"}
     expect(User.export_fields(options)).to eq(["name","full name","email","role","parent","email on submission","email on review","email on metareview","handle"])
   end

   it 'exports only personal_details' do
     options={"personal_details"=>"true","role"=>"false","parent"=>"false","email_options"=>"false","handle"=>"false"}
     expect(User.export_fields(options)).to eq(["name","full name","email"])
   end

   it 'exports only current role and parent' do
     options={"personal_details"=>"false","role"=>"true","parent"=>"true","email_options"=>"false","handle"=>"false"}
     expect(User.export_fields(options)).to eq(["role","parent"])
   end

   it 'exports only email_options' do
     options={"personal_details"=>"false","role"=>"false","parent"=>"false","email_options"=>"true","handle"=>"false"}
     expect(User.export_fields(options)).to eq(["email on submission","email on review","email on metareview"])
   end

   it 'exports only handle' do
     options={"personal_details"=>"false","role"=>"false","parent"=>"false","email_options"=>"false","handle"=>"true"}
     expect(User.export_fields(options)).to eq(["handle"])
   end
 end

 describe '.from_params' do
   it 'returns user by user_id fetching from params' do
     params = {
       :user_id => 1,
     }
     allow(User).to receive(:find).and_return(user)
     expect(User.from_params(params)).to eq user
   end
   it 'returns user by user name fetching from params' do
     params = {
       :user => {
         :name => 'abc'
       }
     }
     allow(User).to receive(:find_by_name).and_return(user)
     expect(User.from_params(params)).to eq user
   end
   it 'raises an error when Expertiza cannot find user' do
     params = {
       :user => {
         :name => 'ncsu'
       }
     }
     allow(user).to receive(:nil?).and_return(true)
     expect {User.from_params(params)}.to raise_error("Please <a href='http://localhost:3000/users/new'>create an account</a> for this user to continue.")
   end
 end

 describe '#is_teaching_assistant_for?' do
   it 'returns false if current user is not a TA' do
     allow(user1).to receive_message_chain("role.ta?"){ false }
     expect(user1.is_teaching_assistant_for?(user)).to be_falsey
   end

   it 'returns false if current user is a TA, but target user is not a student'do
     allow(user1).to receive_message_chain("role.ta?"){true }
     allow(user).to receive_message_chain("role.name").and_return('teacher')
     expect(user1.is_teaching_assistant_for?(user)).to be_falsey
   end

   it 'returns true if current user is a TA of target user'do
   allow(Ta).to receive(:find).and_return(user1)
   allow(user1).to receive_message_chain("role.ta?"){ true }
   allow(user).to receive_message_chain("role.name").and_return('Student')
   c1=Course.new
   allow(user1).to receive_message_chain(:courses_assisted_with,:any?).and_yield(c1)
   allow_any_instance_of(Course).to receive_message_chain(:assignments,:map,:flatten,:map,:include?,:user).and_return(true)
   expect(user1.is_teaching_assistant_for?(user)).to be true
   end
 end

 describe '#is_teaching_assistant?' do
   it 'returns true if current user is a TA' do
     allow(user).to receive_message_chain("role.ta?"){ true }
     expect(user.is_teaching_assistant?).to be true
   end

   it 'returns false if current user is not a TA' do
     allow(user).to receive_message_chain("role.ta?"){ false }
     expect(user.is_teaching_assistant?).to be_falsey
   end
 end

end