Issue #3618835: Give the copied manager form, signed token, domains form, storage schema and row accessors one owner each

A duplication audit over the whole project compared 2,527 non-test method bodies and found five concepts written more than once. Each is now written once, with no user-visible change and no string added, changed or removed, so the translations are untouched.

What moved

  • The resource Managers tab. ScopeManagersFormBase already abstracted the four things that differ per scope, and its own docblock said the mechanics were identical to the per-resource form. ResourceManagersForm now extends it (269 lines to 117) and supplies target(), grantMatch(), createDefaults() and emptyMessage(). Two hooks with defaults on the base, userDescription() and duplicateMessage(), keep the resource-specific wording, so nothing shipped is lost. Listing grants now also conditions on scope, as the tenant and type tabs already do: scope is required with a default of resource, and the only code that creates a grant sets it explicitly, so this is the same set of rows and refuses a tenant grant that happened to name a resource.
  • The two signed tokens. CancelToken::open() and TicketDownloadToken::open() were byte-identical over 14 lines, with matching sign() halves, in two submodules that do not reference each other. Both now extend CapabilityToken, and each names only its own purpose. TicketDownloadTokenTest::testCannotReplayCancelToken() already guarded the property that keeps the two apart and still passes.
  • The two Domains tabs. DomainsFormBase carries the form's shape and the storage: the options, the empty state, the checkboxes, the submit and the id filtering. Validation stays in each subclass, because which pairings are contradictory is a rule about the thing being bound and the two rules genuinely differ.
  • The seven storage schema handlers. TableKeysTrait takes core's schema, finds the entity's table and adds what the handler declares. Each handler now states only its indexes, its unique keys, and (for the slot, whose columns live in the data table) which table those belong on.
  • The row accessors. getWeight() was the same content-entity read in 11 classes, getAllotment() in 4, getAllotmentId() in 8 and getResourceId() in 5. Four traits carry them: EntityWeightTrait, AllotmentReferenceTrait, AllotmentRowLabelTrait and EntityReferenceIdTrait. Tenant and BookingChannel keep their own getWeight(), because they are config entities reading a property rather than a field.

What was deliberately not touched

The create() factories (groups of 12, 9 and 9) and the ListBuilder buildHeader() methods (6, 4 and 3) share a shape but no content. That is Drupal's idiom, not duplication, and collapsing it would cost the per-plugin clarity it buys.

Testing

AllotmentIdentityTest is new. Two of the five unique keys had identity tests; the three allotment ones had none, so the guarantee they are the only defence for was unproven and the refactor of those three could not have been shown to preserve it. Each of its refusals was watched to fail with the key removed, and the slot tariff key likewise, which is also what proved the runs were reaching this branch rather than the deployed code.

Green locally before pushing: phpstan level 3 [OK], phpcs Drupal,DrupalPractice over 1,211 files clean, cspell clean once the project word list is subtracted, ModuleBoundariesTest (34 checks, which is what the new cross-module trait imports had to satisfy) plus both token unit test classes, and 14 kernel classes covering the schemas, the domains tabs, the allotment arithmetic and the placement entities.

Merge request reports

Loading
Loading