Add scheduled visibility for assessments #7697

lxyhan · 2025-10-10T17:03:08Z

Proposed Changes

Added datetime-based visibility scheduling for assessments. Instructors can now set visible_on and visible_until dates to automatically show/hide assessments instead of manually toggling them.

Right now instructors can only mark assessments as visible or hidden with a boolean. They've been requesting the ability to schedule when assessments become visible (like "October 1 - December 31") so they can set it up at the start of the semester and not worry about manually publishing assignments later.

My changes in this PR

Database:

Added visible_on and visible_until datetime columns to assessments and assessment_section_properties tables
Updated the check_repo_permissions SQL function to enforce datetime visibility for git access

Models:

Updated Student#visible_assessments to check datetime ranges
Added validations to ensure visible_on < visible_until
Datetime columns override is_hidden when they're set

API:

Exposed visible_on and visible_until fields in assignments and grade entry forms API endpoints

Tests:

Added validation tests for datetime columns
Added 18 test cases for visibility logic (global, section-specific, datetime ranges, edge cases)
Added 9 policy tests for datetime visibility authorization
If visible_on/visible_until are set, they override the is_hidden boolean
Section-specific datetime settings override global settings (same pattern as section due dates)
Backwards compatible - if datetime columns are null, falls back to is_hidden
Can set just visible_on, just visible_until, or both

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)	x
🐛 Bug fix (non-breaking change that fixes an issue)
🎨 User interface change (change to user interface; provide screenshots)
♻️ 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.)

Add visible_on/visible_until columns to schedule assessment visibility automatically. Datetime columns override is_hidden when set. Section-specific datetime overrides global datetime. Updated models, policies, SQL function, API controllers, and added comprehensive tests.

coveralls · 2025-10-13T13:29:21Z

Pull Request Test Coverage Report for Build 18783844054

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

193 of 198 (97.47%) changed or added relevant lines in 10 files are covered.
55 unchanged lines in 4 files lost coverage.
Overall coverage increased (+0.06%) to 91.574%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
app/models/assessment_section_properties.rb	6	7	85.71%
app/lib/repository.rb	13	17	76.47%

Files with Coverage Reduction	New Missed Lines	%
app/models/result.rb	2	99.17%
app/javascript/Components/Result/marks_panel.jsx	17	43.02%
app/models/assignment.rb	18	93.82%
app/javascript/Components/graders_manager.jsx	18	41.64%

Totals
Change from base Build 18414815512:	0.06%
Covered Lines:	42751
Relevant Lines:	45906

💛 - Coveralls

david-yz-liu

@lxyhan nice work! I left a few inline comments. Note that my comments on the queries apply to both branches of the if statement.

In addition to the inline comments:

Add tests for the SQL function in spec/db/check_repo_permissions_spec.rb
You also need to update Repository.visibility_hash and Assignment#visibility_changed?

david-yz-liu · 2025-10-14T01:54:34Z

Changelog.md

 ### 🚨 Breaking changes

 ### ✨ New features and improvements
+- Added datetime-based visibility scheduling for assessments with `visible_on` and `visible_until` columns


Make sure to include the PR number here

david-yz-liu · 2025-10-14T02:00:22Z

app/models/assessment.rb


+  def visible_dates_are_valid
+    return if visible_on.nil? || visible_until.nil?
+    return unless visible_on.respond_to?(:>=) && visible_until.respond_to?(:>=)


I don't think this line is necessary, if it's reach then shouldn't these attributes be dates?

david-yz-liu · 2025-10-14T02:02:42Z

app/models/assessment_section_properties.rb


  has_one :course, through: :assessment
  validate :courses_should_match
+  validate :visible_dates_are_valid


Validations are typically run in top-down order, so it would be more typical to write this validation below the next two (so that the types are checked first)

david-yz-liu · 2025-10-14T02:14:37Z

app/models/student.rb

