Prevent failing import from file system from deleting assessment files

[20wildmanj]

Summary

Summary generated by Reviewpad on 25 Jul 23 04:32 UTC

This pull request includes three patches.

The first patch (PATCH 1/3) makes changes to the assessments_controller.rb file. It adds a check to ensure that each assessment has an associated yaml file with a name field that matches the filename. It also handles errors when displaying existing assessments.

The second patch (PATCH 2/3) also modifies the assessments_controller.rb file. It updates the comments to clarify the requirements for each assessment's associated yaml file, and it fixes some formatting issues.

The third patch (PATCH 3/3) addresses comments from a previous pull request. It renames the isTarImport parameter to cleanup_on_failure and updates the corresponding code. It also improves error handling and removes unnecessary code.

Overall, these patches enhance the functionality and error handling of the assessments_controller.rb file.

Description

• use isTarImport parameter toimportAssessment to determine whether import is from tarball (aka from importAsmtTar) or from file system
• delete assessment files only is import was from a tarball import, otherwise leave files as-is
• Add pre-check in install_assessment to check for invalid name field in yaml to prevent installation of invalid assessment (and thus prevent yaml from getting wiped out, which happens if the error was only caught during deserialization)

Motivation and Context

• Closes Assessment directory deleted as side effect of "Error loading yaml: Name in yaml doesn't match" #1944

How Has This Been Tested?

• Delete existing assessment (go to "Edit Assessment"->"Delete Assessment")
• Find assessment in file system, navigate to yaml, change name field to not match filename
• Go to "Install Assessment," see following error appears and that assessment doesn't show up under "Import from File System"
  
• Fix the yaml name, then change config rb module name so that it doesn't match the filename / assessment name
• Try installing the assessment, see following error:
  
• verify that assessment directory is still there

Regression:

• import from tarball a buggy assessment (such as spec/fixtures/assessments/homework02-yaml-name-field-wrong.tar), and try importing it
• see error is displayed and that assessment directory is deleted from course directory
• Upload a normal tarball / import from file system a normal directory and see that import succeeds
• Run bundle exec rake spec multiple times and see that tests pass

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting

[damianhxy]

Followed testing plan successfully, LGTM (other than some comments)