API page: https://api.drupal.org/api/drupal/modules%21field%21field.attach.inc/gro...

Enter a descriptive title (above) relating to Field Storage API, then describe the problem you have found:

I don't think the first few functions belong in this topic. Eg, hook_field_display_alter(), hook_field_display_ENTITY_TYPE_alter() -- are these really to do with field storage? There are several that don' have 'storage' in the name, and don't look like they belong.

Comments

jhodgdon’s picture

Good catch -- they probably have the wrong @ingroup or are part of a @defgroup { } or @addtogroup { } that they shoudn't be.

That would probably be a good Novice project to look at the various field-related topics and move the functions around to the right groups. Start with
https://api.drupal.org/api/drupal/core!modules!field!field.module/group/...
and look at the various sub-topics, to see which functions really belong where.

Also... maybe not a novice project, but those Field array/object structures on the top-level topic are not relevant for Drupal 8 are they? That is probably a separate issue...

ThisIsDog’s picture

There is no alternative field storage for Drupal 8. As a result there will be no Field Storage API section for 8. I went ahead through the Drupal 8 Field documentation functions and they seem to be sorted properly.

This seems like it would be a Drupal 7 documentation issue. I also don't see the array/object structures mentioned by jhodgdon in the current documentation, so I'm going to update the issue to Drupal 7.

ThisIsDog’s picture

Version: 8.x-dev » 7.x-dev
Issue summary: View changes
jhodgdon’s picture

Issue tags: -Needs backport to D7

Agreed, this is now D7 only. Thanks for checking into this!

raulmuroc’s picture

Assigned: Unassigned » raulmuroc
raulmuroc’s picture

.

jhodgdon’s picture

Hi Raul, thanks for volunteering to take this on!

A few clarifications:

- hook_* functions should *not* be moved into .inc files. They should be left where they are. [hook_* functions are not really functions -- they are documentation with example function bodies -- and they need to be left in *.api.php files so that they are never loaded into Drupal.]

- To change the Topic/Group that a function is listed in, you need to make a documentation patch. The way that a function appears on a Topic page is one of the following:

a) In the function's documentation header, there could be a line saying

* @ingroup topic_id

where topic_id is the machine name of the topic/group.

b) It could be between a @defgroup topic_id @{ and a @defgroup @}.

c) It could be between a @addtogroup topic_id @{ and a @addtogroup @}.

So in all cases, to move a function between topic_a and topic_b, you would need to change the topic_id in the *.api.php file.

In this case, the "Field Storage API" topic has ID "field_storage", and the other topics these belong in have other IDs.

Read more about @defgroup, @ingroup, and @addtogroup at
https://drupal.org/node/1354#defgroup

Hope that is enough information for you...

filijonka’s picture

Status: Active » Needs review
StatusFileSize
new7.99 KB
jhodgdon’s picture

Status: Needs review » Needs work

Well, that will probably work... The patch is a bit hard to review though (hard to tell if these functions that are moving are unmodified)...

How about if instead of changing the order of the functions in the .api.php file, you instead just move up the "end of addtogroup" and "end of defgroup" statements instead, so that functions are not being added to the topics/groups that aren't supposed to be?

And then you can just add one line to each of the functions that needs to be part of a different topic, like:
@ingroup foo

That will make the patch a lot easier to understand, help with git blame in the future, etc.

filijonka’s picture

Hi

this was the approach I had with this

1. The functions weren't at the end of an @defgroup, so to just exclude (by moving enddef) that function wasn't a good solution, I would have had to changed several functions docheads.

2. The code should be easy to read and follow, if you wanna read about a defgroup functions you should be able to easily follow them in the file (incase you're not able to reach the api)

If I had choosed to do by 1 imo I would have to do so many other changes that isn't really necessary and also the readability of the file would have gone really bad. And I think it's more important of the readability in the file than in the patch but i'll take another look but anyone who feel for it is just to jump in.

filijonka’s picture

Assigned: raulmuroc » Unassigned
Status: Needs work » Needs review
StatusFileSize
new2.39 KB

must been an error that ingroup was used on field_attach so changed that to addtogroup

raulmuroc’s picture

Assigned: Unassigned » raulmuroc
Status: Needs review » Active
filijonka’s picture

Status: Active » Needs review

sorry RaulMuroc, didn't see the assignment first time so made a patch but we still need a review of this so changing back to needs review

jhodgdon’s picture

Status: Needs review » Needs work

Looks pretty good! A few things to fix:

a) Syntax:

 /**
+ * @addtogroup field_types
+ * @{
  * Alters the display settings of a field before it gets displayed.

The @addtogroup field_types ... @{ needs to be in its own /** */ docblock, not merged in with the docs for the next hook.

