Problem/Motivation

Complete the documentation of every implementation of hook_ENTITY_TYPE_*.

A good command to find all the occurrences is git grep hook_ENTITY_TYPE_.

Proposed resolution

Change the document comment to Implements hook_ENTITY_TYPE_prepare_form() for ENTITY_TYPE entities. replacing ENTITY_TYPE with the entity type that the hook is implementing. You should find it in the function name. You can find a good example in #2331783: hook_ENTITY_TYPE_prepare_form() is not documented

CommentFileSizeAuthor
#17 2472669-17.patch4.85 KBrpayanm
#11 interdiff-2472669-9-11.txt722 byteskeopx
#11 2472669-11.patch5.55 KBkeopx
#9 2472669-9.patch5.55 KBkeopx
#9 interdiff-2472669-7-9.txt704 byteskeopx
#7 interdiff-2472669-2-7.txt4.78 KBkeopx
#7 2472669-7.patch5.53 KBkeopx
#2 2472669-2.patch5.68 KBkeopx

Comments

rteijeiro’s picture

rteijeiro commented
Issue summary: View changes
keopx’s picture

keopx
Primary language Basque
Location Basque Country
commented
Status: Active » Needs review
StatusFileSize
new 2472669-2.patch5.68 KB
rteijeiro’s picture

rteijeiro commented
Category: Bug report » Task
rteijeiro’s picture

rteijeiro commented
Status: Needs review » Reviewed & tested by the community

It's good for me. Nice job @keopx!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Close, but not quite!

Several of these are wrong. Here are a couple of examples:

  1. +++ b/core/modules/config/tests/config_test/config_test.module
@@ -29,7 +29,7 @@ function config_test_cache_flush() {
- * Implements hook_ENTITY_TYPE_create().
+ * Implements hook_ENTITY_TYPE_create() for 'test_config_test'.
  */

    The module is called config_test, the entity is config_test, not test_config_test. 

  2. +++ b/core/modules/system/tests/modules/entity_test/entity_test.module
@@ -324,7 +324,7 @@ function entity_test_mulrev_load($id, $reset = FALSE) {
- * Implements hook_ENTITY_TYPE_insert().
+ * Implements hook_ENTITY_TYPE_insert() for 'test_entity_test'.
  */
 function entity_test_entity_test_insert($entity) {
   if ($entity->name->value == 'fail_insert') {

    Same here. Module is entity_test, entity is also entity_test.

Many more also need updates.

keopx’s picture

keopx
Primary language Basque
Location Basque Country
commented
Assigned: Unassigned » keopx
keopx’s picture

keopx
Primary language Basque
Location Basque Country
commented
Assigned: keopx » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2472669-7.patch5.53 KB
new interdiff-2472669-2-7.txt4.78 KB

Correct errors.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work

Most are right now, thanks! One more to fix:

+++ b/core/modules/config/tests/config_entity_static_cache_test/config_entity_static_cache_test.module
@@ -8,7 +8,7 @@
- * Implements hook_ENTITY_TYPE_load().
+ * Implements hook_ENTITY_TYPE_load() for 'config_test'.
  */
 function config_entity_static_cache_test_config_test_load($entities) {
   static $random;

Not quite!

So this is a function in the config_entity module.

It implements the entity loading hook for the static_cache_test_config_test entity.

So this should say:

Implements hook_ENTITY_TYPE_load() for 'static_cache_test_config_test_entity'.

keopx’s picture

keopx
Primary language Basque
Location Basque Country
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2472669-7-9.txt704 bytes
new 2472669-9.patch5.55 KB

Oh sorry, you're right

Kudos for @jhodgdon and @rteijeiro

rteijeiro’s picture

rteijeiro commented
Status: Needs review » Needs work 
+++ b/core/modules/config/tests/config_entity_static_cache_test/config_entity_static_cache_test.module
@@ -8,7 +8,7 @@
+ * Implements hook_ENTITY_TYPE_load() for 'static_cache_test_config_test_'.

Please, remove the trailing underscore. The correct comment is:

Implements hook_ENTITY_TYPE_load() for 'static_cache_test_config_test'.

keopx’s picture

keopx
Primary language Basque
Location Basque Country
commented
Status: Needs work » Needs review
StatusFileSize
new 2472669-11.patch5.55 KB
new interdiff-2472669-9-11.txt722 bytes
rteijeiro’s picture

rteijeiro commented
Status: Needs review » Reviewed & tested by the community

Looks good now! Good job @keopx!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Agreed, looks good now!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2472669-11.patch, failed testing.

rpayanm queued 11: 2472669-11.patch for re-testing.

The last submitted patch, 11: 2472669-11.patch, failed testing.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
commented
Status: Needs work » Needs review
StatusFileSize
new 2472669-17.patch4.85 KB

Rerolled.

keopx’s picture

keopx
Primary language Basque
Location Basque Country
commented
Status: Needs review » Needs work 
+++ b/core/modules/config/tests/config_entity_static_cache_test/config_entity_static_cache_test.module
@@ -8,7 +8,7 @@
+ * Implements hook_ENTITY_TYPE_load() for 'static_cache_test_config_test'.

You lost this change

keopx queued 11: 2472669-11.patch for re-testing.

keopx’s picture

keopx
Primary language Basque
Location Basque Country
commented
Status: Needs work » Reviewed & tested by the community

I guess its RTBC again

But use #2472669-11: Complete documentation for every implementation of hook_ENTITY_TYPE_* patch.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Getting this one in while it's hot!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 257abe1 on 8.0.x 
    Issue #2472669 by keopx, rpayanm, rteijeiro, jhodgdon: Complete...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.