Implement views_handler_field_multiple

Patch?

subscribing

Applied patch and it works but I'm now receiving multiple errors:

Notice: Undefined index: default_formatter in views_handler_field_field->option_definition() (line 62 of /Users/sal/Sites/herrang/sites/all/modules/views/modules/field/views_handler_field_field.inc).

subscribing

subscribing

Subscribing. On a side note, it would be nice if you could just click 'watch post' without having to post subscribe.

Subscribing... and telling ryan.armstrong to donate to the effort to kill subscribe comments.

Thanks for the link, I'll chip in! Seems crazy this hasn't been addressed, especially with the redesign.

With the patch applied as above, I'm not getting fields displayed correctly in my views: some fields are returning empty results.

The culprit appears to be:

class views_handler_field_field extends views_handler_field {
  /**
   * Stores the fieldapi field definition.
   */
  var $field;

  function init(&$view, &$options) {
    parent::init($view, $options);
    $this->field = field_info_field($this->definition['field_name']);
  }

in views_handler_field_field.inc. If I change the $field = $this->field; back to $field = field_info_field($this->definition['field_name']); everything works as expected.

Shouldn't this be fieldapi data? That's why I couldn't find it when searching for issues around fieldapi.

I just posted a bug report which I'm told was a duplicate of this bug. Its disconcerting that something that must surely be a common problem problem has not seen any resolution since August?

I'm encountering this problem on file upload files with an arbitrary number of uploads per field, rendering views with fields useless at the moment. Anyone with an image field that accepts unlimited images will be encountering the same problem. Surely these common use scenarios justify this be promoted to a high priority?

Its disconcerting that something that must surely be a common problem problem has not seen any resolution since August?

You'll note that Views has only a -dev development release. We haven't even been comfortable with it enough to release an alpha. Changes in D7, particularly in field API, have been very time consuming to deal with and the Drupal 7 port of Views has been entirely done by volunteers contributing their time to the project. I, as Views' maintainer, have given almost no time to this port as I have many other responsibilities as well.

If it is a problem for you that this feature has not been implemented, then I suggest you roll up your sleeves and dig into the code and offer to help, which is the open source tradition.

I remember dereine asking me if efq_views had this multiple handler. It doesn't since (because of functioning at the entity level) it works like this by default. In fact, there's no way to get the current views behaviour (one row for each field value).

Anyway, gave this patch a shot.

views_handler_field_field changes need to be reverted (that part dropped from the patch).
This is because init() is called after option_definition(), so $this-field is empty.

Also, this issue depends on #936196: Don't use entity_load to display fields (which got unblocked today), so we need to get that one in first.

This may be what bojanz is saying, but the entity_load() method currently used makes multiple values handling easy (we already have a single row and access to all the values of the field). When the core handling goes back to the old method of retrieving a value for each table row, the method for managing multiple values should involve doing what the field handler currently does -- retrieve only the etid, do an entity_load() and then render the desired values.

No need to touch entity_load at all.

Views currently already does the joins that produce multiple rows if the field has multiple values. That's before the remove_entity_load.patch even lands.
What we need is a CCK-esque multiple handler (dereine's words), nothing more.

Having the 'multiple' handler go through entity_load() (and thus through hook_field_load()) while the 'regular' handler chews raw db data and bypasses hook_field_load() would probably be confusing.

subscribe

how is development going on this? is there a patch or anything I can test?

Subscribe, I would be happy to test a patch when a new one is available.

Work on this issue can finally continue, since we fixed a bunch of field api integration issues today, and brought back entity_load().

What's interesting is that by default the new code doesn't join the field tables, so it behaves just like with views_handler_field_multiple. However, as soon as you add a filter/sort/grouping on the field, the join is made, and duplicates are once again present.

For them we'll figure out a way to add the join (ensure_my_table()) explicitly...

subscribing

subscribe

Where is the patch? :)) good work :))

Not there yet. I did some work on it, so I will update the issue later today.

I got distracted many times today....

