CSC/ECE 517 Fall 2021 - E2160. Implementing and testing import export controllers
Jump to navigation
Jump to search
Team
- Akash Sarda, aksarda
- Sairam Sakhamuri, svsakham
- Sidhant Arora, sarora22
- Sai Harsha Nadendla, snadend2
Import and Export Functionality
In Expertiza, many kinds of files can be imported or exported: lists of users for whom accounts are to be created, lists of teams that have been created or need to be created, lists of topics that users are to be able to sign up for, or instructor or peer scores that have been given for a particular assignment. Historically, all of these import and export methods were written individually, using similar code patterns. The instructor might end up adding those in excel or having that data in excel - this functionality allows the expertiza to accept that csv file and tabulate it infront of the user. The user can then select to assign columns to that data and then import it into the database. Eg. list of topics or feedback comments from the students can be tabular if it is say a MCQ questionnaire. The instructor will then have to either enter each entry manually through the UI or can upload this file to automate it.
The export functionality is however is already quite refactored to suit the strategy pattern, which is not changed by the previous group as well.
Problem Description
Initially the different classes (Topics, Assignments, Course Participants) had different implementations of taking in the csv file which was tightly coupled with the feature / model class. However, it was discovered that instead of defining the new method again and again, this process can be automated, by pushing common code of reading the headers and writing to the database into a single generic function. This would basically make it really easy to add this functionality to other model classes and to the new features yet to come to Expertiza.
Existing Import Functionality
The current import functionality is done through the ImportFileController and there are different import class methods implemented in different models that are performing import function.
In the SignUpTopic and User models use helper classes and the attributes are taken from a hash and new ActiveRecord object is created.
Controllers
- import_file_controller, methods:
- Methods used to process files:
- #get_delimiter - Sets delimiter for filetype
- #parse_line - Processes line of the file
- #parse_to_grid - parses the file into 2D array
- #parse_to_hash - parses file into hash where 'header' stores header row and 'body' stores all contents.
- #hash_rows_with_headers - Creates hash for each row of file. Keys are headers, values are row values.
- Import methods:
- #import_from_hash - Primary import functionality. Creates objects for hashed rows (from #hash_rows_with_headers).
- #import - Larger controller of import, sets error messages and displays.
- Methods used to process files:
Helpers
- import_file_helper, the list of methods in the file are the following:
- ::define_attributes - Sets and returns attributes for User object from hash.
- ::create_new_user - Makes a user object in the database.
- import_topics_helper, the list of methods in the file are the following:
- ::define_attributes - Sets and returns attributes for a SignUpTopic from hash.
- ::create_new_sign_up_topic - Makes SignUpTopic objects in the database.
Models
We have found that the below mentioned models are using the import functionality defined in ImportFileController. SignUpTopic and User are dependent on the helper methods to use import functionality.
- assignment_participant
- assignment_team
- course_participant
- course_team
- team
- metareview_response_map
- question
- review_response_map
- sign_up_sheet
- sign_up_topic
- user
Design Plan
The design plan is to use Strategy patter to implement import functionality, so that all the import requests present in different models are routed through the ImportFileController. Right now since the import functionality is implemented in various model methods this leads to many if and else statements to check the type of model. So, we intent to generalize the import functionality by placing common code in the ImportFileController. But each model will still have its own import method. With this approach the redundancy is reduced by moving common code to ImportFileController and code will become DRY.
Other helper methods such as ImportFileHelper and ImportTopicHelper that are used to perform import functionality will also be removed, which keeps import functionality consistent. We will be using method overloading and overriding for the methods in ImportFileController to eliminate unnecessary if and else blocks.
To summarize our plan of changes:
- Redirect all import calls through ImportFileController
- Refactor ImportFileController by removing the redundant code and making code generic.
- Insert object creation conditions into all relevant ::import functions and into the ImportFileController form.
Implementation Details
Models
- ImportFileController is changed and has been made more generalized, many case statements are removed. The ImprortFileController is made so small and understandable. The import functionality is moved into the corresponding models. Previously code was present in different modules which was confusing now the functionality was all in one place.
- Note:- We have not removed the previous implementation of the import as some of the models are yet to be moved to new import framework and removing the previous code will break backward compatibility. The following code changes are made:
- Previous Implmentation
def import_from_hash(session, params)
if params[:model] == "AssignmentTeam" or params[:model] == "CourseTeam"
contents_hash = eval(params[:contents_hash])
@header_integrated_body = hash_rows_with_headers(contents_hash[:header],contents_hash[:body])
errors = []
begin
@header_integrated_body.each do |row_hash|
if params[:model] == "AssignmentTeam"
teamtype = AssignmentTeam
else
teamtype = CourseTeam
end
options = eval(params[:options])
options[:has_teamname] = params[:has_teamname]
Team.import(row_hash, params[:id], options, teamtype)
end
rescue
errors << $ERROR_INFO
end
elsif params[:model] == "ReviewResponseMap"
contents_hash = eval(params[:contents_hash])
@header_integrated_body = hash_rows_with_headers(contents_hash[:header],contents_hash[:body])
errors = []
begin
@header_integrated_body.each do |row_hash|
ReviewResponseMap.import(row_hash,session,params[:id])
end
rescue
errors << $ERROR_INFO
end
elsif params[:model] == "MetareviewResponseMap"
contents_hash = eval(params[:contents_hash])
@header_integrated_body = hash_rows_with_headers(contents_hash[:header],contents_hash[:body])
errors = []
begin
@header_integrated_body.each do |row_hash|
MetareviewResponseMap.import(row_hash,session,params[:id])
end
rescue
errors << $ERROR_INFO
end
elsif params[:model] == 'SignUpTopic' || params[:model] == 'SignUpSheet'
contents_hash = eval(params[:contents_hash])
if params[:has_header] == 'true'
@header_integrated_body = hash_rows_with_headers(contents_hash[:header],contents_hash[:body])
else
if params[:optional_count] == '0'
new_header = [params[:select1], params[:select2], params[:select3]]
@header_integrated_body = hash_rows_with_headers(new_header,contents_hash[:body])
elsif params[:optional_count] == '1'
new_header = [params[:select1], params[:select2], params[:select3], params[:select4]]
@header_integrated_body = hash_rows_with_headers(new_header,contents_hash[:body])
elsif params[:optional_count] == '2'
new_header = [params[:select1], params[:select2], params[:select3], params[:select4], params[:select5]]
@header_integrated_body = hash_rows_with_headers(new_header,contents_hash[:body])
elsif params[:optional_count] == '3'
new_header = [params[:select1], params[:select2], params[:select3], params[:select4], params[:select5], params[:select6]]
@header_integrated_body = hash_rows_with_headers(new_header,contents_hash[:body])
end
end
errors = []
begin
@header_integrated_body.each do |row_hash|
session[:assignment_id] = params[:id]
Object.const_get(params[:model]).import(row_hash, session, params[:id])
end
rescue
errors << $ERROR_INFO
end
elsif params[:model] == 'AssignmentParticipant' || params[:model] == 'CourseParticipant'
contents_hash = eval(params[:contents_hash])
if params[:has_header] == 'true'
@header_integrated_body = hash_rows_with_headers(contents_hash[:header], contents_hash[:body])
else
new_header = [params[:select1], params[:select2], params[:select3], params[:select4]]
@header_integrated_body = hash_rows_with_headers(new_header, contents_hash[:body])
end
errors = []
begin
if params[:model] == 'AssignmentParticipant'
@header_integrated_body.each do |row_hash|
AssignmentParticipant.import(row_hash, session, params[:id])
end
elsif params[:model] == 'CourseParticipant'
@header_integrated_body.each do |row_hash|
CourseParticipant.import(row_hash, session, params[:id])
end
end
rescue
errors << $ERROR_INFO
end
else # params[:model] = "User"
contents_hash = eval(params[:contents_hash])
if params[:has_header] == 'true'
@header_integrated_body = hash_rows_with_headers(contents_hash[:header],contents_hash[:body])
else
new_header = [params[:select1], params[:select2], params[:select3]]
@header_integrated_body = hash_rows_with_headers(new_header, contents_hash[:body])
end
errors = []
begin
@header_integrated_body.each do |row_hash|
User.import(row_hash, nil, session)
end
rescue StandardError
errors << $ERROR_INFO
end
end
- Current Implementation
def import
errors = import_from_hash(session, params)
err_msg = "The following errors were encountered during import.Other records may have been added. A second submission will not duplicate these records."
errors.each do |error|
err_msg = err_msg + error.to_s
end
err_msg += </ul>
if errors.empty?
ExpertizaLogger.info LoggerMessage.new(controller_name, session[:user].name, "The file has been successfully imported.", request)
undo_link("The file has been successfully imported.")
else
ExpertizaLogger.error LoggerMessage.new(controller_name, session[:user].name, err_msg, request)
flash[:error] = err_msg
end
redirect_to session[:return_to]
end
- Assignment Participant is changed to use the newly implemented #import functionality. Changes are as shown below:
def self.import(row_hash, session, id)
raise ArgumentError, "Record does not contain required items." if row_hash.length < self.required_import_fields.length
user = User.find_by(name: row_hash[:name])
user = User.import(row_hash, session, nil) if user.nil?
raise ImportError, "The assignment with id #{id} was not found." if Assignment.find(id).nil?
unless AssignmentParticipant.exists?(user_id: user.id, parent_id: id)
new_part = AssignmentParticipant.new(user_id: user.id, parent_id: id)
new_part.set_handle
end
end
def self.required_import_fields
{"name" => "Name",
"fullname" => "Full Name",
"email" => "Email"}
end
def self.optional_import_fields(id=nil)
{}
end
def self.import_options
{}
end
- Assignment Team is also changed to use the newly implemented #import functionality.
def self.import(row_hash, session = nil, id, options)
raise ArgumentError, "Record does not contain required items." if row_hash.length < self.required_import_fields.length
raise ImportError, "The assignment with the id \"" + id.to_s + "\" was not found. <a href='/assignment/new'>Create</a> this assignment?" if Assignment.find_by(id: id).nil?
Team.import_helper(row_hash, id, options, prototype)
end
def self.required_import_fields
{"teammembers" => "Team Members"}
end
def self.optional_import_fields(id=nil)
{"teamname" => "Team Name"}
end
def self.import_options
{"handle_dups" => {"display" => "Handle Duplicates",
"options" => {"ignore" => "Ignore new team name",
"replace" => "Replace the existing team with the new team",
"insert" => "Insert any new team members into the existing team",
"rename" => "Rename the new team and import"}}}
end
- Course_team is is also changed to use the newly implemented #import functionality.
def self.import(row_hash, session, id, options)
raise ArgumentError, "Record does not contain required items." if row_hash.length < self.required_import_fields.length
raise ImportError, "The course with the id \"" + id.to_s + "\" was not found. <a href='/course/new'>Create</a> this course?" if Course.find(id).nil?
Team.import_helper(row_hash, id, options, prototype)
end
def self.required_import_fields
{"teammembers" => "Team Members"}
end
def self.optional_import_fields(id=nil)
{"teamname" => "Team Name"}
end
def self.import_options
{"handle_dups" => {"display" => "Handle Duplicates",
"options" => {"ignore" => "Ignore new team name",
"replace" => "Replace the existing team with the new team",
"insert" => "Insert any new team members into the existing team",
"rename" => "Rename the new team and import"}}}
end
- Review response map's #import functionality is also updated to use the newly implemented #import functionality.
def self.import(row_hash, _session, assignment_id)
raise ArgumentError, "Record does not contain required items." if row_hash.length < self.required_import_fields.length
reviewee_user_name = row_hash[:reviewee].to_s
reviewee_user = User.find_by(name: reviewee_user_name)
raise ArgumentError, "Cannot find reviewee user." unless reviewee_user
@@ -57,7 +58,7 @@ def self.import(row_hash, _session, assignment_id)
team_node = TeamNode.create(parent_id: assignment_id, node_object_id: reviewee_team.id)
TeamUserNode.create(parent_id: team_node.id, node_object_id: t_user.id)
end
row_hash[:reviewers].split.each do |reviewer|
reviewer_user_name = reviewer.to_s
reviewer_user = User.find_by(name: reviewer_user_name)
raise ArgumentError, "Cannot find reviewer user." unless reviewer_user
@@ -71,6 +72,20 @@ def self.import(row_hash, _session, assignment_id)
end
end
- Refactored the Course_participant.rb to use newly added import functionality.
def self.import(row_hash, session, id)
byebug
raise ArgumentError, "The record does not have enough items." if row_hash.length < self.required_import_fields.length
user = User.find_by(name: row_hash[:name])
user = User.import(row_hash, session, nil) if user.nil?
user = User.find_by(name: row_hash[:name])
if user.nil?
raise ArgumentError, "The record containing #{row_hash[:name]} does not have enough items." if row_hash.length < 4
attributes = ImportFileHelper.define_attributes(row_hash)
user = ImportFileHelper.create_new_user(attributes, session)
end
course = Course.find_by(id)
raise ImportError, "The course with id " + id.to_s + " was not found." if course.nil?
unless CourseParticipant.exists?(user_id: user.id, parent_id: id)
CourseParticipant.create(user_id: user.id, parent_id: id)
end
end
def self.required_import_fields
{"name" => "Name",
"fullname" => "Full Name",
"email" => "Email"}
end
def self.optional_import_fields(id=nil)
{}
end
def self.import_options
{}
end
With these new changes to #import functionality. The following modules review_response_map.rb, course_team.rb,assignment_team.rb,assignment_participant.rb,course_participant.rb have the "self.import" which will use the import functionality in the ImportFileController and they are made as concise as possible. Each module has specific checks while importing the file, so we can't have same code for all the module.
Test Plan
Manual Testing
- Login with expertiza credentials.
- Navigate to Course Participant page by clicking on Manage => Courses => Add Participant.
- Click on Import course participant link and and browse for the file to import. Select the correct options for the type of file to be imported from Import Course Participant page. Please make sure the CSV file is in correct format as expected by the Import functionality. For course participant the csv should contain username | fullname | e-mail | password. Also make sure the users that are going to be imported are already present in expertiza database.
- Click in import participant button and the selected users should be imported.
Automated Tests
The import method has been implemented in a bunch of models which have been listed above. After preliminary analysis, we assume that the import functionality in a few of the models might have to be amended. These models are:
- assignment_participant
- assignment_team
- course_participant
- course_team
- review_response_map
Scenarios:
1. when assignment found and assignment participant does not exist, creates a new user and participant 2. when assignment cannot be found, creates a new user then raises an ImportError 3. when the assignment team does not have the required fields, raises ArgumentError 4. check what import/export actions are allowed for admin, instructor, TA, student 5. when course found and course participant does not exist, creates a new user and participant 6. when the course team does not have the required fields, raises ArgumentError
Newly added rspec test case to test the #import functionality in review_response_map_spec.rb
it '#import' do
expect {ReviewResponseMap.import({reviewee: "name"}, nil, 1)}.to raise_error(ArgumentError, "Record does not contain required items.")
row_hash = {reviewee: "name", reviewers: "name1"}
#row_hash = {reviewee: "name", reviewers: ["name1"]}
session = nil
assignment_id = 1
# when reviewee user = nil
allow(User).to receive(:find_by).and_return(nil)
expect { ReviewResponseMap.import(row_hash, session, 1) }.to raise_error(ArgumentError, "Cannot find reviewee user.")
# when reviewee user exists but reviewee user is not a participant in this assignment
allow(User).to receive(:find_by).with(name: "name").and_return(student)
allow(AssignmentParticipant).to receive(:find_by).with(user_id: 1, parent_id: 1).and_return(nil)
expect { ReviewResponseMap.import(row_hash, session, 1) }.to raise_error(ArgumentError, "Reviewee user is not a participant in this assignment.")
end
Newly added rspec test case to test the #import functionality in course_team_spec.rb
describe ".import" do
let(:row) do
{teammembers: 'none'}
end
context "when a course team does not exist with id" do
it "raises ImportError" do
course_id = 1
allow(Course).to receive(:find).with(course_id).and_return(nil)
error_message = "The course with the id \"" + course_id.to_s + "\" was not found. <a href='/course/new'>Create</a> this course?"
expect { CourseTeam.import(row, nil, course_id, nil) }.
to raise_error(ImportError, error_message)
end
end
context "when the course team does not have the required fields" do
it "raises ArgumentError" do
expect { CourseTeam.import([], nil, 1, nil) }.
to raise_error(ArgumentError)
end
end
context "when a course team with the same id already exists" do
it "gets imported through Team.import" do
course_id = 1
options = []
allow(Course).to receive(:find).with(course_id).and_return(course)
expect(Team).to receive(:import_helper).with(row, course_id, options, instance_of(CourseTeam))
CourseTeam.import(row, nil, course_id, options)
end
end
end
Newly added rspec test case to test the #import functionality in course_participant_spec.rb
describe "CourseParticipant" do
let(:course) { build(:course, id: 1, name: 'ECE517') }
let(:assignment) { build(:assignment, id: 1, name: 'no assignment', participants: [participant], teams: [team]) }
let(:assignment_participant) {build(:participant, id: 1)}
'''describe "#copy" do
#before(:each) do
# byebug
# assignment = build(:assignment)
# course_participant = build(:course_participant)
# @assignment_participant = build(:participant)
#end
it "create a copy of participant" do
allow(AssignmentParticipant).to receive(:create).and_return(participant)
allow(assignment_participant).to receive(:set_handle).and_return(true)
expect(course_participant.copy(@assignment.id)).to be_an_instance_of(AssignmentParticipant)
end
it "returns nil if copy exist" do
allow(AssignmentParticipant).to receive(:where).and_return(AssignmentParticipant)
allow(AssignmentParticipant).to receive(:first).and_return(participant)
allow(assignment_participant).to receive(:set_handle).and_return(true)
expect(course_participant.copy(assignment.id)).to be_nil
end
end'''
Test cases for Assignment_team.rb
describe ".import" do
context "when an assignment team does not already exist with the same id" do
it "cannot be imported" do
assignment_id = 1
allow(Assignment).to receive(:find_by).with(id: assignment_id).and_return(nil)
error_message = "The assignment with the id \"" + assignment_id.to_s + "\" was not found. <a href='/assignment/new'>Create</a> this assignment?"
expect { AssignmentTeam.import([], assignment_id, []) }.
to raise_error(ImportError, error_message)
end
end
context "when an assignment team with the same id already exists" do
it "gets imported through Team.import" do
row = []
assignment_id = 1
options = []
allow(Assignment).to receive(:find_by).with(id: assignment_id).and_return(assignment)
allow(Team).to receive(:import).with(row, assignment_id, options, instance_of(AssignmentTeam))
expect(Team).to receive(:import).with(row, assignment_id, options, instance_of(AssignmentTeam))
AssignmentTeam.import(row, assignment_id, options)
end
end
end
Added test for the assignment_participant_team.rb
describe ".import" do
context 'when record is empty' do
it 'raises an ArgumentError' do
expect { AssignmentParticipant.import({}, nil, nil, nil) }.to raise_error(ArgumentError, 'No user id has been specified.')
end
end
context 'when no user is found by offered username' do
context 'when the record has less than 4 items' do
it 'raises an ArgumentError' do
row = {name: 'no one', fullname: 'no one', email: 'no_one@email.com'}
expect(ImportFileHelper).not_to receive(:create_new_user)
expect { AssignmentParticipant.import(row, nil, nil, nil) }.to raise_error('The record containing no one does not have enough items.')
end
end
context 'when new user needs to be created' do
let(:row) do
{name: 'no one', fullname: 'no one', email: 'name@email.com', role:'user_role_name', parent: 'user_parent_name'}
end
let(:attributes) do
{role_id: 1, name: 'no one', fullname: 'no one', email: 'name@email.com', email_on_submission: 'name@email.com',
email_on_review: 'name@email.com', email_on_review_of_review: 'name@email.com'}
end
let(:test_user) do
{name: 'abc', email: 'abcbbc@gmail.com'}
end
it 'create the user and number of mails sent should be 1' do
ActionMailer::Base.deliveries.clear
allow(ImportFileHelper).to receive(:define_attributes).with(row).and_return(attributes)
allow(ImportFileHelper).to receive(:create_new_user) do
test_user = User.new(name: 'abc', fullname: 'abc bbc', email: 'abcbbc@gmail.com')
test_user.id = 123
test_user.save!
test_user
end
#allow(ImportFileHelper).to receive(:create_new_user).with(attributes, {}).and_return()
allow(Assignment).to receive(:find).with(1).and_return(assignment)
allow(User).to receive(:exists?).with(name: 'no one').and_return(false)
allow(participant).to receive(:set_handle).and_return('handle')
allow(AssignmentParticipant).to receive(:exists?).and_return(false)
allow(AssignmentParticipant).to receive(:create).and_return(participant)
allow(AssignmentParticipant).to receive(:set_handle)
expect{(AssignmentParticipant.import(row, nil, {}, 1))}.to change { ActionMailer::Base.deliveries.count }.by(1)
end
end
Future Scope
There are still some models that have import function but are not using the new framework like:-
- team
- metareview_response_map
- sign_up_sheet
- sign_up_topic
- user
Future work will include migrating these models to use new import framework and writing test cases for these scenarios. Also the previous implementation can then be safely removed.