CSC/ECE 517 Fall 2019 - E1938. OSS project Duke Blue: Fix import glitches

From Expertiza_Wiki
Jump to navigation Jump to search

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. This issue is not a bug but an additional feature to the existing import topic functionality.

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

Fixes

Issue 918

Problem: The application is not able to import topics with non ascii characters in its description field.
Solution: Before the topic object is persisted into the database, check for any non ascii characters in the description field and strip them.
Code Changes
New code introduced is as follows.
File:app/helpers/import_topics_helper.rb
Here, a new function 'trim_non_ascii' has been added which iterates a given string character by character, and replaces each non ascii character with a white space character. 

 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

Problem: 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.

Solution: 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.
The handling of handle_dups in options was done assuming that the parameter was a symbol. But when tested, it was found to be a string and hence the handling was updated accordingly. 

     name = handle_duplicate(team, name, id, options["handle_dups"], teamtype)

Also, the if and else conditions for checking class type of used was wrong. This was updated based on how the variable was assigned in the import_file_controller.rb#import_from_hash 

     if(teamtype.is_a?(CourseTeam.class))
         ...
     elsif  teamtype.is_a?(AssignmentTeam.class)
         ...



After fixing changes


Issue 329

Problem: When importing teams there are different options to handle conflicting team names as shown in the screenshot below. We’d like to add another option to rename EXISTING team when a conflict exist. Introduce an option to rename the existing team in import_file/start.html.erb, handle this option in import_file_controller.rb and team.rb→import.

Solution: 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

Requirement: 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.

Solution: We have modified the existing implementation for import topics to assign the teams to the topics. We have changed the files to handle another column where the teams have been assigned to topics. The column is optional because there might be topics that have not yet been assigned to any teams

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
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 way is to extend the existing select tag with a new option for the teams assigned. All the select tags has been changed with an additional option "Assigned Team". 

           <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
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' + '&nbsp&nbsp|&nbsp&nbsp' + 'Topic Name' + '&nbsp&nbsp|&nbsp&nbsp' + 'Max Choosers' + '&nbsp&nbsp|&nbsp&nbsp' + 'Topic Category (optional)' + '&nbsp&nbsp|&nbsp&nbsp' + 'Topic Description (optional)' + '&nbsp&nbsp|&nbsp&nbsp' + 'Topic Link (optional)' + '&nbsp&nbsp|&nbsp&nbsp' + 'Assigned Team (optional)' Changes in the application with new implementation 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

  • RSpec:
We have added new testcases for the new pieces of code that were added by us.
[Problem 1 (918)]
New testcases have been added to test this functionality. We use it to check if description with Ascii characters are being trimmed. These test cases has been added to import_topics_helper_spec.rb
[Problem 2 (153)]
As discussed with the mentor, there were no test cases for the SignUpTopic model which is being used to import the topics. As our additional functionality is to have another column to assign team to the topic. We have to have the import test case and the assigned team change could be applied to the same test case.
[Problem 3 (329)]
New testcases have been added to test this functionality. We use it to check if the older team gets the new name and the new team gets the requested name. These testcases run when we run bundle exec for the teams_spec.rb
[Problem 4 (328)]
As discussed with the Mentor this was a bug fix in the existing functionality. There are no new test cases needed for this.'


  • UI Testing:
[Problem 1 (918)]
1. Login to Expertiza using username: instructor6 & password: password.
2. Go to Assignments and click on edit for any of the listed assignments.
3. Click on Topics tab.
4. Scroll down to the bottom of the page and click on Import topics.
5. Click on Topic Description checkbox and import the following file. (sample.csv)
6. Click on next and make the header for 4th column as 'Description' from the dropdown.
7. Import topics.
8. After this, a new topic should be imported which can be viewed from the aforementioned Topics tab. This newly imported topic will not have any non-ascii characters.
[Problem 2 (153)]
1. Login to Expertiza using username: instructor6 & password: password.
2. Select Manage >Assignments > Edit Assignment > Topics
3. Scroll down to the screen and click on import topics
4. Upload a file with the topic details and a team assigned to topic
5. Check the Teams Assigned optional column
6. Click on import and map the columns in the mapping page
6. After submitting, team will be assigned to appropriate topic
[Problem 3 (329)]
1. Login to Expertiza using username: instructor6 & password: password.
2. Select Manage > Courses or Assignments > Add Teams button > Import Team
3. In the screen that loads, select "rename the existing team and import" in the handle conflicts section.
4. Upload a file with the team details and select the appropriate delimiter.
5. Click on Create
6. Verify that the new team got created with the assigned name and the older team's name got changed.
[Problem 4 (328)]
1. Login to Expertiza using username: instructor6 & password: password.
2. Select Manage > Courses or Assignments > Add Teams button > Import Team
3. In the screen that loads, select "Insert any new members into existing team" in the handle conflicts section.
4. Upload a file with the team details and select the appropriate delimiter.
5. Click on Create
6. Verify that the existing team got updated with the new team members

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.

Non Ascii character imports

The non ascii character issue exists for topic identifier and topic name fields as well. This should also be fixed in a similar way as is done for Issue #918 above.

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. Demo Video link: https://drive.google.com/open?id=1mPnOsO5n_44lcVrPuYlbZrX3gDXLA9j7

5. Expertiza project documentation wiki: http://wiki.expertiza.ncsu.edu/index.php/CSC/ECE_517_Fall_2019_-_E1938._OSS_project_Duke_Blue:_Fix_import_glitches

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2019_-_E1938._OSS_project_Duke_Blue:_Fix_import_glitches&oldid=129249"