Okay, so this is very raw. This means the code is ugly at places, lacks comments, and may have bugs.
Still, it shows the approach we want to take.

1) Remove all multiple-field-specific code from views_handler_field_field
2) Make all multiple fields use views_handler_field_field_multiple
3) Do our magic in views_handler_field_field_multiple

Note that the "Show X items from item Y" is now usable regardless of whether row grouping is enabled.
This is intentional, since we need to have the option to limit by delta (and we can't do that through a filter anymore).
The "reversed" functionality is "row grouping" specific, since I can't do that through SQL.

For some reason I killed the fieldset around the "Show X items from item Y reversed?" options, which makes it a bit ugly. I should probably put it back.

If/when #975400: Refactor field render() functions to accept $value, allowing reuse lands, we no longer need to override the whole render() function, just get_value()...
Of course, this is assuming we want both handlers to have the same rendering logic (wrapping html? yes/no?)

Edit: Also, there's a problem of changing field cardinalty (a field gets added with the ordinary handler, then becomes a multiple, or a multiple field stops being multiple). The previous patch had code which checked if the field is still multiple, and if it's not, fell back on the old code. That should probably return to a new patch.

Wow..it's good..the group value it's ok..
the problem is with paginator.. if insert a value to show with the pager,the pager mini,full not show..O_o

same error with distinct feature :))

subscribing...

@bojanz : "there's a problem of changing field cardinalty"
Yes, that's why in CCK D6 the 'multiple' handler acted as a proxy, deferring to the 'single' handler if applicable at run time. I can't say I was in love with the approach. Convoluted.
This being said, I can't remember right now why simply clearing cached 'views data' on field_update() couldn't do the trick...

subscribing

uhm...

Here's a new version of the patch.
I've decided to forget about having a separate handler, and do it all in the main field handler (views_handler_field_field).
That's the only way we can gracefully adapt to cardinality changes.

The code should be a bit less ugly now, so give it a look.

@morxe
This patch doesn't touch the pager, so your issue might just be unrelated (I haven't managed to reproduce it, but will try once again when this patch is done).

Sorry bojanz... i have tested the new patch and i have this problem...

My scenario is 1 content type with 1 images field with multiple value....

The problem is if i want to show only 1 pic for page..working perfect with views 6.xx... in this scenario the pager not show at all...

Actually i am confused and i not say if the problem is the "show #" the "pager" or the "distinct"...

