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

Expertiza

Expertiza is an open source project based on Ruby on Rails framework. Expertiza allows the instructor to create new assignments and customize new or existing assignments. It also allows the instructor to create a list of reviews the students can sign up for. Students can form teams in Expertiza to work on various projects and assignments. Students can also peer review other students' submissions. Expertiza supports submission across various document types, including the URLs and wiki pages.

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

• Context Diagram

• Key Components
  • Controllers:
    • ReviewBidsController
    • SignUpSheetController
  • Models:
    • AssignmentParticipant
    • Assignment
    • ReviewResponseMap
    • SignUpTopic
    • SignedUpTeam
    • ReviewBid
  • ServiceObjects:
    • BidsAlgorithmService (proposed rename)
    • 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

• MVC Architecture

• Controller
  • 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 accordingly (implements fallback algorithm)
• Service Object Integration
  • BidsAlgorithmService
    • Encapsulates bidding logic, processes review bids, and supports a fallback algorithm
  • CompletedReviewsCounter:
    • Isolates business logic for counting completed reviews

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

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 will be refactored to be kept thin by moving business logic into service objects (BidsAlgorithmService)

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

• 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
  • Break down complex methods into smaller, self contained functions for better readability and easier testing.

• Testability
  • Code should be able to facilitate unit and integration testing, ensuring all branches are covered, including error cases.

• Reusability
  • Employ consistent naming and clear structure and purpose allows easy reuse across the expertiza environment

Component Design

The controller includes 8 methods that handle the review bidding process. The public methods cover authorization, rendering views, saving user bids, and coordinating review assignment. The private methods support these functions by abstracting data gathering and algorithm coordination logic.

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.

Method: fetch_reviewer_ids (Private)

This helper method retrieves all AssignmentParticipant IDs for a given assignment. These IDs represent the reviewers participating in the bidding process.

Method: process_bidding (Private)

This helper method delegates the logic for running the bidding algorithm to the service class BidsAlgorithmService. It takes the assignment ID and list of reviewers as input and returns matched topic assignments.

Method: ensure_valid_topics (Private)

This method ensures that all reviewers have valid topic assignment entries, even if the algorithm returns nothing (e.g., due to web service failure). It initializes empty topic arrays for any missing reviewers.

API/Webservice Integration

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
 }

Output

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

• To be implemented after full implementation
  • Plan:
    • Document timeout configurations for external calls
    • Outline fallback strategies when the primary service is unavailable

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
• Expand test coverage to include additional branches of conditional logic
• Ensure error-handling paths are exercised to verify failures are managed correctly
• Increase comments / documentation in the webservice repository.

Testing Plan

• Unit Tests
  • Unit tests will be written for each model, ensuring validations, associations, and business logic are correct
  • Unit tests will also be written for service objects with mocked external calls

• Each controller will also be tested that it sets correct instance variables, renders correct views, and handles redirection correctly

• The system will also be end to end tested from bid submission to processing including a fallback scenario

• Edge cases will be handled appropriately (error responses) in testing the service models

• We will monitor code coverage targets and ensure all conditional logic is exercised.