Same with

 /**
+ * @addtogroup field_storage
+ * @{
  * Alters the widget properties of a field instance on a given entity type

down below.

b) I applied the patch, and looked through field.api.php to see if I thought all the hooks were in the right "group". The topics/groups we have are listed on the main "Field API" topic (@defgroup field -- https://api.drupal.org/api/drupal/modules!field!field.module/group/field/7 -- machine names of the topics in parens):

(field_types) Field Types API. Defines field types, widget types, and display formatters. Field modules use this API to provide field types like Text and Node Reference along with the associated form elements and display formatters.

And this topic lists: field_widget and field_formatter as sub-topics.

(field_crud) Field CRUD API. Create, updates, and deletes fields, bundles (a.k.a. "content types"), and instances. Modules use this API, often in hook_install(), to create custom data structures.

(field_attach) Field Attach API. Connects entity types to the Field API. Field Attach API functions load, store, generate Form API structures, display, and perform a variety of other functions for field data connected to individual entities. Fieldable entity types like node and user use this API to make themselves fieldable.

(field_info) Field Info API. Exposes information about all fields, instances, widgets, and related information defined by or with the Field API.

(field_storage) Field Storage API. Provides a pluggable back-end storage system for actual field data. The default implementation, field_sql_storage.module, stores field data in the local SQL database.

(field_purge) Field API bulk data deletion. Cleans up after bulk deletion operations such as field_delete_field() and field_delete_instance().

(field_language) Field language API. Provides native multilingual support for the Field API.

The hooks in field.api.php are, after this patch:
hook_field_extra_fields (and alter) - no group - should be in field_types perhaps?
hook_field_info (and alter) - field_types [good]
hook_field_schema (and alter) - field_types [good]
hook_field_* where * == load, prepare_view, validate, presave, insert, update, update_field, delete, delete_revision, is empty - field_types [good]
hook_field_prepare_translation - field_types - should this also be in field_language? (could add @ingroup to docs header)
hook_field_widget_* - field_widget [good]
hook_field_formatter_* - field_formatter [good]
hook_field_attach_* - field_attach [good]
hook_field_language_alter() - field_attach - shouldn't this be in field_language, or in addition?
hook_field_available_languages_alter() - field_attach - shouldn't this be in field_language, or in addition?
hook_field_storage_* - field_storage [good]
hook_field_info_max_weight - field_info [good]
hook_field_display_alter - field_types [good]
hook_field_display_ENTITY_TYPE_alter - field_types [good]
hook_field_extra_fields_display_alter - field_crud - should be in field_types instead, I think? field_crud is about actually deleting field types or detaching them from entities, not about reading field data from the database or displaying it.
hook_field_widget_properties_ENTITY_TYPE_alter - field_storage - should be field_widget I think?
hook_field_create_field/instance - field_crud [good]
hook_field_update_forbid - field_crud [good]
hook_field_update_field/instance - field_crud [good]
hook_field_delete_field/instance - field_crud [good]
hook_field_read_field/instance - field_crud [good]
hook_field_purge_field/instance - field_crud [good]
hook_field_storage_purge_field, purge_field_instance, and purge - field_crud - needs to be in field_storage also
hook_field_access - no group - should be in field_types I think

So I think we need to adjust just a little more. Thanks!

And by the way, I checked Drupal 8 just now, and there is only one hook there that is a problem. So rather than delay this on that one hook, I filed a separate issue:
#2260035: hook_field_info_max_weight in field.api.php is not in a group/topic

filijonka’s picture

StatusFileSize
new5.64 KB
new6.71 KB

Incase of not sure I used to add an ingroup so e.g the language that the review was uncertain about

hmm we have no file doc header of this..

filijonka’s picture

Status: Needs work » Needs review

forgott to change status..

jhodgdon’s picture

Title: surplus functions in Field Storage API topic » Several field hooks are in the wrong topics/groups
Status: Needs review » Needs work

Whoops, sorry for not being clear!

 /**
  * @defgroup field_types Field Types API
  * @{
+*/
+
+/**
  * Define field types.
  *
  * In the Field API, each field has a type, which determines what kind of data

This was correct before the patch. When you *define* a group with @defgroup, the rest of that doc block is the definition of the group/topic (summary line and paragraphs of docs). So that should be all in the same docblock.

It's when you are *adding* to a group with @addtogroup that you want to have two separate doc blocks. Sorry for the confusion! So that needs to be fixed in several places in the latest patch.

So... One more fix, and I think this is ready to go -- thanks!

filijonka’s picture

ok np. perhaps update the doc about that so it's clearer #1354: [Obsolete] API documentation and comment standards?

raulmuroc’s picture

Assigned: raulmuroc » Unassigned
filijonka’s picture

Status: Needs work » Needs review
StatusFileSize
new1.24 KB
new6.25 KB
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

RE #18 - https://drupal.org/node/1354#defgroup - it seems to show, in the examples, how to define a @defgroup, and how to use @addtogroup?

RE #20 - Looks excellent! Thanks for all the iterations!

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Thanks again! Committed to 7.x.

  • Commit 076802c on 7.x by jhodgdon:
    Issue #2048395 by filijonka: Fix up group/topic membership for field...

Status: Fixed » Closed (fixed)

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