Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
248 changes: 191 additions & 57 deletions app/controllers/assignments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def index
@user = User.find_by_id(session[:user_id])
@guest_user = GuestUsersDetail.find_by_user_id(@user.id)
# obtain assignment to be shown if is a guest user
@assignment_to_be_shown = @guest_user.assignment_id
#@assignment_to_be_shown = @guest_user.assignment_id

@assignments = @course.assignments
@empty_assignments = @course.empty_assignments
Expand Down Expand Up @@ -101,82 +101,203 @@ def create
return render action: "new" unless @assignment.save

isMapEnabled = (params[:assignment]["mapbox"] == "Yes")? true : false;

# to check whether user clicked on optional options
isOptionalOptionsEnabled = (params[:assignment]["mapbox"] == "Yes")? true : false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please follow Ruby/Rails naming convention.


# to check whether user uploaded the students code zip file
isCodeFileUploaded = (params[:assignment]["file"].nil? == true)? false : true;
codeFile = params[:assignment]["file"];
isValidCodeFile = false # to determine whether valid code file

# to check whether user uploaded a map file
isMapFileUploaded = (params[:assignment]["mapfile"].nil? == true)? false : true;
mapFile = (isOptionalOptionsEnabled)? params[:assignment]["mapfile"] : nil;
isValidMapFile = false # to determine whether valid map file

# to check whether the uploaded students code zip file is structured by ...
isCodeFileStructureAllFiles = (params[:assignment]["fileStructure"] == "all")? true : false;

# no student submission file was uploaded
if !(isCodeFileUploaded)
# create asssignment but don't process it
redirect_to course_assignments_url(@course), notice: 'Assignment was successfully created. Please upload the student submission files to preview files and/or process the assignment'

# ascertain that all the fields are valid
else
# verify codeFile and mapFile (if uploaded)
isValidCodeFile = is_valid_zip?(codeFile.content_type, codeFile.path)
isValidMapFile = is_valid_map_or_no_map?(isOptionalOptionsEnabled, mapFile)

# No student submission file was uploaded
if params[:assignment]["file"].nil?
# Create asssignment but don't process it
redirect_to course_assignments_url(@course), notice: 'Assignment was successfully created. Please upload the student submission files and mapping file (if any) to process the assignment'
# Student submission file is a valid zip
elsif (is_valid_zip?(params[:assignment]["file"].content_type, params[:assignment]["file"].path))
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
if (is_valid_map_or_no_map?(isMapEnabled, params[:assignment]["mapfile"]))
self.start_upload(@assignment, params[:assignment]["file"], isMapEnabled, params[:assignment]["mapfile"])
redirect_to course_assignments_url(@course), notice: 'Assignment was successfully created. Please refresh this page after a few minutes to view the similarity results.'
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
elsif (isMapEnabled && params[:assignment]["mapfile"].nil?)
@assignment.errors.add(:mapfile, "containing mapped student names need to be uploaded if the 'Upload map file' box is ticked")
return render action: "new"
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
else
@assignment.errors.add :mapfile, "containing mapped student names must be a valid csv file"
return render action: "new"
# uploaded student code file was not valid
if (!isValidCodeFile)
@assignment.errors.add :file, "Containing student submission files must be a valid zip file"
hasErrorParams = true
end
# Student submission file is not a valid zip file
else
@assignment.errors.add :file, "containing student submission files must be a valid zip file"
if (params[:assignment]["mapfile"].nil? && isMapEnabled)
@assignment.errors.add :mapfile, "containing mapped student names need to be uploaded if the 'Upload map file' box is ticked"
elsif !(is_valid_map_or_no_map?(isMapEnabled, params[:assignment]["mapfile"]))
@assignment.errors.add :mapfile, "containing mapped student names must be a valid csv file"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In MVC/Rails, people usually follow "Fat models, skinny controllers" convention for code's readability and maintenance. Fat model is not always maintainable. So, people introduce service layer to do validation or to interact with models.

In this case, it's suggested to have assignment validation service for code segments: is_valid_zip?, is_valid_map_or_no_map? ... and to have a file helper service for code segment: extract_uploaded_files ...
It will help to make the code low coupling, easy to read and maintain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knmnyn do you have any comments on it?

