CSC/ECE 517 Spring 2025 - E2524 Reimplement student review controller

From Expertiza_Wiki
Jump to navigation Jump to search

About 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 topics 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

The goal of this project is to reimplement the `StudentReviewController` and its associated views. The reimplementation will introduce a new service layer, streamline logic, and remove obsolete code (e.g., references to metareviews). The new design prioritizes clarity, modularity, and maintainability, following Rails best practices and RESTful conventions.

Purpose and Scope

This reimplementation addresses longstanding design issues in the original controller, including overly complex methods, redundant logic, and tight coupling between controller and view layers.

Objectives

  • Redesign the controller to delegate all business logic to a new service class
  • Eliminate obsolete functionality (e.g., metareviews)
  • Improve testability by enabling unit tests on the service layer
  • Simplify the view templates by preparing data in the controller beforehand
  • Adhere to Rails RESTful conventions and best practices.
  • Improve maintainability, localization support, and scalability.

Requirements

Reimplement the `StudentReviewController` and ensure all associated components function correctly. The components involved include:

Controller

student_review_controller.rb

Views

Located at app/views/student_review:

  • `list.html.erb` – main controller view
  • `_responses.html.erb` – renders each review
  • `_set_dynamic_review.html.erb` – dynamic review form
  • `_set_dynamic_metareview.html.erb` – dynamic metareview form (to be removed)

Models Used

  • `AssignmentParticipant`
  • `Assignment`
  • `SignedUpTeam`
  • `ReviewResponseMap`
  • `SampleReview`
  • `MetareviewResponseMap` (obsolete – all references to be removed)

Existing Issues

  • Controller Complexity:
 - The `list` method is too long and handles multiple responsibilities: data loading, logic, rendering setup, and redirects.  
 - Business logic is intertwined with HTTP concerns (e.g., redirection, instance variables).
  • Redundant Logic:
 - The `action_allowed?` method checks for student privileges multiple times.
  • Hard-Coded Values:
 - Literal strings like `"list"` and `"submitter"` are used instead of constants.
  • Obsolete Code:
 - The `MetareviewResponseMap` model is no longer relevant but still referenced.
  • Lack of Separation of Concerns:
 - Controller handles business logic that should reside in service/model layers.

Implementation

To be added after reimplementation and testing are complete.

Controller Diagram

To be added once the service class and interface are finalized.


Design / Proposed Solution

Separation of Concerns

This reimplementation is based on a clear separation of responsibilities:

  • The controller handles only routing, access validation, and view setup.
  • The service object encapsulates all business logic and data handling.
  • Obsolete functionality such as `MetareviewResponseMap` is eliminated to simplify the design.

Architectural Overview

The new design will be based on the following structure:

  • A `StudentReviewController` that handles only HTTP responsibilities.
  • A new `StudentReviewService` that encapsulates all business logic.
  • Continued use of models like `AssignmentParticipant`, `Assignment`, and `ReviewResponseMap` (excluding `MetareviewResponseMap`).
  • Views adjusted to work with service-driven data.

Breakdown of Controller Responsibilities

  • Validate access using a simplified `action_allowed?` method
  • Set locale using controller_locale
  • Call `StudentReviewService.new(id, current_user).perform` to gather data
  • Assign service output to instance variables for the view
  • Redirect if bidding is enabled

Breakdown of Service Object Responsibilities

  • Retrieve `AssignmentParticipant` and associated `Assignment`
  • Calculate `topic_id` using `SignedUpTeam.topic_id`
  • Determine review phase using `assignment.current_stage`
  • Get reviewer via `participant.get_reviewer`
  • Load and (if applicable) sort review mappings (`ReviewResponseMap`)
  • Count completed and in-progress reviews
  • Load sample reviews (`SampleReview`) and extract `response_ids`
  • Check if bidding is enabled

Removal of Obsolete Code

  • All logic involving `MetareviewResponseMap` will be deleted
  • Related counters for metareview progress will be removed from both controller and views

