Display Grace Day usage on submission history table, improve management of assessment penalty settings

[damianhxy]

Summary

Summary generated by Reviewpad on 01 Nov 23 03:23 UTC

This pull request includes several changes across multiple files. Here's a summary of the changes:

• In the file _edit_penalties.html.erb, changes were made to text fields, labels, and checkboxes related to assessment penalties. The changes aim to improve user experience and provide more flexibility in setting penalties for assessments.
• In the file assessments_controller.rb, modifications were made to the edit and update actions to handle penalty-related parameters and provide warning messages for instructors. These changes enhance the functionality and flexibility of the assessment controller.
• The file courses/_courseFields.html.erb includes modifications to the placeholder values of certain input fields.
• Changes in the FormBuilderWithDateTimeInput class consolidate and improve the arguments and options passed to various methods.
• The file diff related to the Penalties tab in the assessment editor introduces event listeners for checkboxes, logic to enable or disable input fields, and triggering change events on page load.
• Changes in the ScoreAdjustment model's to_s method enhance consistency and formatting.
• The app/views/assessments/_submission_history_table.html.erb file modifies a label in the submission history table.
• CSS selectors in another file now exclude elements with a specific class and target additional elements, adding a bottom padding.
• The edit.html.erb file in the app/views/assessments directory removes error message handling code.
• A new file config/initializers/core_ext.rb is being added, defining class extensions for string-to-boolean conversions.
• Changes in the app/views/assessments/_submission_history_row.html.erb file enhance the display of submission details.

These changes improve functionality, user experience, and consistency in different parts of the codebase.

Description

