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

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

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

1. E2238 GitHub Repository
2. E2238 Pull Request
3. Demo Video

E2401 Current Implementation

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

• Export UML Diagram

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.

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

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

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

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 :

1. 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 will modify the layout to ensure the export link redirects users to the export view page, allowing them to choose column options.
2. 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

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.