Skip to content

Commit e9161b3

Browse files
committed
fix: ensure submission history uses archive and moves
- use archive folder when unit archived - move submission history to archive folder when unit archived - move submission history on task abbr change - move submission history on username change - delete submission history on task delete
1 parent bf113fa commit e9161b3

File tree

4 files changed

+66
-3
lines changed

4 files changed

+66
-3
lines changed

app/helpers/file_helper.rb

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -658,13 +658,39 @@ def latest_submission_timestamp_entry_in_dir(path)
658658
sorted_timestamp_entries_in_dir(path)[0]
659659
end
660660

661+
def root_submission_history_dir(archived: false)
662+
file_server = if archived
663+
archive_root
664+
else
665+
student_work_root
666+
end
667+
668+
"#{file_server}/submission_history/" # trust the server config and passed in type for paths
669+
end
670+
671+
def unit_submission_history_dir(unit, archived: true)
672+
dst = if (unit.archived && archived) || (archived == :force)
673+
"#{archive_root}/"
674+
else
675+
"#{student_work_root}/"
676+
end
677+
678+
dst << sanitized_path('submission_history', "#{unit.code}-#{unit.id}")
679+
end
680+
681+
def project_submission_history_dir(project, username: nil, archived: true)
682+
username = project.student.username.to_s if username.nil?
683+
dst = unit_submission_history_dir(project.unit, archived: archived)
684+
685+
File.join(dst, sanitized_path(username))
686+
end
687+
661688
def task_submission_identifier_path(type, task)
662-
file_server = Doubtfire::Application.config.student_work_dir
663-
"#{file_server}/submission_history/#{sanitized_path("#{task.project.unit.code}-#{task.project.unit.id}", task.project.student.username.to_s, type.to_s, task.id.to_s)}"
689+
"#{project_submission_history_dir(task.project)}/#{sanitized_path(type.to_s, task.id.to_s)}"
664690
end
665691

666692
def task_submission_identifier_path_with_timestamp(type, task, timestamp)
667-
"#{task_submission_identifier_path(type, task)}/#{timestamp.to_s}"
693+
"#{task_submission_identifier_path(type, task)}/#{sanitized_path(timestamp.to_s)}"
668694
end
669695

670696
# Apply line wrapping to a given file, returns true when line wrapping is necessary.
@@ -747,6 +773,9 @@ def line_wrap(path, width: 160)
747773
module_function :process_audio
748774
module_function :sorted_timestamp_entries_in_dir
749775
module_function :latest_submission_timestamp_entry_in_dir
776+
module_function :root_submission_history_dir
777+
module_function :unit_submission_history_dir
778+
module_function :project_submission_history_dir
750779
module_function :task_submission_identifier_path
751780
module_function :task_submission_identifier_path_with_timestamp
752781
module_function :known_extension?

app/models/unit.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,21 +2576,34 @@ def move_files_to_archive
25762576
FileUtils.mv(original_work_path, archive_work_path)
25772577
end
25782578

2579+
# Move portfolios
25792580
archive_portfolio_path = FileHelper.unit_portfolio_dir(self, create: false, archived: :force)
25802581
original_portfolio_path = FileHelper.unit_portfolio_dir(self, create: false, archived: false)
25812582

25822583
if File.exist?(original_portfolio_path) && ! File.exist?(archive_portfolio_path)
25832584
FileUtils.mv(original_portfolio_path, archive_portfolio_path)
25842585
end
2586+
2587+
# Move submission history
2588+
archive_submission_history_path = FileHelper.unit_submission_history_dir(self, archived: :force)
2589+
original_submission_history_path = FileHelper.unit_submission_history_dir(self, archived: false)
2590+
2591+
if File.exist?(original_submission_history_path) && ! File.exist?(archive_submission_history_path)
2592+
FileUtils.mkdir_p(FileHelper.root_submission_history_dir(archived: true))
2593+
FileUtils.mv(original_submission_history_path, archive_submission_history_path)
2594+
end
25852595
end
25862596

25872597
private
25882598