Sorry for my bad english,and language scenario explanation.. i not have the conoscence to help us with hard coding,.. my limit is small testing only :(

Any update?....I am totally ready to go online with my projects,but it's impossible without this option.. :(

Today i have download the new dev version and patched.. i have been created a view block..activated and assigned a region for this view block and i receive this error....

Undefined variable: offset in views_handler_field_field->query() (linea 131 di .......\sites\all\modules\views\modules\field\views_handler_field_field.inc)

The scenario is the explicated before..

This patch should work without generating that notice (offset wasn't declared, silly omission).
I won't have any time to push this further until the weekend.

sub

subscribing. Views slideshows needs this for the multiple images per field scenario, as mentioned here: http://drupal.org/node/1026416

Rewrote the insane inline logic.
Added more comments.
Added a function that tells us if the field table should be joined in (dereine's idea from a while back) -> it's name could be different, didn't have any inspiration.

I always miss something stupid.

Now with less trailing whitespace.

Edit: Testing shows that the Reversed checkbox value doesn't stay checked when ctools dependent is used.

Now the reversed option and value start its'ok.. only problem is the 1 value per page in field image multiple with pager...

Totally blocked in my project for this problem :(

morxe, I can't reproduce your problem.

I specify "Show 1 value(s)", tell a pager to paginate, and it works. You'll need to share more info about how this is occurring.
Maybe you could try to reproduce it outside of this issue, you might be running into some unrelated bug.

i have only pathauto,token,views dev and ctools in a new and clean installation of drupal...

I have 1 fieldimage multiple... i want to show the images 1 value for pages.. if i insert "show 1 value" (without group multiple values) the pager not show...if i try to insert "show 2 values" the pager appear and this is an error because the items per page is 1...this is my problem.. for me the rest of function it's ok... the reversed,and start from working very good

Actually, that's how it should work.

1) Show 1 value, without grouping rows
In this case, a WHERE clause gets added to the query, specifying only one image, so you get one row with one image.

2) Show 2 values, without grouping rows
Since grouping rows is off, a join is added to the field table, and since you are showing two images, you get two rows, hence the pager.

If you don't want duplicates, enable row grouping.

One thing the current patch can't do is: if you have a field with three images, show three rows, each with 1 different picture.
As far as I know this wasn't possible in 6.x as well, no?

Actually..if i want to show only 1 image per page this is impossible... if i insert 3 in "show" and i insert in pager option 1 item,the image show in 1 page is 3.. the error is the pager because the pager appear and if i try to navigate to page 2,the pager not go to next page because the images to show it's ended.... (the error is the pager show)

If i want to show only 1 image per page with pager correctly, i have to use d6....in d6 this is possible,i have been used this function in five project to construct beautifull gallery without any other modules... .

I have a huge problem with understanding what you're saying. Could you create a screencast (or at least a ton of screenshots) detailing your problem?

Other people following this issue, can you give the patch a test and tell me if this solves your problems and behaves like it should?
Thanks.

Ok.. this is the screenshot step.. Same settings different result...

Now I understand. Finally! Thank you.

morxe's use case is the one I mentioned in #52:

One thing the current patch can't do is: if you have a field with three images, show three rows, each with 1 different picture.

Since this was possible in 6.x, we need it here too.

So there are two remaining issues:
1) Figure out what to do with the Reversed checkbox conflicting with ctools dependency (even when #default_value is TRUE, it's rendered as unchecked. If I remove the #process call, it's good)
2) Figure out how to support morxe's use case (how does 6.x do it?)
When "group rows" is disabled, then the offset needs to be incremented somehow.
Since we actually need duplicate rows here, we can't add the LIMIT to the query.

When "group rows" is enabled, it should act like it does currently.

Not setting to "needs work" since this actually still needs review, even though we are not there yet.

Ahhh ok... :))) ...thanks bojanz..i attempt the feature... sorry for my english explanation... :))

good work...

1) Right! I remember that from before, not well enough obviously :)

2) If I have a field with three images, and the join gets added, then I get three rows, each with all three images. If I set the delta to 0, it will show the first image for all three rows. How to tell it to show the first, then second, then third? I don't think it's going to do itself :P

Here's an updated patch.

Changes:
1) Fixed the Reversed checkbox as dereine suggested in #58
2) Removed the field limit from the query completely. This was only done when row grouping was off. We now realize that we can't limit in that case, since we want duplicated rows, so that we can support the missing use case (one row per field value, different field value for each row).
This change has a downside: specifying "add fields to query" as TRUE in the definition will now never limit the values, but return all of them instead. I don't see any way to avoid this (maybe limit JUST when "add fields to query" is TRUE?).

What's left to be done is calculating the right offset (per entity id) for each multiple field with row grouping off.
Any smart&clean ideas? :)
Extending the entity object feels more unclean than just having a static array of counters.

This change has a downside: specifying "add fields to query" as TRUE in the definition will now never limit the values, but return all of them instead. I don't see any way to avoid this (maybe limit JUST when "add fields to query" is TRUE?).

Here's how that would work.
Done for tonight.

hi bojanz, i have the same truble with the multiple field of images of d7. The problem is i can't install you patch 895046_6, i use the IDE Netbeans6.5.

With last patch all right in the middle in my case (2) :))..

the pager is correct, it displays the correct number of fields .. only a problem if i try to go to page 2, the pager disappears, no image is displayed and the view disappears :))

bojanz, i installed the patch and found those questions
1. filter->fields->image-delta dispappered
2. when i use the slideshow view, the view can't slideshow

one thing more, for now, if any field use multiple values, this field cannot be printed without all the wrappers using new semantic options.

