Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
Binary file added .DS_Store
Binary file not shown.
2 changes: 1 addition & 1 deletion app/controllers/assignments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def varying_rubrics_by_round?
end
end
end

private
# Only allow a list of trusted parameters through.
def assignment_params
Expand Down
87 changes: 87 additions & 0 deletions app/controllers/submitted_content_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,41 @@ def list_files
render_error("Failed to list directory contents: #{e.message}. Please try again.", :internal_server_error)
end

# GET /submitted_content/:id/view_submissions
def view_submissions
assignment = Assignment.find_by(id: params[:id])
return render json: { error: "Assignment not found" }, status: :not_found if assignment.nil?

submissions = assignment.teams.map do |team|
members = team.teams_users.includes(:user).map do |tu|
user = tu.user
{
full_name: user&.full_name,
github: "",
email: user&.email
}
end

links = current_team_links(team, assignment.id)
files = current_team_files(team, assignment.id)

{
id: team.id,
team_id: team.id,
team_name: team.name,
members: members,
links: links,
files: files
}
end

render json: {
assignment_id: assignment.id,
assignment_name: assignment.name,
submissions: submissions
}, status: :ok
end

private

# Before action callback: Sets @submission_record for the show action
Expand Down Expand Up @@ -377,4 +412,56 @@ def create_submission_record_for(record_type, content, operation)
operation: operation # Operation description (e.g., 'Submit File')
)
end

def current_team_links(team, assignment_id)
hyperlink_records = SubmissionRecord
.where(team_id: team.id, assignment_id: assignment_id, record_type: 'hyperlink')
.order(created_at: :desc)

team.hyperlinks.each_with_index.map do |hyperlink, index|
matching_record = hyperlink_records.find { |record| record.content == hyperlink }

{
id: matching_record&.id || index + 1,
url: hyperlink,
display_name: hyperlink,
name: hyperlink,
type: 'Hyperlink',
modified: matching_record&.created_at || team.updated_at
}
end
Comment on lines +421 to +432

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Potential ID collisions when mixing record IDs with index-based fallbacks.

The pattern id: matching_record&.id || index + 1 mixes database IDs (which could be any integer) with sequential indices starting at 1. If a record exists with ID 2 and another hyperlink falls back to index + 1 = 2, you'd have duplicate IDs in the response.

Consider using a different fallback strategy or a separate identifier namespace.

