CSC/ECE 517 Fall 2019 - E1938. OSS project Duke Blue: Fix import glitches
About Expertiza
Expertiza is an open source project based on Ruby on Rails framework and the code is available on Github. Expertiza allows the instructor to create new assignments as well as edit new or existing assignments. Instructors can also create a list of topics the students can sign up for and specify deadlines for completion of various tasks. Students can form teams in Expertiza to work on various projects and assignments as well as peer review other students' submissions. Expertiza supports submission across various document types, including the URLs Wiki pages.
Problem Statement
The import feature is the most helpful feature for instructors to set up assignments. The instructors usually have a list of students, teams, etc from their learning management system. Being able to import these into expertiza saves a lot of time when setting up an assignment. Unfortunately this feature has some bugs that were identified as the following list.
Issue 918: It seems that non-ASCII characters in the description (e.g., a curly apostrophe, and perhaps also an en-dash) caused a SQL error in inserting the description string into the db. I had to edit the import file to remove non-ASCII characters. Any Unicode character ought to be allowed in a description.
Issue 153: There should be a way to import a list of who has signed up for which topic. When a signup sheet is used in Expertiza, users are expected to sign up for topics. But, the instructor might've taken signups offline, e.g., by passing around a signup sheet in class. There should be a way to import this list to Expertiza, rather than impersonate all the students one by one and sign them up for a topic.
Issue 329: When importing teams there are different options to handle conflicting team names
Issue 328: When importing teams, and a conflict exists there is an option to merge the two teams by inserting the new members into an existing team. Unfortunately, this feature is broken. we tried to import a team and selected 'Insert any new members into existing team'. No new team members were in fact inserted into the team
Issue 153 is not a bug but an additional feature to the existing import topic functionality.
Fixes
Issue 918
New code introduced is as follows.
File:app/helpers/import_topics_helper.rb
Code:
def self.define_attributes(row_hash)
attributes = {}
if !row_hash[:description].nil? and !row_hash[:description].ascii_only?
row_hash[:description] = self.trim_non_ascii(row_hash[:description])
puts row_hash[:description]
end
attributes["topic_identifier"] = row_hash[:topic_identifier].strip
attributes["topic_name"] = row_hash[:topic_name].strip
attributes["max_choosers"] = row_hash[:max_choosers].strip
attributes
end
def self.trim_non_ascii(string)
string.split().each do |char|
!char.ascii_only? ? string.tr!(char, ' ') : nil
end
string.gsub!(/\s+/, ' ')
end
Issue 328
This was an issue to fix the functionality of Insert any new members into existing team. This was due to faulty handling of the option handle_dups and incorrect checking of parameters.
Code was modified to ensure minimal impact to existing testcases and functionalities.
Code:
name = handle_duplicate(team, name, id, options["handle_dups"], teamtype)
After fixing changes
Issue 329
This was a request to add an option to rename existing team if there was a conflict in the team name.
New code introduced was as follows:
In start.html.erb
In this file, the new option as displayed to the user is added
<option value="rename_existing">rename the existing team and import</option>
In team.rb
In this file, the handling of the new tag "rename_existing" is added wherein, the old team is updated with a new name.
# E1938: Added handling for renaming old team when conflict arises
if handle_dups == "rename_existing"
if teamtype.is_a?(CourseTeam.class)
team.update(name: self.generate_team_name(Course.find(id).name))
elsif teamtype.is_a?(AssignmentTeam.class)
team.update(name: self.generate_team_name(Assignment.find(id).name))
end
return name
end
The state of D before conflicting import:
After fixing changes
As you can see the new team got the name D and the old file got renamed to Team_17.
Issue 153
This was a new requirement to add an option to upload topics along with the teams assigned.
Expectations from the import file:
Assigned team name should exist in the system, otherwise the import will fail. Instructor has to make sure that the teams which are assigned in the import file should exist with same names in the system or database.
Instructor also need to ensure that the topic for which the team is being assigned should not be full or can accommodate new team assignment to the respective topic.
Code Changes:
import_file/_sign_up_topic.html.erb (All the select tags has been changed)
As there's already a screen with select options to map the columns with the fields for import. We are just adding another column so the best to extend the existing select tag with a new option for the teams assigned.
<select name="select1" id="select1" class="form-control" style="background-color:lightgrey">
<option value="topic_identifier">Topic Identifier (required)</option>
<option value="topic_name">Topic Name (required)</option>
<option value="max_choosers">Max Choosers (required)</option>
<option value="category">Category (optional)</option>
<option value="description">Description (optional)</option>
<option value="link">Link (optional)</option>
<option value="assigned_team">Assigned Team (optional)</option>
</select>
model/sign_up_topic.rb
The existing import function in the model import the topics to the topic table. As per the requirement and validations, topic should be imported first and then only the team can be assigned to the topic. So the existing import method has been modified to assign the team once the topic is imported in the database table. As per the edge case mentioned in the requirement document that few topics may not have any team assigned to it, so we first check if topic has any team assigned, then only it tries to map the topic with the team.
def self.import(row_hash, session, _id = nil)
if row_hash.length < 3
raise ArgumentError, "The CSV File expects the format: Topic identifier, Topic name, Max choosers, Topic Category (optional), Topic Description (Optional), Topic Link (optional), Assigned Team Name (optional)."
end
topic = SignUpTopic.where(topic_name: row_hash[:topic_name], assignment_id: session[:assignment_id]).first
if topic.nil?
attributes = ImportTopicsHelper.define_attributes(row_hash)
topic_new_id = ImportTopicsHelper.create_new_sign_up_topic(attributes, session)
unless row_hash[:assigned_team].nil?
team = Team.where(name: row_hash[:assigned_team]).first
ImportTopicsHelper.assign_team_topic(topic_new_id, team.id)
end
else
topic.max_choosers = row_hash[:max_choosers]
topic.topic_identifier = row_hash[:topic_identifier]
topic.save
unless row_hash[:assigned_team].nil?
team = Team.where(name: row_hash[:assigned_team]).first
newteam = SignedUpTeam.where(topic_id: topic.id, team_id: team.id).first
if newteam.nil?
ImportTopicsHelper.assign_team_topic(topic.id, team.id)
else
newteam.team_id = team.id
newteam.save
end
end
end
end
helpers/import_topics_helper.rb
Having the assign_team_topic in the helper would organize the code but also in the unit testing. We can isolate the model logic and test this method separately by writing the test cases.
def self.assign_team_topic(topic_id, assigned_team)
attributes = {}
attributes["topic_id"] = topic_id
attributes["team_id"] = assigned_team
assign_team = SignedUpTeam.new(attributes)
assign_team.save!
end
def self.create_new_sign_up_topic(attributes, session)
sign_up_topic = SignUpTopic.new(attributes)
sign_up_topic.assignment_id = session[:assignment_id]
sign_up_topic.save
# sign_up_topic
sign_up_topic.save!
sign_up_topic.id
end
controllers/import_file_controller.rb
Existing implementation for import have many flaws and design issues. Most of the code has been written with nested if else, which is not the practice to be followed. Also the names of the fields have been hardcoded.
As we cannot change the complete import implementation. So i had to use the existing one the way it is and make changes to extend the functionality.
if (params[:assigned_team] == 'true')
@optional_count += 1
end
controllers/import_file_controller.rb (import_from_hash)
elsif params[:optional_count] == '4'
new_header = [params[:select1], params[:select2], params[:select3], params[:select4], params[:select5], params[:select6], params[:select7]]
@header_integrated_body = hash_rows_with_headers(new_header,contents_hash[:body])
views/sign_up_sheet/_add_topics.html.erb
This code has been used to display the columns in the footer.As we already have implementation for other columns, so another column name 'Assigned Team' is added here in this part of the code.
<%= link_to 'Import topics',
{
:controller => 'import_file',
:action => 'start',
:model => 'SignUpTopic',
:id => params[:id],
:expected_fields => 'Topic Identifier' +
'  |  ' +
'Topic Name' +
'  |  ' +
'Max Choosers' +
'  |  ' +
'Topic Category (optional)' +
'  |  ' +
'Topic Description (optional)' +
'  |  ' +
'Topic Link (optional)' +
'  |  ' +
'Assigned Team (optional)'
An option column checkbox (Teams Assigned) has been added to the UI
A new column Assigned Team is added where the columns are being mapped.
Test Plan
Possible Enhancements
Import functionality
The existing import implementation does not follow DRY principle, rails code practices and even basic design principles. Most of the code is written based with nested if else conditions. This implementation can be improved.
Exception Handling: Before starting to work on this requirement. I tried few imports and in many cases, import was not successful and i was not able to figure out what's going wrong. My suggestion would be to develop a screen to show which rows were successfully imported and the failure reason which didn't.
Also, if there's no team in the system and instructor has used it in the import file so a new pop up can be displayed where the new team can be created or the import can be extended to create teams while assigning the topic.
References
1. Expertiza on GITHUB: https://github.com/expertiza/expertiza
2. GitHub Project Repository Fork: https://github.com/NisargC/expertiza
3. Demo link: http://152.46.19.138:8080/
4. Expertiza project documentation wiki: http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_E1938._OSS_project_Duke_Blue:_Fix_import_glitches