Final patch? dereine & me couldn't figure out how to make it prettier.
Test, people!

@kervi
I know, but that's a separate issue, won't be handled here.

Actually, that was wrong.

Hi bojanz.. now the pager it'ok for my case... only 1 problem.. only first field is displayed in all pages...

I can't reproduce. Try clearing your cache, and seeing if you applied the right patch.

You are now seeing what you should have seen with patch #61, not with patch #67.

Ok,i have been repatched with #67, cleared all cache and created new view.. this is the screenshot step with error..

Post the generated query, it might help.

SELECT node.nid AS nid, field_data_field_post_imgs.delta AS field_data_field_post_imgs_delta, field_data_field_post_imgs.language AS field_data_field_post_imgs_language, field_data_field_post_imgs.bundle AS field_data_field_post_imgs_bundle, field_data_field_post_imgs.field_post_imgs_fid AS field_data_field_post_imgs_field_post_imgs_fid, field_data_field_post_imgs.field_post_imgs_alt AS field_data_field_post_imgs_field_post_imgs_alt, field_data_field_post_imgs.field_post_imgs_title AS field_data_field_post_imgs_field_post_imgs_title, 'node' AS field_data_field_post_imgs_node_entity_type FROM {node} node LEFT JOIN {field_data_field_post_imgs} field_data_field_post_imgs ON node.nid = field_data_field_post_imgs.entity_id AND (field_data_field_post_imgs.entity_type = :views_join_condition_0 AND field_data_field_post_imgs.deleted = :views_join_condition_1) WHERE (( (field_data_field_post_imgs.language = 'it') )) LIMIT 1 OFFSET 0

EDIT: I now realize what's happening.
The pager completely removes any advantage of drupal_static(), since a new page starts from scratch.

The right answer is using the delta! Cleaner, too.
Not sure if I'm missing something, it's really late.

Even better. Why did I even overcomplicate it in the first place?
Good night.

EDIT: The one above is probably a safer bet, ignore this one.

Ok, I have tried the two patch #74 #75 .. the result is the same ... 1 field
per page (very well) .. The problem is the pager .. on click to next page,
the pager not redirect,load and still on page 1..

:))

sub

@morxe
I don't know what could cause that.

I'm leaving you with my last patch. Don't have time for more. Added some comments and cleanups.
Works in my limited testing (tested the things which were a problem before in this issue). Anyway, it's as far as I got.

after apply this patch:

Notice: Undefined index: delta_limit in views_handler_field_field->init() (line 47 of /home/u3220/domains/**********/sites/all/modules/views/modules/field/views_handler_field_field.inc).

sub

sub

sub

The attached patch:
- corrects the display of the selected number of values by using an associative array of options for 'delta_limit'
- changes the caption of the 'Group multiple values' checkbox to 'Display a single row per entity'
- amplifies the description of this checkbox
- changes the caption of the 'Show n values' select to 'Display n values'

Stepping back, it seems we could use a roadmap for what this functionality is intended to provide. There are references above to what CCK did in D6. However, I have not seen a summary in one place. In particular, grouping is not compatible with multi-value fields. The resulting group by query will usually include n1 x n2 x ... rows, where nk = number of values for field k. This is due to SQL doing a grouping on the values in each row (not the entity id) which could result in grouping across disparate delta values but still returning multiple rows per entity.

In general, returning multiple rows returned per entity seems a bit odd, although the "image gallery" use case mentioned above by @morxe seems plausible.

Also, the combination of settings can produces unwanted results. For example, my test data consisted of:
- nodes with two fields: field A with up to 2 values, field B with up to 4 values
- actual data has 2 field A values on each node and 2,3, or 4 values in field B

With settings of:
- Field A: 'display one row per entity' is checked; 'display 2 values'
- Field B: 'display one row per entity' is unchecked for a 'display 3 values'

The query returns 2-4 rows per entity (as this is the number of field B values). The display shows the two values for field A in each row. For field B, the first entity has 4 values; 3 values are displayed on the first row for this entity, with the 4th value on the second row; rows 3-4 for this entity have no value for field B.

