CSC/ECE 517 Spring 2025 - E2531 Refactor participants controller.rb

Introduction

From the project description: "This project aims to refactor the participants_controller.rb and its helper class, participants_helper.rb, in the Expertiza system. The focus will be on improving code structure by implementing DRY and SOLID principles, enhancing readability with meaningful comments and clear naming conventions, and ensuring robustness through comprehensive RSpec testing." Since this is a refactoring project, we will also be utilizing Large Language Models (LLMs) to aid in development. More information on how we used them can be found in the section below.

Use of Large Language Models (LLMs)

For this project, we are using ChatGPT as an LLM helper in determining what issues there are within the code. The use of LLMs in this project will likely be limited to probing for potential issues that exist within the system.

Below we have organized the following prompts and a summary of the output in a table:

Prompts	LLM Output Summary
[Code from participants_controller.rb] Are there any ways where we can refactor the code to improve readability, DRYness, and clarity of error handling?	It recognized some of the issues we, as a team, had already identified and suggested fixes that were similar to what we outlined: Redundant JSON Rendering Code[1] and Simplifying the Assignment of Participant Fields[2]. Additionally, it suggested minor changes that we could implement, such as using Participant.where(...) instead of Participant.all to filter out the code more efficiently instead of getting all database instances every time, as well as the use of strong parameters in add() and update_authorization() to avoid having to implement custom parsing techniques. It also recommends renaming the add() function to something more specific in relation to its purpose (adding a Participant to an Assignment.
Potential Redundant Tests in participants_controller_spec.rb	The tests on line 222 and 231 are redundant as only the authorization differs. (Note: This should be investigated to see whether or not this is necessary)
Checking for issues with the current testing files	The "Get Participant by ID call" test was returning a 201 status code instead of the more suitable 200 code

For more detailed information on what prompts we used and the output we got, please visit the page here: [3].

Requirements

Objectives:

• Apply DRY principles to eliminate redundant code.
• Ensure adherence to SOLID principles for better maintainability.
• Use appropriate design patterns to optimize code architecture.
• Write clear, informative comments and use meaningful naming conventions.
• Test the refactored code extensively with RSpec to verify functionality.

Deliverables:

• Refactored code for participants_controller.rb and helper class.
• Comprehensive test suite and coverage report.
• Updated and detailed documentation.
• Video demonstration of the API functionality.

Existing Issues

• There are some API functions that are currently not functioning properly, making it somewhat difficult to test
• Some API functions are not returning the correct HTML status messages (e.g. test on line 136 returns 201 instead of 200)
• Documentation and comments can be somewhat vague
• In-line comments should be more descriptive, explaining what the code does in more details
• Tests are not described via comments, but in name alone; some more descriptive comments that explain the tests in more detail would be ideal
• Eliminate instances of redundant code
• The method naming should be more descriptive

Design / Proposed Solution

Based on the previous implementation's controller diagram, we have created a new version that contains our proposed changes and modifications to participants_controller.rb.

Simplifying Redundant JSON Rendering Code

Many of the functions in the current implementation contain a conditional if-else statement that determines what HTML status code is returned alongside any necessary values that are requested. Due to its simple nature, it might be possible to reduce the amount of space this takes up by implementing a private helper method that would shorten the process.

Example:

  if participants.nil?
    render json: participants.errors, status: :unprocessable_entity
  else
    render json: participants, status: :ok
  end

Simplifying the Assignment of Participant Fields

The add() and update() both take up a significant amount of space to assign values to database fields and are essentially the same in functionality (in terms of this specific section. We can remove redundancy by creating a method that can remove this redundancy.

Example

  permissions = retrieve_participant_permissions(authorization)

  participant.authorization = authorization
  participant.can_submit = permissions[:can_submit]
  participant.can_review = permissions[:can_review]
  participant.can_take_quiz = permissions[:can_take_quiz]
  participant.can_mentor = permissions[:can_mentor]

Provide More Clarity in Function Names

There are some functions that are somewhat unclear/vague in participants_controller.rb. For example, the list_user_participants() and list_assignment_participants().

- Suggested Method Name Improvements

Old Method Name	New Method Name	Notes
list_user_participants	get_participants_by_user	Clearer and more descriptive
list_assignment_participants	get_participants_by_assignment	Clearer and more descriptive and is consistent with the other get method
add	add_participant_to_assignment	Describes both creation and authorization setup of participant to assignment
filter_user_participants	filter_participants_by_user	Highlights the purpose of the method
filter_assignment_participants	filter_participants_by_assignment	Highlights the purpose of the method

Improve Documentation and Comments

Some of the documentation and in-line comments can be somewhat vague. This can lead to misconceptions about how some functions work or how some of the code can function. Below are potential improvements we plan to make for function descriptions, though these may change with the final implementation.

Improved Method Comments

Method	Original Comment	Refactored Comment
list_user_participants	Return a list of participants for a given user	Retrieves all participants associated with the specified user ID
list_assignment_participants	Return a list of participants for a given assignment	Retrieves all participants linked to the specified assignment ID
show	Return a specified participant	Fetch and return the participant with the given ID
add	Assign the specified authorization to the participant and add them to an assignment	Creates a new participant for a given assignment and assigns appropriate authorization and permissions
update_authorization	Update the specified participant to the specified authorization	Updates the participant’s authorization role and related permissions
destroy	Delete a participant	Deletes the participant identified by the given ID and returns a confirmation message
participant_params	Permitted parameters for creating a Participant object	Strong parameters: list of attributes allowed for participant creation or update
filter_user_participants	Filters participants based on the provided user. Returns participants ordered by their IDs	Returns a sorted list of participants filtered by user, if provided
filter_assignment_participants	Filters participants based on the provided assignment. Returns participants ordered by their IDs	Returns a sorted list of participants filtered by assignment, if provided
validate_authorization	Validates that the authorization parameter is present and is one of the following valid authorizations: reader, reviewer, submitter, mentor	Ensures authorization param is valid and returns it in lowercase; otherwise returns error response
find_user / find_assignment / find_participant	Finds a user/assignment/participant by ID and returns it	Fetches the corresponding record by ID; renders a not-found error if record does not exist

Proposed Testing Changes

• Add more descriptive comments for tests
• Add test that checks that authorizations are updated correctly
• Add test that checks that inputted path parameters are not missing and valid inputs
• Tests at Line 222 and Line 231 are redundant, as the only difference in the test is the authorization, which would not affect what happens if a participant is not found

Implementation

As this is a refactoring project, most of our work on the codebase involved cleaning up the current codebase to better align with foundational code style principles rather than introducing any of kind of new functionality. Our primary focus is to make the code more readable and easier to work with than before. We also took the liberty of fixing some minor errors that we found along the way.

Changelog

• Naming changes to multiple functions for better clarity and consistency
• Changed HTML status message for show() from 201 (Created) to 200 (OK) as it is a better fit
• Created the assign_participant_permissions() method
• Introduced two additional tests and removed a redundant one (see Testing Plan for more details)
• Removed redundant queries to Participant.all() in both filtering methods
• Updated method comments to be more descriptive (see comments table in Design)

Function Name Changes

The previous implementation had several functions that were strangely named or had names that could be easily misconstrued. The name changes remain the same from the proposal and design section, but for the purpose of clarity, the rationale for each of the functions will be provided below. The update_authorization method is not

list_user_participants and list_assignment_participants

These two functions are meant to return a list of participants given a User or Assignment respectively. Based on the name alone, it is difficult to tell that the function would return a User/Assignment or Participants. In order to remedy this, we chose the names get_participants_by_user and get_participants_by_assignment. The word "list" was replaced by "get" in order to better denote that it is returning something and the "by" is added to emphasize that the User and Assignment are being used as parameters to search for the Participants. This provides a much clearer picture of what these functions should do compared to before.

add

The add function's name was too vague to be particularly useful in identifying its purpose. To be more accurate, the function was meant to add the user to a given assignment, which would return a participant from the assignment's add_participant function. The new name of the function provides much more context to what it does (add_participant_to_assignment).

filter_user_participants and filter_assignment_participants

Similar to the list methods that were previously mentioned, it is difficult to tell what exactly these two functions do. While it can be easily seen that it filters something, what is being filtered can be easily mistaken (e.g. does it filter users by participants or participants by users?). The new names, filter_participants_by_user and filter_participants_by_assignment are much clearer in defining that the User and Assignment are being used as the filter parameters for the function.

HTML Status Code Change for GET Call

The status code being used for this was previously 201, denoting "successful creation". We have corrected this to a simple 200 code, which signifies an "ok" status message. This issue was actually identified by ChatGPT while we were dowsing for potential issues.

Test modified

# Tests whether a participant returns properly if provided with a valid Participant ID
response '200', 'Returns a participant' do
  let(:id) { participant2.id }

  run_test! do |response|
    data = JSON.parse(response.body)
    expect(data['user_id']).to eq(studenta.id)
    expect(data['assignment_id']).to eq(assignment2.id)
  end
end

Redundant Participant.all Queries Removed

This issue was initially caught by the LLM, ChatGPT, that the team was using in order to find potential issues. We found that simply using Participant.where() removed the need for the Participant.all query entirely while changing nothing else about the function, reducing the amount of queries necessary to call the filtering methods, filter_participants_by_user and filter_participants_by_assignment.

Example:

# New function
def filter_participants_by_user(user)
  participants = Participant.where(user_id: user.id) if user # This would simply get the participants given the proper user ID
  participants.order(:id)
end

# Old function
def filter_participants_by_user(user)
  participants - Participant.all() # This would obtain the entire list of participants rather than just the necessary ones
  participants = participants.where(user_id: user.id) if user
  participants.order(:id)
end

New Private Method: assign_participant_permissions()

The reasoning behind the addition of this method is because of a repeated piece of code that existed in the previous implementation:

permissions = retrieve_participant_permissions(authorization)

participant.authorization = authorization
participant.can_submit = permissions[:can_submit]
participant.can_review = permissions[:can_review]
participant.can_take_quiz = permissions[:can_take_quiz]
participant.can_mentor = permissions[:can_mentor]

This was repeated in two different contexts: when the Participant was updated or created. This is a violation of the DRY principle, as the same code is repeated multiple times where it could have been reduced to make it more easily readable. In order to resolve this issue, we simply turned this code snippet into a private method that can be called within participants_controller.rb:

  # An authorization string containing the participant's role is taken and used to determine
  # what permissions will be allocated to the participant. Each of these permissions will
  # then be assigned to the Participant's database permission attributes.
  #
  # @param [String] authorization: An authorization string that represents the participant's role
  # @param [Participant] participant: The participant whose authorization permissions are being updated
  def assign_participant_permissions(authorization, participant)
    # Call helper method from participants_helper to retrieve a dictionary containing the
    # appropriate permission boolean values for the role specified by the authorization string
    permissions = retrieve_participant_permissions(authorization)

    # Assigns each of the boolean permission values to their respective database counterparts
    participant.authorization = authorization
    participant.can_submit = permissions[:can_submit]
    participant.can_review = permissions[:can_review]
    participant.can_take_quiz = permissions[:can_take_quiz]
    participant.can_mentor = permissions[:can_mentor]
  end

Description:

An authorization string containing the participant's role is taken and used to determine what permissions will be allocated to the participant. Each of these permissions will then be assigned to the Participant's database permission attributes.

Parameters:

• authorization: An authorization string that represents the participant's role. This can be either reader, reviewer, submitter, or mentor.
• participant: The participant whose attributes are being updated. This is passed in to be modified by the function to be used to assign each of the participant's attribute values that are representative of its assigned role.

Behavior

• Use retrieve_participant_permissions from participants_helper in order to get all necessary permission boolean values for a given participant role
• Assign each of the permission boolean values (can_submit, can_review, can_take_quiz, can_mentor)
from the helper method to their respective database attribute counterparts in Participant

Testing Plan

From the previous implementation, there are 24 tests that are all passing. For more information about the previous testing plan, please refer to the previous implementation's page here: [4]

New Tests

Test ID	Test Description
25	Check whether Participant properly assigns the appropriate permissions when updating authorizations to each new role
26	Check the validity of incoming path parameters (Invalid: Non-Existent)

Modified Tests

Test ID	Status	Modification Description
22	Removed	This test was removed as it was redundant. While it is important to check the authorization of the participant that would be invalid, the invalid ID used for this test is a duplicate of the previous test, meaning the result is guaranteed to match that of the previous test regardless of what is meant to be tested.

Team

Mentor

• Aniruddha Rajnekar <aarajnek@ncsu.edu>

Students

• Akhil Adusumilli <aadusum@ncsu.edu>

• Vansh Dodiya <vkdodiya@ncsu.edu>

• Brian Huynh <bhhuynh@ncsu.edu>

Relevant Links

Pull Request (Unimplemented) []
GitHub Repository [5]
GitHub Project Board [6]
participants_controller.rb in Old Expertiza [7]
participants_helper.rb in Old Expertiza [8]