CSC/ECE 517 Fall 2017/E1776 Enhance Imports: Difference between revisions
No edit summary |
|||
(57 intermediate revisions by the same user not shown) | |||
Line 1: | Line 1: | ||
<div style="padding: 25px 25px 25px 25px | <div style="padding: 25px 25px 25px 25px"> | ||
= Team Contact = | = Team Contact = | ||
Line 18: | Line 18: | ||
= Topic Description = | = Topic Description = | ||
=== Backgroud === | === Backgroud === | ||
Line 23: | Line 24: | ||
Expertiza is an open-source web application devloped on the Ruby on Rails platform that helps students create reusable learning objects through peer review, and also supports document submission and team projects. | Expertiza is an open-source web application devloped on the Ruby on Rails platform that helps students create reusable learning objects through peer review, and also supports document submission and team projects. | ||
Expertiza includes several variations of import functionality, and allows | Expertiza includes several variations of import functionality, and allows instructors to import following data: | ||
* A list of users. | * A list of users. | ||
Line 44: | Line 45: | ||
All of these import functions are routed through the Import File Controller, which is responsible for parsing file data and delegating the import process to the appropriate model. | All of these import functions are routed through the Import File Controller, which is responsible for parsing file data and delegating the import process to the appropriate model. | ||
=== Problem Statement === | === Problem Statement === | ||
The following is an Expertiza-based OSS project which deals primarily with the Import File Controller. This project is associated with fixing [https://github.com/expertiza/expertiza/issues/110 issue #110], detailed on the Expertiza Github. A significant problem with the existing import functions is that they are not implemented consistently, and there are rigid restrictions placed on how the columns should be ordered in the given import files. In the current implementation, rows are taken is arrays, with the column numbers of specific fields hard-coded into the helper methods for different imports. The aim of this project is to improve the import functionality as well as provide a flexible and user-friendly interface so that users can easily and reliably take advantage of Expertiza's various import functions. | The following is an Expertiza-based OSS project which deals primarily with the Import File Controller. This project is associated with fixing [https://github.com/expertiza/expertiza/issues/110 issue #110], detailed on the Expertiza Github. A significant problem with the existing import functions is that they are not implemented consistently, and there are rigid restrictions placed on how the columns should be ordered in the given import files. In the current implementation, rows are taken is arrays, with the column numbers of specific fields hard-coded into the helper methods for different imports. The aim of this project is to improve the import functionality as well as provide a flexible and user-friendly interface so that users can easily and reliably take advantage of Expertiza's various import functions. | ||
=== Solution === | === Solution === | ||
To resolve | To resolve these issue, we have enhanced the interface and the import process, keeping the core methods of import intact, but expanding its functionality to increase its reliability, robustness, and ease-of-use. In the new interface, the system will display what will be imported in an easy-to-read grid before the import is finalized, letting the user verify the correctness of the data and, if necessary, choose from a dropdown menu which columns need to be rearranged. The default ordering is the same as what is currently required by Expertiza, unless a specific order is provided by the user as a header in the import file. | ||
In our implementation of the import method, we have made the use of hash data structure. During the import process, we create an array of hashes, in which each row of the array corresponds to a row of the original import file. In each hash, keys refer to the field name, and values refer to the actual value of that field that will be saved to the database. This greatly improved the flexibility of the import process, since the order of the data in the import files is no longer required to be in one specific order only. | In our implementation of the import method, we have made the use of hash data structure. During the import process, we create an array of hashes, in which each row of the array corresponds to a row of the original import file. In each hash, keys refer to the field name, and values refer to the actual value of that field that will be saved to the database. This greatly improved the flexibility of the import process, since the order of the data in the import files is no longer required to be in one specific order only. | ||
= Plan of Work and Implementation Process = | = Plan of Work and Implementation Process = | ||
In this project we have | In this project we have made the import process more reliable, robust, and easy to use. As stated in the previous section, the previous import functions required that files have a specific order to there columns. We have lifted this restriction, allowing users to import files with the columns given in any order and. If a file has a header, the new import function will use the column names defined in the header to ensure that information is saved in the proper field of the model. If a file does not have a header, the user is given options to specify which columns refer to which fields. | ||
We will work through the User import process as an example. | |||
The general flow of the original import process is as follows: | |||
<ol> | |||
<li>The user navigates to the "Import Users" link.</li> | |||
<li>The link forwards the user to the <code style="font-size:1.5em">start</code> view for <code style="font-size:1.5em">import_file</code>, and passes <code style="font-size:1.5em">User</code> in a variable called <code style="font-size:1.5em">@model</code>.</li> | |||
<li>The user selects a file and clicks "Import".</li> | |||
<li>This submits a form, which calls the <code style="font-size:1.5em">import</code> method of the <code style="font-size:1.5em">import_file_controller</code>.</li> | |||
<li>The <code style="font-size:1.5em">import</code> method calls a secondary <code style="font-size:1.5em">importFile</code> method, which parses the file and returns a 2D array representing the data.</li> | |||
<li>The <code style="font-size:1.5em">importFile</code> method then dispatches actions of saving to the database to the <code style="font-size:1.5em">self.import</code> method of the <code style="font-size:1.5em">User</code> model, based on the previously-populated <code>@model</code> variable.</li> | |||
<li>The <code style="font-size:1.5em">self.import</code> method makes use of methods in the <code style="font-size:1.5em">import_file_helper</code> to create or update the users listed in the file appropriately.</li> | |||
</ol> | |||
However, this process was brittle for a number of reasons, the main one being demonstrated by the code below: | |||
<pre style="font-size:1. | <pre style="font-size:1.25em"> | ||
user = User.find_by_name(row[0]) | user = User.find_by_name(row[0]) | ||
Line 84: | Line 95: | ||
</pre> | </pre> | ||
This was the reason why columns were | This was the reason why columns were required to be in such a restrictive order. In our approach, we divide the rows of data provided in import files into hashes. Accessing data from hashes is flexible as we just need the keys to access certain values, which we specify during our import process. Following is a snippet from the updated <code style="font-size:1.5em">self.import</code> method from <code style="font-size:1.5em">user.rb</code> to demonstrate our usage of hash. | ||
<pre style="font-size | <pre style="font-size: 1.25em"> | ||
user = User.find_by_name(row_hash[:name]) | user = User.find_by_name(row_hash[:name]) | ||
Line 102: | Line 113: | ||
</pre> | </pre> | ||
In order to achieve the flexibility afforded by hashes, we created several protected methods in the <code style="font-size:1.5em">import_file_controller</code> to change the "intermediary" data structure used during import. | |||
Previously, data would be represented as a 2D array, with the first "row" being the header (if the file had a header). | |||
As an aside, another problem with the original code is how it checked for headers. Previous Expertiza developers had left several comments in the <code style="font-size:1.5em">importFile</code> method of <code style="font-size:1.5em">import_file_controller</code> warning of these problems. We decided to give users the option to specify whether or not their file had a header, rather than rely on the brittle existing methodology. | |||
We also split the <code style="font-size:1.5em">importFile</code> functionality, which was originally responsible for reading from a file, parsing the data of the file, and calling the appropriate <code style="font-size:1.5em">self.import</code> method for the given model. | |||
File upload and parsing is now achieved through several protected methods we wrote, and a new method we wrote called <code style="font-size:1.5em">import_from_hash</code> is now responsible for delegating import calls to respective models. | |||
Below is an example of the previous "intermediary" data structure: | |||
<pre style="font-size:1.25em"> | |||
[ [ 'name', 'fullname', 'email' ], | |||
[ 'ppatel16', 'Pushpendra Patel', 'ppatel@ncsu.edu' ], | |||
[ 'tkothar', 'Tanay Kothari', 'tkothar@ncsu.edu' ], | |||
[ 'tmdement', 'Timothy Dement', 'tmdement@ncsu.edu' ] ] | |||
[ | |||
</pre> | </pre> | ||
Under our new implementation, the "intermediary" data structure would be: | |||
<pre style="font-size:1.25em"> | |||
[ { :name => 'ppatel16', | |||
:fullname => 'Pushpendra Patel', | |||
:email => 'ppatel16@ncsu.edu' }, | |||
{ :name => 'tkothar', | |||
:fullname => 'Tanay Kothari', | |||
:email => 'tkothar@ncsu.edu' }, | |||
{ :name => 'tmdement', | |||
[ | :fullname => 'Timothy Dement', | ||
:email => 'tmdement@ncsu.edu } ] | |||
</pre> | </pre> | ||
= Test Plan and Demonstrations = | In addition to theses changes, we have added several options that a user can select during the import process, and have also created a new <code style="font-size:1.5em">import_file_controller</code> action <code style="font-size:1.5em">show</code>, along with corresponding views, so that a user can view and amend the data to be imported before it is finalized and saved to the database. | ||
Detail concerning these additional features are outlined in the screencasts and sections below. | |||
= Test Plan = | |||
In order to address the specifications laid out in the project description, we needed to make significant changes to the Expertiza code base, which had cascading effects to all models with import methods, all of which we needed to address in order to preserve the exiting import functions. | |||
As such, several of the existing specs were not suited to the new code. | |||
We have adapted the appropriate unit tests in <code style="font-size:1.5em">spec/models/course_participant_spec.rb</code> to pass with the existing code. | |||
We have commented out integration tests in <code style="font-size:1.5em">spec/features/instructor_interface_spec.rb</code> and <code style="font-size:1.5em">spec/controllers/airbrake_exception_errors_controller_tests_spec.rb</code> per the instructor's advice, and have added comments above these tests describing how they will need to be adapted to our new code in the future. | |||
All other tests defined in the spec directory are passing, as evidenced by the Travis CI pass on our pull request. | |||
We have thoroughly tested our new import implementations for all models involved, for each of the permutations of options available. | |||
A few of these permutations are outlined in the video demonstrations below, but the videos are not exhaustive. | |||
= Video Demonstrations = | |||
Line 172: | Line 199: | ||
This video demonstrates the enhanced import features for the Review Response Map and Metareview Response Map models with several varying examples. | This video demonstrates the enhanced import features for the Review Response Map and Metareview Response Map models with several varying examples. | ||
=== Topic Import === | |||
[https://youtu.be/58o9u4qgJ8M Topic Import Demo] | |||
This video demonstrates the enhanced import features for the Sign Up Topic model with several varying examples. | |||
= Updates to Existing Views = | |||
=== User Import === | === User Import === | ||
<strong>Improvements:</strong> | |||
<ul> | |||
<li>Added descriptive title</li> | |||
<li>Fixed form alignment</li> | |||
<li>Added "Header" radio button</li> | |||
<li>Changed "Expected Columns" to be more legible</li> | |||
</ul> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Old-User-Import.png|none|frame|Old User Import View]] | [[File:Old-User-Import.png|none|frame|Old User Import View]] | ||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:New-User-Import.png|none|frame|New User Import View]] | [[File:New-User-Import.png|none|frame|New User Import View]] | ||
</div> | |||
=== Assignment Participant and Course Participant Import === | |||
<strong>NOTE: </strong>Course Participant changes are identical. | |||
<strong>Improvements:</strong> | |||
<ul> | |||
<li>Added descriptive title</li> | |||
<li>Fixed form alignment</li> | |||
<li>Added "Header" radio button</li> | |||
<li>Changed "Expected Columns" to be more legible</li> | |||
</ul> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Old-Assignment-Participant-Import.png|none|frame|Old Assignment Participant Import View]] | |||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:New-Assignment-Participant-Import.png|none|frame|New Assignment Participant Import View]] | |||
</div> | |||
=== Assignment Team and Course Team Import === | |||
<strong>NOTE: </strong>Course Team changes are identical. | |||
<strong>Improvements:</strong> | |||
<ul> | |||
<li>Added descriptive title</li> | |||
<li>Fixed form alignment</li> | |||
<li>Added "Header" radio button</li> | |||
<li>Added "Team Name" radio button</li> | |||
<li>Changed "Expected Columns" to be more legible</li> | |||
</ul> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Old-Assignment-Team-Import.png|none|frame|Old Assignment Team Import View]] | |||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:New-Assignment-Team-Import.png|none|frame|New Assignment Team Import View]] | |||
</div> | |||
=== Reviewer Import === | |||
<strong>Improvements:</strong> | |||
<ul> | |||
<li>Added descriptive title</li> | |||
<li>Fixed form alignment</li> | |||
<li>Added "Header" radio button</li> | |||
<li>Added "Contributor" radio button</li> | |||
<li>Changed "Expected Columns" to be more legible</li> | |||
</ul> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Old-Review-Import.png|none|frame|Old Reviewer Import View]] | |||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:New-Review-Import.png|none|frame|New Reviewer Import View]] | |||
</div> | |||
=== Metareviewer Import === | |||
<strong>Improvements:</strong> | |||
<ul> | |||
<li>Added descriptive title</li> | |||
<li>Fixed form alignment</li> | |||
<li>Added "Header" radio button</li> | |||
<li>Added "Contributor and Reviewer" radio button</li> | |||
<li>Changed "Expected Columns" to be more legible</li> | |||
</ul> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Old-Metareview-Import.png|none|frame|Old Metareviewer Import View]] | |||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:New-Metareview-Import.png|none|frame|New Metareviewer Import View]] | |||
</div> | |||
=== Topic Import === | |||
<strong>Improvements:</strong> | |||
<ul> | |||
<li>Added descriptive title</li> | |||
<li>Fixed form alignment</li> | |||
<li>Added "Header" radio button</li> | |||
<li>Added "Optional Columns" checkbox list</li> | |||
<li>Fixed bug where "Expected Columns" would not display if an assignment had no prior topics set up</li> | |||
<li>Changed "Expected Columns" to be more legible</li> | |||
</ul> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Old-Topic-Import.png|none|frame|Old Topic Import View]] | |||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:New-Topic-Import.png|none|frame|New Topic Import View]] | |||
</div> | |||
= New Views = | |||
=== User Import === | |||
<br/> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-User-With-Header.png|none|frame|An example of the new User Import view when the given file contains a header.]] | |||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-User-No-Header.png|none|frame|An example of the new User Import view when the given file does not contain a header.]] | |||
</div> | |||
=== Assignment Participant and Course Participant Import === | === Assignment Participant and Course Participant Import === | ||
<strong>NOTE: </strong>The new view for Course Participant is identical. | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-Participant-With-Header.png|none|frame|An example of the new Assignment Participant Import view when the given file contains a header.]] | |||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-Participant-No-Header.png|none|frame|An example of the new Assignment Participant Import view when the given file does not contain a header.]] | |||
</div> | |||
=== Assignment Team and Course Team Import === | === Assignment Team and Course Team Import === | ||
=== Reviewer | |||
<strong>NOTE: </strong>The new view for Course Team is identical. | |||
<strong>NOTE: </strong>The new view for Teams is the same whether or not there is a header. | |||
<strong>NOTE: </strong>As shown in the video demonstrations, the notice on this page is changed according to the order of columns specified in the previous import step. | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-Team.png|none|frame|An example of the new Assignment Team Import view.]] | |||
</div> | |||
=== Reviewer Import === | |||
<strong>NOTE: </strong>As shown in the video demonstrations, the notice on this page is changed according to the order of columns specified in the previous import step. | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-Reviewer.png|none|frame|An example of the new Reviewer Import view.]] | |||
</div> | |||
=== Metareviewer Import === | |||
<strong>NOTE: </strong>As shown in the video demonstrations, the notice on this page is changed according to the order of columns specified in the previous import step. | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-Metareviewer.png|none|frame|An example of the new Metareviewer Import view.]] | |||
</div> | |||
=== Topic Import === | === Topic Import === | ||
= Files Changed = | <br/> | ||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-Topic-With-Header.png|none|frame|An example of the new Topic Import view when the given file contains a header.]] | |||
</div> | |||
<div style="padding: 0px 0px 0px 25px"> | |||
[[File:Show-Topic-No-Header.png|none|frame|An example of the new Topic Import view when the given file does not contain a header.]] | |||
</div> | |||
= Overview of Files Changed or Added = | |||
=== Assets (1 file) === | |||
<br/> | |||
* <code style="font-size:1.5em">app/asstes/javascripts/shared.js</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">checkForFile()</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">checkIfUserColumnDuplicate()</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">checkForParticipantColumnDuplicate()</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">checkTopicForDuplicatesAndRequiredColumns(optional_count)</code> | |||
The methods above were written to address pre-existing file-import bugs, and prevent users from importing files improperly under the new implementation. | |||
=== Config (1 file) === | |||
<br/> | |||
* <code style="font-size:1.5em">config/routes.rb</code> | |||
Routes were added to accommodate the new controller action and view written for the enhanced import process. | |||
=== Controllers (1 file) === | |||
<br/> | |||
* <code style="font-size:1.5em">app/controllers/import_file_controller.rb</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">show</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">import_from_hash</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">hash_rows_with_headers</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">parse_to_hash</code> | |||
** <strong>NEW: </strong><code style="font-size:1.5em">parse_to_grid</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">import</code> | |||
The new controller action was written to enable the review and editing of the import data before finalization. | |||
The methods above were written to facilitate the new hash-based data structure used for import, and to separate different functionality from the old <code style="font-size:1.5em">importFile</code> into distinct methods. | |||
The old <code style="font-size:1.5em">import</code> method was changed to use our new <code style="font-size:1.5em">import_from_hash</code> method. | |||
=== Helpers (2 files) === | |||
<br/> | |||
* <code style="font-size:1.5em">app/helpers/import_file_helper.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.define_attributes</code> | |||
* <code style="font-size:1.5em">app/helpers/import_topics_helper.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.define_attributes</code> | |||
The methods above were adapted to use the new hash-based data structure used for import. | |||
=== Models (8 files) === | |||
<br/> | |||
* <code style="font-size:1.5em">app/models/assignment_participant.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.import</code> | |||
Adapted to use the new hash-based data structure used for import. | |||
* <code style="font-size:1.5em">app/models/course_participant.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.import</code> | |||
Adapted to use the new hash-based data structure used for import. | |||
* <code style="font-size:1.5em">app/models/course_team.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">add_member</code> | |||
Fixed a pre-existing bug caused by a mismatch of arguments during import. | |||
* <code style="font-size:1.5em">app/models/metareview_response_map.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.import</code> | |||
Adapted to use the new hash-based data structure used for import. | |||
* <code style="font-size:1.5em">app/models/review_response_map.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.import</code> | |||
Adapted to use the new hash-based data structure used for import. | |||
* <code style="font-size:1.5em">app/models/sign_up_topic.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.import</code> | |||
Adapted to use the new hash-based data structure used for import. | |||
* <code style="font-size:1.5em">app/models/team.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">import_team_members</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.import</code> | |||
Adapted to use the new hash-based data structure used for import. | |||
* <code style="font-size:1.5em">app/models/user.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">self.import</code> | |||
Adapted to use the new hash-based data structure used for import. | |||
=== Spec (3 files) === | |||
<br/> | |||
* <code style="font-size:1.5em">spec/controllers/airbrake_exception_errors_controller_tests_spec.rb</code> | |||
** <strong>COMMENTED OUT: </strong><code style="font-size:1.5em">it 'will catch the error info if the tempfile cannot be obtained from params[:file]'</code> | |||
This spec is no longer suited to the new import methodology. See the comments included above the tests in the source code for more detail. | |||
* <code style="font-size:1.5em">spec/features/instructor_interface_spec.rb</code> | |||
** <strong>COMMENTED OUT: </strong><code style="font-size:1.5em">it 'should be valid file with 3 columns'</code> | |||
** <strong>COMMENTED OUT: </strong><code style="font-size:1.5em">it 'should be valid file with 3 or more columns'</code> | |||
** <strong>COMMENTED OUT: </strong><code style="font-size:1.5em">it 'should be a invalid csv file'</code> | |||
** <strong>COMMENTED OUT: </strong><code style="font-size:1.5em">it 'should be a random text file'</code> | |||
These specs are no longer suited to the new import methodology. See the comments included above the tests in the source code for more detail. | |||
* <code style="font-size:1.5em">spec/models/course_participant_spec.rb</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">it 'raise error if record does not have enough items'</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">it 'raise error if course with id not found'</code> | |||
** <strong>EDITED: </strong><code style="font-size:1.5em">it 'creates course participant form record'</code> | |||
These specs were adapted to accurately test the new import methodology. | |||
=== Views (15 files) === | |||
<br/> | |||
*<strong>EDITED: </strong><code style="font-size:1.5em">app/views/course_participant/list.html.erb</code> | |||
*<strong>EDITED: </strong><code style="font-size:1.5em">app/views/participants/list.html.erb</code> | |||
*<strong>EDITED: </strong><code style="font-size:1.5em">app/views/review_mapping/_list_review_mappings.html.erb</code> | |||
*<strong>EDITED: </strong><code style="font-size:1.5em">app/views/review_mapping/list_sortable.html.erb</code> | |||
*<strong>EDITED: </strong><code style="font-size:1.5em">app/views/sign_up_sheet/add_topics.html.erb</code> | |||
*<strong>EDITED: </strong><code style="font-size:1.5em">app/views/teams/list.html.erb</code> | |||
*<strong>EDITED: </strong><code style="font-size:1.5em">app/views/users/list.html.erb</code> | |||
The above files were edited to promote readability of the expected columns message, and in the case of topics to address a pre-existing bug where this information was not displayed properly. | |||
*<strong>EDITED: </strong><code style="font-size:1.5em">app/views/import_file/start.html.erb</code> | |||
The above file was edited to provide new options to the user during the import process, based on the model being imported. | |||
Various other cosmetic changes were made, as described in the appropriate Wiki section. | |||
= | *<strong>NEW: </strong><code style="font-size:1.5em">app/views/import_file/show.html.erb</code> | ||
The above file was created to accommodate the new controller action made for the improved import process. | |||
*<strong>NEW: </strong><code style="font-size:1.5em">app/views/import_file/_metareviewer.html.erb</code> | |||
*<strong>NEW: </strong><code style="font-size:1.5em">app/views/import_file/_participant.html.erb</code> | |||
*<strong>NEW: </strong><code style="font-size:1.5em">app/views/import_file/_reviewer.html.erb</code> | |||
*<strong>NEW: </strong><code style="font-size:1.5em">app/views/import_file/_sign_up_topic.html.erb</code> | |||
*<strong>NEW: </strong><code style="font-size:1.5em">app/views/import_file/_team.html.erb</code> | |||
*<strong>NEW: </strong><code style="font-size:1.5em">app/views/import_file/_user.html.erb</code> | |||
The above files are partials rendered in the <code style="font-size:1.5em">show.html.erb</code> view based on the appropriate model. | |||
Latest revision as of 01:43, 3 November 2017
Team Contact
- MEMBERS
- Pushpendra Patel: ppatel16@ncsu.edu
- Tanay Kothari: tkothar@ncsu.edu
- Timothy Dement: tmdement@ncsu.edu
- MENTOR
- Ferry Pramudianto: fferry@ncsu.edu
Topic Description
Backgroud
Expertiza is an open-source web application devloped on the Ruby on Rails platform that helps students create reusable learning objects through peer review, and also supports document submission and team projects.
Expertiza includes several variations of import functionality, and allows instructors to import following data:
- A list of users.
- A list of participants for an existing assignment.
- A list of participants for an existing course.
- A list of teams for an existing assignment
- A list of teams for an existing course.
- A list of reviewers (reviewing contributors to assignments).
- A list of meta-reviewers (reviewing reviewers).
- A list of topics for an existing assignment.
These imports are done by uploading a file containing rows of data, with each individual value of a row separated by a given delimiter.
Expertiza allows four specifications for delimiters:
- Comma
- Space
- Tab
- Other (any custom delimiter provided by the user as text)
All of these import functions are routed through the Import File Controller, which is responsible for parsing file data and delegating the import process to the appropriate model.
Problem Statement
The following is an Expertiza-based OSS project which deals primarily with the Import File Controller. This project is associated with fixing issue #110, detailed on the Expertiza Github. A significant problem with the existing import functions is that they are not implemented consistently, and there are rigid restrictions placed on how the columns should be ordered in the given import files. In the current implementation, rows are taken is arrays, with the column numbers of specific fields hard-coded into the helper methods for different imports. The aim of this project is to improve the import functionality as well as provide a flexible and user-friendly interface so that users can easily and reliably take advantage of Expertiza's various import functions.
Solution
To resolve these issue, we have enhanced the interface and the import process, keeping the core methods of import intact, but expanding its functionality to increase its reliability, robustness, and ease-of-use. In the new interface, the system will display what will be imported in an easy-to-read grid before the import is finalized, letting the user verify the correctness of the data and, if necessary, choose from a dropdown menu which columns need to be rearranged. The default ordering is the same as what is currently required by Expertiza, unless a specific order is provided by the user as a header in the import file.
In our implementation of the import method, we have made the use of hash data structure. During the import process, we create an array of hashes, in which each row of the array corresponds to a row of the original import file. In each hash, keys refer to the field name, and values refer to the actual value of that field that will be saved to the database. This greatly improved the flexibility of the import process, since the order of the data in the import files is no longer required to be in one specific order only.
Plan of Work and Implementation Process
In this project we have made the import process more reliable, robust, and easy to use. As stated in the previous section, the previous import functions required that files have a specific order to there columns. We have lifted this restriction, allowing users to import files with the columns given in any order and. If a file has a header, the new import function will use the column names defined in the header to ensure that information is saved in the proper field of the model. If a file does not have a header, the user is given options to specify which columns refer to which fields.
We will work through the User import process as an example.
The general flow of the original import process is as follows:
- The user navigates to the "Import Users" link.
- The link forwards the user to the
start
view forimport_file
, and passesUser
in a variable called@model
. - The user selects a file and clicks "Import".
- This submits a form, which calls the
import
method of theimport_file_controller
. - The
import
method calls a secondaryimportFile
method, which parses the file and returns a 2D array representing the data. - The
importFile
method then dispatches actions of saving to the database to theself.import
method of theUser
model, based on the previously-populated@model
variable. - The
self.import
method makes use of methods in theimport_file_helper
to create or update the users listed in the file appropriately.
However, this process was brittle for a number of reasons, the main one being demonstrated by the code below:
user = User.find_by_name(row[0]) if user.nil? attributes = ImportFileHelper.define_attributes(row) user = ImportFileHelper.create_new_user(attributes, session) password = user.reset_password # the password is reset MailerHelper.send_mail_to_user(user, "Your Expertiza account has been created.", "user_welcome", password).deliver else user.email = row[2].strip # ***** Use of hard-coded index values ***** user.fullname = row[1].strip user.parent_id = (session[:user]).id user.save end
This was the reason why columns were required to be in such a restrictive order. In our approach, we divide the rows of data provided in import files into hashes. Accessing data from hashes is flexible as we just need the keys to access certain values, which we specify during our import process. Following is a snippet from the updated self.import
method from user.rb
to demonstrate our usage of hash.
user = User.find_by_name(row_hash[:name]) if user.nil? attributes = ImportFileHelper.define_attributes(row_hash) user = ImportFileHelper.create_new_user(attributes, session) password = user.reset_password MailerHelper.send_mail_to_user(user, "Your Expertiza account has been created.", "user_welcome", password).deliver else user.email = row_hash[:email] # ***** No hard-coded index values required ***** user.fullname = row_hash[:fullname] # ***** Direct use of field name for setting up user parameters for import ***** user.parent_id = (session[:user]).id user.save end
In order to achieve the flexibility afforded by hashes, we created several protected methods in the import_file_controller
to change the "intermediary" data structure used during import.
Previously, data would be represented as a 2D array, with the first "row" being the header (if the file had a header).
As an aside, another problem with the original code is how it checked for headers. Previous Expertiza developers had left several comments in the importFile
method of import_file_controller
warning of these problems. We decided to give users the option to specify whether or not their file had a header, rather than rely on the brittle existing methodology.
We also split the importFile
functionality, which was originally responsible for reading from a file, parsing the data of the file, and calling the appropriate self.import
method for the given model.
File upload and parsing is now achieved through several protected methods we wrote, and a new method we wrote called import_from_hash
is now responsible for delegating import calls to respective models.
Below is an example of the previous "intermediary" data structure:
[ [ 'name', 'fullname', 'email' ], [ 'ppatel16', 'Pushpendra Patel', 'ppatel@ncsu.edu' ], [ 'tkothar', 'Tanay Kothari', 'tkothar@ncsu.edu' ], [ 'tmdement', 'Timothy Dement', 'tmdement@ncsu.edu' ] ]
Under our new implementation, the "intermediary" data structure would be:
[ { :name => 'ppatel16', :fullname => 'Pushpendra Patel', :email => 'ppatel16@ncsu.edu' }, { :name => 'tkothar', :fullname => 'Tanay Kothari', :email => 'tkothar@ncsu.edu' }, { :name => 'tmdement', :fullname => 'Timothy Dement', :email => 'tmdement@ncsu.edu } ]
In addition to theses changes, we have added several options that a user can select during the import process, and have also created a new import_file_controller
action show
, along with corresponding views, so that a user can view and amend the data to be imported before it is finalized and saved to the database.
Detail concerning these additional features are outlined in the screencasts and sections below.
Test Plan
In order to address the specifications laid out in the project description, we needed to make significant changes to the Expertiza code base, which had cascading effects to all models with import methods, all of which we needed to address in order to preserve the exiting import functions.
As such, several of the existing specs were not suited to the new code.
We have adapted the appropriate unit tests in spec/models/course_participant_spec.rb
to pass with the existing code.
We have commented out integration tests in spec/features/instructor_interface_spec.rb
and spec/controllers/airbrake_exception_errors_controller_tests_spec.rb
per the instructor's advice, and have added comments above these tests describing how they will need to be adapted to our new code in the future.
All other tests defined in the spec directory are passing, as evidenced by the Travis CI pass on our pull request.
We have thoroughly tested our new import implementations for all models involved, for each of the permutations of options available.
A few of these permutations are outlined in the video demonstrations below, but the videos are not exhaustive.
Video Demonstrations
User Import
This video demonstrates the enhanced import features for the User model with several varying examples, and also demonstrates that the original error messaging system is preserved with our new implementation.
Assigment Participant and Course Participant Import
This video demonstrates the enhanced import features for the Assignment Participant and Course Participant models with several varying examples.
Assignment Team and Course Team Import
This video demonstrates the enhanced import features for the Assignment Team and Course Team models with several varying examples.
Reviewer and Meta-reviewer Import
This video demonstrates the enhanced import features for the Review Response Map and Metareview Response Map models with several varying examples.
Topic Import
This video demonstrates the enhanced import features for the Sign Up Topic model with several varying examples.
Updates to Existing Views
User Import
Improvements:
- Added descriptive title
- Fixed form alignment
- Added "Header" radio button
- Changed "Expected Columns" to be more legible
Assignment Participant and Course Participant Import
NOTE: Course Participant changes are identical.
Improvements:
- Added descriptive title
- Fixed form alignment
- Added "Header" radio button
- Changed "Expected Columns" to be more legible
Assignment Team and Course Team Import
NOTE: Course Team changes are identical.
Improvements:
- Added descriptive title
- Fixed form alignment
- Added "Header" radio button
- Added "Team Name" radio button
- Changed "Expected Columns" to be more legible
Reviewer Import
Improvements:
- Added descriptive title
- Fixed form alignment
- Added "Header" radio button
- Added "Contributor" radio button
- Changed "Expected Columns" to be more legible
Metareviewer Import
Improvements:
- Added descriptive title
- Fixed form alignment
- Added "Header" radio button
- Added "Contributor and Reviewer" radio button
- Changed "Expected Columns" to be more legible
Topic Import
Improvements:
- Added descriptive title
- Fixed form alignment
- Added "Header" radio button
- Added "Optional Columns" checkbox list
- Fixed bug where "Expected Columns" would not display if an assignment had no prior topics set up
- Changed "Expected Columns" to be more legible
New Views
User Import
Assignment Participant and Course Participant Import
NOTE: The new view for Course Participant is identical.
Assignment Team and Course Team Import
NOTE: The new view for Course Team is identical.
NOTE: The new view for Teams is the same whether or not there is a header.
NOTE: As shown in the video demonstrations, the notice on this page is changed according to the order of columns specified in the previous import step.
Reviewer Import
NOTE: As shown in the video demonstrations, the notice on this page is changed according to the order of columns specified in the previous import step.
Metareviewer Import
NOTE: As shown in the video demonstrations, the notice on this page is changed according to the order of columns specified in the previous import step.
Topic Import
Overview of Files Changed or Added
Assets (1 file)
app/asstes/javascripts/shared.js
- NEW:
checkForFile()
- NEW:
checkIfUserColumnDuplicate()
- NEW:
checkForParticipantColumnDuplicate()
- NEW:
checkTopicForDuplicatesAndRequiredColumns(optional_count)
- NEW:
The methods above were written to address pre-existing file-import bugs, and prevent users from importing files improperly under the new implementation.
Config (1 file)
config/routes.rb
Routes were added to accommodate the new controller action and view written for the enhanced import process.
Controllers (1 file)
app/controllers/import_file_controller.rb
- NEW:
show
- NEW:
import_from_hash
- NEW:
hash_rows_with_headers
- NEW:
parse_to_hash
- NEW:
parse_to_grid
- EDITED:
import
- NEW:
The new controller action was written to enable the review and editing of the import data before finalization.
The methods above were written to facilitate the new hash-based data structure used for import, and to separate different functionality from the old importFile
into distinct methods.
The old import
method was changed to use our new import_from_hash
method.
Helpers (2 files)
app/helpers/import_file_helper.rb
- EDITED:
self.define_attributes
- EDITED:
app/helpers/import_topics_helper.rb
- EDITED:
self.define_attributes
- EDITED:
The methods above were adapted to use the new hash-based data structure used for import.
Models (8 files)
app/models/assignment_participant.rb
- EDITED:
self.import
- EDITED:
Adapted to use the new hash-based data structure used for import.
app/models/course_participant.rb
- EDITED:
self.import
- EDITED:
Adapted to use the new hash-based data structure used for import.
app/models/course_team.rb
- EDITED:
add_member
- EDITED:
Fixed a pre-existing bug caused by a mismatch of arguments during import.
app/models/metareview_response_map.rb
- EDITED:
self.import
- EDITED:
Adapted to use the new hash-based data structure used for import.
app/models/review_response_map.rb
- EDITED:
self.import
- EDITED:
Adapted to use the new hash-based data structure used for import.
app/models/sign_up_topic.rb
- EDITED:
self.import
- EDITED:
Adapted to use the new hash-based data structure used for import.
app/models/team.rb
- EDITED:
import_team_members
- EDITED:
self.import
- EDITED:
Adapted to use the new hash-based data structure used for import.
app/models/user.rb
- EDITED:
self.import
- EDITED:
Adapted to use the new hash-based data structure used for import.
Spec (3 files)
spec/controllers/airbrake_exception_errors_controller_tests_spec.rb
- COMMENTED OUT:
it 'will catch the error info if the tempfile cannot be obtained from params[:file]'
- COMMENTED OUT:
This spec is no longer suited to the new import methodology. See the comments included above the tests in the source code for more detail.
spec/features/instructor_interface_spec.rb
- COMMENTED OUT:
it 'should be valid file with 3 columns'
- COMMENTED OUT:
it 'should be valid file with 3 or more columns'
- COMMENTED OUT:
it 'should be a invalid csv file'
- COMMENTED OUT:
it 'should be a random text file'
- COMMENTED OUT:
These specs are no longer suited to the new import methodology. See the comments included above the tests in the source code for more detail.
spec/models/course_participant_spec.rb
- EDITED:
it 'raise error if record does not have enough items'
- EDITED:
it 'raise error if course with id not found'
- EDITED:
it 'creates course participant form record'
- EDITED:
These specs were adapted to accurately test the new import methodology.
Views (15 files)
- EDITED:
app/views/course_participant/list.html.erb
- EDITED:
app/views/participants/list.html.erb
- EDITED:
app/views/review_mapping/_list_review_mappings.html.erb
- EDITED:
app/views/review_mapping/list_sortable.html.erb
- EDITED:
app/views/sign_up_sheet/add_topics.html.erb
- EDITED:
app/views/teams/list.html.erb
- EDITED:
app/views/users/list.html.erb
The above files were edited to promote readability of the expected columns message, and in the case of topics to address a pre-existing bug where this information was not displayed properly.
- EDITED:
app/views/import_file/start.html.erb
The above file was edited to provide new options to the user during the import process, based on the model being imported.
Various other cosmetic changes were made, as described in the appropriate Wiki section.
- NEW:
app/views/import_file/show.html.erb
The above file was created to accommodate the new controller action made for the improved import process.
- NEW:
app/views/import_file/_metareviewer.html.erb
- NEW:
app/views/import_file/_participant.html.erb
- NEW:
app/views/import_file/_reviewer.html.erb
- NEW:
app/views/import_file/_sign_up_topic.html.erb
- NEW:
app/views/import_file/_team.html.erb
- NEW:
app/views/import_file/_user.html.erb
The above files are partials rendered in the show.html.erb
view based on the appropriate model.
User Import
To begin the User import process, first select the "User" link from the "Manage" drop-down menu.
Next, scroll to the bottom of the page and click the "Import Users" link.
You will then be redirected to the User import page.
Assignment Participant Import
To begin the Assignment Participant import process, first select the "Assignments" link from the "Manage" drop-down menu.
Make sure that "Assignments" is highlighted on the "Manage content" page, then locate the appropriate assignment and click the "Add Participants" button.
Finally, scroll to the bottom of the page and click the "Import assignment participants" link.
You will then be redirected to the Assignment Participant import page.
Course Participant Import
To begin the Course Participant import process, first select the "Courses" link from the "Manage" drop-down menu.
Make sure that "Courses" is highlighted on the "Manage content" page, then locate the appropriate course and click the "Add Participants" button.
Finally, scroll to the bottom of the page and click the "Import course participants" link.
You will then be redirected to the Course Participant import page.
Assignment Team Import
To begin the Assignment Team import process, first select the "Assignments" link from the "Manage" drop-down menu.
Make sure that "Assignments" is highlighted on the "Manage content" page, then locate the appropriate assignment and click the "Create Teams" button.
Finally, scroll to the bottom of the page and click the "Import Teams" link.
You will then be redirected to the Assignment Team import page.
Course Team Import
To begin the Course Team import process, first select the "Courses" link from the "Manage" drop-down menu.
Make sure that "Courses" is highlighted on the "Manage content" page, then locate the appropriate assignment and click the "Create Teams" button.
Finally, scroll to the bottom of the page and click the "Import Teams" link.
You will then be redirected to the Course Team import page.
Reviewer and Metareviewer Import
To begin the Reviewer and Metareviewer import process, first select the "Assignments" link from the "Manage" drop-down menu.
Make sure that "Assignments" is highlighted on the "Manage content" page, then locate the appropriate assignment and click the "Assign reviewers" button.
Finally, scroll to the bottom of the page, and click either the "Import reviewer mappings" link or the "Import meta reviewer mappings" link.
You will then be redirected to either the Reviewer import page or the Metareviewer import page.
Topic Import
To begin the Topic import process, first select the "Assignments" link from the "Manage" drop-down menu.
Make sure that "Assignments" is highlighted on the "Manage content" page, then locate the appropriate assignment and click the "Edit" button.
Next, select the "Topics" tab, scroll to the bottom of the page and click the "Import topics" link.
If topics have not been set up yet on the given assignment, you will receive an alert that will need to be accepted before continuing.
You will then be redirected to the Topic import page.