What the bug is: LMS looks up the list of "course" group types once and then never looks again. When a recipe installs LMS, Drupal core creates those group types only after the install step. So LMS remembers "there are no course types" and crashes as soon as the recipe creates a group. This affects every release since 1.2.0.

Problem/Motivation

If a recipe installs LMS and then creates groups in the same run, the recipe fails partway through with:

Drupal\Core\Entity\EntityStorageException: Query condition
'group_relationship_field_data.group_type IN ()' cannot be empty.

This affects 1.2.0 and later. I found it while applying LMS Demo Kit (a recipe I maintain), but it also happens with a minimal recipe that only installs lms and lms_classes and creates one class group (attached).

Cause. TrainingManager::getCourseBundleIds() stores its result in a property the first time it runs, and never recalculates it. When a recipe installs modules, core deliberately skips creating config entities (RecipeRunner::installModules() sets the config installer to syncing mode) and imports them afterwards. The router rebuild at the end of module install calls RouteSubscriber::alterRoutes(), which calls getCourseBundleIds() while no group types exist yet, so an empty list is stored. The recipe then imports the lms_course group type, but the same TrainingManager instance keeps returning the empty list for the rest of the process.

When the recipe then saves a group, the creator membership is validated, group permissions are calculated, and ClassPermissionCalculator calls loadGroupRelationshipsByCourseBundles(), which queries group_type IN (). That's the exception above.

Smaller side effect. Any router rebuild later in that same process saves the five course routes (lms.course.start, lms.course.reset_test, lms.group.answer_form, lms.group.results, lms.group.self_results) with bundle: []. Core's EntityConverter treats an empty list as "any bundle", so until the next cache rebuild these routes also match non-course groups. In my (ahem, Claude) testing, /course/{class id}/start returned 403 instead of 404. I didn't find anything worse than that.

Steps to reproduce

Automated: the attached patch includes a kernel test, CourseBundleIdsTest. On 1.2.x without the fix it fails with the same IN () exception.

Manually (Drupal 11.4.7, Group 3.3.5, LMS 1.2.x-dev; reproduced on SQLite, and first seen on MySQL/MariaDB):

  1. Install a fresh site: drush site:install standard -y
  2. Unzip the attached lms-recipe-repro.zip into recipes/.
  3. drush recipe /full/path/to/recipes/lms_min_repro
  4. The recipe fails while creating content, with the exception above.

(The zip contains two recipes. group_base imports Group's group_roles field storage first. It's needed only because Group expects that field to exist when a group type is saved, and has nothing to do with this bug.)

Proposed resolution

  1. Stop caching in getCourseBundleIds(). Core's EntityTypeBundleInfo already caches bundle info, so calculating the list again is just a loop over the group bundles. I measured about 0.4 µs per call uncached vs 0.03 µs cached. I counted 0–3 calls per page load (3 on a cold /courses page, 0 with warm caches), so the cost is around a microsecond per request.
  2. Return [] early from loadGroupRelationshipsByCourseBundles() when there are no course bundles, since IN () isn't valid SQL. A site can have no course bundles at a given moment even without the caching problem, e.g. partway through an install.

Both changes are needed. I tested each on its own:

Variant Minimal recipe Course route bundle afterwards New kernel test
Unpatched 1.2.x fails (IN ()), 3/3 runs n/a fails
Empty-list guard only applies [] (stale) fails
Cache removal only applies ["lms_course"] fails (IN () before any course type exists)
Full patch applies, 3/3 runs ["lms_course"] passes

An alternative would be to keep the cache and clear it when a group type is created or deleted. I didn't go that way because it adds code to save a fraction of a microsecond per request.

Verified locally: LMS's existing unit and kernel tests pass with the patch; phpcs (the project's phpcs.xml, plus DrupalPractice), phpstan (the project's phpstan.neon, level 6) and cspell report no issues on the changed files. The patch applies cleanly to 1.2.x at 5666cfe. I haven't run the FunctionalJavascript test locally.

Remaining tasks

  • Review.
  • Maybe a follow-up: other getCourseBundleIds() callers also pass the result into IN conditions or route options without checking for an empty list (UnpublishedParentConstraintValidator, LmsParentEntityFilter, CoursePermission, LmsEntityViewsDataProvider). I haven't seen them fail, so they're not changed here.

User interface changes

None.

API changes

None. The private $courseBundleIds property is removed; public method signatures are unchanged.

Data model changes

Comments

markfelton created an issue. See original summary.

markfelton’s picture

Status: Needs review » Closed (duplicate)

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.