# uploaded map file was not valid
if (isMapFileUploaded && !isValidMapFile)
@assignment.errors.add :mapfile, "Containing mapped student names must be a valid CSV file"
hasErrorParams = true
end

codeFile_path = ""

# if there are no errors with the uploaded files
if (!hasErrorParams)

require 'submissions_handler'

# extract codefile and mapfile (if uploaded)
codeFile_path = SubmissionsHandler.process_upload(
codeFile, isMapFileUploaded, mapFile, @assignment
)

# if there are errors with the uploaded files, display error in the UI
else
return render action: "new"
end
return render action: "new"

extract_uploaded_files(
@assignment, isCodeFileStructureAllFiles, codeFile_path, isMapFileUploaded
)

end
else
render action: "new"
end
end

# PUT /courses/1/assignments/1
def update
@assignment = Assignment.find(params[:id])

isMapEnabled = (params[:assignment]["mapbox"] == "Yes")? true : false;
# Get all the parameters

# to check whether user clicked on optional options
isOptionalOptionsEnabled = (params[:assignment]["mapbox"] == "Yes")? true : false;

# to check whether user uploaded the students code zip file
isCodeFileUploaded = (params[:assignment]["file"].nil? == true)? false : true;
codeFile = params[:assignment]["file"];
isValidCodeFile = false # to determine whether valid code file

# to check whether user uploaded a map file
isMapFileUploaded = (params[:assignment]["mapfile"].nil? == true)? false : true;
mapFile = (isOptionalOptionsEnabled)? params[:assignment]["mapfile"] : nil;
isValidMapFile = false # to determine whether valid map file

# to check whether the uploaded students code zip file is structured by ...
isCodeFileStructureAllFiles = (params[:assignment]["fileStructure"] == "all")? true : false;

# no student submission file was uploaded
if !(isCodeFileUploaded)
@assignment.errors.add :file, "containing student submission files need to be uploaded"
hasErrorParams = true
# ascertain that all the fields are valid
else
# verify codeFile, mapFile (if uploaded) and refFile (if uploaded)
isValidCodeFile = is_valid_zip?(codeFile.content_type, codeFile.path)
isValidMapFile = is_valid_map_or_no_map?(isOptionalOptionsEnabled, mapFile)
codeFile_path = ""

# No student submission file was uploaded
if params[:assignment]["file"].nil?
@assignment.errors.add :file, "containing student submission files need to be uploaded to process the assignment"
if (params[:assignment]["mapfile"].nil? && isMapEnabled)
@assignment.errors.add :mapfile, "containing mapped student names need to be uploaded if the 'Upload map file' box is ticked"
elsif !(is_valid_map_or_no_map?(isMapEnabled, params[:assignment]["mapfile"]))
@assignment.errors.add :mapfile, "containing mapped student names must be a valid csv file"
end
return render action: "show"
elsif (is_valid_zip?(params[:assignment]["file"].content_type, params[:assignment]["file"].path))
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
if (is_valid_map_or_no_map?(isMapEnabled, params[:assignment]["mapfile"]))
self.start_upload(@assignment, params[:assignment]["file"], isMapEnabled, params[:assignment]["mapfile"])
redirect_to course_assignments_url(@course), notice: 'SSID will start to process the assignment now. Please refresh this page after a few minutes to view the similarity results.'
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
elsif (isMapEnabled && params[:assignment]["mapfile"].nil?)
@assignment.errors.add(:mapfile, "containing mapped student names need to be uploaded if the 'Upload map file' box is ticked")
return render action: "show"
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
else
if (!isValidCodeFile)
@assignment.errors.add :file, "containing student submission files must be a valid zip file"
hasErrorParams = true
end

if (isMapFileUploaded && !isValidMapFile)
@assignment.errors.add :mapfile, "containing mapped student names must be a valid csv file"
hasErrorParams = true
end

if (!hasErrorParams)

require 'submissions_handler'

# extract codefile and mapfile (if uploaded)
codeFile_path = SubmissionsHandler.process_upload(
codeFile, isMapFileUploaded, mapFile, @assignment
)

else
return render action: "show"
end
# Student submission file is not a valid zip file
else
@assignment.errors.add :file, "containing student submission files must be a valid zip file"
if (params[:assignment]["mapfile"].nil? && isMapEnabled)
@assignment.errors.add :mapfile, "containing mapped student names need to be uploaded if the 'Upload map file' box is ticked"
elsif !(is_valid_map_or_no_map?(isMapEnabled, params[:assignment]["mapfile"]))
@assignment.errors.add :mapfile, "containing mapped student names must be a valid csv file"

extract_uploaded_files(
@assignment, isCodeFileStructureAllFiles, codeFile_path, isMapFileUploaded
)
end
end

def extract_uploaded_files(
assignment, isCodeFileStructureAllFiles, codeFile_path, isMapEnabled
)

@dirSize = 0

# Create directory for code comparison, delete first if necessary
code_compare_dir = File.join(codeFile_path, "_compare")
FileUtils.rm(code_compare_dir, force: true) if File.exist? code_compare_dir
FileUtils.mkdir_p(File.join(codeFile_path, "_compare"))

# For each student submission, combine the code files into one
# Only match files in the current directory
# For file based comparision, no need to combine code files
if (isCodeFileStructureAllFiles)

Dir.glob(File.join(codeFile_path, "**/*")).each do |file|
next if File.directory?(file) # skip the loop if the file is a directory
if (File.basename(file, File.extname(file)) != 'skeleton')
@dirSize += 1
end
arr = []
split_str = file.split('/')
filepath = split_str[split_str.count - 1]
arr.push(filepath)
dest_folder = code_compare_dir + "/" + File.basename(file, File.extname(file)) + File.extname(file)
FileUtils.cp(file, dest_folder)
end

# For folder comparision, combine code files and write into compare dir as new file with same name, remove ext if any
else

Dir.glob(File.join(codeFile_path, "*")).each { |subpath|
next if subpath == code_compare_dir || (File.file?(subpath) && subpath.split('.').last.to_s.downcase == 'csv')

if (Dir.exist?(subpath))
split_str = subpath.split('/')
foldname = split_str[split_str.count - 1]
if (foldname != 'skeleton')
@dirSize += 1
end
return render action: "show"
files = Dir.entries(subpath).reject!{|file_name| [".","..",".gitignore"].include?(file_name)}
end

File.open(File.join(code_compare_dir, File.basename(subpath, File.extname(subpath))), 'w') { |f|
f.puts string_from_combined_files(subpath)
}

}
end

# Check the contents size inside zip
if (@dirSize == 0)
@assignment.errors.add :file, "Containing student submission files cannot be empty"
return render action: "show"
elsif (@dirSize == 1)
@assignment.errors.add :file, "Containing student submission files must have at least 2 student folders/files"
return render action: "show"
end

require 'submissions_handler'
SubmissionsHandler.process_submissions(code_compare_dir, @assignment, isMapEnabled)

redirect_to course_assignments_url(@course), notice: 'Assignment was successfully created. Please refresh this page after a few minutes to view the similarity results.'

end


# DELETE /courses/1/assignments/1
def destroy
@assignment = Assignment.find(params[:id])
Expand Down Expand Up @@ -244,6 +365,19 @@ def is_valid_map_or_no_map?(isMapEnabled, mapFile)
end
end

def string_from_combined_files(path)
strings = []
if File.directory? path
Dir.glob(File.join(path, "*")).sort.each { |subpath|
strings << string_from_combined_files(subpath)
}
else
strings << File.open(path).readlines.join
end

strings.join("\n")
end

end


10 changes: 10 additions & 0 deletions app/models/assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,20 @@ class Assignment < ActiveRecord::Base
ocaml: "ocaml"
}

# enum to store whether plagiarism detection needs to recursively look at files or folders
FILE_STRUCTURES = {
all: "By Files",
dir: "By Directory"
}

def self.options_for_languages
LANGUAGES.to_a.collect { |pair| pair.reverse }
end

def self.options_for_file_structure
FILE_STRUCTURES.to_a.collect { |pair| pair.reverse }
end

