CSC/ECE 517 Fall 2025 - E2551. Reimplementing SubmittedContentController

From Expertiza_Wiki
Revision as of 23:39, 28 October 2025 by Agaudan (talk | contribs)
Jump to navigation Jump to search

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 and code duplication.

 A previous project team (E2417) attempted reimplementation in PR #78 but their work was not merged due to several issues:

 * Law of Demeter violations (deep method chains like participant.team.path.to_s)
 * Overuse of instance variables
 * Inconsistent helper method usage (mixed class/instance methods)
 * Missing critical functionality - no way to view uploaded files
 * Missing directory_num database column causing runtime errors
 * Sparse documentation

 ===Problem Statement===
 This project (E2551) aims to reimplement the SubmittedContentController in a more maintainable way, learning from E2417's feedback and addressing all identified issues.

 ===Goals===
 * Use single unified controller with organized helper modules
 * Eliminate Law of Demeter violations using Rails delegation
 * Reduce instance variables to minimal required set
 * Standardize helper method patterns
 * Implement missing list_files endpoint
 * Add missing database columns
 * Provide comprehensive documentation
 * Use proper HTTP status codes and error messages

 ==Design==

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

 Rationale:
 * File and hyperlink operations share validation/error handling patterns
 * Reduces code duplication
 * Unified API namespace
 * Easier to maintain consistent responses
 * E2417's split approach led to inconsistencies

 ===Files Created===
 * app/controllers/api/v1/submitted_content_controller.rb - Main controller (375 lines)
 * app/helpers/file_helper.rb - File utilities (34 lines)
 * app/helpers/submitted_content_helper.rb - File operations (248 lines)
 * app/models/submission_record.rb - Audit trail model (21 lines)
 * spec/requests/api/v1/submitted_content_spec.rb - Tests (829 lines)

 ===Files Modified===
 * app/models/assignment_participant.rb - Added delegation
 * app/models/assignment_team.rb - Added hyperlink methods, delegation
 * config/routes.rb - Added routes
 * swagger/v1/swagger.yaml - API documentation

 ==Implementation==

 ===API Endpoints Implemented===
Endpoint Method Functionality
/api/v1/submitted_content GET List all submission records
/api/v1/submitted_content/:id GET Get specific record
/api/v1/submitted_content POST Create submission record
/api/v1/submitted_content/submit_hyperlink POST Submit hyperlink
/api/v1/submitted_content/remove_hyperlink POST Remove hyperlink
/api/v1/submitted_content/submit_file POST Upload file
/api/v1/submitted_content/list_files GET List directory contents (NEW)
/api/v1/submitted_content/folder_action POST File operations
/api/v1/submitted_content/download GET Download file 
 ===Fixing Law of Demeter Violations===

 Problem in E2417:

  # Deep chaining - violates Law of Demeter
  current_directory = File.join(team.path.to_s, current_folder)
  # Chains: team → assignment → path

 Our Solution - Added Delegation:

 In AssignmentParticipant:

  class AssignmentParticipant < Participant
    delegate :path, to: :team, prefix: true, allow_nil: true
  end

 In AssignmentTeam:

  class AssignmentTeam < Team
    delegate :path, to: :assignment, prefix: true
    
    def path
      "#{assignment_path}/#{directory_num}"
    end
  end

 In Controller:

  # Clean delegation instead of deep chaining
  current_directory = File.join(@participant.team_path.to_s, current_folder)

 ===Reducing Instance Variables===

 E2417: Used many instance variables throughout

 Our Solution: Only 2 instance variables

  class Api::V1::SubmittedContentController < ApplicationController
    before_action :set_participant, only: [:submit_hyperlink, :submit_file, ...]
    
    private
    
    def set_participant
      @participant = AssignmentParticipant.find(params[:id])
    end

    def set_submission_record
      @submission_record = SubmissionRecord.find(params[:id])
    end
  end

 ===Standardizing Helper Methods===

 E2417: Mixed usage

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

 Our Solution: Consistent instance methods

  module FileHelper
    def sanitize_filename(file_name)
      just_filename = File.basename(file_name)
      clean_path(just_filename)
    end
  end

  # In controller
  include FileHelper
  # All calls: sanitize_filename(name)

 ===Critical Missing Feature: list_files===

 E2417: No endpoint to view uploaded files

 Our Implementation:

  def list_files
    team = participant_team
    team.set_student_directory_num
    
    folder_path = sanitize_folder(params.dig(:folder, :name) || '/')
    base_path = @participant.team_path.to_s
    full_path = folder_path == '/' ? base_path : File.join(base_path, folder_path)
    
    files = []
    folders = []
    
    Dir.entries(full_path).each do |entry|
      next if entry == '.' || entry == '..'
      entry_path = File.join(full_path, entry)

      if File.directory?(entry_path)
        folders << { name: entry, modified_at: File.mtime(entry_path) }
      else
        files << {
          name: entry,
          size: File.size(entry_path),
          type: File.extname(entry).delete('.'),
          modified_at: File.mtime(entry_path)
        }
      end
    end

    render json: {
      current_folder: folder_path,
      files: files.sort_by { |f| f[:name] },
      folders: folders.sort_by { |f| f[:name] },
      hyperlinks: team.hyperlinks
    }, status: :ok
  end

 ===Hyperlink Operations===

 Submit Hyperlink:

  def submit_hyperlink
    team = participant_team
    submission = params[:submission].to_s.strip
    
    return render_error('Hyperlink cannot be blank.', :bad_request) if submission.blank?
    return render_error('Already submitted.', :conflict) if team.hyperlinks.include?(submission)

    begin
      team.submit_hyperlink(submission)
      create_submission_record_for('hyperlink', submission, 'Submit Hyperlink')
      render_success('Link successfully submitted.')
    rescue StandardError => e
      render_error("Invalid URL: #{e.message}", :bad_request)
    end
  end

 Remove Hyperlink:

  def remove_hyperlink
    team = participant_team
    index = params['chk_links'].to_i
    hyperlink = team.hyperlinks[index]

    return render_error('Hyperlink not found.', :not_found) unless hyperlink

    team.remove_hyperlink(hyperlink)
    create_submission_record_for('hyperlink', hyperlink, 'Remove Hyperlink')
    head :no_content
  end

 ===File Upload===

  def submit_file
    uploaded = params[:uploaded_file]
    return render_error('No file provided.', :bad_request) unless uploaded
    
    # Validate size (5MB limit)
    file_size_limit_mb = 5
    unless check_content_size(uploaded, file_size_limit_mb)
      return render_error("File must be < #{file_size_limit_mb}MB", :bad_request)
    end

    # Validate extension
    unless check_extension_integrity(uploaded_file_name(uploaded))
      return render_error('Invalid file type.', :bad_request)
    end

    # Save file
    file_bytes = uploaded.read
    current_folder = sanitize_folder(params.dig(:current_folder, :name) || '/')

    team = participant_team
    team.set_student_directory_num

    current_directory = File.join(@participant.team_path.to_s, current_folder)
    FileUtils.mkdir_p(current_directory) unless File.exist?(current_directory)

    safe_filename = sanitize_filename(uploaded_file_name(uploaded)).gsub(' ', '_')
    full_path = File.join(current_directory, File.basename(safe_filename))

    File.open(full_path, 'wb') { |f| f.write(file_bytes) }

    # Optional unzip
    if params[:unzip] && file_type(safe_filename) == 'zip'
      SubmittedContentHelper.unzip_file(full_path, current_directory, true)
    end

    create_submission_record_for('file', full_path, 'Submit File')
    render_success('File submitted successfully.', :created)
  end

 ===Database Migrations===

 Add directory_num (Critical - was missing in E2417):

  class AddDirectoryNumToTeams < ActiveRecord::Migration[8.0]
    def change
      add_column :teams, :directory_num, :integer
    end
  end

 Create SubmissionRecords:

  class CreateSubmissionRecords < ActiveRecord::Migration[8.0]
    def change
      create_table :submission_records do |t|
        t.text :record_type
        t.text :content
        t.string :operation
        t.integer :team_id
        t.string :user
        t.integer :assignment_id
        t.timestamps
      end
    end
  end

 Add Hyperlinks Storage:

  class AddSubmittedHyperlinksToTeams < ActiveRecord::Migration[8.0]
    def change
      add_column :teams, :submitted_hyperlinks, :text
    end
  end

 ==Testing==

 ===Test Coverage===
 Created 48 comprehensive test cases:

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

 ===Running Tests===

  docker compose exec app bundle exec rspec spec/requests/api/v1/submitted_content_spec.rb

 Results: 40/48 passing (83%)

 The 8 failing tests are test infrastructure issues (mock conflicts), not functionality bugs.

 ===Swagger Testing===
 All endpoints documented in swagger/v1/swagger.yaml

 Access Swagger UI:

  docker compose up
  # Visit: http://localhost:3002/api-docs

 ==Comparison with E2417==