🛡️ Suggested fix using negative indices for fallbacks
-      {
-        id: matching_record&.id || index + 1,
+      {
+        id: matching_record&.id || -(index + 1),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
team.hyperlinks.each_with_index.map do |hyperlink, index|
matching_record = hyperlink_records.find { |record| record.content == hyperlink }
{
id: matching_record&.id || index + 1,
url: hyperlink,
display_name: hyperlink,
name: hyperlink,
type: 'Hyperlink',
modified: matching_record&.created_at || team.updated_at
}
end
team.hyperlinks.each_with_index.map do |hyperlink, index|
matching_record = hyperlink_records.find { |record| record.content == hyperlink }
{
id: matching_record&.id || -(index + 1),
url: hyperlink,
display_name: hyperlink,
name: hyperlink,
type: 'Hyperlink',
modified: matching_record&.created_at || team.updated_at
}
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/controllers/submitted_content_controller.rb` around lines 421 - 432, The
current mapping in team.hyperlinks.each_with_index mixes database IDs
(matching_record&.id) with index-based fallbacks (index + 1), risking
collisions; change the fallback to a non-colliding identifier namespace—e.g.,
return id: (matching_record&.id || "temp-#{index + 1}") or id:
(matching_record&.id || - (index + 1))—so update the block that builds the hash
(the each_with_index loop using matching_record and hyperlink_records) to use a
string-prefixed or negative numeric fallback instead of a plain positive index.

end

def current_team_files(team, assignment_id)
team.set_team_directory_num
base_path = team.path.to_s
return [] unless File.directory?(base_path)

file_records = SubmissionRecord
.where(team_id: team.id, assignment_id: assignment_id, record_type: 'file')
.order(created_at: :desc)

file_entries = Dir.glob(File.join(base_path, '**', '*')).select { |entry| File.file?(entry) }.sort

file_entries.each_with_index.map do |entry, index|
relative_path = entry.delete_prefix("#{base_path}/")
current_folder = File.dirname(relative_path)
current_folder = current_folder == '.' ? '/' : "/#{current_folder}"
file_name = File.basename(relative_path)
matching_record =
file_records.find do |record|
record.content.to_s == entry || File.basename(record.content.to_s) == file_name
end
Comment on lines +435 to +454

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

File matching by basename alone may cause false matches.

The matching logic on lines 452-454 checks if File.basename(record.content.to_s) == file_name. If two files in different folders share the same name (e.g., /folder1/report.pdf and /folder2/report.pdf), the first record found will match both files incorrectly.

Consider including the folder path in the match criteria.

🛡️ Suggested fix to include path in matching
       matching_record =
         file_records.find do |record|
-          record.content.to_s == entry || File.basename(record.content.to_s) == file_name
+          record.content.to_s == entry ||
+            record.content.to_s.end_with?("/#{relative_path}")
         end
🧰 Tools
🪛 RuboCop (1.86.1)

[convention] 435-466: Assignment Branch Condition size for current_team_files is too high. [<12, 32, 11> 35.9/20]

(Metrics/AbcSize)


[convention] 435-466: Cyclomatic complexity for current_team_files is too high. [9/7]

(Metrics/CyclomaticComplexity)


[convention] 435-466: Method has too many lines. [26/20]

(Metrics/MethodLength)


[convention] 435-466: Perceived complexity for current_team_files is too high. [9/8]

(Metrics/PerceivedComplexity)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/controllers/submitted_content_controller.rb` around lines 435 - 454, The
current_team_files method is matching SubmissionRecord entries by basename only
which can produce false positives when files share names in different folders;
update the matching logic in current_team_files to compare the record's path
relative to team.path (e.g., normalize record.content.to_s to a full or relative
path) against the computed relative_path/current_folder plus file_name (or build
the expected absolute path using base_path + relative_path) so matching_record
only returns when the record's content path equals the file entry path (or its
relative equivalent) rather than just matching
File.basename(record.content.to_s).


{
id: matching_record&.id || index + 1,
url: "/submitted_content/download?download=#{URI.encode_uri_component(file_name)}&current_folder[name]=#{URI.encode_uri_component(current_folder)}",
display_name: file_name,
name: file_name,
size: File.size(entry),
type: File.extname(file_name).delete('.').upcase,
modified: File.mtime(entry)
}
end
end
end
46 changes: 40 additions & 6 deletions app/helpers/submitted_content_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,17 @@ def copy_selected_file
def delete_selected_files
# Wrap the delete operation with error handling
handle_file_operation_error('deleting') do
files_to_delete = resolved_files_for_deletion

if files_to_delete.empty?
render json: { error: 'No files were selected for deletion.' }, status: :bad_request
return
end

# Track successfully deleted files for response
deleted_files = []

# Iterate through each file index in the chk_files param
Array(params[:chk_files]).each do |idx|
# Build the full file path for this index
file_path = File.join(params[:directories][idx], params[:filenames][idx])

files_to_delete.each do |file_path|
# Check if file exists before attempting deletion
if File.exist?(file_path)
# Remove file or directory recursively
Expand All @@ -231,7 +234,7 @@ def delete_selected_files
deleted_files << file_path
else
# File doesn't exist, return error
render json: { error: "Cannot delete '#{params[:filenames][idx]}': File does not exist. It may have already been deleted." }, status: :not_found
render json: { error: "Cannot delete '#{File.basename(file_path)}': File does not exist. It may have already been deleted." }, status: :not_found
return
end
end
Expand All @@ -258,4 +261,35 @@ def create_new_folder
render json: { message: "Directory '#{params[:faction][:create]}' created successfully." }, status: :created
end
end

def resolved_files_for_deletion
current_folder_param =
params.dig(:current_folder, :name) ||
params.dig('current_folder', 'name') ||
params[:current_folder] ||
params['current_folder']
delete_target = params.dig(:faction, :delete) || params.dig('faction', 'delete')

if current_folder_param.present? && delete_target.present?
team = participant_team
team.set_team_directory_num

current_folder = clean_folder(current_folder_param)
base_path = team.path.to_s
directory_path = current_folder == '/' ? base_path : File.join(base_path, current_folder)

return Array(delete_target).filter_map do |entry_name|
sanitized_name = clean_filename(entry_name)
File.join(directory_path, sanitized_name) if sanitized_name.present?
end
end

Array(params[:chk_files]).filter_map do |idx|
directory = params.dig(:directories, idx) || params.dig(:directories, idx.to_s)
filename = params.dig(:filenames, idx) || params.dig(:filenames, idx.to_s)
next if directory.blank? || filename.blank?

File.join(directory, filename)
end
end
end
26 changes: 15 additions & 11 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,21 @@
end
end

resources :submitted_content do
collection do
get :download
get :list_files
delete :remove_hyperlink
post :submit_file
post :submit_hyperlink
post :folder_action
end

member do
get :view_submissions
end
end

resources :bookmarks, except: [:new, :edit] do
member do
get 'bookmarkratings', to: 'bookmarks#get_bookmark_rating_score'
Expand Down Expand Up @@ -108,17 +123,6 @@
delete :remove_advertisement
end
end

resources :submitted_content do
collection do
get :download
get :list_files
delete :remove_hyperlink
post :submit_file
post :submit_hyperlink
post :folder_action
end
end

resources :join_team_requests do
member do
Expand Down
2 changes: 1 addition & 1 deletion db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 51 additions & 0 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,57 @@
end
end

puts "creating submission records (hyperlinks and files)"
hyperlink_samples = [
"https://github.com/ncsu-csc/project-repo",
"https://github.com/ncsu-csc/demo-app",
"https://youtu.be/dQw4w9WgXcQ",
"https://docs.google.com/presentation/d/example",
"https://github.com/student-team/final-project"
]

file_samples = [
{ name: "report.pdf", url: "https://www.w3.org/WAI/UR/terms/media/sample.pdf", type: "pdf" },
{ name: "slides.pptx", url: "https://file-examples.com/storage/sample.pptx", type: "pptx" },
{ name: "readme.md", url: "https://raw.githubusercontent.com/github/docs/main/README.md", type: "md" }
]

num_teams.times do |i|
team = AssignmentTeam.find(team_ids[i])
participant = AssignmentParticipant.find_by(team_id: team.id)
next unless participant

assignment_id = team.parent_id

# Add 1-2 hyperlinks per team
rand(1..2).times do |j|
url = hyperlink_samples[(i + j) % hyperlink_samples.length]
# Store on the team's hyperlinks list
team.submit_hyperlink(url) rescue nil
# Create audit record
SubmissionRecord.create(
record_type: 'hyperlink',
content: url,
operation: 'Submit Hyperlink',
team_id: team.id,
user: participant.user&.name || "student#{i}",
assignment_id: assignment_id,
created_at: Faker::Time.backward(days: 14)
)
end

file = file_samples[i % file_samples.length]
SubmissionRecord.create(
record_type: 'file',
content: file[:url], # store real URL instead of local path
operation: 'Submit File',
team_id: team.id,
user: participant.user&.name || "student#{i}",
assignment_id: team.parent_id,
created_at: Faker::Time.backward(days: 7)
)
end

rescue ActiveRecord::RecordInvalid => e
puts e, 'The db has already been seeded'
end
Loading
Loading