Skip to content

Course: add Delete operation to /api/courses/{id} (admin only)#8675

Open
addow wants to merge 1 commit into
chamilo:masterfrom
webstone:feat/course-delete-api
Open

Course: add Delete operation to /api/courses/{id} (admin only)#8675
addow wants to merge 1 commit into
chamilo:masterfrom
webstone:feat/course-delete-api

Conversation

@addow

@addow addow commented Jun 29, 2026

Copy link
Copy Markdown

Summary

Adds DELETE /api/courses/{id} to the Course #[ApiResource], restricted to ROLE_ADMIN.

  • Uses API Platform's built-in remove processor (EntityManager::remove() + flush)
  • The database-level onDelete: CASCADE on resource_node_id handles cleanup of the linked ResourceNode
  • Mirrors the pattern used by other destructive operations on Course (Patch) and Session (Delete)

Notes

CourseVoter::DELETE grants course teachers delete rights, which would be too permissive for a course-level delete. The explicit is_granted('ROLE_ADMIN') sidesteps the voter and restricts access correctly.

Physical files (e.g. course illustrations on disk) are not automatically removed on delete — this is a pre-existing limitation shared by the legacy CourseRepository::deleteCourse().

@addow

addow commented Jun 30, 2026

Copy link
Copy Markdown
Author

Follow-up: filesystem and ResourceNode cleanup are not handled

After further investigation, I want to flag a limitation of this implementation before it is merged.

What happens when DELETE /api/courses/{id} is called:

API Platform's default remove processor calls EntityManager::remove($course) + flush(). This only deletes the course DB row. The following are not cleaned up:

Left behind Root cause
resource_node row (and all child rows) The FK is on course.resource_node_id → resource_node.id. onDelete: CASCADE means "delete course when resource_node is deleted" — not the reverse. AbstractResource only has cascade: ['persist'], not cascade: ['remove'], so Doctrine never schedules the ResourceNode for removal.
All resource_file rows ResourceNode.resourceFiles has cascade: ['persist', 'remove'], but this only fires if the ResourceNode is explicitly removed via Doctrine — which it isn't.
Physical files in var/upload/resource/ VichUploader's RemoveListener only fires when a #[Vich\Uploadable] entity (ResourceFile) goes through Doctrine's preRemove. Since no ResourceFile is ever Doctrine-removed, no files are deleted from disk.

The correct approach would be a custom StateProcessor for the Delete operation that walks the ResourceNode tree, removes each ResourceFile via Doctrine (so VichUploader fires), and then removes the ResourceNode — before the Course row itself is deleted. IllustrationRepository::deleteIllustration() already demonstrates this pattern for the illustration sub-resource.

The commented-out node-walking code in CourseRepository::deleteCourse() appears to be a scar from a previously abandoned attempt at the same problem.

Options going forward:

  1. Merge as-is with the understanding that orphaned files need manual cleanup (same behaviour as the legacy CourseRepository::deleteCourse())
  2. Add a custom CourseDeleteProcessor that properly cleans up the ResourceNode hierarchy before deletion

Happy to implement option 2 if that is the preferred direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant