Limit module editing, deletion, and course assignment to the owner

[albarin]

Closes https://linear.app/a8c/issue/SEN-79

Proposed Changes

Teachers hold the manage_modules capability so they can manage their own
modules, but that capability was applied without any per-module ownership
check. This restricts two operations to the module's owner:

• Editing / deleting a module: WordPress maps the edit_term and
  delete_term meta capabilities straight to manage_modules, ignoring which
  module it is. A map_meta_cap filter now denies those operations on a module
  the current non-admin user does not own.
• Assigning a module to courses: The "Course(s)" field on the
  module edit screen synced a module's course associations from the submitted
  IDs with no permission check. save_module_course() now skips any course the
  current user cannot edit, in both the attach and detach loops.

Administrators are unaffected. The restriction applies to all non-admin
holders of manage_modules, teachers and editors (editors were already
scoped to their own modules on the read side, so this aligns write access with
what they could already see).

Tip

Might be easier to review commit by commit, since they are quite self-contained.

Testing instructions

Manual testing instructions can be found in the Linear issue

Automated tests

make test-php-filter FILTER="Sensei_Class_Modules_Test"

[github-actions]

WordPress Playground Preview

The changes in this pull request can previewed and tested using a WordPress Playground instance.

[Copilot]

Pull request overview

This PR tightens authorization around Sensei “module” taxonomy management so that non-admin users with manage_modules can only edit/delete modules they own, and can only attach/detach modules to courses they’re allowed to edit.

Changes:

• Added a map_meta_cap filter to deny edit_term / delete_term on module terms not owned by the current non-admin user.
• Updated module ↔ course sync on the module edit screen to skip attaching/detaching for courses the current user cannot edit.
• Added PHPUnit coverage for module list scoping, term capability enforcement, and course assignment restrictions; added a changelog entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
includes/class-sensei-modules.php	Adds ownership-based term-cap restriction and course-edit-cap checks during module↔course syncing.
tests/unit-tests/test-class-modules.php	Adds unit tests validating the new ownership and course-assignment authorization behavior.
changelog/fix-module-ownership-authorization	Records the user-facing fix in the changelog.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[albarin]

@claude please review

[claude]

Code Review

The approach is solid — locking down edit_term/delete_term via map_meta_cap is the right WordPress mechanism, and the save_module_course guards are correct. Tests are well-structured, follow the project naming convention, and cover the meaningful cases. Two issues worth addressing before merge:

---

1. Redundant DB query in restrict_module_term_management (efficiency)

map_meta_cap fires on every capability check in WordPress, not only module-related ones. For edit_term/delete_term on a module term the code reaches:

$author = self::get_term_author( $term->slug );

get_term_author() calls get_term_by( 'slug', $slug, 'module' ) internally — a fresh DB query — even though the term was already fetched a few lines above. That is two queries for one term per capability check.

Fix: read the meta directly (cached after the first access in a request):

$author_id = (int) get_term_meta( $term->term_id, 'module_author', true );
// Legacy: fall back to slug-encoded ownership if meta is absent.
if ( ! $author_id ) {
    $author    = self::get_term_author( $term->slug );
    $author_id = $author ? (int) $author->ID : 0;
}
if ( $author_id !== (int) $user_id ) {
    $caps[] = 'do_not_allow';
}

---

2. get_term( $args[0] ) should specify the taxonomy (correctness)

$term = get_term( $args[0] );

Without a taxonomy argument, WordPress looks up the term_id across all taxonomies. If the same term_id ever appears in more than one taxonomy (the schema allows it), WordPress may return the non-module entry. The subsequent 'module' !== $term->taxonomy check would then bail out without applying the restriction — silently skipping the ownership guard.

Fix is a one-word change:

$term = get_term( $args[0], 'module' );

This also makes the intent immediately clear to future readers.

[donnapep]

Left a few comments, a couple of which I'd consider blockers.

[donnapep]

All good! 🎉