Comments

roam2345’s picture

Status: Active » Needs review
StatusFileSize
new2.65 KB

This patch adds the ability to sort on the entities position in the queue.

roam2345’s picture

StatusFileSize
new11.66 KB

Missed some field in previous patch

mrfelton’s picture

StatusFileSize
new11.62 KB

Not sure why, but that wasn't applying for me. Had a whitespace issue too. Here is one that applies cleanly.

bibo’s picture

Status: Needs review » Reviewed & tested by the community

@mrfelton, sorry but your patch didnt apply (not to the latest july-7 version), while jucallmes patch in #2 worked as expected.

However, even that patch failed when it was used with drush make to build our site. It turned out that the problem was related to .info files and a certain bug in drush. That bug has been fixed in drush 5.7. After upgrading drush 5.4 -> 5.7, our drush make runs fine.

Marking as RTBC.

mrfelton’s picture

Thanks for the heads up @bibo. It was a drush make issue that I was experiencing too, so will try upgrading there.

bibo’s picture

Status: Reviewed & tested by the community » Needs work

The sorting seemed to work, but only partially and caused weird timeout errors. Currently we use the patch but not really the filters (hope to firgure it out later).

Also: the patch applying informed about a lot of offset (success still), so I wonder if it should be redone for the current version?

eft’s picture

StatusFileSize
new10.78 KB

Here is another patch. I created it by first applying @mrfelton's one from #3. Then I ran git diff again and excluded the prefixes (so the patch credits have been removed). Tested with Drush Make (Drush 4.5 even) and it applied fine. Just had to remember to specify the correct branch in the entityqueue repo.

Testing showed the Subqueue: Position field in the field list as expected and the sort list but the field was not in the list of filterable attributes.

Thanks to everyone behind the effort to move the project forward.

bibo’s picture

@eft : which entityqueue dev-version are you using? Do you have a link to the commit?

It seemed to fail against my current version:
projects[entityqueue][download][revision] = "f34c4214819b2bcce9dcb2f9ed0a5d132aa59edb"

Will try with latest dev.

bibo’s picture

Just had to remember to specify the correct branch in the entityqueue repo.

Please elaborate.

My make file fails the patching with these settings:

projects[entityqueue][type] = "module"
projects[entityqueue][subdir] = "contrib"
projects[entityqueue][download][type] = "git"
projects[entityqueue][download][url] = "http://git.drupal.org/sandbox/amateescu/1429904.git"
projects[entityqueue][download][revision] = "f34c4214819b2bcce9dcb2f9ed0a5d132aa59edb"
; Patch for views sorting support: http://drupal.org/node/1680962
projects[entityqueue][patch][] = http://drupal.org/files/1680962.7-entityqueue-sort-position.patch

I also tried manually with "patch -p1 <", "patch -p0 -" and "git patch", and they all fail with various erros, such as:

Hunk #1 succeeded at 15 with fuzz 2 (offset -6 lines).
(Stripping trailing CRs from patch.)
patching file entityqueue.module
Hunk #1 succeeded at 420 with fuzz 1 (offset -222 lines).
...
patch unexpectedly ends in middle of line
patch: **** malformed patch at line 368:
..
etc

eft’s picture

This is how I configure my make file:

projects[entityqueue][type] = "module"
projects[entityqueue][download][type] = "git"
projects[entityqueue][download][url] = "http://git.drupal.org/sandbox/amateescu/1429904.git"
projects[entityqueue][download][branch] = "7.x-1.x-ctools"

;patches
projects[entityqueue][patch][] = "http://drupal.org/files/1680962.7-entityqueue-sort-position.patch"
bibo’s picture

Thanks eft, but for some reason the patch still fails with those settings in drush makefile.

Could it be that the branch youre using has changed meanwhile? I would patch only against a specific commit, since branches might evolve fast.

Im using drush 5.7.

paulihuhtiniemi’s picture

StatusFileSize
new10.64 KB

I had also problems applying patch in comment #7, so here's a re-created version of the same patch.

paulihuhtiniemi’s picture

I also noticed that there were problems with views where relationship to queue was not required. Here's another patch (apply after previous patch), that updates the entityqueue_sort() and render() functions. Not the most elegant solution, but seems to work for me.

Sorting is now done with custom usort() comparison function. It kind of sounds something that should rather be done with SQL, or what do you think?

paulihuhtiniemi’s picture

Forget that patch in #13, it doesn't actually work :( As PHP manual says: "The cmp_function doesn't keep the original order for elements comparing as equal." http://php.net/manual/en/function.usort.php

jojonaloha’s picture

There are a couple issues I see with these approaches.

  1. Like #13 mentioned, this would probably be better if it was done in SQL
  2. All the patches are hard-coded to 'eq_node', basically rendering the entity part of entityqueue useless

I thought there was already a way to sort on the delta column of a field, but it looks like we are wiping that out in entityqueue_views_data_alter() when unset($data[$field_table]); is called.

I tried removing that and clearing all caches and then I was able to add the sort on delta, but then my view was joining the field_data_eq_node table a second time.
Example views query:

