CSC/ECE 517 Spring 2025 - E2542. Refactor review bids controller.rb

Introduction

Purpose

The purpose of the Review Bids Controller is to create review bids on reviews, send HTTP requests to an outside webservice (hosted on a VCL), and handle the overall review biddings the students intend to interact with. This project is intended to expand on our own previous implementation of the OSS project, which includes cleaning up functions, increasing maintainability, and reducing repeated code, and eliminating single responsibility principle violations.

Scope

The Review Bids Controller handles the review biddings, processes the review selections as they are returned from the python webservice, whether in a valid or invalid response state (handled by fallback algorithm) and handles serving responses to the views.

System Overview

• Data Flow Diagram

• User Flow Diagram

• Key Components
  • Controllers:
    • ReviewBidsController
    • SignUpSheetController
  • Models:
    • AssignmentParticipant
    • Assignment
    • ReviewResponseMap
    • SignUpTopic
    • SignedUpTeam
    • ReviewBid
  • ServiceObjects:
    • BidsAlgorithmService
    • BidsBiddingService
    • BidsPriorityService
    • CompletedReviewsCounter
• Functionality Summary
  • Bid creation is done by users through expertiza UI - Users drag and drop reviews, organizing them according to how their specific priority aligns with what want to work on
  • Updating bid priorities is done through a similar view and can be done by rearranging the table
  • Review sign up

Architecture and Design

• Review_Bids_Controller Overview
  • Acts as intermediary between UI and webservice we use to run the bidding algorithm on our python module.
  • Receives processed input from the webservice which comes from our views
  • Validates and processes incoming data
  • Handles success / error responses from the webservice. We have implemented a timeout and better responses from the Service Object employed by the controller.
  • In the case of failure, the controller will use a fallback algorithm (implemented as a round-robin method, which will still assign students topics)
• Service Object Integration
  • BidsAlgorithmService
    • Encapsulates bidding logic, processes review bids, and supports a fallback algorithm
  • CompletedReviewsCounterService:
    • Isolates business logic for counting completed reviews
  • AssignBiddingService
    • Isolates the logic for the process of assigning bidding to students
  • BidsPriorityService
    • Sets the priority of topic bids

• The data flow of our application is highlighted in the above diagram!

Frontend View in Expertiza

Specific Objectives

• General Code Refactoring and Readability improvements - simplifying logic, variable naming, improving documentation
• Method Specific improvements - validations, role checks, caching repeated queries to reduce database calls
  • e.g. action_allowed?, index, etc.
• BidsAlgorithmService - encapsulates the logic to call the WebService to run the bidding algorithm.
• WebService and UI Enhancements - documentation, refactoring tables that users bid with

Design Principles and Refactoring Goals

• Single Responsibility Principle
  • Controllers have been refactored to be kept thin by moving business logic into service objects (BidsAlgorithmService, AssignBiddingService, CompletedReviewCounterService, BidsPriorityService)
  • The service objects listed above have been utilized in the review_bids_controller to shorten methods and keep them more concise.

• Separation of Concerns
  • We will distinguish between HTTP request handling (controllers) and business logic (service objects - BidsAlgorithmService, AssignBiddingService, CompletedReviewCounterService, BidsPriorityService)