+      )
+
+      # Use datetime if set, otherwise fall back to is_hidden
+      visible.where(


This can be simplified using built-in ActiveRecord methods. In this case you can use both .where.not(...) and .or

david-yz-liu · 2025-10-14T02:24:26Z

app/models/student.rb

-      visible = visible.where(is_hidden: false)
+      # No section assigned - just check global visibility
+      visible = visible.where(
+        '(assessments.visible_on IS NULL OR assessments.visible_on <= ?) AND ' \


Similar to my comments below, I'd prefer using the ActiveRecord methods where possible. In this case the part with the <= is fine, but you can use .or instead of SQL OR and chained .wheres instead of AND

… SQL function tests

david-yz-liu · 2025-10-16T17:46:58Z

app/models/student.rb

+      section_visible = section_visible
+                        .where.not(assessment_section_properties: { visible_on: nil })
+                        .or(section_visible.where.not(assessment_section_properties: { visible_until: nil }))
+                        .or(section_visible.where(assessment_section_properties: { is_hidden: false }))


This is something subtle here with the is_hidden section property.

In the case that for the section properties, visible_on: nil, visible_until: nil, is_hidden: true, then the assignment should not be visible to students in that section, regardless of what the global properties are. I don't think this is currently being implemented correctly by this logic.

You handled this differently in the visibility_hash method by first checking whether any of the three section properties were non-nil before overriding the global properties.

And in the SQL function, you're only checking assessment_section_properties.is_hidden IS NOT NULL but should probably be checking all three.

david-yz-liu · 2025-10-16T17:47:42Z

spec/models/student_spec.rb

+        expect(student.visible_assessments).not_to include(assignment)
+      end
+
+      it 'shows assessment when datetime range is valid (overrides is_hidden=true)' do


The word "valid" is not useful in this context. Better wording would be "when the datetime range includes the current time". This repeats a few times in the test descriptions.

…rove test descriptions

david-yz-liu

@lxyhan nice work, I just left two small comments

david-yz-liu · 2025-10-18T18:49:09Z

app/models/student.rb

+
+      # Check section-specific visibility first (takes precedence when any section property is set)
+      section_visible = visible.where('assessment_section_properties.section_id': self.section_id)
+                               .where('assessment_section_properties.is_hidden IS NOT NULL OR ' \


this is a place where you can use ActiveRecord methods like .or and .not

david-yz-liu · 2025-10-18T18:52:16Z

app/models/student.rb

+
+      # Check global visibility (when no section-specific settings exist)
+      global_visible = visible.where('assessment_section_properties.section_id': nil)
+                              .or(visible.where('assessment_section_properties.is_hidden IS NULL AND ' \


a similar comment here, you can both use : nil and pass multiple conditions to .where instead of relying on raw SQL

david-yz-liu

Oh @lxyhan I just realized that in 322c3a8 you modified structure.sql directly, but did not modify the migration file. You should always modify migrations rather than structure.sql.

In this case, you can first rollback the existing migration, and then modify it, and then run it again. You won't see any changes in git to the structure.sql file, but you will see the changes to your migration file, which should be committed and pushed.

david-yz-liu

Nice work, @lxyhan!

lxyhan added 2 commits October 10, 2025 12:52

Update changelog and improve code comments

bbdb5a5

lxyhan changed the title ~~Add scheduled assessment visibility~~ Add scheduled visibility for assessments Oct 10, 2025

Merge branch 'master' into add-scheduled-assessment-visibility

316469c

lxyhan marked this pull request as ready for review October 11, 2025 20:16

lxyhan requested a review from david-yz-liu October 11, 2025 20:18

lxyhan added 2 commits October 12, 2025 16:22

Fix visible_on/visible_until validation and update test expectations

122d7be

Update test helpers to include visible_on/visible_until fields

9629b85

david-yz-liu reviewed Oct 14, 2025

View reviewed changes

Use ActiveRecord range syntax for datetime visibility queries and add…

4af096b

… SQL function tests

lxyhan requested a review from david-yz-liu October 15, 2025 22:08

david-yz-liu reviewed Oct 16, 2025

View reviewed changes

Fix section property override logic to check all three fields and imp…

322c3a8

…rove test descriptions

lxyhan requested a review from david-yz-liu October 18, 2025 12:36

david-yz-liu reviewed Oct 18, 2025

View reviewed changes

Use ActiveRecord Query methods instead of Raw SQL in student.rb

5069f57

lxyhan requested a review from david-yz-liu October 19, 2025 12:55

david-yz-liu reviewed Oct 19, 2025

View reviewed changes

Move section property override logic to migration file

d198c38

lxyhan requested a review from david-yz-liu October 24, 2025 16:05

david-yz-liu approved these changes Oct 24, 2025

View reviewed changes

david-yz-liu merged commit 2ca6a7b into MarkUsProject:master Oct 24, 2025
7 checks passed

david-yz-liu mentioned this pull request Oct 24, 2025

Add penalty_type option to submission rules for percentage_of_mark penalties #7709

Open

9 tasks

Add scheduled visibility for assessments #7697

Add scheduled visibility for assessments #7697

Conversation

lxyhan commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

My changes in this PR

Type of Change

Checklist

Questions and Comments

Uh oh!

coveralls commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 18783844054

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

david-yz-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lxyhan commented Oct 10, 2025 •

edited

Loading

coveralls commented Oct 13, 2025 •

edited

Loading