inaccurate docs for hook_field_views_data()

Working on a solution for this issue.

Updated documentation.

@kellyimagined you are creating the patch in some wrong way. Try with this:

cd your-drupal-folder
git diff > hook-field-views-data-2730227-8.patch

@dagmar,
Thank you for the feedback, please let me know if this works.

@dagmar,
I must have a love for punishment. Maybe you can tell me why it is failing?

Updated according to docs and removed white space according to fail.

Good catch, and thanks for the patches!

However, the latest one still needs some work:

+++ b/core/modules/views/views.api.php
@@ -503,9 +503,8 @@ function hook_views_data_alter(array &$data) {
+ * The views invoke hook_views_data() in views_views_data(). The field module
+ * no longer implements this hook in D8.

The first sentence is not grammatical, and it doesn't make sense to me.

Also, we don't want to document history in the API docs. So remove the part about "no longer". Just document what is currently true.

Also, incidentally, please don't use the 'documentation' tag in Drupal Core. See https://www.drupal.org/node/1208166 .

Modified.

+++ b/core/modules/views/views.api.php
@@ -503,9 +503,7 @@ function hook_views_data_alter(array &$data) {
+ * Implements the hook_views_data().

No, this is not accurate either. This is not a hook implementation at all, it's documentation for a hook.

I think what we need to do is to replace this text:

The field module's implementation of hook_views_data() invokes this for each field storage, in the module that defines the field type. It is not invoked in other modules.

If no hook implementation exists, hook_views_data() falls back to views_field_default_views_data().

with something like this:

When collecting the views data, views_views_data() invokes this hook for each field storage definition, on the module that provides the field storage definition. If the return value is empty, the result of views_field_default_views_data() is used instead. Then the result is altered by invoking hook_field_views_data_alter() on all modules.

It also seems that hook_field_views_data_views_data_alter() is no longer invoked. And the documentation for hook_field_views_data_alter() needs a similar fix to this issue. I think we should combine all of that into one issue/patch?

@jhodgdon, is this what you are requesting?

Close, thanks!

A few things to address:

1. +++ b/core/modules/views/views.api.php
   @@ -503,12 +503,10 @@ function hook_views_data_alter(array &$data) {
   + * When collecting the views data, views_views_data() invokes this hook for each field
   + * storage definition, on the module that provides the field storage definition. If the return
   + * value is empty, the result of views_field_default_views_data() is used instead. Then the
   + * result is altered by invoking hook_field_views_data_alter() on all modules.
   

   This section needs to be rewrapped. We want all documentation lines to be less than 80 characters.

2. +++ b/core/modules/views/views.api.php
   @@ -587,9 +585,10 @@ function hook_field_views_data_alter(array &$data, \Drupal\field\FieldStorageCon
     * Alter the Views data on a per field basis.
     *
   - * The field module's implementation of hook_views_data_alter() invokes this for
   - * each field storage, in the module that defines the field type. It is not
   - * invoked in other modules.
   + * When collecting the views data, views_views_data() invokes this hook for each field
   + * storage definition, on the module that provides the field storage definition. If the return
   + * value is empty, the result of views_field_default_views_data() is used instead. Then the
   + * result is altered by invoking hook_field_views_data_alter() on all modules.
     *
     * Unlike hook_field_views_data_alter(), this operates on the whole of the views
     * data. This allows a field type to add data that concerns its fields in
   

   I think this is the docs header for hook_field_views_data_views_data_alter()... I had thought that this hook should be removed, but I was wrong about that. It does need an update though, but exactly not this update. What it needs to say instead is that it is invoked on the module that provides the field storage definition, and that it is invoked from views_views_data_alter().

3. Then we also need an update to the documentation on hook_field_views_data_alter(), similar to what is in this patch.

Hi jhodgdon,

Can you please elaborate second point of suggestion as it is little confusing ?

Sure, and sorry!

OK. Ignore comment #22. Here is hopefully some clearer information.

So we have 3 hooks:

a) hook_field_views_data() -- this hook documentation header needs the fix in the #21 patch (but all documentation needs to be wrapped to 80 character lines).

b) hook_field_views_data_alter() -- needs a fix similar to hook_field_views_data() -- but not exactly, because the text says "this hook" (referring to hook_field_views_data() in that text) -- so it needs a small edit so it is relevant to hook_field_views_data_alter() instead of hook_field_views_data(). Note that the patch in #21, by mistake, put the fix in the wrong this documentation header (see (c) below).

c) hook_field_views_data_views_data_alter() -- This hook documentation was mistakenly changed in the last patch. It does need a change, but the text that was put in there was for the wrong hook. What it needs instead is some text saying that it is that it is invoked from views_views_data_alter(), and only invoked on the module that provides the field storage definition.

Thanks! Hope this is clearer.

Given it a try!

Please review.

Thanks for trying, but this isn't quite right.

Looking at #25...
a) Looks good.

b) This wasn't done at all.

c)

+++ b/core/modules/views/views.api.php
@@ -587,9 +586,11 @@ function hook_field_views_data_alter(array &$data, \Drupal\field\FieldStorageCon
+ * storage definition. If the return value is empty, the result of
+ * views_field_default_views_data() is used instead. Then the result is altered
+ * by invoking hook_field_views_data_alter() on all modules.

This part about the return value doesn't apply to this hook.

@jhodgdon
Updated patch as per your suggestion.
Removed c point changes from patch (https://www.drupal.org/node/2730227#comment-11225311) .
Fix other comment.

Unassigned as @mohit_aghera has already uploaded a patch for review, and he can't review his own patch :) Also, I'm hoping that someone will be able to review this at the Drupal North 2016 coding sprint tomorrow.

Points a and b look alright, but we are still missing point c in it's entirety.

As suggested by jhodgdon in #25, made changes to docs of hook_field_views_data_views_data_alter().

+++ b/core/modules/views/views.api.php
@@ -590,6 +588,9 @@ function hook_field_views_data_alter(array &$data, \Drupal\field\FieldStorageCon
+ * ¶
+ * Views implmentation of hook_views_data_alter() invokes this on the module ¶
+ * that provides the field storage definition.

Some empty spaces seem to have crept in.

Removing empty spaces.

Fixed a typo.

@jhodgdon fixed for 8.6.x please review.

Hi,

- here is a reroll minus hook_field_views_data_views_data_alter() as suggested in #43
- the new issue about hook_field_views_data_views_data_alter() : https://www.drupal.org/project/drupal/issues/2999325

Thanks

Hi,
Thanks for the review
here is another try :)

Here it is :)

Hi,
thanks for the review :)
Here it is

Patch applies to 9.4.x. The text changes look fine to me but I'll ask in #bugsmash for a second opinion.

Gave it a detailed read and checked the logic to make sure it still matches what happens in views_views_data() and views_views_data_alter() and it does, so this looks good to me.

Re-uploading the patch so it tests with 9.4.x

Random fail.

Committed and pushed 9b9dc205ec to 10.0.x and be38b68647 to 9.4.x and 6e7d1e6e21 to 9.3.x. Thanks!