Expertiza_Wiki - User contributions [en]

CSC/ECE 517 Spring 2024 - E2401 Implementing and testing import & export controllers

2024-04-27T19:56:23Z

Mpatel29: /* Test Plan */

This wiki page is for the description of changes made under E2401 OSS assignment for Spring 2024, CSC/ECE 517.

== Team==
=== Mentor ===
* Ed Gehringer

=== Team Members ===
* John Nolan - jrnolan2@ncsu.edu
* Meet Patel - mpatel29@ncsu.edu
* Deana Franks - dlfranks@ncsu.edu

== Expertiza Background ==
Expertiza is an open source Ruby on Rails application for teachers to manage classrooms. Expertiza is maintained by the teaching staff at NC State along with the very students who use it. Expertiza has many different models saved inside it including assignments, questions, and users. Relevant to our project, these models can be imported or exported through the website as CSV files for ease of uploading or editing many objects at once.

== Description of project ==
[[File:Team E2401 Import Shared Functionality.PNG|thumb|right|Import page is now common to all models. See the model type is a parameter in the web address bar.]]
Our project's description changed late into the project submission period. We were originally tasked with using the new implementation of the import_file_controller.rb [[CSC517 Spring 2019/E1923 New Framework For Import/Export|created by team E1923]]. Their project involved consolidating all the import functions into one common functionality. Before team 1923, each model used their own import code. However, these import codes shared a lot of functionality and were ripe for refactoring. E1923's import_file_controller.rb used polymorphism to fix this duplicate code. Each model had import specific methods that the import file controller calls and receives the information necessary to create and save the respective models. We spent the early stages of the project understanding the previous team's code. Our original goal was to take several models not yet utilizing this import_file_controller.rb and extending them to use the common code.

However, later into the project we learned that unfortunately, [https://github.com/expertiza/expertiza/pull/1438 team E1923's code was never merged into the main Expertiza repository]. In 2019, the project was tagged with merge, however, it never actually was and later closed in 2021. Upon this discovery, our project's mission changed dramatically. Instead of getting other models to work with the existing import_file_controller.rb, our goal was to redo the changes of team 1923. Our goal is to get justice for the work of E1923 by getting their changes into Expertiza. However, our project was not as easy as just copying over the changes from E1923. The codebase has changed significantly over the previous 5 years which meant that much of their code and tests had to be fast forwarded to integrate into the current repository. Additionally, E1923 finished their project without the static analysis tools that currently check for styling and vulnerabilities. There were 100s of errors on GitHub Actions related to styling, test failures, and vulnerabilities that we had to correct given the scale of E1923's changes. Because of the late-term revelation, Dr. Ed graciously extended our deadline by a few days to allow our new goal to be completed.
== Our changes ==
Our changes can be split into three main phases. All changes worked towards a common goal to have E1923's code integrated into the current repository.

=== 1. Copying changes over ===
Under our mentor's advice, we could not directly merge E1923's code into the current Expertiza. This is simply because there have been so many changes over the last 5 years that the merge conflicts would be infeasible to sift through. Instead, we had to manually go file by file and make the changes matching E1923. We had to pay attention to two main diffs for each of the 47 files that E1923 changed. We first compared the changes that E1923 made to the 2019 Expertiza repo then compare the original 2019 repo against the modern repo. We had to resolve all the conflicts that arose from changes in these files throughout the years. For example, if a method was renamed during the last 5 years, we had to make sure the copied code was changed to use that new method.

=== 2. Test fixing ===
Upon completion of the copying phase, we discovered that many tests were failing. Luckily, many of these tests had common causes (originally just a few compilation errors). Despite our scrupulousness, a few bugs had still crawled into our updated code. We had to add some missing methods. Eventually, we got to the bugs not caused by simple copy errors as well. Some bugs were caused by outdated tests and deeper routed issues that occurred from trying to integrate the 2019 and current codebases. We rewrote tests and had to refactor E1923's code until we were finally to have all tests fixing.

=== 3. Static analysis fixing ===
[[File:Team E2401 Codeclimate Errors.png|thumb|right|Original codeclimate errors after copying and integrating E1923's code]]
As aforementioned, the previous team's project occurred at a time before GitHub Actions verified many aspects of the code using static analysis. This resulted in many, many errors being brought forth when we copied the previous team's code. Most severe were a handful of security vulnerabilities that existed in E1923's code. All the found security vulnerabilities existed in import_file_controller.rb.

The first, involved constantize. The error existed because constantize was being run on user provided input, therefore, a malicious actor could provide unintended classes to the import method and receive information about these classes and maybe even have these other classes added to the website's database. To fix this error, we created a simple ALLOWED_MODELS whitelist array that includes the strings of all the permitted class types. We then call constantize on the match one of these strings to avoid the GitHub vulnerability message. This way, only the classes we desire to be imported will actually be allowed by the allowed_model method.

The second vulnerability, involved the eval method. Upon further research, this is a '''terrible''' method to use in general in Ruby. The eval method will run the code of whatever is inside the string provided to it. While this is powerful, a malicious user can use this to run arbitrary code on Expertiza. The eval method was overkill for what it was really trying to do, parse a hash. We replaced the eval function with JSON.parse() which fixes this vulnerability. After the security vulnerabilities, we had 404 style errors thrown by codeclimate. Many of these changes were simple yet very tedious. For example, many suggested change double quotes to single quotes, removing trailing or excess whitespace, correcting indentation, removing unnecessary .to_s method calls, and adding spaces around the curly braces of hashes. After these changes, only 33 remain. Reviewing the GitHub Actions, we recognize that codeclimate is one the checks that is not required and we would recommend that many of the remaining suggestions are unhelpful and some may be suppressed.

For example, the recommended fixes for "Use the new Ruby 1.9 hash syntax" involves changing the rocket (=>) to a colon (:), however, this change in our context will actually cause a compilation error. Furthermore, some of the refactoring suggestions are unnecessary. For example, the branch condition size was determined to high for the get_topic_attributes method in sign_up_topic.rb, however, the method is simply only adding fields if they exist and splitting this code might make it more confusing to read, not less.

=='''Project 4 - DESIGN DOC'''==
The team has implemented a new framework for import and export functionality, focusing first on specific models due to the large number of models requiring import/export capabilities. The schema.rb file is affecting the pull request data, likely due to Rails migration commands. At present, the Travis CI pipeline is experiencing test failures, preventing the project's merge. We will ensure these tests are passing and expand the import and export functionality to include at least one more class.

=== Previous Implementations ===
#[https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Spring_2022_-_E2238._Implementing_and_testing_Import_%26_Export_controllers E2238 GitHub Repository]
#[https://github.com/expertiza/expertiza/pull/2396 E2238 Pull Request]
#[https://drive.google.com/file/d/1obg-vu1lSX28kILlATQkyTFkGGZXnoVq/view?usp=sharing Demo Video]

=== E2401 Current Implementation ===
#[https://github.com/mpatel029/expertiza E2401 GitHub Repository]
#[https://github.com/expertiza/expertiza/pull/2801 Pull Request]
#[https://youtu.be/jQKY-wI_PRw Demo Video]

=== Design Pattern ===
Polymorphism: The import_file_controller and export_file_controller dynamically determines which model to interact with based on the input model name provided. Each model adheres to a common interface for listing columns and fetching the data to import and export, which allows for polymorphic behavior.
*Import UML Diagram
[[File:Import file controller.jpg | 450px]]
*Export UML Diagram
[[File:Export file controller.jpg | 400px]]

== Current Status ==
=== import_file_controller ===
* Participants: The feature to import participants, involved in various course-related activities, is currently functional. However, there's ongoing work to improve error handling and streamline the import process for a smoother user experience.
* Questionnaire with a set of questions: The import functionality for questionnaires, consisting of predefined sets of questions for assessments, is partially implemented. Our goal is to finalize it for seamless integration with the assessment module.
* sign_up_topics: The import feature for sign-up topics, which are crucial for facilitating collaborative work and task allocation, is currently being worked on. We aim to enable users to efficiently import a list of sign-up topics, enhancing the assignment creation process.
* signed_up_teams: Importing signed-up teams, which is important for organizing group activities and submissions, is currently under development. We're striving to provide a streamlined import process for teams, ensuring accurate data representation and alignment with assignments.
* Teams: Importing teams, essential for collaborative assignments and peer evaluations, is partially implemented. Our focus is on enhancing the import functionality to support comprehensive team data importation and seamless integration with the assignment workflow.
* Users: Importing users, which is fundamental for user management and assignment participation, is currently operational. However, we're continuously working on improving data validation and error handling during user importation to maintain data integrity and system stability.

=== export_file_controller ===
Currently, the implementation for exporting features for Grades, Sign-up Topics, Teams, and Answer Tags is incomplete. Our team plans to finalize these features in the final project, with a focus on enhancing code readability and understandability.
* Grades for submission : As it stands, clicking the "Export" button on the export view page for "Grade_for_submission", which is associated with the assignment model, results in an error. Using instance variables (@variable) inside a class method can lead to errors if those instance variables are not initialized within the same context.
[[File:Grade-error.jpg | 600px]]

* Sign up topics : The export functionality for sign_up_topics, developed under the topic E2238, has not yet been integrated into the main Expertiza platform. Consequently, users are currently unable to export the list of topics from the Topics tab on the assignment page.
[[File:SignedupTopic-no-expot.jpg | 600px]]

* Teams : The feature for exporting the Team list successfully exports basic fields from the team model but fails to include all selected fields or properly organize data and headers are not properly aligned under the 'Team Members' header column in the CSV file.
[[File:Team-Not-aligned.jpg | 400px]]

== Proposed Solution ==
=== import_file_controller ===
*Participants: Enhance error handling mechanisms to provide clearer error messages and improve the import process by optimizing performance and user interface interactions. Additionally, consider reverting changes to shared.js and removing email as a required field for participants. Also, review the "required_import_fields" method to ensure it returns the necessary fields for participant importation. Furthermore, rename "assignment_participant" and "course_participant" to "participant" for consistency. Look at the database table for optional fields and include missing fields in error messages by implementing a new method to identify missing fields. Consider allowing participant users to call the import user method for easier user importation. Lastly, document the row_hash format for better understanding and maintainability.
*Questionnaire with a set of questions: Complete the implementation of questionnaire import functionality, ensuring compatibility with various question types and providing a user-friendly interface for mapping imported data to questionnaire templates.
*sign_up_topics: Develop robust import functionality for sign-up topics, allowing users to import topics from external sources with ease, and provide options for data validation and error handling during import.
*signed_up_teams: Enhance the import process for teams by implementing features such as automatic assignment of team members based on imported data, validation of team information against assignment criteria, and providing feedback on import status.
*Teams: Complete the implementation of team import functionality by adding support for importing additional team attributes, such as team roles and responsibilities, and integrating with assignment creation and management features for seamless workflow integration.
*Users: Implement robust data validation checks during user importation to ensure consistency and integrity of user data, and enhance error handling mechanisms to provide informative feedback to users in case of import errors.

=== export_file_controller ===
* Grades for submission : To accomplish this, we've modified the self.export method within assignment.rb by transitioning from instance variables to local variables. This change is crucial because using instance variables inside a class method can lead to errors. This is due to the class method being called on the class itself, not on an instance of the class.
* Sign up topics : We will add the “Export topics” link, and transfer the code from E2238 repository to our own. Subsequent steps will involve identifying and rectifying any bugs present.
* Teams : We will revise and rearrange the code block to properly align the data with the headers.

== Final Changes ==
=== Export controller ===
1. Grades for submission (assignment.rb)
* Old code: The exporting logic was largely handled within static methods, which led to code that was less modular and harder to maintain.
<pre>
def self.export(csv, parent_id, options)
@assignment = Assignment.find(parent_id)
@questions = {}
questionnaires = @assignment.questionnaires
questionnaires.each do |questionnaire|
if @assignment.varying_rubrics_by_round?
round = AssignmentQuestionnaire.find_by(assignment_id: @assignment.id, questionnaire_id: @questionnaire.id).used_in_round
questionnaire_symbol = round.nil? ? questionnaire.symbol : (questionnaire.symbol.to_s + round.to_s).to_sym
else
questionnaire_symbol = questionnaire.symbol
end
@questions[questionnaire_symbol] = questionnaire.questions
end
@scores = @assignment.review_grades(@assignment, @questions)
return csv if @scores[:teams].nil?
export_data(csv, @scores, options)
end

EXPORT_FIELDS = { team_score: ['Team Max', 'Team Min', 'Team Avg'], submitted_score: ['Submitted Max', 'Submitted Min', 'Submitted Avg'], metareview_score: ['Metareview Max', 'Metareview Min', 'Metareview Avg'], author_feedback_score: ['Author Feedback Max, Author Feedback Min, Author Feedback Avg'], teammate_review_score: ['Teammate Review Max', 'Teammate Review Min', 'Teammate Review Avg'] }.freeze

def self.export_fields(options)
fields = []
fields << 'Team Name'
fields << 'Team Member(s)'
EXPORT_FIELDS.each do |key, value|
next unless options[key.to_s] == 'true'

value.each do |f|
fields.push(f)
end
end
fields.push('Final Score')
fields
end
</pre>
* New code: We introduce a new class, ExportAssignment, which encapsulates the exporting functionality. This class-based approach allows better separation of concerns and makes the codebase more organized. We break down the tasks into smaller ones so that each method in the ExportAssignment class adheres to the single responsibility principle. We implemented a class method for exporting fields, allowing you to extract fields without needing to instantiate the class and use it elsewhere.
<pre>
def self.export(csv, parent_id, options)
export_assignment = ExportAssignment.new(csv, parent_id, options)
csv = export_assignment.export()
end
def self.export_fields(options)
fields = ExportAssignment.export_fields(options)
end
</pre>

2 Sign up Topic (sign_up_topic.rb)

* Old code: We inherited the export method from the previous team's (E2238) implementation. They used instance variables (@assignment, @signuptopics, @slots_filled, @slots_waitlisted, @signedupteam, @users). However, since self.export is a class method, using instance variables in this context is inappropriate.

* New code: Currently, the Expertiza site does not support exporting the sign-up topic list by assignment. Therefore, we added a UI button on the view page and included a new column for the assignment name in the CSV file. Additionally, we have switched from using instance variables to local variables in the self.export method. This change is appropriate for a method defined with self.
[[File:SignedupTopic-no-expot.jpg | 600px]] [[File:SignedupTopic-expot.jpg | 600px]]

3 Team (team.rb)

* Old code: The export function for the course team list is not working, and the team member list is not properly aligned under the "Team Members" header.
<pre>
def self.export(csv, parent_id, options, teamtype)
if teamtype.is_a?(CourseTeam)
teams = CourseTeam.where(parent_id: parent_id)
elsif teamtype.is_a?(AssignmentTeam)
teams = AssignmentTeam.where(parent_id: parent_id)
end
teams.each do |team|
output = []
output.push(team.name)
if options[:team_name] == 'false'
team_members = TeamsUser.where(team_id: team.id)
team_members.each do |user|
output.push(user.name)
end
end
csv << output
end
csv
end
</pre>
* New code: The current Expertiza site includes a UI for exporting course team lists, but it was not functioning properly. To address this, we updated and refactored the code to ensure the feature is operational. Additionally, we added the course name to the course CSV file and the assignment name to the assignment CSV file. We also organized the team members list under the "Team Members" column.
<pre>
def self.export(csv, parent_id, options, teamtype)
team_parent = nil
# Determine the context and get the teams based on the team type
if teamtype.is_a?(CourseTeam)
team_parent = Course.find_by(id: parent_id)
elsif teamtype.is_a?(AssignmentTeam)
team_parent = Assignment.find_by(id: parent_id)
end
teams = Team.where(parent_id: parent_id)
return csv if team_parent.nil? # Exit if no team_parent is found
teams.each do |team|
# Start with team_parent name (course or assignment) and team name
output = [team_parent.name, team.name]
# Append team members' names if options[:team_name] is false
if options[:team_name] == 'false'
team_members = TeamsUser.where(team_id: team.id)
team_members.each do |user|
output.push(user.name)
end
end
# Append the output array to the CSV
csv << output
end
csv
end
</pre>

== Files modified ==
* expertiza/app/controllers/import_file_controller.rb
* expertiza/app/controllers/export_file_controller.rb
* expertiza/app/models/user.rb
* expertiza/app/models/assignment.rb
* expertiza/app/models/sign_up_topic.rb
* expertiza/app/models/team.rb
* expertiza/app/models/assignment_team.rb
* expertiza/app/models/course_team.rb
* expertiza/lib/scoring.rb
* expertiza/app/models/assignment_participant.rb

== Test Plan ==
For the export_file_controller class, we identified the following methods to be tested:

*action_allowed:
** when someone is logged in:
*** allows certain action for admin
*** allows certain action for instructor
*** allows certain action for ta
*** refuses certain action for student
** when no one is logged in refuses certain action
*Start action:
**given valid parameters
**does not assign the expected mode
**does not assign the expected id
*export:
** exports a csv file with User model data
**export grade data for submissions for an assignment into a CSV file
**export sign up topic data for an assignment into a CSV file
**export assignment teams for an assignment into a CSV file
**export assignment teams for an assignment into a CSV file

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.

*'''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.

*'''parse_to_hash''' - This block contains tests for the parse_to_hash method of the controller. "when has_header is true": Tests whether the method correctly parses the contents grid into a hash when the has_header parameter is true.b. "when has_header is false": Tests whether the method correctly parses the contents grid into a hash when the has_header parameter is false.

*'''parse_to_grid''' - This block contains tests for the parse_to_grid method of the controller. "when given contents and delimiter": Tests whether the method correctly parses the contents into a grid of parsed lines.
*'''parse_line''' - This block contains tests for the parse_line method of the controller. "when the delimiter is a comma": Tests whether the method correctly splits the line by comma and handles double quotes correctly. "when the delimiter is not a comma": Tests whether the method correctly splits the line by the given delimiter and removes double quotes.
*import:
**redirects to user list page when import data for User model succeeds
**redirects to user list page after flashing error message when import data for User model fails and raises an exception
**redirects to user Asignments/:id page when import data for SignUpTopic model succeeds with no optional parameters
**redirects to user Asignments/:id page when import data for SignUpTopic model succeeds with one (category) optional parameters
**redirects to user Asignments/:id page when import data for SignUpTopic model succeeds with two (category, description) optional parameters
**redirects to user Asignments/:id page when import data for SignUpTopic model succeeds with all (category, description, link) optional parameters

== Execution of Test Plan ==

We focused on writing test cases for the high-priority models—Users, Grades for submission, Assignment teams, and Course teams —that were assigned to us. Our test coverage so far is as follows:

=== Rspec Test ===
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'' ''''

=== CSV File Results ===
1. Grades for submission

[[File:Assignment-new.jpg | 1000px]]

2. Sign up topics

[[File:SignUpTopic-new.jpg | 600px]]

3. Assignment teams

[[File:Assignment-team-new.jpg | 300px]]

4. Course teams

[[File:Course-Team-new.jpg | 300px]]

CSC/ECE 517 Spring 2024 - E2401 Implementing and testing import & export controllers

2024-04-10T02:24:22Z

Mpatel29: /* import_file_controller */

This wiki page is for the description of changes made under E2401 OSS assignment for Spring 2024, CSC/ECE 517.

== Team==
=== Mentor ===
* Ed Gehringer

=== Team Members ===
* John Nolan - jrnolan2@ncsu.edu
* Meet Patel - mpatel29@ncsu.edu
* Deana Franks - dlfranks@ncsu.edu

== Expertiza Background ==
Expertiza is an open source Ruby on Rails application for teachers to manage classrooms. Expertiza is maintained by the teaching staff at NC State along with the very students who use it. Expertiza has many different models saved inside it including assignments, questions, and users. Relevant to our project, these models can be imported or exported through the website as CSV files for ease of uploading or editing many objects at once.

== Description of project ==
[[File:Team E2401 Import Shared Functionality.PNG|thumb|right|Import page is now common to all models. See the model type is a parameter in the web address bar.]]
Our project's description changed late into the project submission period. We were originally tasked with using the new implementation of the import_file_controller.rb [[CSC517 Spring 2019/E1923 New Framework For Import/Export|created by team E1923]]. Their project involved consolidating all the import functions into one common functionality. Before team 1923, each model used their own import code. However, these import codes shared a lot of functionality and were ripe for refactoring. E1923's import_file_controller.rb used polymorphism to fix this duplicate code. Each model had import specific methods that the import file controller calls and receives the information necessary to create and save the respective models. We spent the early stages of the project understanding the previous team's code. Our original goal was to take several models not yet utilizing this import_file_controller.rb and extending them to use the common code.

However, later into the project we learned that unfortunately, [https://github.com/expertiza/expertiza/pull/1438 team E1923's code was never merged into the main Expertiza repository]. In 2019, the project was tagged with merge, however, it never actually was and later closed in 2021. Upon this discovery, our project's mission changed dramatically. Instead of getting other models to work with the existing import_file_controller.rb, our goal was to redo the changes of team 1923. Our goal is to get justice for the work of E1923 by getting their changes into Expertiza. However, our project was not as easy as just copying over the changes from E1923. The codebase has changed significantly over the previous 5 years which meant that much of their code and tests had to be fast forwarded to integrate into the current repository. Additionally, E1923 finished their project without the static analysis tools that currently check for styling and vulnerabilities. There were 100s of errors on GitHub Actions related to styling, test failures, and vulnerabilities that we had to correct given the scale of E1923's changes. Because of the late-term revelation, Dr. Ed graciously extended our deadline by a few days to allow our new goal to be completed.

=='''Project 4 - DESIGN DOC'''==
The team has implemented a new framework for import and export functionality, focusing first on specific models due to the large number of models requiring import/export capabilities. The schema.rb file is affecting the pull request data, likely due to Rails migration commands. At present, the Travis CI pipeline is experiencing test failures, preventing the project's merge. We will ensure these tests are passing and expand the import and export functionality to include at least one more class.

=== Previous Implementations ===
#[https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Spring_2022_-_E2238._Implementing_and_testing_Import_%26_Export_controllers E2238 GitHub Repository]
#[https://github.com/expertiza/expertiza/pull/2396 E2238 Pull Request]
#[https://drive.google.com/file/d/1obg-vu1lSX28kILlATQkyTFkGGZXnoVq/view?usp=sharing Demo Video]

=== E2401 Current Implementation ===
#[https://github.com/mpatel029/expertiza E2401 GitHub Repository]

=== Design Pattern ===
Polymorphism: The import_file_controller and export_file_controller dynamically determines which model to interact with based on the input model name provided. Each model adheres to a common interface for listing columns and fetching the data to import and export, which allows for polymorphic behavior.
*Import UML Diagram
[[File:Import file controller.jpg | 450px]]
*Export UML Diagram
[[File:Export file controller.jpg | 400px]]

== Current Status ==
=== import_file_controller ===
* Participants: The feature to import participants, involved in various course-related activities, is currently functional. However, there's ongoing work to improve error handling and streamline the import process for a smoother user experience.
* Questionnaire with a set of questions: The import functionality for questionnaires, consisting of predefined sets of questions for assessments, is partially implemented. Our goal is to finalize it for seamless integration with the assessment module.
* sign_up_topics: The import feature for sign-up topics, which are crucial for facilitating collaborative work and task allocation, is currently being worked on. We aim to enable users to efficiently import a list of sign-up topics, enhancing the assignment creation process.
* signed_up_teams: Importing signed-up teams, which is important for organizing group activities and submissions, is currently under development. We're striving to provide a streamlined import process for teams, ensuring accurate data representation and alignment with assignments.
* Teams: Importing teams, essential for collaborative assignments and peer evaluations, is partially implemented. Our focus is on enhancing the import functionality to support comprehensive team data importation and seamless integration with the assignment workflow.
* Users: Importing users, which is fundamental for user management and assignment participation, is currently operational. However, we're continuously working on improving data validation and error handling during user importation to maintain data integrity and system stability.

=== export_file_controller ===
Currently, the implementation for exporting features for Grades, Sign-up Topics, Teams, and Answer Tags is incomplete. Our team plans to finalize these features in the final project, with a focus on enhancing code readability and understandability.
* Grades for submission : As it stands, clicking the "Export" button on the export view page for "Grade_for_submission", which is associated with the assignment model, results in an error. Using instance variables (@variable) inside a class method can lead to errors if those instance variables are not initialized within the same context.
[[File:Grade-error.jpg | 600px]]

* Sign up topics : The export functionality for sign_up_topics, developed under the topic E2238, has not yet been integrated into the main Expertiza platform. Consequently, users are currently unable to export the list of topics from the Topics tab on the assignment page.
[[File:SignedupTopic-no-expot.jpg | 600px]]

* Teams : The feature for exporting the Team list successfully exports basic fields from the team model but fails to include all selected fields or properly organize data and headers are not properly aligned under the 'Team Members' header column in the CSV file.
[[File:Team-Not-aligned.jpg | 600px]]

* Tagged Answers : The current implementation does not adhere to the polymorphism approach like other models within the new export_file_controller. Therefore, we plan to eliminate the export_tag method. Instead, we'll employ the start action to direct users to the consolidated export view page, where they can select the relevant fields of the requested model.
[[File:Answer tags.jpg | 600px]]

== Proposed Solution ==
=== import_file_controller ===
*Participants: Enhance error handling mechanisms to provide clearer error messages and improve the import process by optimizing performance and user interface interactions. Additionally, consider reverting changes to shared.js and removing email as a required field for participants. Also, review the "required_import_fields" method to ensure it returns the necessary fields for participant importation. Furthermore, rename "assignment_participant" and "course_participant" to "participant" for consistency. Look at the database table for optional fields and include missing fields in error messages by implementing a new method to identify missing fields. Consider allowing participant users to call the import user method for easier user importation. Lastly, document the row_hash format for better understanding and maintainability.
*Questionnaire with a set of questions: Complete the implementation of questionnaire import functionality, ensuring compatibility with various question types and providing a user-friendly interface for mapping imported data to questionnaire templates.
*sign_up_topics: Develop robust import functionality for sign-up topics, allowing users to import topics from external sources with ease, and provide options for data validation and error handling during import.
*signed_up_teams: Enhance the import process for teams by implementing features such as automatic assignment of team members based on imported data, validation of team information against assignment criteria, and providing feedback on import status.
*Teams: Complete the implementation of team import functionality by adding support for importing additional team attributes, such as team roles and responsibilities, and integrating with assignment creation and management features for seamless workflow integration.
*Users: Implement robust data validation checks during user importation to ensure consistency and integrity of user data, and enhance error handling mechanisms to provide informative feedback to users in case of import errors.

=== export_file_controller ===
* Grades for submission : To accomplish this, we've modified the self.export method within assignment.rb by transitioning from instance variables to local variables. This change is crucial because using instance variables inside a class method can lead to errors. This is due to the class method being called on the class itself, not on an instance of the class.
* Sign up topics : We will add the “Export topics” link, and transfer the code from E2238 repository to our own. Subsequent steps will involve identifying and rectifying any bugs present.
* Teams : We will revise and rearrange the code block to properly align the data with the headers.
* Tagged Answers :
#We plan to eliminate the export_tag method from the export_file_controller, add TagPromptDeployment model to the polymorphism list, and implement the required methods within the TagPromptDeployment class to enable these functionalities. Additionally, on the view page, we'll adjust the layout of the export link to direct users to the export page, where they can generate a list of answer tags.
#The CSV file for exporting answer tags currently lacks some of the relevant data found on the answer tag report page, presumably because it only retrieves data from the main model's attributes. We plan to include the data displayed on the answer tags report page, which showcases amounts of data across multiple years using tabs. However, since the export_files_controller is set up only for CSV exports, which cannot accommodate multiple sheets like an Excel file, we propose a workaround. Our idea is to organize the data by year in the CSV file, separating each year's data with a blank row. This is feasible since all years share the same headers.

== Test Plan ==
Previous testing issue: The team has used the new framework for import and export. They prioritized the specific models as there were initially a lot of models to add import/export. Schema.rb is skewing the pull request data due to probably rails migrate commands. Currently, the Travis CI pipeline has tests failing.
=== export_file_controller ===

Three methods in the export_file_controller class that needed to be tested:

*action_allowed: This method's role is to check if the current user has the necessary permissions (in this case, teaching assistant privileges or higher) to perform the export operation. This can involve mocking or simulating the current_user_has_ta_privileges? method's return values to represent different user scenarios.

*export: We must verify the accurate retrieval and formatting of metadata from the specified models. This entails comparing the export method's output with expected values for provided inputs, to confirm consistent and correct behavior across various export configurations.

*'''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.

== Our changes ==
Our changes can be split into three main phases. All changes worked towards a common goal to have E1923's code integrated into the current repository.

=== 1. Copying changes over ===
Under our mentor's advice, we could not directly merge E1923's code into the current Expertiza. This is simply because there have been so many changes over the last 5 years that the merge conflicts would be infeasible to sift through. Instead, we had to manually go file by file and make the changes matching E1923. We had to pay attention to two main diffs for each of the 47 files that E1923 changed. We first compared the changes that E1923 made to the 2019 Expertiza repo then compare the original 2019 repo against the modern repo. We had to resolve all the conflicts that arose from changes in these files throughout the years. For example, if a method was renamed during the last 5 years, we had to make sure the copied code was changed to use that new method.

=== 2. Test fixing ===
Upon completion of the copying phase, we discovered that many tests were failing. Luckily, many of these tests had common causes (originally just a few compilation errors). Despite our scrupulousness, a few bugs had still crawled into our updated code. We had to add some missing methods. Eventually, we got to the bugs not caused by simple copy errors as well. Some bugs were caused by outdated tests and deeper routed issues that occurred from trying to integrate the 2019 and current codebases. We rewrote tests and had to refactor E1923's code until we were finally to have all tests fixing.

=== 3. Static analysis fixing ===
[[File:Team E2401 Codeclimate Errors.png|thumb|right|Original codeclimate errors after copying and integrating E1923's code]]
As aforementioned, the previous team's project occurred at a time before GitHub Actions verified many aspects of the code using static analysis. This resulted in many, many errors being brought forth when we copied the previous team's code. Most severe were a handful of security vulnerabilities that existed in E1923's code. All the found security vulnerabilities existed in import_file_controller.rb.

The first, involved constantize. The error existed because constantize was being run on user provided input, therefore, a malicious actor could provide unintended classes to the import method and receive information about these classes and maybe even have these other classes added to the website's database. To fix this error, we created a simple ALLOWED_MODELS whitelist array that includes the strings of all the permitted class types. We then call constantize on the match one of these strings to avoid the GitHub vulnerability message. This way, only the classes we desire to be imported will actually be allowed by the allowed_model method.

The second vulnerability, involved the eval method. Upon further research, this is a '''terrible''' method to use in general in Ruby. The eval method will run the code of whatever is inside the string provided to it. While this is powerful, a malicious user can use this to run arbitrary code on Expertiza. The eval method was overkill for what it was really trying to do, parse a hash. We replaced the eval function with JSON.parse() which fixes this vulnerability. After the security vulnerabilities, we had 404 style errors thrown by codeclimate. Many of these changes were simple yet very tedious. For example, many suggested change double quotes to single quotes, removing trailing or excess whitespace, correcting indentation, removing unnecessary .to_s method calls, and adding spaces around the curly braces of hashes. After these changes, only 33 remain. Reviewing the GitHub Actions, we recognize that codeclimate is one the checks that is not required and we would recommend that many of the remaining suggestions are unhelpful and some may be suppressed.

For example, the recommended fixes for "Use the new Ruby 1.9 hash syntax" involves changing the rocket (=>) to a colon (:), however, this change in our context will actually cause a compilation error. Furthermore, some of the refactoring suggestions are unnecessary. For example, the branch condition size was determined to high for the get_topic_attributes method in sign_up_topic.rb, however, the method is simply only adding fields if they exist and splitting this code might make it more confusing to read, not less.

== Future improvements ==
Like our preceding team, E1923, we have not touched the exporting code. Exporting could receive a mirroring change to that of importing to allow for a similar sharing of code. Additionally, as mentioned in the static analysis fixing section, there are some codeclimate errors remaining in our final pull request. If the teaching staff believes all of these should be fixed as the automated system describes, this would be a future necessary task, and potentially a good option as a good continuation for our team into Project 4.

CSC/ECE 517 Spring 2024 - E2401 Implementing and testing import & export controllers

2024-04-10T02:14:37Z

Mpatel29: /* import_file_controller */

User:Mpatel29

2024-03-26T04:31:27Z

Mpatel29: /* New Features Added */

== 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.

*'''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):'''

[[Image: badUML.png]]

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'''

[[Image: goodUML.png ]]

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.

== 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'''''

[[File:Fix1 |800x520px]]

*'''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.
[[File:Incorrect grouped column.jpeg|800x520px]]

*'''''Code Changes for Fix 2: Incorrect headers in the exported file'''''

[[File:Fix2 |800x520px]]

*'''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'''''

[[File:Fix |800x520px]]

== 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%'''''

[[File:Export test |800x520px]]

*'''''import_file_controller test coverage - 52.43%'''''

[[File:Import |800x520px]]

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.

== '''''Link to Pull Request:''''' ==

User:Mpatel29

2024-03-26T02:56:09Z

Mpatel29:

== 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.

*'''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):'''

[[Image: badUML.png]]

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'''

[[Image: goodUML.png ]]

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:Export |800x520px]]

[[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'''''

[[File:Fix1 |800x520px]]

*'''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.
[[File:Incorrect grouped column.jpeg|800x520px]]

*'''''Code Changes for Fix 2: Incorrect headers in the exported file'''''

[[File:Fix2 |800x520px]]

*'''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'''''

[[File:Fix |800x520px]]

== 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%'''''

[[File:Export test |800x520px]]

*'''''import_file_controller test coverage - 52.43%'''''

[[File:Import |800x520px]]

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.

== '''''Link to Pull Request:''''' ==

User:Mpatel29

2024-03-26T02:53:06Z

Mpatel29: 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..."

== 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.

*'''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):'''

[[Image: badUML.png]]

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'''

[[Image: goodUML.png ]]

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:Export Topics.png|800x520px]]

[[File:Customization.png|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:Bug1.png|800x520px]]

*'''''Code Changes for Fix 1: Export grades into a file successfully'''''

[[File:Fix1 code.png|800x520px]]

*'''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.
[[File:Incorrect grouped column.jpeg|800x520px]]

*'''''Code Changes for Fix 2: Incorrect headers in the exported file'''''

[[File:Fix2 code.png|800x520px]]

*'''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:E3.png|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:E3 1.png|800x520px]]

*'''''Code Changes for Fix 3: Only export selected columns from the UI'''''

[[File:Fix3 code.png|800x520px]]

[[File:B3.jpeg|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'''''

[[File:Fix 4.png|800x520px]]

== 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%'''''

[[File:Export test coverage.png|800x520px]]

*'''''import_file_controller test coverage - 52.43%'''''

[[File:Import test coverage.png|800x520px]]

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.

== '''''Link to Pull Request:''''' https://github.com/expertiza/expertiza/pull/2396 ==