action_allowed? Method

  • Eliminate redundant privilege check: `|| current_user_has_student_privileges?`
  • Use constants instead of literal strings (`LIST_ACTION = 'list'`)
  • Restructure logic as:
return false unless current_user_has_student_privileges?
return are_needed_authorizations_present?(params[:id], SUBMITTER_ROLE) if action_name == LIST_ACTION
true

controller_locale Method

  • Ensure the `controller_locale` method sets the locale appropriately for student users, supporting internationalization and accessibility.

list Method

The `list` method will be removed and replaced with a simplified call to the service: 

def list
  return unless valid_participant?
  @data = StudentReviewService.new(params[:id], current_user.id).perform
  redirect_to bidding_path if @data[:bidding_enabled]
end

View Refactoring Plan

  • `list.html.erb`: Simplify to only display data passed in from `@data`
  • `_responses.html.erb`: Remove inline logic (e.g., topic ID lookups) and rely on passed-in values
  • `_set_dynamic_*`: May be removed if dynamic review logic is relocated or rewritten
  • View logic for metareviews will be removed
  • Ensure views follow Rails i18n localization conventions by replacing any hardcoded strings with translation keys where appropriate.

Testing Plan

To ensure the integrity of the reimplementation, both the service objects and controller actions should have dedicated tests. It is recommended to generate test skeletons as soon as the methods are in place.

Service Object Testing

Thorough unit tests will be added to validate each responsibility of `StudentReviewService`:

Key Test Areas:

  • Participant and assignment lookup
  • Topic ID calculation
  • Reviewer existence
  • Review mapping fetching and sorting (including calibrated assignments)
  • Completed/in-progress review counts
  • Sample review loading and ID extraction
  • Bidding check
  • Validate that the service object correctly handles all business logic and returns expected outcomes.
  • Retrieve the participant using the provided ID.
  • Load the associated assignment.
  • Calculate topic ID using the participant’s parent ID and user ID.
  • Determine the current assignment phase based on the topic ID.
  • Check for existence of a reviewer.
  • Load correct review mappings based on reviewer existence.
  • Sort review mappings correctly for calibrated assignments.
  • Count total number of review mappings.
  • Count completed reviews by checking submitted responses.
  • Calculate number of in-progress reviews.
  • Load sample reviews for the assignment.
  • Extract and collect response IDs from sample reviews.
  • Identify whether bidding is enabled for the assignment.

Controller Testing

  • Verify that the `StudentReviewController` interacts properly with the service object and handles HTTP responses appropriately.

Key Test Areas:

  • `action_allowed?` method:
 * Test both cases: when the user has student privileges and when they do not.
 * Confirm that `false` is returned when privileges are absent, and that the extra check is done for the `list` action when privileges are present.
  • `list` action:
 * Ensure controller delegates business logic to the service object.
 * Validate correct setting of instance variables based on service output.
 * Test redirection to bidding page when bidding is enabled.
 * Confirm correct behavior when the current user is unauthorized.
  • `controller_locale` method:
 * Verify that the locale is correctly set for student users.
 * Test fallback behavior if locale cannot be determined.

View Testing

  • Test rendering of `list.html.erb` with full and empty review sets
  • Validate rendering with various review statuses
  • Confirm removal of metareview-related UI elements


Design Principles Applied (Planned)

  • Single Responsibility Principle (SRP):
 Each component is focused on a distinct task.
  • DRY (Don't Repeat Yourself):
 Shared logic is extracted into reusable service methods.
  • Maintainability and Testability:
 A modular design makes future updates and testing easier.
  • Use of Constants:
 Magic strings are replaced with named constants for clarity.
  • RESTful Best Practices:
 Routes and controller actions follow Rails conventions
  • Localization and Internationalization:
 Support for multiple languages via proper use of the `controller_locale` method and Rails i18n framework.

Team

Mentor

  • Janice Uwujaren

Students

  • Kenil Patel (kpatel47)
  • Smit Patel (spatel68)
  • Katerina Vilkomir (evilkom)

Relevant Links

Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Spring_2025_-_E2524_Reimplement_student_review_controller&oldid=163401"