-
Notifications
You must be signed in to change notification settings - Fork 194
E2603: Implement ViewSubmissions frontend #312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0e8a314
2acbe96
9714ef8
e35a29e
0465dce
74c82ed
35fc36a
65f045d
4826d67
ee0e183
de88372
f7a6950
620473b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -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 | ||
| 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. File matching by basename alone may cause false matches. The matching logic on lines 452-454 checks if 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 (Metrics/AbcSize) [convention] 435-466: Cyclomatic complexity for (Metrics/CyclomaticComplexity) [convention] 435-466: Method has too many lines. [26/20] (Metrics/MethodLength) [convention] 435-466: Perceived complexity for (Metrics/PerceivedComplexity) 🤖 Prompt for AI Agents |
||
|
|
||
| { | ||
| id: matching_record&.id || index + 1, | ||
| url: "/submitted_content/download?download=#{URI.encode_uri_component(file_name)}¤t_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 | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential ID collisions when mixing record IDs with index-based fallbacks.
The pattern
id: matching_record&.id || index + 1mixes 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 toindex + 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
📝 Committable suggestion
🤖 Prompt for AI Agents