Skip to content

[Bugfix:CourseMaterials] Fix Course Materials Directory Deletion#12064

Merged
bmcutler merged 5 commits intomainfrom
fix-course-materials-directory-deletion
Sep 19, 2025
Merged

[Bugfix:CourseMaterials] Fix Course Materials Directory Deletion#12064
bmcutler merged 5 commits intomainfrom
fix-course-materials-directory-deletion

Conversation

@RyanStyron
Copy link
Contributor

Why is this Change Important & Necessary?

Fixes #11923 by invoking CourseMaterialsCleanup#cleanupEmptyDirectoriesAfterDeletion following a change to a file's path.

What is the New Behavior?

Prior to this fix, switching the directory of the example file cell1_source.txt (from the attached issue) to the top directory would yield entries in the course materials section of the given course as follows:
472082065-45c2a0ba-0417-4ca0-8608-423e29859fb4
Now, the entries with IDs of 260 and 261 would no longer be present in the database as they are empty and as such have been removed.

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

Described in the attached issue.

Automated Testing & Documentation

Other information

@codecov
Copy link

codecov bot commented Sep 15, 2025

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.69%. Comparing base (25d7aee) to head (d3d0d11).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #12064      +/-   ##
============================================
- Coverage     21.69%   21.69%   -0.01%     
  Complexity     9587     9587              
============================================
  Files           268      268              
  Lines         36591    36593       +2     
  Branches        475      475              
============================================
  Hits           7940     7940              
- Misses        28180    28182       +2     
  Partials        471      471              
Flag Coverage Δ
autograder 21.39% <ø> (ø)
js 2.07% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.71% <0.00%> (-0.01%) ⬇️
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 90.72% <ø> (ø)

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.

Copy link
Contributor

@JManion32 JManion32 left a comment

Choose a reason for hiding this comment

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

I reproduced the issue on main and confirmed that this PR resolves it. Just make sure you resolve that small PHP lint failure, the failing Cypress tests are unrelated. Great work!

@github-project-automation github-project-automation bot moved this from Seeking Reviewer to Awaiting Maintainer Review in Submitty Development Sep 16, 2025
@bmcutler bmcutler merged commit b9fc23a into main Sep 19, 2025
44 of 48 checks passed
@bmcutler bmcutler deleted the fix-course-materials-directory-deletion branch September 19, 2025 21:12
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.

Empty course material folders

3 participants