• DRY (Don't repeat yourself)
  • We will consolidate duplicated logic like bid processing into reusable modules and services.

• Consistency
  • Update naming conventions (BidsAlgorithmService).
  • Standardize response formats and error handling in service methods.

• Modularity
  • Methods have been refactored to use ServiceObjects which have been listed above

• Testability
  • The code tests all the service objects and the controller thoroughly and we have also expanded the tests in the Python web service.

• Reusability
  • We employed consistent naming structure throughout all the updates and code

Component Design: ReviewBidsController

The ReviewBidsController manages the full lifecycle of the review bidding process for an assignment in Expertiza. It provides functionality for both privileged users (e.g., instructors, administrators) and students, handling permissions, view rendering, bid creation and update, and review topic assignment.

Method: action_allowed?

This method controls user access to controller actions. It allows 'Instructor', 'Teaching Assistant', 'Administrator', 'Super-Administrator', and 'Student' roles to access show, set_priority, and index. Other actions are restricted to non-student roles. It ensures proper permissions for accessing review bidding features.

Method: index

This method gathers information for the "Your Work" page, where users can view their assigned reviews. It checks user identity, retrieves associated reviews, and counts completed reviews. The view rendered is sign_up_sheet/review_bids_others_work.View: others_work.html.erb

Method: show

This method renders the bidding page where users select topics they'd prefer to review. It fetches the assignment, available topics, and any existing bids. Already assigned or signed-up topics are excluded. The page also lists any assigned reviews.View: show.html.erb

Method: set_priority

This method stores or updates a participant's topic preferences. It removes old bids not in the current selection and creates or updates the remaining bids with new priorities. This method is triggered when users submit their topic rankings on the bidding page.

Method: assign_bidding

This method coordinates the assignment of review topics. It gathers reviewer IDs and invokes the bidding algorithm using a service (BidsAlgorithmService). It validates the output and uses ReviewBid.assign_review_topics to perform the actual assignment. Bidding is then disabled.

Service: AssignBiddingService

This service object takes the logic from assign_bidding and compacts it into a single responsibility service object where it can be used to eliminate bloat from the controller

Service: CompletedReviewCounterService

This service object counts the number of completed reviews it is passed

Service: BidsPriorityService

This service object shows the priority of bids

Method: set_participant (Private)

Fetches the AssignmentParticipant record from the params[:id]. Redirects to the home page if the participant is not found.

Method: set_assignment (Private)

Sets the @assignment instance variable based on the current participant’s associated assignment.

Method: authorize_participant (Private)

Verifies that the logged-in user matches the participant’s user ID. If not, redirects to the home page.

Method: delete_all_bids_and_redirect (Private)

Helper used when a participant deselects all topics. It deletes all current bids for the participant and redirects them to the bidding page.

API/Webservice Sample Throughput

Sample case:

Request:

{
 {
   "tid": [4427, 4428, 4429, 4430],
   "users": {
     "40763": {
         "bids": [
         { "tid": 4430, "priority": 3, "timestamp": "Sun, 15 Nov 2020 17:16:34 EST -05:00" },
         { "tid": 4428, "priority": 2, "timestamp": "Sun, 15 Nov 2020 17:16:35 EST -05:00" },
         { "tid": 4427, "priority": 1, "timestamp": "Sun, 15 Nov 2020 17:16:37 EST -05:00" }
       ],
       "otid": 4429
     },
     "40764": {
       "bids": [
         { "tid": 4429, "priority": 3, "timestamp": "Sun, 15 Nov 2020 17:16:34 EST -05:00" },
         { "tid": 4430, "priority": 2, "timestamp": "Sun, 15 Nov 2020 17:16:35 EST -05:00" },
         { "tid": 4428, "priority": 1, "timestamp": "Sun, 15 Nov 2020 17:16:37 EST -05:00" }
       ],
       "otid": 4427
     },
     "40765": {
       "bids": [
         { "tid": 4427, "priority": 3, "timestamp": "Sun, 15 Nov 2020 17:17:15 EST -05:00" },
         { "tid": 4428, "priority": 1, "timestamp": "Sun, 15 Nov 2020 17:17:16 EST -05:00" },
         { "tid": 4429, "priority": 2, "timestamp": "Sun, 15 Nov 2020 17:17:17 EST -05:00" }
       ],
       "otid": 4428
     }
   },
   "max_accepted_proposals": 3
 }

Response

The webservice algorithm reflects the conflicts of topics and assigns the users who bid first the topic correctly.

{
   "40763": [
       4427,
       4428,
       4430
   ],
   "40764": [
       4428,
       4430,
       4429
   ],
   "40765": [
       4429,
       4427,
       4430
   ]
}

Timeout and Retry Policies

• Plan:
  • We implemented a 10 second timeout to the RestClient post request. This gives us a certain threshold to wait if the webservice is unresponsive.
  • If the primary webservice fails, a fallback "round robin" algorithm was implemented in the previous OSS project as a way for students to still be assigned topics.

Implementation Plan

Affected Repositories

• The changes will be made on the expertiza/expertiza repository
• Changes will be made using ruby and python - Python for WebService, Ruby for expertiza.

Refactoring action_allowed? method

• Move the arrays of roles into constants so they are easier to manage
• Use if/else statements instead of a ternary statement to increase readability
• Find opportunities to return early if a condition is not met. e.g. if a user's role is not part of allowed roles
• Add comments for each conditional branch
• Use one method (params[:action] or action_name) to check current action for clarity.

Refactoring index method

• Move the logic for counting completed reviews into a service object called CompletedReviewCounter. (Violates SRP)
• Use before_action filters to laod participant and assignment and check auth to align to DRY principle.
• Refactor variable names (num_reviews_completed -> completed_reviews_count)
• Include error handling for cases where participant cannot be found

Refactoring set_priority method

• Move bid processing logic into a dedicated service object to thin out controller
• Assign repeated queries like fetching review bids or assignment id's to local variables to avoid multiple database calls
• Rename variables to closer reflect their intended purpose (params[:topic] -> selected_topic_ids)
• Utilize early returns to simplify conditional logic and reduce nesting in method
• Move logic for removing unselected bids, creating new bids, updating existing bids to a service object

Additional Changes

• Rename bidding service to better reflect its purpose
• Consistency in response structure in send_bidding_request
• Add timeout parameter to prevent indefinite waits if external service becomes unresponsive
• Improve comments and documentation throughout expertiza and the Python Webservice
• Expand test coverage to include additional branches of conditional logic
• Ensure error-handling paths are exercised to verify failures are managed correctly

Testing Plan

Review Bids Controller

• TC1 - action_allowed? method - Passed
  • Tests if certain user roles are authorized to run certain actions.

• TC2 - index method - Passed
  • Ensures the others_view page is rendered when the index method is called

• TC3 - show method - Passed
  • Ensures the bidding page is rendered when the show method is called

• TC4 - set_priority method - Passed
  • Tests when topics are selected
  • Tests when no topics are selected
  • Checks the response redirects to root_path

• TC5 - assign_bidding method - Passed
  • Tests that the user gets redirected based on success or fail of bidding assignment

• TC6 - Authorization issues - Passed
  • Tests participant not found and when user is not authorized
  • Expects a redirect to the root path

AssignBiddingService

• TC7 - call_by_assignment method - Passed:
  • Tests when the external service succeeds
  • Tests when the external webservice fails and the fallback algorithm is used
  • Tests when a reviewer has no topics assigned
  • Tests when unexpected exceptions occur

BidsAlgorithmService

• TC8 - run_bidding_algorithm method - Passed:
  • Tests when the web service is available and tests the timeout
  • Tests when the webservice is unavailable

• TC9 - process_bidding method - Passed:
  • Tests when web service succeeds and passes sample data
  • Tests when fallback is used and provides sample data.

BidsPriorityService

• TC10 - process_bids method - Passed:
  • Tests the action of processing bids
  • Tests when no topics are selected or removed

CompletedReviewsCounterService

• TC11 - .count_reviews method - Passed
  • Tests when all reviews are submitted
  • Tests when some are not submitted
  • Tests when some have empty responses

External Webservice

This was tested via pytest in its own repository

• TC12 - BiddingAlgorithm Webservice - Passed
  • Tests a valid POST request
  • Tests an invalid POST Request
  • Tests when all users choose the same topic and same priority
  • Tests when users bid more than is allowed

Relevant Links

• Pull Request
• Video