This result points out that empty display rows will result if 'display one row per entity' is unchecked and 'display n values' has k>n>1 for a multi-value field with k values. Is this something to warn the user of, or do we simply let them figure it out for themselves? Or, do we disable the 'display n values' field and set n=1 if the box is not checked?

Nice!

The most important change being:

- corrects the display of the selected number of values by using an associative array of options for 'delta_limit'

I agree that "Group multiple values" is not very clear, it's copied from the D6 CCK version.
However, I'm not sure we want to mention the word "entity".
Views currently knows nothing about the entity concept, and I'm not sure if the word itself is mentioned anywhere.
In any case, the new names are better then the previous ones.

+      '#description' => t('If checked, then one row per entity will be used to display the number of values selected below. If unchecked, then a display row will be created for each value in this multiple value field with a single value displayed in each row. However, the number of distinct values displayed from this field will equal the number of values selected below (which, if less than the number of field values, may appear to cause duplicates). This setting is not compatible with click-sorting in table displays nor grouping.'),

Not sure if the new description for the "group" checkbox is better, seems a bit too verbose.
Perhaps something like:

+      '#description' => t('Check in order to show all field values in the same row. If unchecked, each value in the field will create a new row, which may appear to cause duplicates. This setting is not compatible with click-sorting in table displays nor grouping.'),

I think what we need to do is just keep the D6 CCK behavior.
By now it's understood that this setting has it's limitations, depending on whether the join itself is added.

Let me sum up my understanding:
1) If the "group" checkbox is not enabled, the join is explicitly added, giving us as many rows as there are values. Setting "Show X values" means that it should show X different values for each row, depending on the delta. So if I selected "Show 1 values", then each row gets a different image from a multiple value field, that's @morxe's use case.
2) If the "group" checkbox is enabled, the join is not added, hence no duplicate rows, only one row per entity. We take the number of deltas to display from settings, chop away at the entity, and then display it.
However, even if "group" is enabled, a join might be added which messes up things (through a filter, sort, clicksort, grouping, or even another image field which creates it's own duplicates).

Just an FYI. I am making extensive changes to this patch for various reasons, but one is to make it more match prerender_list

Great, looking forward to seeing the new version :)

This kills a couple of birds and stones. We get to format multiple fields the way Views users expect. We get to get rid of all that extraneous entity api markup unless we specifically say we want it.

with last patch, instead of display images, it display some numbers...

Right, drupal_render() still confuses me sometimes. This one is better I think

Ok, solotandem points out that when you have a field that can be multiple, if you uncheck the group multiple (and yes, better terminology would be good, though I did actually remove a lot of the text there. It must be short text there), if you don't set the delta field to 1 you get unexpected behavior.

When that box is not checked, I am not certain why anybody would ever use a value other than 1.

I had initially thought that the reason those were not hidden when that box is unchecked is that they would modify the query, but it doesn't seem to work that way, and I actually can't see how it would. So probably I think we should go ahead and hide those fields and not use them when not grouping.

bojanz: I'm not entirely sure what you were trying to accomplish with that field, though. I could be missing something?

Ok, this version of the patch makes the delta stuff work as I believe bojanz intended. When NOT grouping values together on a row, it limits the deltas in the query. It also allows for IS NULL; that IS NULL actually possibly should be optional. Now that I think on it, I'm less sure about the IS NULL bit.

What I saw when testing this is that when using delta limiting, I wasn't getting fields that had no values, like I do with no delta limiting. The thing is, I can see users wanting it both ways. Maybe it should be a checkbox?

Still TODO on this patch:

1) Wording change on 'group multiple values'. Looking for suggestions.
2) Change reference to entity api markup to field api markup. Specifically reference field.tpl.php

Anything I"m missing?

Thanks merlinofchaos for all your work.

For the "Group Multiple Values" option, I'm thinking this might make more sense:

[ ] Create new row for each value in field.
Leave unchecked to group all values as one field row.

