Add custom handling for Views [#117621]

We need to add a way for modules to opt out of the standard CCK views handling. I am working on the pollfield module (http://drupal.org/node/116567) which is one that needs to handle some of its own Views functions.

My thought is to do it this way:

1) Allow fields to provide their own field and query handlers in the same way they provide filters, in field_settings. If provided, those handlers will be used in place of the ones the content module provides.

2) Allow fields a way to opt completely out of the content module views_tables() and views_arguments() handling. I would do that using the custom callback method we're using for handling the node view and default values. If a field sets the callback to custom, the content module will skip those fields completely when creating the Views tables and/or arguments.

How does that sound? I can make a patch or maybe someone will have other ideas on how to do it.

Comment	File	Size	Author
#9	content_views.inc2_.patch	8.49 KB	yched
#4	content_views.inc__0.patch	9.21 KB	yched
#1	content_views.inc_.patch	9.98 KB	KarenS

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

KarenS CreditAttribution: KarenS commented 14 February 2007 at 17:25

Status:

Active

» Needs review

File	Size
content_views.inc_.patch	9.98 KB

Here's my patch. This creates two custom handing options 'views tables' and 'views arguments'. If a module adds a callback for custom handling of either one, that modules fields are skipped when creating the CCK tables and/or arguments.

Also, since sometimes all you really want is to add your own field handler to the one that the content module creates rather than create the whole thing from scratch, I also added in an optional 'field_name' parameter for those two arguments so you can call them from your field module to get the tables/arguments that the content module creates to use as a starting point, then make whatever changes are needed, as an alternative to doing them completely from scratch.

The patch looks messy because it indents nearly every line in the file, but there are really only a few lines that change.

Comment #2

yched CreditAttribution: yched commented 17 February 2007 at 18:37

I'm trying to grasp the implications and the code duplication this might mean (studying this with pollfield).
Wouldn't the 'views_tables_alter / views_arguments_alter' patch in http://drupal.org/node/105620 be a simpler solution ?
Of course, this means getting merlin into committing this (the patch nearly got in a few weaks ago, but it was broken)

My point is that content_views is a good place to provide interesting features (such as http://drupal.org/node/101050) or deal with complex situations (multiple values...). A module completely opting out means that it "has to" reimplement it's own from scratch.

Or maybe as an intermediate solution, we could call our own callback after we built the views table for the field, allowing the field module to alter the default stuff we built for him ?

Well, your pollfield and datefield modules obviously have the richest views features, so you definitely know better.

Comment #3

KarenS CreditAttribution: KarenS commented 19 February 2007 at 11:07

You can do that with this patch. You use the callback to opt out of the default handling and the content module will not build the tables in its views_tables hook. The field module then must create its own views_tables hook which can either build the tables completely from scratch or can start with the tables that the content module would have created and alter them as needed. The field module can get the content module to return the tables it would have created by calling content_views_tables(field_name). The field_name parameter is not used by Views, only if you call the function from a field module.

This way it is not dependent on getting any patches into Views, plus I think it is a failry simple solution. I am open to other ideas if you have something better.

Comment #4

yched CreditAttribution: yched commented 19 February 2007 at 14:03

File	Size
content_views.inc__0.patch	9.21 KB

You can do that with this patch.
Yes, actually I realized that after posting- sorry for having you explain that again.

I would simply propose a slight reformulation to make this work internally more like other field ops :
content_views_tables() iterates through all fields and calls a method to build the table for each field.
If callback setting == DEFAULT, default method content_views_field_tables($field) returns the table as it currently exists
If callback setting == CUSTOM, module_views_field_tables($field) gets called (and can internally call the default method to build upon the default tables)

Same for arguments.

This unifies rather well with other 'iterations' / 'callback switching' performed in several places (_content_field_invoke, _content_field_invoke_default, content_field_view...), which I'd like to unify at some point.

Attached patch does that - I have to say that I did not actually test it to check it works OK.

Comment #5

KarenS CreditAttribution: KarenS commented 19 February 2007 at 16:32

For the most part, this looks OK, except it's a little bit inconsistent in the way the field module handles things. For all our other callbacks, we have a hook operation that provides the callback value and another hook operation that executes the custom code, but here we're calling a function instead.

For instance, if we do custom a custom view, the content module calls date_field('view', $node, $field, $node_field, $teaser, $page). If we do a custom default value, the content module calls date_widget('default value', $node, $field, $items). We also get the views filters with date_field('filters', $node, $field, $node_field, $teaser, $page).

Based on that pattern, I would think we would now be calling date_field('views tables', $node, $field, $node_field, $teaser, $page) and date_field('views arguments', $node, $field, $node_field, $teaser, $page), but instead we call date_views_field_arguments() and date_view_field_tables().

I'm not sure which method is better, but I think we should be consistent one way or the other.

Also, interestingly, your method means the field module could also still use the regular views hook to create other tables since that hook hasn't been used, or the field module could use CUSTOM_CALLBACK_NONE and then use the regular views_tables() hook to generate its tables, so it seems pretty flexible.

Comment #6

yched CreditAttribution: yched commented 19 February 2007 at 21:28

Yes, I see what you mean. But hook_field is dedicated to field values inside a node - all ops operate in that context, and the arguments of hook_field ($node, $items, $teaser, $page...) reflect that and would make little sense for a 'view tables' op. Sticking a 'views table' op in hook_field would really feel weird IMO.

hook_field_settings seems more appropriate - 'filters' op is already there. But that does not really solve the inconsistency you point out : so far, the 'callbacks' settings only deal with hook_field / hook_widget ops.

I think the bottomline is that 'generic' hooks with $op arguments (a la hook_nodeapi) are a bit constraining... The idea of splitting them in hook_field_load, hook_field_validate, etc. has been mentioned before, I think. Well, not for now, obviously.

Meanwhile, I think we could go this way :
- make 'views tables' a hook_field_settings op.
- we do not query hook_field_settings('callbacks') for this op, since it is not a hook_field op
- content_views_tables() iterates all fields, calls their hook_field_settings('views tables'), and if no result (the field module does not implement the op), it calls its own default content_views_field_tables($field).

That is yet another way of handling default behaviour vs. custom behaviour, which does not make me happy either, but at least it's semi-consistent.

Comment #7

KarenS CreditAttribution: KarenS commented 19 February 2007 at 21:58

Of course, you're right, it should be hook_field_settings which puts it in the same place as the filters. That seems the most logical. Then, as you say, it is no longer a callback, but is at least consistent.

And, yes, we surely do need to clean all this up sometime, but since it will break all the contrib modules, that will have to wait for another day :-)

Comment #8

yched CreditAttribution: yched commented 24 February 2007 at 01:10

Attached patch uses hook_field_settings for views tables and arguments, as per #6 above.
I have named the ops 'tables' and 'arguments' (instead of 'views tables' and 'views arguments' in my previous patch) to stay consistent with the existing 'filters' op.

Some additional review is probably welcome before we commit.