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

From Expertiza_Wiki
Jump to navigation Jump to search

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)

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 (TEMP)

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

In 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 participants_by_user Clearer and more descriptive
list_assignment_participants participants_by_assignment Consistent with the above
show show_participant Explicit about what is shown
add create_participant_with_authorization Describes both creation and auth setup
update_authorization update_participant_authorization Makes the subject (participant) clear
destroy destroy_participant More descriptive for further use
filter_user_participants participants_for_user Highlights the purpose of the method
filter_assignment_participants participants_for_assignment Highlights the purpose of the method
validate_authorization validated_authorization Suggests return value is validated

Improve Documentation and Comments

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

Testing Plan (TEMP)

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/Modified 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)

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]