Attached patch adds the following drupal_alter hooks.
It's a D7 equivalent of quicksketch's #417122: Allow drupal_alter() on Field and Widget Settings for CCK D6.

- hook_fieldable_info_alter()
example: if #460320: Standarized, pluggable entity loading (nodes, users, taxonomy, files, comments) gets in, node-level cache is doable in contrib. The module will then need to change the 'cacheable' property of node fieldable_infos

- hook_field_info_alter()
example: #501404: Field Permissions in Core mentions "selectively enable access control for a field to avoid UI bloat on the access page". That would require field_permissions module to add an 'access_control' setting to all field types.
or : change the default widget, default formatter...

- hook_field_widget_info_alter(), hook_field_formatter_info_alter()
examples:
1) let a 3rd party module 'enrich' a widget or formatter by adding new settings
That's the original reason for quicksketch's similar patch for CCK D6 in #417122: Allow drupal_alter() on Field and Widget Settings : allow fieldfield widget enhancers (JS crop, browse existing files...) to cohabitate.
Also, from #394720-16: Migrate profile module data to field API: "a generic link builder for formatters, similar to the one added in Views 2.2 or 2.3, would be super useful (link to the object, link to a custom path with a pattern...). But we don't want each formatter (for text fields, for number fields, for image fields...) to reimplement this".
2) let a contrib field type reuse an existing widget (e.g optionwidgets)

- hook_field_schema_alter()
No actual use case I can think of right now, but I see no reason not to add this...

This will also require additional alter hooks in the UI to allow form elements for the alter-added settings in the field config screens.

CommentFileSizeAuthor
#12 field_alter-502522-9.patch4.13 KBwebchick
#9 field_alter-502522-9.patch4.13 KByched
#7 field_alter-502522-6.patch4.19 KByched
#6 field_alter-502522-5.patch4.19 KByched
field_alter.patch4.55 KByched
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

yched CreditAttribution: yched commented
Status: Active » Needs review

er, s/fieldfield/filefield in the OP...

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

Nice explanation. Nice docs. RTBC.

bjaspan’s picture

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

I do not think we should provide hook_field_schema_alter() because and for the same reason there is no hook_schema_alter(). Too dangerous, not clear what the use cases are.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Barry, are you sure? http://api.drupal.org/api/function/hook_schema_alter

yched’s picture

yched CreditAttribution: yched commented

Hm, Barry is probably right.

Quicksketch's patch for CCK D6 has a hook to alter field columns.
- I'm not sure he has actual use cases for it, or if was just for consistency. I'll ask over there.
- CCK D6 had code for on-the-fly db migration, which we chose not to support in D7 (no field_update()). So alter field columns is possibly OK in D6, but more problematic in D7 (would only affect newly created fields, not updating existing ones)

Meanwhile, it's probably best to remove hook_field_schema_alter.
I'll reroll shortly.

yched’s picture

yched CreditAttribution: yched commented
FileSize
field_alter-502522-5.patch4.19 KB

Without hook_field_schema_alter(), and with actual sample code for the hooks (illustrating the examples from the OP)

yched’s picture

yched CreditAttribution: yched commented
FileSize
field_alter-502522-6.patch4.19 KB

Fixed capitalization typo.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

These examples look great. I agree with leaving _schema_alter() to another patch (or out completely).

We don't normally prefix the inline comments on example hook implementations with example though - I think we should just drop that and make them look like regular inline comments. Otherwise RTBC.

yched’s picture

yched CreditAttribution: yched commented
FileSize
field_alter-502522-9.patch4.13 KB

Fair enough.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs review » Reviewed & tested by the community

Works for me. We're going to have actual implementations of these hooks in upcoming patches so don't think we need special tests here. RTBC as long as the test bot doesn't knock it back.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

This seems reasonable to me. Nice to finally have some Field API hook docs with examples. :)

There are a couple of minor things with the docs, such as "informations" and the summary should be in third-person, but I can clean those up prior to commit. However, I'm currently on a "commit freeze" until some folks on IRC do their magic. Marking for later.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
FileSize
field_alter-502522-9.patch4.13 KB

Grrr testing bot. Uploading patch again.

webchick’s picture

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

There we go! Committed to HEAD!

mattyoung’s picture

mattyoung CreditAttribution: mattyoung commented

No

HOOK_widget_settings_alter()

and

HOOK_field_settings_alter()

for D7?

I don't know CCK in D7 so I'm just wondering about this. Because these are in quicksketch's #417122: Allow drupal_alter() on Field and Widget Settings patch and I'm using them now.

yched’s picture

yched CreditAttribution: yched commented

@mattyoung: hook_field_settings() are hook_widget_settings() gone in D7, the list of settings and default values is provided in hook_field_info() and hook_field_widget_info(), and thus can be altered through the alter hooks that were committed.

Status: Fixed » Closed (fixed)

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

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented

@yched

In regards to your comment "CCK D6 had code for on-the-fly db migration, which we chose not to support in D7" is this still the case?

This is going to cause a lot of redundant data on fields that require a dynamic schema. I think that I have about 5 modules loosely based on this behavior, the name module is the only one in contrib, but was planning to move imagefield extended in this direction too.

The name module has between 2 - 5 columns depending on the user selected combo, which could lead up to, well a lot of combos (see below - some combos would never happen 'middle name' & 'generational suffix', but you get the point). Two clients have wanted all 5 columns, and both latter dropped down to 3 & 2 respectively. After trying dynamic columns an early dev version, this killed the Drupal installation with multiple one off errors that appeared to be impossible to fix and not reported anywhere. So I guess this would force the best solution to be varchar(255) on all 5 and just put up with NULL's on dynamic fields? Or force the user to be restricted to just a subset of the potential combo. Both are not optimal solutions.

If so, what was the argument for this? Surely it can not be data loss as if that was the case, you should never be able to have uninstall scripts. Both actions change the underlying schema with the loss of data, one is just a more extreme version of the first!

anyway, the combos, I think this would contain about 8-10 desirable combos, with 13-15 unwanted ones.

ab
ac
ad
ae
bc
bd
be
cd
ce
de
abc
abd
abe
acd
ace
bcd
bce
cde
abcd
abde
acde
abcde

Sorry for pinging a closed thread, but this has been the first time that I have seen that this issue on the forums for what I had considered to be a bug, being suggested that it is "by design".

Cheers

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

Yes, you would typically create 5 columns with NULL default.

None of the CCK maintainers were comfortable with on the fly data migration that we do in D6. We had numerous reports of fatal data loss.

Note that your module can completely take over field storage if you want so I'm pretty sure you can still do what you want if you really have to.

Alan D.’s picture

Alan D. CreditAttribution: Alan D. commented

I stopped trying to port to D7 months ago because of this, thinking that the issues were my lack of field knowledge / core bugs. At least now I can plan ahead

I think that NULL columns would be the best bet. KISS is always the best principle :)

Thanks Moshe.