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.

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.

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
2. Pull Request
3. 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

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

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.

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

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

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

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.

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.

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

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

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

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

2. Sign up topics

3. Assignment teams

4. Course teams