thanks again!

I tried that originally, but the checkbox controls the visibility of other items, and as such the UI really sucks if it's reversed.

Maybe "Display all values in the same row", with the description something like this: "If checked, multiple values for this field will be shown in the same row. If not checked, each value in this field will create a new row."

It's a variation of solotandem's version.

With this patch is not possible show only 1 item per page with pager... the pager not show at all if i select "show 1 value"... grrrrrr

EDIT: With ajax on i receive this error:

Ops.. Sorry.. error for the comment #97

subscribe

#97: I can't understand what you mean. I've tested this on my system with the pager, and it's working. Your sentence isn't very coherent.

I've gone ahead and cleaned this up a little based upon comments, and committed. Actual patch committed in case we want to do refinements is attached.

Attached is a small follow-up patch that also renames the entity api to field api in the setting name, not just in the user facing text.
Also, changed one comment which was backwards.

I've tested the patch, and it does good in two cases:
1) Grouping values
2) Acting as a delta filter
But it fails in morxe's use case, where he expects the D6 behavior:
If you don't select "display all values in single row", and don't specify a number for the "show x values" setting, it then shows a row for each image, but a different image in each row.

I have no idea how to squeeze that in as well (an extra checkbox?). We are trying to get too many things from too few UI elements.

This patch restores the way it worked in D6.

1) Delta limiting is once again done with a filter. I said this couldn't be done 20 days ago, but I'm a new man now.
2) The "Show X" textboxes are once again tied to the main checkbox ("Display all values in a single row").

So, this should work in all use cases. The changes are pretty simple, now that it builds off of the committed patch.

That patch works brilliantly for me!

i'm sorry, but after download last dev version (with latest patch from merlinofchaos) and:

1.create using Devel a lof of nodes with multivalue fields
2. create new view.
3. add field for one of this fields (for example phones).

Views output me all the values of this field. ok. But i need to output only the first value of each node.

so, uncheck "Display all values in the same row", and set "Display 1 values starting from 0".

the output is empty.

the query is:

SELECT node.title AS node_title, node.nid AS nid, field_data_field_phone.delta AS field_data_field_phone_delta, field_data_field_phone.language AS field_data_field_phone_language, field_data_field_phone.bundle AS field_data_field_phone_bundle, field_data_field_phone.field_phone_value AS field_data_field_phone_field_phone_value, field_data_field_phone.field_phone_format AS field_data_field_phone_field_phone_format, 'node' AS field_data_field_phone_node_entity_type, 'node' AS field_data_field_fax_node_entity_type, 'node' AS field_data_field_www_node_entity_type, 'node' AS field_data_field_email_node_entity_type, 'node' AS field_data_field_schedule_node_entity_type, 'node' AS field_data_field_info_node_entity_type, 'node' AS field_data_field_photo_node_entity_type
FROM 
{node} node
LEFT JOIN {field_data_field_phone} field_data_field_phone ON node.nid = field_data_field_phone.entity_id AND (field_data_field_phone.entity_type = :views_join_condition_0 AND field_data_field_phone.deleted = :views_join_condition_1)
WHERE (( (node.type IN  ('directory')) AND (node.nid = '123') AND (field_data_field_phone.language = 'en') ))
ORDER BY node_title ASC
LIMIT 20 OFFSET 0

Oh my god.. Wooooooooooow...Now it's ok.. :P

I have attached a screenshot with the great result FOR MY CASE

With views dev 25 jan and only the patch #102 it's all ok...

Thankssss Bojanzzzzzzz... :-)))))) now my project go to next step :P

So if I download the newest 7-x-dev, then this patch is included?

Edit: yes, answered own question.

Another confirmation that this is now fixed and working well.
Many thanks!!

seems this patch doesn't work on the 7.x-3.0-alpha1 release of views

Why would you expect it to?

Please let this issue rest in peace. And do the same with alpha1. It's dead, Jim (Tim?)
This code is part of the latest -dev release.

Found the answer myself. The new Views interface is awesome!