Skip to content

[Feature:Notifications] Notification defaults for future courses#12912

Merged
bmcutler merged 77 commits into
mainfrom
notification-default-future
Jul 6, 2026
Merged

[Feature:Notifications] Notification defaults for future courses#12912
bmcutler merged 77 commits into
mainfrom
notification-default-future

Conversation

@dagemcn

@dagemcn dagemcn commented Jun 15, 2026

Copy link
Copy Markdown
Member

Why is this Change Important & Necessary?

Closes issue #11732

What is the New Behavior?

A new checkbox inside the sync notifications popup has been added that will update a table in the master database with the information of a course whose notification settings we should copy defaults from. There are also UI updates including banners in the notification settings page to show when a course is the default or if another course already is set as the default. Also, the course you are changing notification settings for will no longer appear in the sync notifications menu.

Before:
image

After:
image
image

What steps should a reviewer take to reproduce or test the bug or new feature?

  1. Run submitty_install
  2. Log in as any student
  3. Go to any existing course's notification settings
  4. Check/uncheck desired notification settings
  5. Click Sync notification settings
  6. Click 'set as future course default'
  7. Log in as instructor
  8. Create a new course
  9. Create a registration section
  10. Add the student with notification defaults set up to the course
  11. Log in as the original student
  12. Go to the new course's notification settings
  13. See defaults that were saved earlier

Also verify the correct banners are displayed in the notification settings page for each course.

Automated Testing & Documentation

Included in this PR.

Other information

This is not a breaking change.
This PR does include a migration.
No known security concerns.

@github-project-automation github-project-automation Bot moved this to Seeking Reviewer in Submitty Development Jun 15, 2026
@automateprojectmangement automateprojectmangement Bot moved this from Seeking Reviewer to Work in Progress in Submitty Development Jun 15, 2026
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 105 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.35%. Comparing base (c33d434) to head (eab332e).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #12912      +/-   ##
============================================
- Coverage     21.41%   21.35%   -0.06%     
- Complexity    10002    10027      +25     
============================================
  Files           277      277              
  Lines         37418    37519     +101     
  Branches        512      512              
============================================
  Hits           8013     8013              
- Misses        28904    29005     +101     
  Partials        501      501              
Flag Coverage Δ
autograder 21.16% <ø> (ø)
js 1.98% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.30% <0.00%> (-0.07%) ⬇️
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 91.13% <ø> (ø)
vue 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dagemcn dagemcn changed the title [Feature:Notifications] Set notification defaults for future courses [Feature:Notifications] Notification defaults for future courses Jun 15, 2026
@dagemcn
dagemcn marked this pull request as ready for review June 18, 2026 16:53
@automateprojectmangement automateprojectmangement Bot moved this from Work in Progress to Seeking Reviewer in Submitty Development Jun 18, 2026
@bmcutler

Copy link
Copy Markdown
Member

@dagemcn please update migration name to be < 7 days old.

@bmcutler
bmcutler requested a review from NicholasCiuica June 25, 2026 20:46
@automateprojectmangement automateprojectmangement Bot moved this from Awaiting Maintainer Review to In Review in Submitty Development Jun 25, 2026
@NicholasCiuica

Copy link
Copy Markdown
Member

Only issue I'm seeing on my rereview is that the new sync_notification spec is not passing locally on the last test. It has something to do with student first and last name failing to auto-complete when you select their id in the New Student popup of the Manage Students page.

Comment thread site/app/controllers/admin/UsersController.php Outdated
Comment thread site/app/controllers/admin/UsersController.php Outdated
Comment thread site/app/libraries/database/DatabaseQueries.php Outdated

@jndlansh jndlansh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I did both, a code review and a functionality check. It works as described.
Cypress test coverage is great. Code is overall very good. A few nitpicking comments I have added, just address them - I would be happy to approve then!

@github-project-automation github-project-automation Bot moved this from In Review to Work in Progress in Submitty Development Jul 1, 2026
@dagemcn
dagemcn requested a review from jndlansh July 1, 2026 18:26

@jndlansh jndlansh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing my comments. Great Work!

@github-project-automation github-project-automation Bot moved this from Work in Progress to Awaiting Maintainer Review in Submitty Development Jul 1, 2026
@automateprojectmangement automateprojectmangement Bot moved this from Awaiting Maintainer Review to In Review in Submitty Development Jul 1, 2026
@bmcutler
bmcutler merged commit af07c0d into main Jul 6, 2026
51 of 53 checks passed
@bmcutler
bmcutler deleted the notification-default-future branch July 6, 2026 13:06
@github-project-automation github-project-automation Bot moved this from In Review to Done in Submitty Development Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants