Skip to content
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

Propagate start_at during assessment import, remove unused db columns #1932

Merged
merged 7 commits into from
Jul 9, 2023

Conversation

damianhxy
Copy link
Member

@damianhxy damianhxy commented Jun 25, 2023

Summary

Summary generated by Reviewpad on 09 Jul 23 08:01 UTC

This pull request includes various changes across multiple files:

  1. A new migration file 20230625125740_remove_visible_at_from_assessments.rb has been added. It removes the visible_at column from the assessments table in the database.
  2. Another new migration file remove_has_autograde_old_from_assessments.rb has been added. It removes the has_autograde_old column from the assessments table in the database.
  3. The file confirmInstall.html.erb has been deleted. This file contained a JavaScript function newCat() and a form for creating a new assessment.
  4. The file assessment.rb has undergone changes related to the visible_at attribute in the dates hash and its handling in the serialize and deserialize methods.
  5. The file assessments_controller.rb has multiple changes, including the removal of code related to visibility and embedded quiz functionality.
  6. Another new migration file RemoveEmbeddedQuizFormFromAssessments.rb has been added. It removes the embedded_quiz_form column from the assessments table in the database.
  7. The file assessment_autograde_core.rb has a change where the visible_at attribute is no longer assigned to the @visible_at variable.
  8. Another new migration file 20230625130526_remove_has_scoreboard_old_from_assessments.rb has been added. It removes the has_scoreboard_old column from the assessments table in the database.
  9. The file lib/tasks/autolab.rake has changes related to the removal of the visible_at attribute.
  10. The file app/views/assessments/confirmInstall.html.erb has been deleted. It contained JavaScript code and a form for creating assessments.
  11. The schema.rb file has various modifications, including changes to the schema version and columns in the annotations and assessments tables.

These are the main changes found in the diff. Please review the individual summaries for more details.

Description

  • Remove unused visible_at column from assessments table
  • Remove unused has_autograde_old column from assessments table
  • Remove unused has_scoreboard_old column from assessments table
  • Remove unused embedded_quiz_form column from assessments table. Delete :embedded_quiz_form from edit_assessment_params as a result. Note that the form would have already been read into embedded_quiz_form_data in update
  • Remove unused confirmInstall pages (they reference a doInstall action that doesn't exist)
  • Update deserialize logic to utilize start_at date for all other dates if any of them are missing
  • Remove dead embedded forms code from assessments_controller.rb#create. Note that the front end code was commented out and removed in Github OAuth2 integration #1388 and Add user interface to upload config files, revamp file input fields #1883 respectively.

Motivation and Context

Simplify the creation of assessments via scripting methods, especially when they are intended to be non-submittable.

Closes #1927

Also removes some unused fields to clean up the codebase.

How Has This Been Tested?

Propagate start_at

  • Successfully create a new assessment from scratch
  • Test only start_at present
    • Delete assessment
    • Other than start_at, delete all other dates from assessment yml file
    • Modify start_at date to some date of your choice
    • Import from file system, observe that all dates are set to the start_at date
  • Test all dates present
    • Delete assessment
    • Modify all dates in assessment yml file to dates of your choice
    • Import from file system, observe that dates are correctly set
  • Test no dates present
    • Delete assessment
    • Delete all dates in assessment yml file
    • Import from file system, observe that dates are set to current date + 1 day

Regression tests

  • Successfully upload and enable an embedded form
  • Successfully replace an embedded form
  • Successfully import an assessment from tar
  • Verify that visible_at, has_autograde_old, has_scoreboard_old, embedded_quiz_form columns are not used anywhere else

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
  • My change requires a change to the documentation, which is located at Autolab Docs
  • I have updated the documentation accordingly, included in this PR

@reviewpad reviewpad bot added the medium Pull request is medium label Jun 25, 2023
@damianhxy damianhxy marked this pull request as ready for review June 26, 2023 17:19
@reviewpad reviewpad bot requested a review from najclark June 26, 2023 17:19
@damianhxy damianhxy changed the title Propagate start_at during asmt import, remove unused asmt db columns Propagate start_at during assessment import, remove unused db columns Jul 1, 2023
app/models/assessment.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@20wildmanj 20wildmanj left a comment

Choose a reason for hiding this comment

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

Followed testing plan and did regression testing successfully, LGTM besides my leftover bug from before the PR

@damianhxy damianhxy enabled auto-merge July 9, 2023 08:03
@damianhxy damianhxy added this pull request to the merge queue Jul 9, 2023
Merged via the queue into master with commit ba39594 Jul 9, 2023
@damianhxy damianhxy deleted the asmt-import-yml branch July 9, 2023 08:15
NicholasMy pushed a commit to UB-CSE-IT/Autolab that referenced this pull request Jan 3, 2024
…autolab#1932)

* Propagate start_at when applicable

* Remove visible_at column

* Remove has_autograde_old and has_scoreboard_old columns

* Remove unused embedded_quiz code from assessments_controller#create

* Remove embedded_quiz_form column

* Update schema.rb

* Fix end_at bug

(cherry picked from commit ba39594)
NicholasMy added a commit to UB-CSE-IT/Autolab that referenced this pull request Jan 4, 2024
* Remove visible_at column

* Remove embedded_quiz_form column

* Update schema.rb

(cherry picked from commit ba39594)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium Pull request is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Propagate start_at date during assessment import if other dates absent
2 participants