SELECT node.title AS node_title, node.nid AS nid, eq_node_node__field_data_eq_node.delta AS eq_node_node__field_data_eq_node_delta
FROM 
{node} node
LEFT JOIN {field_data_eq_node} field_data_eq_node ON node.nid = field_data_eq_node.eq_node_target_id
LEFT JOIN {entityqueue_subqueue} eq_node_node ON field_data_eq_node.entity_id = eq_node_node.subqueue_id AND eq_node_node.name = 'test'
LEFT JOIN {field_data_eq_node} eq_node_node__field_data_eq_node ON eq_node_node.subqueue_id = eq_node_node__field_data_eq_node.entity_id AND (eq_node_node__field_data_eq_node.entity_type = 'entityqueue_subqueue' AND eq_node_node__field_data_eq_node.deleted = '0')
WHERE (( (node.status = '1') ))
ORDER BY eq_node_node__field_data_eq_node_delta ASC
LIMIT 10 OFFSET 0

Other then that, I think removing the unset works as I'd expect. I'm thinking that instead of unsetting the field table, we could override the handlers with our own handler that removes the extra join.

The only other thing is that NULL values are sorted first if you don't want the relationship to be required. With Nodequeue views I typically get around that by adding a sort by Queue ID DESC before the Queue Position sort. I think adding a queue id sort handler would be an easy way to achieve the same result here too.

amateescu’s picture

Other then that, I think removing the unset works as I'd expect. I'm thinking that instead of unsetting the field table, we could override the handlers with our own handler that removes the extra join.

Or we could copy the relevant parts from the field handler before unsetting it.

The only reason for unsetting the field stuff was to not have duplicate ways of dealing with entityqueue in views, so I would be happy with any solution that accomplishes that goal :)

Also, for everyone following this issue, I just merged the 7.x-1.x-ctools branch into 7.x-1.x so we have a unified codebase to work on.

jojonaloha’s picture

Issue summary: View changes
StatusFileSize
new3.42 KB

Attached is a patch that keeps the necessary information for sorting on the delta column. It also adds the queue and subqueue machine names for sorting.

The problem I see is that it still creates another join. Here is a sample query from views:

SELECT node.title AS node_title, node.nid AS nid, eq_node_node.name AS eq_node_node_name, eq_node_node__field_data_eq_node.delta AS eq_node_node__field_data_eq_node_delta
FROM 
{node} node
LEFT JOIN {field_data_eq_node} field_data_eq_node ON node.nid = field_data_eq_node.eq_node_target_id AND (field_data_eq_node.bundle = 'featured' AND field_data_eq_node.deleted = '0')
LEFT JOIN {entityqueue_subqueue} eq_node_node ON field_data_eq_node.entity_id = eq_node_node.subqueue_id AND eq_node_node.name = 'featured'
LEFT JOIN {field_data_eq_node} eq_node_node__field_data_eq_node ON eq_node_node.subqueue_id = eq_node_node__field_data_eq_node.entity_id AND (eq_node_node__field_data_eq_node.entity_type = 'entityqueue_subqueue' AND eq_node_node__field_data_eq_node.deleted = '0')
WHERE (( (node.status = '1') ))
ORDER BY eq_node_node_name DESC, eq_node_node__field_data_eq_node_delta ASC
LIMIT 10 OFFSET 0

From what I can tell the query still returns the results I would expect, but the extra join makes me leery, especially when the relationship is optional.

I had also tried removing the table info, but then it doesn't show in the Sort options, and I think it's because it doesn't know that the table has already been joined? I'm still thinking a simple custom sort handler might be a way around this.

I'm also leaving this in "Needs work" for now because there are issues if you use the MultipleEntityQueueHandler (or any other handler where the $subqueue->name != $queue->name).

jojonaloha’s picture

Status: Needs work » Needs review
StatusFileSize
new5.14 KB

Ok, I think this is the better approach. It adds a new sort handler and instead of keeping the delta sort on the field_data table, it uses the fake entityqueue_relationship field to add the sort handler. Since it relies on the relationship, I added an error message if they try adding the sort without the relationship (not sure if there is a built-in way in views to require that). I had tried to add it to $data['entityqueue_subqueue'][$field_name . '_position'] instead so that the relationship was inherently required, but then for every type of queue (node, user, etc) there is another sort option, which just clutters up the UI and you have to select the right one.

For the MultipleEntityQueueHandler issue, at first I thought we could change entityqueue_get_options() to key by entity id for subqueues (maybe add a third optional parameter). Then instead of adding a join condition on the bundle it would be on the entity_id. The problem I see there is then the views are only exportable if the subqueues have the same entity id.

Given that, I think that option is out. So instead, I am selecting the queue names for the selected subqueues so we can add a join condition on the bundle.

amateescu’s picture

Edit: Scratch that, didn't look close enough..

jojonaloha’s picture

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Cool, let's do this.

jojonaloha’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7e3b9e4

Status: Fixed » Closed (fixed)

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