User:Mpatel29: Difference between revisions
(Created page with "== Team== === Mentor === * Ed Gehringer === Team Members === *Meet, Patel - mpatel29@ncsu.edu *John, Nolan - jrnolan2@ncsu.edu *Gwen, Mason - cpmason@ncsu.edu == Project Purpose & Description == The export/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 export/import these into expertiza saves a lot of time when setting...") |
No edit summary |
||
Line 118: | Line 118: | ||
[[File:Export | [[File:Export |800x520px]] | ||
[[File: | [[File:|800x520px]] | ||
Line 131: | Line 131: | ||
*'''Fix 1: Export grades into a file successfully''' - When exporting grades, self.export method of Assignment.rb is called. This internally calls review_grades method in Scoring.rb. But review_grades method was not being invoked because it was being called from a class method but was imported as an instance method. Then self.export method internally invokes self.export_data method. After fixing that, there were some lines related to the business logic were missing here. We added those lines and it can be seen in the code diff. | *'''Fix 1: Export grades into a file successfully''' - When exporting grades, self.export method of Assignment.rb is called. This internally calls review_grades method in Scoring.rb. But review_grades method was not being invoked because it was being called from a class method but was imported as an instance method. Then self.export method internally invokes self.export_data method. After fixing that, there were some lines related to the business logic were missing here. We added those lines and it can be seen in the code diff. | ||
[[File: | [[File:|800x520px]] | ||
*'''''Code Changes for Fix 1: Export grades into a file successfully''''' | *'''''Code Changes for Fix 1: Export grades into a file successfully''''' | ||
[[File:Fix1 | [[File:Fix1 |800x520px]] | ||
Line 143: | Line 143: | ||
*'''''Code Changes for Fix 2: Incorrect headers in the exported file''''' | *'''''Code Changes for Fix 2: Incorrect headers in the exported file''''' | ||
[[File:Fix2 | [[File:Fix2 |800x520px]] | ||
Line 149: | Line 149: | ||
*'''Fix 3: Only export selected columns from the UI''' - Removed author feedback score from being exported by unchecking that option in the UI as seen in image below. | *'''Fix 3: Only export selected columns from the UI''' - Removed author feedback score from being exported by unchecking that option in the UI as seen in image below. | ||
[[File: | [[File:|800x520px]] | ||
Line 156: | Line 156: | ||
** Problem 2: Also only the headers have changed but all values of all columns are still being exported. | ** Problem 2: Also only the headers have changed but all values of all columns are still being exported. | ||
[[File: | [[File:|800x520px]] | ||
*'''''Code Changes for Fix 3: Only export selected columns from the UI''''' | *'''''Code Changes for Fix 3: Only export selected columns from the UI''''' | ||
[[File: | [[File:|800x520px]] | ||
[[File: | [[File:|800x520px]] | ||
Line 171: | Line 171: | ||
*'''''Code Changes for Fix 4: Category, Description, and Link were not being updated in the database while importing the CSV using the Import Topics button''''' | *'''''Code Changes for Fix 4: Category, Description, and Link were not being updated in the database while importing the CSV using the Import Topics button''''' | ||
[[File:Fix | [[File:Fix |800x520px]] | ||
== Test Plan == | == Test Plan == | ||
Line 199: | Line 199: | ||
*'''''export_file_controller test coverage - 54.84%''''' | *'''''export_file_controller test coverage - 54.84%''''' | ||
[[File:Export test | [[File:Export test |800x520px]] | ||
*'''''import_file_controller test coverage - 52.43%''''' | *'''''import_file_controller test coverage - 52.43%''''' | ||
[[File:Import | [[File:Import |800x520px]] | ||
Line 216: | Line 216: | ||
== '''''Link to Pull Request:''''' | == '''''Link to Pull Request:''''' == |
Revision as of 02:56, 26 March 2024
Team
Mentor
- Ed Gehringer
Team Members
- Meet, Patel - mpatel29@ncsu.edu
- John, Nolan - jrnolan2@ncsu.edu
- Gwen, Mason - cpmason@ncsu.edu
Project Purpose & Description
The export/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 export/import these into expertiza saves a lot of time when setting up an assignment.
Expertiza provides multiple export and import features for eg. export students, teams etc. Essentially what it does is it fetches some data from the database and saves it as a file in the desired format. However, the same functionality is implemented multiple times for exporting and importing different things. The aim of this project is to design a generic export/import feature that can be used across the application. This will help in maintaining the codebase and keep things consistent throughout.
Current Project Scope
A generic import_file_controller has already been experimented with in Project E1923: https://github.com/expertiza/expertiza/pull/1438 and Project E2238: https://github.com/expertiza/expertiza/pull/2396 But as discussed with the professor, the PR hasn’t been merged, and it’s not known whether the code was correct or it had issues. And given the existing code has lots of if-else, it would be better to rewrite it from scratch.
Below listed are our primary goals for this project:
- Redo the code for import_file_controller.rb and export_file_controller.rb, by referring to the code in the pull request. Our plan of action is to rewrite the code than try to refactor it, as it’s not known whether the code in the PR is correct.
- Write test cases for the new import_file_controller and the existing export_file_controller.
- Find and fix the existing bugs in the import and export process flows.
Extended Project Scope
Below listed is the stretch goal for this project. We'll try to pick it only if the time permits.
- After the functionality is fixed, the next step is to leverage the import and export controllers for importing/exporting participants, users, topics, teams, questionnaires, and grade_for_submission. Initially, High Priority modules are users, topics, and grade_for_submission, and then move on to the other modules.
Existing Import/Export Controller design
The existing import and export functionality primarily uses the import_file_controller and export_file_controller.
SignUpTopic and User, rely on helper classes that extract attributes from a hash and create an ActiveRecord object.
The questionnaire relies on a helper method that can import Question objects (objects that make up a Questionnaire) from a CSV and adjust the size of the associated QuestionAdvice (the words that pop up after you pick a certain number of stars). However, these functions might be deprecated, as it appears that Question importing is now routed through the ImportFileController unsuccessfully. More detail about specific functions is provided below.
Controllers
- import_file_controller, the list of methods in the controller are the following:
- File processing methods:
- #get_delimiter - Sets proper delimiter for filetype
- #parse_line - Processes line (row) of the file
- #parse_to_grid - Turns file into 2D array
- #parse_to_hash - Turns 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.
- File processing methods:
- questionnaires_controller, the list of methods in the controller are the following:
- ::import - Allows import from CSV using QuestionnaireHelper (Appears to be deprecated/unused)
- export_file_controller, the list of methods in the controller are the following:
- ::find_delim_filename - Sets and returns attributes for User object from hash.
- ::exportdetails - Generates the metadata for the data that needs to be exported.
- ::export - Converts the data into CSV format files.
- ::export_advices - Export question advice data to CSV file.
- ::export_tags - Export the tags.
Helpers
- import_file_helper, the list of methods in the file are the following:
- ::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 - Find the filename and delimiter.
- questionnaire_helper (Appears to be deprecated/unused), the list of methods in the file are the following:
- ::get_questions_from_csv - Allows Question and QuestionAdvice import/creation with CSV file.
Proposed Design Change
- Search for occurrences of “import” and “export” to find all the valid models that can be imported or exported.
- Identify the flow of import/export functionality involving those models and figure out whether they are using import_file_controller/export_file_controller or using their own logic to perform these actions.
- Once all the involved pieces are identified, we will change the code to ensure that all the involved modules make use of the new import_file_controller/export_file_controller.
- Once everything is in working condition, we will proceed with testing.
In a more generic way, this is how the import functionality is working as of now (taking one example for simplicity):
Import_file_controller is used for importing the data of team and course_participant models in a CSV. Questionnaires_controller is being used to import data of the question model.
What we want it to look like: UML Diagram
A generic import_file_controller/export_file_controller should be used for importing/exporting the data of all modules i.e., team, course_participant and questions model.
Refer to the above PR (https://github.com/expertiza/expertiza/pull/1438) for reference.
The major part of our effort will be targeted at rewriting this import_file_controller controller and removing the pieces which are breaking. Once we achieve that, we will create helper methods specific to each method and leverage the controllers to do the import and export in a consistent fashion across the application.
Analysis of Import Functionality Code Flow
We conducted an in-depth analysis of the code flow within the import_file_controller.rb file to understand how it handles the import functionality across different models. Our focus was primarily on two key models: Users and Topics. Through our analysis, we identified certain areas where the import functionality was being utilized by these models. However, we encountered several bugs within the import functionality, particularly in the handling of Topics. We addressed these issues and implemented fixes, ensuring smooth operation of the import process.
Bug Fixes
We identified and resolved several bugs within the import functionality, particularly in the handling of Topics. These fixes were essential for ensuring the correct import of data and the smooth functioning of the import process.
Test Cases for Import File Controller
Following the completion of our analysis and the implementation of necessary fixes for the import functionality, we proceeded to write comprehensive test cases for the import_file_controller.rb file. Since no existing test cases were available, we employed a combination of the previous team's test cases and the test skeletons provided by the professor and TA's guidance. Our test cases primarily focus on the functionality related to the Users and Topics models. The methods within these controllers are complex and contain numerous conditional statements tailored to different models. Therefore, our test cases are designed to thoroughly evaluate the behavior of these methods across various scenarios.
New Features Added
- Export Topics - The Export Topics Feature was not present in the application. You can navigate to Export Topics by going to Assignments > Click on Edit (pencil icon) > Click on Topics menu. Once you click the Export Topics button, you will be taken to a customizations page where you can select what columns you want in your CSV file. You can select/deselect the options by clicking on the checkboxes provided. Once you select the desired cloumns, click on the export button to generate the CSV file. The Export Topics and Customizations pages are attached below.
[[File:|800x520px]]
Bug Fixes
On analyzing the code flow for import and export functionality we found that the Export Grades and Import Topics functionality was broken. There were a few things that were creating problems in this flow. Mentioned below are the fixes that we made in this flow:
- Fix 1: Export grades into a file successfully - When exporting grades, self.export method of Assignment.rb is called. This internally calls review_grades method in Scoring.rb. But review_grades method was not being invoked because it was being called from a class method but was imported as an instance method. Then self.export method internally invokes self.export_data method. After fixing that, there were some lines related to the business logic were missing here. We added those lines and it can be seen in the code diff.
[[File:|800x520px]]
- Code Changes for Fix 1: Export grades into a file successfully
- Fix 2: Incorrect headers in the exported file - During the export, the generated CSV had some issues with the headers. A few headers were not separated into multiple columns which was not the desired result. We fixed those as well.
- Code Changes for Fix 2: Incorrect headers in the exported file
- Fix 3: Only export selected columns from the UI - Removed author feedback score from being exported by unchecking that option in the UI as seen in image below.
[[File:|800x520px]]
But what happens in the exported file?
- Problem 1: The meta-review feedback column is removed but the author feedback header still shows up.
- Problem 2: Also only the headers have changed but all values of all columns are still being exported.
[[File:|800x520px]]
- Code Changes for Fix 3: Only export selected columns from the UI
[[File:|800x520px]]
[[File:|800x520px]]
- Fix 4: Category, Description, and Link were not being updated in the database while importing the CSV using the Import Topics button - We tried importing the topics.csv and it imported all the data except the Category, Description, and Link columns. These columns were optional columns and when we selected the checkboxes for these, the data was not persisted in the database. We debugged this workflow and fixed this bug.
- Code Changes for Fix 4: Category, Description, and Link were not being updated in the database while importing the CSV using the Import Topics button
Test Plan
Writing test cases for import_file_controller and export_file_controller. We would be focussing on writing test cases for the controllers and the helpers that we will create specific to the models.
Now we have only 3 methods in the export_file_controller class that we need to test. Three methods in the export_file_controller class that needed to be tested were:
- action_allowed - This is a method that calls "current_user_has_ta_privileges?" to ensure that the current user has the proper privileges to perform the export operation. We write a test for this method expecting that a user calling the method has required permissions.
- exportdetails - This method fetches the delimiter and the file name required to perform the export operation. It also stores additional metadata from the Assignments class in a CSV file. We will test this method by verifying whether the export process works with supported model types.
- export - This is the main method that does the actual exporting task. It gathers all the necessary data such as - assignment, course, participant id, team, questions, answers, reviews, etc, and saves the data unless the model is not supported. We will test this method by verifying whether the export process works with supported model types.
For the import_file_controller class, we identified the following methods to be tested:
- action_allowed - Similar to the one in export_file_controller, this is a method that in turn calls "current_user_has_ta_privileges?" to ensure that the current user has the proper privileges to perform the import operation. We write a test for this method expecting that a user calling the method has required permissions.
- show - This method stores necessary metadata required to perform import operations such as - user id, delimiters, header, and also additional information such as the assignment id and team number. We write a test to check whether the necessary variables in the method are populated.
- import - The import method is used to verify the hash header or body, and generate error messages if needed. We will be writing a test to verify if this method can throw an error if something goes wrong with the import process.
Execution of Test Plan
To run the test cases for export_file_controller use the command - rspec spec/controllers/export_file_controller_spec.rb
To run the test cases for import_file_controller use the command - rspec spec/controllers/import_file_controller_spec.rb '
We wrote the test cases for the above-mentioned methods focussing on the models (Users, Topics and Grades) since these were the high priority models assigned to us. Our Test Coverage so far is as follows:
- export_file_controller test coverage - 54.84%
- import_file_controller test coverage - 52.43%
The reason for low test coverage is the fact that our test cases revolve around the high-priority models mentioned above. We covered all the scenarios associated with those models only. Future work will involve writing test cases for other models as well.
We also implemented fixtures for testing the generated CSVs. This is a new concept that was not implemented earlier in this project and can be leveraged in the future to test the CSVs. This provides the developers ability to test the Export and Import functionalities for CSV files.
We did not attach the code difference screenshots for the test cases because it was quite big. Kindly refer to the PR to view the code changes for the test cases corresponding to export_file_controller and import_file_controller. Our Test cases are quite exhaustive, kindly refer to the PR to check the different contexts for each test case in both the controllers. For the sake of simplicity and to keep redundant stuff out we did not write each context on the Wiki page.