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 :
• Questionnaire with a set of questions :
• sign_up_topics :
• signed_up_teams :
• Teams :
• Users :

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.

• Answer Tags : 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

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.
• Answer Tags :

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'll adjust the layout of the export link to direct users to the export page, where they can generate a list of answer tags.
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.

• 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: you should check that any metadata from the Assignments class is accurately fetched and formatted. This might involve comparing the output of export against expected values for given inputs, ensuring that the method behaves correctly across different 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.