Feature E2417 E2551 (Ours)
Law of Demeter Violations present Fixed with delegation
Instance Variables Overused Only 2 required
Helper Methods Inconsistent Standardized
list_files endpoint Missing Implemented
directory_num column Missing Added
Documentation Sparse Comprehensive
HTTP Status Codes Inconsistent Proper codes
Test Coverage Unknown 48 tests (83% pass) 
 ==Improvements Over E2417==

 * Fixed all Law of Demeter violations using delegation
 * Reduced instance variables from many to 2
 * Standardized helper method usage throughout
 * Added critical missing list_files endpoint
 * Added missing directory_num database column
 * Comprehensive inline documentation
 * Proper HTTP status codes for all scenarios
 * Clear, actionable error messages
 * Audit trail via SubmissionRecord model

 ==Known Limitations==

 * 8 tests fail due to test infrastructure (not functionality)
 * Path sanitization could be more robust
 * No authorization layer (per project requirements)
 * No team storage quotas

 ==Future Work==

 * Fix test infrastructure issues
 * Enhanced path sanitization (URL encoding, null bytes)
 * Add authorization layer
 * Implement storage quotas
 * Add chunked uploads for large files
 * Performance optimizations

 ==Conclusion==

 Successfully reimplemented SubmittedContentController with major improvements over E2417:

 ✓ Single controller with organized helpers

 ✓ Law of Demeter violations eliminated

 ✓ Consistent helper usage

 ✓ Complete functionality including list_files

 ✓ Comprehensive documentation

 ✓ Proper HTTP codes and error messages

 ✓ Audit trail implemented

 ✓ 48 test cases written


 All E2551 requirements completed. Ready for deployment pending authorization layer.

 ==References==

 * E2417 Previous Implementation
 * E2417 Wiki Page
 * Rails Guides

 ==Repository==

 Repository: https://github.com/aasthagaudani/reimplementation-back-end

 Branch: aastha

 Key Commits:
 * 753821f - Add list_files and directory_num
 * 7c7bcdd - Fix Law of Demeter violations
 * d166ffe - Add documentation
 * c02c628 - Initial integration
Retrieved from "https://wiki.expertiza.ncsu.edu/index.php?title=CSC/ECE_517_Fall_2025_-_E2551._Reimplementing_SubmittedContentController&oldid=166807"