ISSUE-7677: Display late penalty selection

[Naragod]

Proposed Changes

This is a 🎨 User interface change.

We wish to notify users if a late penalty has been applied to an extension at first glance. Currently, users have to click the due date extension link before being able to view the application of late penalties to their assignments. More information can be found in the issue description.

Implementation

We already keep track of an extension's late submission policy. Let's display it to the user when viewing the group table assignments.

Let's also add a filter that enables us to filter this column to show

1. All groups
2. Groups without extension
3. Groups with extension but no late submissions
4. Groups with extensions and that allow late submissions.

Screenshots of your changes (if applicable)

Associated documentation repository pull request (if applicable)

Type of Change

(Write an X or a brief description next to the type or types that best describe your changes.)

Type	Applies?
🚨 Breaking change (fix or feature that would cause existing functionality to change)
✨ New feature (non-breaking change that adds functionality)
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)	X
♻️ Refactoring (internal change to codebase, without changing functionality)
🚦 Test update (change that only adds or modifies tests)
📦 Dependency update (change that updates a dependency)
🔧 Internal (change that only affects developers or continuous integration)

Checklist

(Complete each of the following items for your pull request. Indicate that you have completed an item by changing the [ ] into a [x] in the raw text, or by clicking on the checkbox in the rendered description on GitHub.)

Before opening your pull request:

• I have performed a self-review of my changes.
  • Check that all changed files included in this pull request are intentional changes.
  • Check that all changes are relevant to the purpose of this pull request, as described above.
• I have added tests for my changes, if applicable.
  • This is required for all bug fixes and new features.
• I have updated the project documentation, if applicable.
  • This is required for new features.
• If this is my first contribution, I have added myself to the list of contributors.

After opening your pull request:

• I have updated the project Changelog (this is required for all changes).
• I have verified that the pre-commit.ci checks have passed.
• I have verified that the CI tests have passed.
• I have reviewed the test coverage changes reported by Coveralls.
• I have requested a review from a project maintainer.

Questions and Comments

(Include any questions or comments you have regarding your changes.)

[david-yz-liu]

Hi @Naragod, nice work making this PR! Adding an extra column table is not a bad idea (and you've done so correctly), but I think this will make UI a bit more cluttered on smaller screens, which I'd like to avoid.

Instead I suggest conditionally adding the text "(late submissions accepted)" directly in the "Extension" column, after the extension period. Note: user-facing string literals shouldn't be added to the components directly, but instead added to the locale files under config/locales and then referred to by calling I18n.t().

I'd also be good with adding a custom filter to this column to show (1) all groups, (2) groups without extension, (3) groups with extension but no late submissions, and (4) groups with extensions and that allow late submissions.

[coveralls]

Pull Request Test Coverage Report for Build 18383183058

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 20 of 26 (76.92%) changed or added relevant lines in 2 files are covered.
• 37 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.4%) to 91.516%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
app/javascript/Components/Helpers/table_helpers.jsx	7	13	53.85%

Files with Coverage Reduction	New Missed Lines	%
app/controllers/assignments_controller.rb	37	79.07%

Totals
Change from base Build 18331268749:	-0.4%
Covered Lines:	42494
Relevant Lines:	45647

---

💛 - Coveralls

[Naragod]

Hi @david-yz-liu,

I have taken into account your comment and a few things remains unclear:

• The issue's original intent was to demonstrate to the user whether or not a penalty was applied to a user's late submission. I am unsure why we would want to add the text late submissions accepted when the time extension is already displayed, i.e. 1 week. Do we want to make it more clear to the user that a time extension has been granted?

• How do you recommend we mark late penalties?

• I was under the impression that submitting an assignment after it had been extended was a late submission. I am unsure what you mean by groups with extension but no late submissions, can you please clarify? I perhaps lack the context to understand exactly what you mean.

[donny-wong]

Extension due date allows submissions to be submitted without being considered late. By selecting "Apply late submission policy when collecting this group's submissions" checkbox, the late submission policy will take into effect once the extension due date has been reached.

For the class in general, the late submission policy will be applied to the original due date.

[david-yz-liu]

Hi @Naragod, it's great to clarify the language and semantics here, as this is quite subtle.

In MarkUs each assignment has a due date, and then on top of that individual groups can get an extension, which adds onto the assignment due due.

But separate from this, assignments can also have a late submission policy (implemented in the SubmissionRule subclasses) that allows all students to submit after the assignment due date.

By default, when an extension is granted then the extended deadline replaces any late submission policy. But if the instructor checks the "Apply late submission policy" checkbox, the group can both get the extension, and additional time on top of the extension based on the configured late submission policy. For example, a group might get a 1 week extension, but then also be able to submit an extra day late (i.e., 8 days after the original due date) with a 10% penalty.

---

Having said all of the above, I agree that the wording "late submissions accepted" may be confusing. I'd be okay with rewording this, though I don't have any brilliant ideas that are short. Something like "extended further by late submission policy", but shorter? You could discuss with Donny to suggest alternate wording.

[Naragod]

Thank you @donny-wong and @david-yz-liu for the clarifications. I have a much better understanding of the task and have applied the feedback given. Please take another look.

[david-yz-liu]

@Naragod I took a quick look, and I think the changes look great. Please go ahead and add test cases; it may be that this component does not have any existing tests, so you might need to spend time adding some.

[david-yz-liu]

@Naragod nice work! I just left a few quick comments. Please also make sure to add your name to the contributors list.

[Naragod]

@david-yz-liu, please take another look.

I made a few small changes:

• Grammatic recommendations implemented.
• Moved where (late submissions accepted) is added. I realized after some testing, that my original approach of appending the late submission accepted string would have resulted in multiple extra additions of this text if multiple time periods were selected. Example: 1 week (late submission accepted), 2 days (late submissions accepted)
• Moved the time extension function into the table_helpers file for reusability. (It comes in handy during my tests).

[david-yz-liu]

@Naragod you addressed all of my inline comments (which is great) and your additional changes also look good to me. But I think you missed the text in my "overall comment" for the review:

@Naragod nice work! I just left a few quick comments. Please also make sure to add your name to the contributors list.

I realized that you did not preserve the PR template that we use for MarkUs. In the future please do so; you can add subsections to the "proposed changes" section, but it's important that you keep the other sections.

[Naragod]

Hi @david-yz-liu

I have added my name to the list of contributors and updated the PR description. I appreciate your patience, I was confused by what your previously comment.

Usually, when a commit is merged, github automatically adds your name to the list of contributors. Once my PR was merged, I would have been added to this list and so, was confused by what you meant.

Let me know if there are any other changes you would like me to make.

[david-yz-liu]

Thanks, @Naragod!