Problem/Motivation

While working on #2616814: Delegate all hook invocations to ModuleHandler, we discovered that workspaces_entity_load() takes its $entities argument by reference, which violates the hook_entity_load() interface.

Proposed resolution

Introduce hook_entity_load_alter() to directly solve the problem in the Workspaces module, and hook_ENTITY_TYPE_load_alter() to make this alteration behavior consistent with the other entity API hooks.

Remaining tasks

None.

User interface changes

None.

API changes

None. These are additions only.

Data model changes

None.

CommentFileSizeAuthor
#8 3001101-8.patch1.61 KBamateescu
#6 drupal_3001101_6.patch12.65 KBxano
#6 interdiff.txt4.68 KBxano
#2 drupal_3001101_2.patch8.73 KBxano

Comments

Xano created an issue. See original summary.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Assigned: xano » Unassigned
Status: Active » Needs review
StatusFileSize
new drupal_3001101_2.patch8.73 KB
xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Title: workspaces_entity_load() violates the hook interface (a.k.a. introdce hook_entity_load_alter() and hook_ENTITY_TYPE_LOAD_alter()) » workspaces_entity_load() violates the hook interface (a.k.a. introdce hook_entity_load_alter() and hook_ENTITY_TYPE_load_alter())
xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Issue tags: -Needs change record

I added a change record.

Status: Needs review » Needs work

The last submitted patch, 2: drupal_3001101_2.patch, failed testing. View results

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt4.68 KB
new drupal_3001101_6.patch12.65 KB
joachim’s picture

joachim commented
Title: workspaces_entity_load() violates the hook interface (a.k.a. introdce hook_entity_load_alter() and hook_ENTITY_TYPE_load_alter()) » workspaces_entity_load() violates the hook interface (a.k.a. introduce hook_entity_load_alter() and hook_ENTITY_TYPE_load_alter())
Component: workspaces.module » entity system
Issue tags: +Needs subsystem maintainer review

Thanks for filing this! :)

The real change here is in the Entity system, and I think this needs input from the maintainer.

amateescu’s picture

amateescu commented
Title: workspaces_entity_load() violates the hook interface (a.k.a. introduce hook_entity_load_alter() and hook_ENTITY_TYPE_load_alter()) » Fix the documentation for entity load hooks
Issue tags: -Needs subsystem maintainer review
StatusFileSize
new 3001101-8.patch1.61 KB

I think the existing hooks can do their job just fine, as proven by the current implementation in the Workspaces module. IMO there's no need to introduce two new hooks, we should fix their documentation instead.

joachim’s picture

joachim commented

> we should fix their documentation instead.

Their documentation is not wrong. Workspaces module is misusing them. This isn't a bugfix, this is an API addition. Some of this discussion was already had over at the issue that spawned this one: #2616814: Delegate all hook invocations to ModuleHandler.

amateescu’s picture

amateescu commented
Status: Needs review » Closed (duplicate)

@joachim, ah, I see! Then this is a duplicate of #2928888: Add a hook_entity_preload() for modules that need to load a different revision than the default one :)

joachim’s picture

joachim commented
Title: Fix the documentation for entity load hooks » workspaces_entity_load() violates the hook interface (a.k.a. introduce hook_entity_load_alter() and hook_ENTITY_TYPE_load_alter())

Yup. I wasn't aware of that older issue.

Restoring the original title so it matches the issue summary.