def prettify_js_lang
"lang-#{PRETTIFY_LANGUAGES[self.language]}"
end
Expand Down
4 changes: 4 additions & 0 deletions app/views/assignments/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
<%= f.label :ngram_size, "Size of n-gram [?]", title: "An n-gram is a contiguous subsequence of n tokens of a given sequence.", class: 'form-label' %>
<%= f.text_field :ngram_size, class: 'form-control' %>
</div>
<div class="col-sm-6">
<%= f.label :fileStructure, "File Structure \u24D8", title: "This refers to the way the files are arranged in your zip folder. \n\nTo recursively search for all files in the provided directories and compare every file with each other file, select \"By Files\". \n\nInstead, if you don't want to compare files in the same leaf directory (for example, if the zip file is split into per-student directories and you don't care about self plagiarism, select \"By Directory\"", class: 'form-label' %>
<%= f.select :fileStructure, Assignment.options_for_file_structure, {}, { :class => 'form-select' } %>
</div>
<div class="col-12">
<%= f.label :file, "Student submissions (zip) [?]", title: "The zip file should contain one folder for each student's code files; folders are named according to student ID; Name your folder as \"skeleton\" for the codes you wish to exclude from the system", class: 'form-label' %>
<%= f.file_field :file, class: 'form-control' %>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddFileStructureToAssignments < ActiveRecord::Migration[6.0]
def change
add_column :assignments, :fileStructure, :string
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new parameter to store whether the user wants to process the uploaded zip file by files or by directory.

16 changes: 14 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2021_06_13_072330) do
ActiveRecord::Schema.define(version: 2022_04_27_061852) do

create_table "announcements", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb3", force: :cascade do |t|
t.string "title"
Expand All @@ -32,6 +32,7 @@
t.datetime "updated_at", precision: 6, null: false
t.text "upload_log"
t.boolean "mapbox", default: false
t.string "fileStructure"
t.index ["course_id"], name: "index_assignments_on_course_id"
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new parameter to store whether the user wants to process the uploaded zip file by files or by directory.


Expand Down Expand Up @@ -138,6 +139,17 @@
t.index ["assignment_id"], name: "index_submission_similarity_processes_on_assignment_id"
end

create_table "submission_similarity_skeleton_mappings", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb3", force: :cascade do |t|
t.bigint "submission_similarity_mapping_id"
t.integer "start_line1"
t.integer "end_line1"
t.integer "start_line2"
t.integer "end_line2"
t.datetime "created_at", precision: 6, null: false
t.datetime "updated_at", precision: 6, null: false
t.index ["submission_similarity_mapping_id"], name: "on_submission_similarity_mapping_id"
end

create_table "submissions", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb3", force: :cascade do |t|
t.text "lines", size: :long
t.integer "assignment_id", null: false
Expand All @@ -161,7 +173,7 @@
create_table "users", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb3", force: :cascade do |t|
t.string "name"
t.string "full_name"
t.string "password_digest", default: "$2a$12$BEjX4o6KQ8HsgdP7JV6NEuVygw6U5kaKQrvxk9U3xtotcS92.0BlG"
t.string "password_digest", default: "$2a$12$I30qed3NhS08Zz7rf.2fy.FEbomtdgXz.4YxtNs7awDjkE3V4EBjy"
t.boolean "is_admin", default: false, null: false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can ignore this.

t.string "id_string"
t.datetime "created_at", precision: 6, null: false
Expand Down
17 changes: 1 addition & 16 deletions lib/submissions_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,22 +128,7 @@ def self.process_upload(file, isMapEnabled, mapfile, assignment)
upload_dir
end

def self.process_submissions(path, assignment, isMapEnabled)
# Create directory for code comparison, delete first if necessary
compare_dir = File.join(path, "_compare")
FileUtils.rm(compare_dir, force: true) if File.exist? compare_dir
FileUtils.mkdir_p(File.join(path, "_compare"))

# For each student submission, combine the code files into one
Dir.glob(File.join(path, "*")).each { |subpath|
next if subpath == compare_dir || (File.file?(subpath) && subpath.split('.').last.to_s.downcase == 'csv')

# Combine code files and write into compare dir as new file with same name, remove ext if any
File.open(File.join(compare_dir, File.basename(subpath, File.extname(subpath))), 'w') { |f|
f.puts string_from_combined_files(subpath)
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above is now done inside app/controllers/assignments_controller.rb

def self.process_submissions(compare_dir, assignment, isMapEnabled)
# Read database configuration
config = Rails.configuration.database_configuration
host = config[Rails.env]["host"]
Expand Down