CSC/ECE 517 Fall 2025 - E2551. Reimplementing SubmittedContentController: Difference between revisions

Latest revision as of 15:09, 31 October 2025

Introduction

Background

The SubmittedContentController in Expertiza manages student-submitted content for assignments, including uploading/deleting files, submitting/removing hyperlinks, handling folders, and downloading content. Over time, the original controller became complex with methods performing multiple responsibilities, inconsistent handling of similar actions, and code duplication.

A previous project team (E2417) attempted reimplementation but their work was not merged due to several issues identified during code review.

Motivation

This project provides students an opportunity to collaborate on an open-source educational platform while learning about Rails, RSpec, REST API design, and software engineering best practices.

The E2417 implementation (PR #78) left several critical issues unresolved:

Law of Demeter violations throughout the codebase
Overuse of instance variables causing tight coupling
Inconsistent helper method usage (mixed class methods and instance methods)
Missing critical functionality - students had no way to view uploaded files
Sparse documentation making code difficult to maintain

Tasks Identified

Ensure that participants can: Submit/remove hyperlinks.
Upload, view, and delete files.
Check file types and directory contents.
Receive clear, concise messages and appropriate HTTP status codes for every action.
Refactor methods to follow the Single Responsibility Principle and eliminate duplicate or inconsistent logic.
Handle file operations and hyperlink operations as analogously as possible; evaluate whether separate controllers would improve clarity.
Reduce overuse of instance variables; use them only when necessary.
Refactor away Law of Demeter violations, e.g., avoid deep chains like participant.team.path.to_s.
Create or reuse supporting models (e.g., submission records).
Keep responses short and descriptive, returning proper HTTP codes for success and error cases.
Add inline comments to document every method.
Skip authorization and logging logic in this phase.

Classes

New Files Created:

controllers/api/v1/submitted_content_controller.rb (375 lines)
helpers/submitted_content_helper.rb (248 lines)
helpers/file_helper.rb (34 lines)
models/submission_record.rb (21 lines)
spec/requests/api/v1/submitted_content_spec.rb (829 lines)

Modified Files:

models/assignment.rb
models/assignment_participant.rb
models/assignment_team.rb

Problem Analysis

Issues with E2417 Implementation

Law of Demeter Violations

The previous implementation had deep method chaining throughout the controller:

# E2417 Code - Violation
current_directory = File.join(team.path.to_s, current_folder)
# Chains: team → assignment → path (3 levels deep)

Overuse of Instance Variables

E2417 used instance variables extensively throughout the controller, creating unnecessary coupling and making the code harder to test and maintain.

Inconsistent Helper Usage

The FileHelper module had mixed usage patterns:

FileHelper.sanitize_filename(name)  # Sometimes class method
sanitize_filename(name)              # Other times instance method

Missing Critical Functionality

The E2417 implementation had no list_files endpoint. Students could upload files but had no way to view what they uploaded, check if uploads succeeded, or browse their folder structure.

Implementation

Architecture Decision

We chose a single unified controller approach rather than splitting into multiple controllers.

Rationale:

File and hyperlink operations share common validation/error handling patterns
Reduces code duplication
Provides unified API namespace (/api/v1/submitted_content)
Easier to maintain consistent response formats
E2417's split approach led to inconsistencies

Core Functionality Implemented

Endpoint	HTTP Method	Functionality
/api/v1/submitted_content	GET	List all submission records
/api/v1/submitted_content/:id	GET	Get specific submission record
/api/v1/submitted_content	POST	Create new submission record
/api/v1/submitted_content/submit_hyperlink	POST/GET	Submit hyperlink with URL validation
/api/v1/submitted_content/remove_hyperlink	POST/GET	Remove hyperlink by index
/api/v1/submitted_content/submit_file	POST/GET	Upload file with validation (5MB limit, extension check)
/api/v1/submitted_content/list_files	GET	NEW: List directory contents with metadata
/api/v1/submitted_content/folder_action	POST/GET	File operations (delete/rename/move/copy/create)
/api/v1/submitted_content/download	GET	Download submitted file

Key Improvements

Fixing Law of Demeter Violations

Before (E2417):

current_directory = File.join(team.path.to_s, current_folder)

After (Our Implementation):

# Added delegation in models
delegate :path, to: :team, prefix: true, allow_nil: true

# Controller now uses clean delegation
current_directory = File.join(@participant.team_path.to_s, current_folder)

Reducing Instance Variables

Reduced from many instance variables to only 2 required by before_action:

before_action :set_participant, only: [:submit_hyperlink, :submit_file, ...]
before_action :set_submission_record, only: [:show]

Standardizing Helper Methods

Before: Mixed FileHelper.method and method calls

After: Consistent instance methods via include FileHelper

Implementing list_files Endpoint

Returns file metadata including name, size, type, and modified date:

files << {
  name: entry,
  size: File.size(entry_path),
  type: File.extname(entry).delete('.'),
  modified_at: File.mtime(entry_path)
}

Other Functionality

Hyperlink operations: Submit with URL validation, remove by index
File upload: 5MB size limit, extension validation, optional unzip for zip files
Folder operations: Delete, rename, move, copy files; create folders
Download: Stream files with validation

Database Migrations

Critical Migrations Added:

directory_num column to teams table
submitted_hyperlinks column to teams table
submission_records table for audit trail

Supporting Models

SubmissionRecord

Provides audit trail for all submission operations with validations for record_type, content, operation, team_id, user, and assignment_id.

AssignmentTeam Enhancements

Added hyperlinks method to parse YAML-stored hyperlinks
Added submit_hyperlink with URL validation
Added remove_hyperlink for deletion
Added set_student_directory_num to assign directory numbers
Added delegation to fix Law of Demeter violations

Comparison with E2417

Aspect	E2417 Implementation	E2551 (Our Implementation)
Architecture	Unclear separation	Single controller + 2 organized helpers
Law of Demeter	Violations present	Fixed with Rails delegation
Instance Variables	Overused	Only 2 required
Helper Methods	Inconsistent	Standardized instance methods
list_files Endpoint	Missing	Implemented with metadata
Error Messages	Generic	Specific, actionable
HTTP Status Codes	Inconsistent	Proper codes
Documentation	Sparse	Comprehensive inline docs
Test Coverage	Incomplete	48 comprehensive tests

Testing

Test Coverage

Created 48 comprehensive test cases:

Submission Record CRUD (5 tests) - List, show, create operations
Hyperlink Operations (8 tests) - Submit, remove, validation, error handling
File Upload (10 tests) - Size check, extension validation, upload, unzip
Folder Operations (12 tests) - Delete, rename, move, copy, create folder
Download (5 tests) - Download file, various error cases
Error Handling (8 tests) - Participant/team not found scenarios

Running Tests

git clone https://github.com/aasthagaudani/reimplementation-back-end.git
cd reimplementation-back-end
git checkout aastha

docker compose up -d
docker compose exec app bundle install
docker compose exec app rails db:create db:migrate
docker compose exec app bundle exec rspec spec/requests/api/v1/submitted_content_spec.rb

Results: 40/48 tests passing (83%)

The 8 failing tests are test infrastructure issues (mock/stub conflicts), not functionality bugs. All endpoints work correctly via Swagger UI.

Swagger Testing

All endpoints documented in swagger/v1/swagger.yaml. Access at http://localhost:3002/api-docs after starting server.

Impact Analysis

Changes Summary

Total Changes: 2,354 lines added, 235 lines modified across 19 files

New Files:

Controller: 375 lines
Helpers: 282 lines
Model: 21 lines
Tests: 829 lines
Migrations: 3 files