• Display grace days used next to number of late days
• Add warnings when late days are allowed but settings do not make sense (e.g. grace days can't be used and/or penalty late days are "free")
• Add checkboxes on penalty tab to easily use course defaults / allow unlimited submissions / allow unlimited grace days

Motivation and Context

Closes #1989

How Has This Been Tested?

Make a late submission on an assessment that allows the use of grace days. Observe the correct number of grace days used displayed.

Allow late submissions on assessment (end at > due at). Set late penalty to 0 and/or max grace days to 0, see warning(s). Also check that they display correctly when course defaults are used for these values.

• Set values for max submissions, max grace days, late penalty, version threshold, version penalty -> ensure it saves successfully
• Check all the checkboxes and save
  • max submissions -> -1
  • max grace days -> (blank)
  • late penalty -> (blank)
  • version threshold -> -1
  • version penalty -> (blank)
• Verify that the above settings take effect
  • make a student account submit once, a few days late
  • they should see unlimited submissions left
  • they should see all their grace days being used (e.g. 3)
  • can test late penalty by toggling between 0 (assessment-specific), and 1 (course default)
  • can test version threshold / penalty in a similar manner

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

[20wildmanj]

If I add a value into one of the fields that can be defaulted, and then click the checkbox, the value in the field doesn't get cleared. Could the value here be cleared / replaced with the default when re-clicked?

[20wildmanj]

There doesn't seem to be enough vertical space between the title and the text box, and also too much space between the checkmark and the textbox, makes it seem like the checkbox is for the field below the actual one

[20wildmanj]

Functionally looks good, if you could try to fix up the frontend stuff that would be great (I know it might be hard so if you can't then that's ok)

[coderabbitai]

Walkthrough

The changes primarily focus on enhancing the user interface and functionality of the assessment system. They introduce new event handlers for checkboxes, modify CSS styling, display warning messages, and update form fields. The changes also improve the handling of late submissions and grace days, and adjust the display of submission details.

Changes

File(s)	Summary
app/assets/javascripts/edit_assessment.js	Introduced event handlers for checkboxes, modifying corresponding input fields based on the checked state. Added trigger events for checkboxes on page load.
app/assets/stylesheets/style.css.scss	Updated CSS styling for input fields and form divs.
app/controllers/assessments_controller.rb	Added warning messages, set variables based on assessment settings, set up assessment penalties, and added parameters to the edit_assessment_params method.
app/form_builders/form_builder_with_date_time_input.rb	Updated the FormBuilderWithDateTimeInput class to include new options and attributes in various methods.
app/models/score_adjustment.rb	Modified the to_s method in the kind class to change the formatting of the output string.
app/views/assessments/_edit_penalties.html.erb	Updated form fields with help text and checkboxes for unlimited submissions and grace days.
app/views/assessments/_submission_history_row.html.erb	Updated the display of submission details related to late submissions and grace days.
app/views/assessments/_submission_history_table.html.erb	Changed the header label in a table from "Late Days Used" to "Days Late (Grace Days Used)".
app/views/assessments/edit.html.erb	Removed error message rendering for @course and @assessment objects in a form.
app/views/courses/_courseFields.html.erb	Updated placeholders and help text for various fields.
config/initializers/core_ext.rb	Introduced two new methods: to_boolean for the String and NilClass classes.

Validation with GitHub issue(Beta)

Assessment of the PR changes against GitHub Issue

Aspect	Aligns with the Linked Issue	Explanation
Submissions history table changes	✅	The changes in app/views/assessments/_submission_history_row.html.erb and app/views/assessments/_submission_history_table.html.erb align with the issue's requirement to differentiate between Penalty Late Days and Grace Days.
Indication of late submissions with zero penalties or grace days	✅	The changes in app/controllers/assessments_controller.rb introduce a warning message for instructors when late submissions are allowed but have zero late penalty or zero maximum grace days, which aligns with the issue's requirements.
Changes to form fields and checkboxes	❓	The changes in app/assets/javascripts/edit_assessment.js, app/views/assessments/_edit_penalties.html.erb, and app/views/courses/_courseFields.html.erb introduce new checkboxes and modify form fields. These changes seem related to the issue's requirements but it's not explicitly clear from the issue summary.
Changes to CSS styling and form builder	❌	The changes in app/assets/stylesheets/style.css.scss and app/form_builders/form_builder_with_date_time_input.rb seem unrelated to the issue's requirements.
Changes to to_boolean methods and ScoreAdjustment model	❌	The changes in config/initializers/core_ext.rb and app/models/score_adjustment.rb seem unrelated to the issue's requirements.

 </details>
 <!-- This is an auto-generated comment: raw summary by coderabbit.ai -->

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai help to get help.
• @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.json

[damianhxy]

PR has been updated so that helper values are displayed when checkboxes are selected

[coderabbitai]

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between e0473f9 and 65f7d2e.

Files selected for processing (10)

• app/assets/javascripts/edit_assessment.js (1 hunks)
• app/assets/stylesheets/style.css.scss (1 hunks)
• app/controllers/assessments_controller.rb (2 hunks)
• app/form_builders/form_builder_with_date_time_input.rb (2 hunks)
• app/models/score_adjustment.rb (1 hunks)
• app/views/assessments/_edit_penalties.html.erb (1 hunks)
• app/views/assessments/_submission_history_row.html.erb (1 hunks)
• app/views/assessments/_submission_history_table.html.erb (1 hunks)
• app/views/assessments/edit.html.erb (1 hunks)
• app/views/courses/_courseFields.html.erb (1 hunks)

Files skipped from review due to trivial changes (7)

• app/assets/stylesheets/style.css.scss
• app/models/score_adjustment.rb
• app/views/assessments/_edit_penalties.html.erb
• app/views/assessments/_submission_history_row.html.erb
• app/views/assessments/_submission_history_table.html.erb
• app/views/assessments/edit.html.erb
• app/views/courses/_courseFields.html.erb

Additional comments: 6

app/form_builders/form_builder_with_date_time_input.rb (3)

• 20-24: The wrap_field method is now called with options instead of help_text and display_name. Ensure that all calls to this method throughout the codebase have been updated to match the new signature.

• 27-39: The score_adjustment_input method now includes an optional placeholder and disabled option for the text field and select field. This is a good practice as it allows for more flexibility when using this method.

• 132-142: The date_helper method now includes a data-date-greater-than attribute in the generated field. This could be useful for validating the date input. The wrap_field method now accepts an options parameter instead of separate help_text and display_name parameters. It also includes an optional wrap_class option for the wrapping <div> element. These changes improve the flexibility and reusability of these methods.

app/assets/javascripts/edit_assessment.js (1)

• 43-115: The event handlers for the checkboxes are well implemented. They correctly update the state and value of the corresponding input fields based on the checked state of the checkboxes. The use of the initial_load variable to prevent clearing of fields on the first page load is a good approach. However, consider adding comments to explain the purpose of the initial_load variable and the logic behind the event handlers for better maintainability.

    // Penalties tab
+   // The initial_load variable is used to determine if the page is loading for the first time.
+   // If it is the first load, the fields should not be cleared when the checkboxes are checked.
    let initial_load = true; 

    // Event handler for the 'unlimited_submissions' checkbox.
    // When checked, it disables the 'max_submissions' input field and sets its value to 'Unlimited submissions'.
    // When unchecked, it enables the 'max_submissions' input field and clears its value (if it's not the first page load).
    $('#unlimited_submissions').on('change', function() {
...

app/controllers/assessments_controller.rb (2)

• 796-801: The variables are correctly set based on the assessment settings.

• 803-807: The penalties are correctly set up if they are not already present.

[coderabbitai]

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 65f7d2e and b76df5b.

Files selected for processing (2)

• app/controllers/assessments_controller.rb (2 hunks)
• config/initializers/core_ext.rb (1 hunks)

Files skipped from review due to trivial changes (1)

• config/initializers/core_ext.rb

Additional comments: 1

app/controllers/assessments_controller.rb (1)

• 798-802: The variables are well defined and will be useful for the penalties tab. However, consider using a more descriptive name for these variables to improve code readability.