Problem/Motivation
A duplication audit over the whole project compared 2,527 non-test method bodies in src and the submodules, hashing each one normalized and again with variables, strings and numbers blinded. It found 53 groups of byte-identical bodies and 84 groups sharing a shape. Excluding framework boilerplate, where create() factories and list builder headers share a shape but no content, and excluding bodies under six significant lines, 10 identical and 23 same-shape groups remain. Those reduce to five concepts that are genuinely written more than once. Each is a rule a later fix would have to find twice.
A managers form that copies its own base. ScopeManagersFormBase exists precisely to carry the grant table, the add field group, the validation and the removal, and it abstracts four hooks: target(), grantMatch(), createDefaults() and emptyMessage(). TenantManagersForm and TypeManagersForm use it and are 100 lines each. ResourceManagersForm extends FormBase instead and reimplements the lot at 269 lines: removeSubmit() is byte-identical to the base, and buildForm(), submitForm(), addValidate() and addSubmit() mirror it with no difference that the four hooks do not already express. Only two things actually differ between them, and both are wording: the copy names the resource in its duplicate-grant error and in the user field's description, where the base is generic. Everything else is the same query and the same form built twice, so a correction to either side reaches only that side.
Two signed tokens with one implementation between them. CancelToken::open() in yoyaku_order and TicketDownloadToken::open() in yoyaku_ticket are byte-identical over 14 lines: the same three-part split, the same expiry comparison, the same hash_equals() check, with the signing halves matching too. A correction to the token format or the expiry handling has to be made in two modules that do not reference each other.
Two domains forms with no shared base. TenantDomainsForm (205 lines) and BookingChannelDomainsForm (231 lines) have byte-identical submittedDomainIds() and domains(), and their getFormId(), title(), buildForm(), validateForm() and submitForm() differ only in the entity they hang off.
Seven storage schema classes that add their table keys the same way. Each one takes core's schema, finds the entity's own table in it, and adds what that type needs, and each writes those three steps out again. BookingStorageSchema and BookingTransactionStorageSchema have byte-identical getEntitySchema() bodies and already document an indexes() hook meant for exactly this; four more add a single unique key rather than indexes, and the slot's adds one to its data table. Only the keys, and which table they belong on, actually differ.
Row accessors written once per entity type. label() is byte-identical across the three allotment row entities. getWeight() is hand-written as the same content-entity read in 11 classes, which is the module's widest single repetition, getAllotmentId() in 8 and getResourceId() in 5, each time as the same read of the same field name.
Proposed resolution
Give each of the five one owner. Make ResourceManagersForm extend ScopeManagersFormBase and supply the four hooks, with two further hooks on the base so each scope keeps its own wording. Move the signed token into one class the two submodules share, keeping each module's own key and lifetime. Give the two domains forms a common base holding the form's shape and the storage, and leave validation in each of them, because which pairings are contradictory is a rule about the thing being bound. Add a trait so all seven schema classes state only their indexes, their unique keys, and which of the entity's tables those belong on. Add traits for the weight, allotment and resource accessors, and let the three allotment rows share their label().
Remaining tasks
The engine-side findings from the same audit are a separate issue, because they are about BookingManager reusing readers it already injects rather than about copied classes.
User interface changes
None intended. The strings the copies carried are kept, so the resource managers form keeps its own empty-table text and field descriptions through the base hooks rather than losing them.
API changes
None. Nothing listed here is public API: the touched methods are protected or private, and the interfaces the accessors satisfy do not move.
Data model changes
None. The storage schema change moves where the keys are declared, not the keys themselves, so the tables are unchanged.
AI-Generated: Yes (Claude Code was used to run the duplication audit behind this issue, to help draft this summary, and to write the code and tests on the merge request. The linters and the named test classes were run before it was pushed; a maintainer reviews it before it merges.)
Issue fork yoyaku-3618835
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
mably commentedComment #4
mably commentedComment #6
mably commentedComment #8
mably commented