25892599
def delete_associated_files
25902600
unit_path = FileHelper.unit_dir(self, create: false)
25912601
unit_portfolio_path = FileHelper.unit_portfolio_dir(self, create: false)
2602+
submission_history_path = FileHelper.unit_submission_history_dir(self)
2603+
25922604
FileUtils.rm_rf unit_path
25932605
FileUtils.rm_rf unit_portfolio_path
2606+
FileUtils.rm_rf submission_history_path
25942607

25952608
FileUtils.cd FileHelper.student_work_dir
25962609
end

app/models/user.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,12 @@ def move_files_on_username_change
428428
project.tasks.where('portfolio_evidence IS NOT NULL').update_all("portfolio_evidence = REPLACE(portfolio_evidence, '#{FileHelper.sanitized_path(old_username)}', '#{FileHelper.sanitized_path(username)}')")
429429
# rubocop:enable Rails/SkipsModelValidations
430430

431+
# Now move submission history files
432+
old_path = FileHelper.project_submission_history_dir(project, username: old_username)
433+
new_path = FileHelper.project_submission_history_dir(project, username: username)
434+
435+
FileUtils.mv(old_path, new_path) if File.exist?(old_path)
436+
431437
# Now move the portfolio folder
432438
old_path = FileHelper.student_portfolio_dir(project.unit, old_username, create: false)
433439
new_path = FileHelper.student_portfolio_dir(project.unit, username, create: false)

test/models/unit_model_test.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,9 +836,15 @@ def test_archive_unit
836836
DatabasePopulator.generate_portfolio(p)
837837
old_portfolio_path = p.portfolio_path
838838

839+
old_submission_history_path = FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123/45')
840+
FileUtils.mkdir_p(old_submission_history_path)
841+
FileUtils.touch(File.join(old_submission_history_path, 'output.txt'))
842+
839843
assert File.exist?(old_path)
840844
assert File.exist?(task_pdf)
841845
assert File.exist?(old_portfolio_path)
846+
assert File.exist?(old_submission_history_path)
847+
assert File.exist?(File.join(old_submission_history_path, 'output.txt'))
842848

843849
unit.move_files_to_archive
844850
unit.archived = true
@@ -853,6 +859,9 @@ def test_archive_unit
853859
assert File.exist?(task.final_pdf_path), "New task file does not exist"
854860
assert_not File.exist?(old_portfolio_path), "Old portfolio file still exists - #{old_portfolio_path}"
855861
assert File.exist?(p.portfolio_path), "New portfolio file does not exist"
862+
assert_not File.exist?(old_submission_history_path), "Old submission history still exists - #{old_submission_history_path}"
863+
assert File.exist?(FileHelper.task_submission_identifier_path(:done, task))
864+
assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt'))
856865

857866
assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist - #{task.final_pdf_path}"
858867

@@ -862,17 +871,23 @@ def test_archive_unit
862871

863872
# File exists after rename
864873
assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist - #{task.final_pdf_path}"
874+
assert File.exist?(FileHelper.task_submission_identifier_path(:done, task))
875+
assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt'))
865876

866877
p.student.update(username: 'NEW_USERNAME')
867878
task.reload
868879
assert File.exist?(task.final_pdf_path), "Portfolio evidence file does not exist after username change - #{task.final_pdf_path}"
869880
assert File.exist?(p.portfolio_path), "New portfolio file does not exist"
881+
assert File.exist?(FileHelper.task_submission_identifier_path(:done, task))
882+
assert File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt'))
870883

871884
unit.destroy!
872885

873886
assert_not File.exist?(td.task_sheet), "New file exists after delete - #{td.task_sheet}"
874887
assert_not File.exist?(task.final_pdf_path), "New task file exists after delete - #{task.final_pdf_path}"
875888
assert_not File.exist?(p.portfolio_path), "New portfolio exists after delete - #{p.portfolio_path}"
889+
assert_not File.exist?(FileHelper.task_submission_identifier_path(:done, task))
890+
assert_not File.exist?(File.join(FileHelper.task_submission_identifier_path_with_timestamp(:done, task, '123_45'), 'output.txt'))
876891
ensure
877892
Doubtfire::Application.config.archive_units = false
878893
end

0 commit comments

Comments
 (0)