Comments

merlinofchaos’s picture

I've abstracted the ability to do export/import into a library as part of the ctools package. I'm not sure if adding reliance on ctools is a good idea or not.

Also, while doing the export/import trick woudl be *very* valuable, it does add some burdensome restrictions -- most notably that you must always load all queues any time you need to sort them; you can no longer rely on pagers (or at least the default pager) and it can become a little more memory intensive.

But it sure is handy for deployment.

ezra-g’s picture

Assigned: mlncn » Unassigned

Which aspects of a queue would you export? The queue definition? All the subqueues and the nids?

I'll check out merlinofchaos's ctools import/export functionality. We could probably make import/export an optional feature with that as a dependency if you're going to use it.

pasqualle’s picture

queue definition

subscribe

amitaibu’s picture

Just for documentation, the way that I add nodequeue in hook_update_N() is:

  // Add simple patterns nodequeue.
  $queue = array(
    'title' => 'my queue',
    'size' => 0,
    'reverse' => 0,
    'roles' => array(),
    'types' => array('story'),
    'i18n' => 1,
    'show_in_links' => FALSE,
    'show_in_tab' => TRUE,  
    'show_in_ui' => TRUE,
    'subqueues' => array(),
    'add_subqueue' => array('my queue'),
    'new' => TRUE, 
  );
  
  $queue = (object) $queue;  
  $qid = nodequeue_save(&$queue); 
Bevan’s picture

Thanks Amitaibu. In Drupal 5 this is almost the same;

function custom_update_N() {
  $ret = array();

  if (!function_exists('nodequeue_save')) {
    $ret[] = array('success' => FALSE, 'query' => 'Enable nodequeue module and re-run custom update N');
    return $ret;
  }

  // From drupal.org/node/373174#comment-1678136
  $queue = new stdClass();
  $queue->title = 'RS featured video';
  $queue->subqueue_title = '';
  $queue->size = 1;
  $queue->link = 'feature this video on the homepage';
  $queue->link_remove = NULL;
  $queue->owner = 'nodequeue';
  $queue->show_in_links = 0;
  $queue->show_in_tab = 1;
  $queue->show_in_ui = 1;
  $queue->reverse = 0;
  $queue->reference = 0;
  nodequeue_save($queue);

  $ret[] = array('success' => TRUE, 'query' => 'Saved RS Featured video nodequeue');
  return $ret;
}
mrfelton’s picture

integration with the features module?

drupalninja99’s picture

i am too looking for integration with features

jenlampton’s picture

I'd love a hook_default_nodequeues that could load your importables too :-)

rickvug’s picture

Issue tags: +CTools exportables

As merlinofchaos states in #1: "the ability to do export/import into a library as part of the ctools package". Features automatically works with exportables using this API so it sounds like the way forward is clear. I'm not sure how major the issues raised in #1 are. I don't see any problems with relying on cTools at this point as it seams to be on trending towards token module like ubiquity. If you're using Features, you're likely using Context, which now has a dependency on cTools.

mrfelton’s picture

Yes, I think ctools is the way to go. Numerous other modules are using if for integration with features (as well as other stuff), so by following this trend you will put yourself in a better position to benefit from the experience and further development of others.

mlncn’s picture

Note that Features Extra is doing a features-dependent export/import in its fe_nodequeue module.

(A different issue, literally, but Drop automatic view creation would help nodequeue with general compatibility with exportable solutions.)

benjamin, agaric

mundanity’s picture

For those interested, I'm proposing a step 1 of get machine names in, which will hopefully let us get to exportables sooner than later. #817558: Machine names

Remon’s picture

+1

q0rban’s picture

subscribe

pcarman’s picture

Subscribe. Would love to see this functionality.

drupalninja99’s picture

+1 on features integration

j0rd’s picture

Anyone working on integration between nodequeue module and the features module? I would love to see this.

--
Ubercart themes

Grayside’s picture

subscribe

drupalninja99’s picture

i mean we have this with features_extra, we we not just roll that functionality into the nodequeue module? It forces you to create a machine name to make it exportable.

dww’s picture

Status: Active » Postponed

Yeah, let's focus on getting #817558: Machine names in first, then we can come back to this.

karma.code’s picture

subscribing

blakehall’s picture

I'm currently working on a site that needs this functionality sooner rather than later, so I've written a nodequeue_export module for Drupal 6 (http://github.com/blakehall/nodequeue_export).

NOTE: This module depends on this patch: http://drupal.org/files/issues/machine-names4.patch (from comment #24 of the machine names issue linked to above).

Once the machine names patch lands, I'll be more than happy to provide this module as a patch to nodequeue itself.

In the mean time if anyone else has need for this functionality and would like to take a look I'd happily accept pull requests on github :)

jaydub’s picture

StatusFileSize
new337 bytes

Regarding #22 we are using this export module and @blakehall I reported a simple PHP warning issue on your github page. I've posted a patch here so that I can pull in the patch for our build via drush_make.

blakehall’s picture

Just as a side note, I've fixed the PHP warning in the github repo. Thanks for the heads up!

ezra-g’s picture

Version: 7.x-2.x-dev » 6.x-2.x-dev
Status: Postponed » Active

#817558: Machine names is fixed in the 6.x-2.x branch. Un-postponing :).

beeradb’s picture

subscribe

rgristroph’s picture

subscribe

rgristroph’s picture

I have followed the famous blog post by Stella

http://civicactions.com/blog/2009/jul/24/using_chaos_tools_module_create...

and I have nodequeues exporting. I am working on the hook necessary to import them, and hope to post an initial patch for people to start tweaking soon.

This is awesome, being able to put nodequeues in a feature will smooth out roll-outs for me immediately.

--Rob

mrconnerton’s picture

Is this any different than what is supported here: http://drupal.org/project/features_extra

rickvug’s picture

@rgristroph Patch or work in progress code please?

rgristroph’s picture

I can post a patch of what I have so far; I'm working on it right now.

The remaining problems aren't that easy:

1) Subqueues need to be exported as well as queues

2) In order to do the above, subqueues need to refer to their parent queue via machine name instead of qui

3) Much of the code needs to be made to work from machine names in the place of qids, for example when programmatically creating, inserting, and deleting from the queues. This may not be necessary for the next release. Ideally the "API" type functions would work their current arguments, but another set of functions or additional optional arguments to the existing ones would allow operating with machine names.

--Rob

rgristroph’s picture

StatusFileSize
new5.13 KB

Ok, here is the patch of what I have so far.

If you look in nodequeue.module for the comment that begins "TODO / BROKEN:" you will see the line where I am currently having trouble.

Even when that is fixed, I think for this to be acceptable we have to add subqueues. Should that be fixed, it is possible that you could make a Feature that had nodequeues that just had their default subqueue and it would work, but we really need to add the subqueues I think, and that implies some schema changes so subqueues have a machine name too.

I don't expect to be able to work on this for another week or so, so if anyone else can make progress that would be great.

--Rob

mrconnerton’s picture

@rgristroph: I'm working on the #32 patch now. I did something that made your todo issue magically work and I'm trying to trace it.

Keeping in mind I'm completely new to the ctools api: Wouldn't ctools_export_load_object('nodequeue_queue', 'all') invoke those includes and return the queues? I'm testing it now.

mrconnerton’s picture

If I replace
$default_nodequeues = module_invoke_all('default_nodequeue_nodequeue');
with
$default_nodequeues = ctools_export_load_object('nodequeue_queue', 'all');
It works as expected for me.

According to the docs ctools_export_load_object will: This function will cache the objects, load subsidiary objects if necessary, check default objects in code and properly set them up. It will cache the results so that multiple calls to load the same objects will not cause problems.

Which calls: _ctools_export_get_defaults: Call the hook to get all default objects of the given type from the export. If configured properly, this could include loading up an API to get default objects.

Can you verify this and roll a new patch if this is correct?

mrconnerton’s picture

I also found that it is not exporting/saving the node types associated with queues

hypertext200’s picture

sub

justafish’s picture

subscribing

vivianspencer’s picture

SUBSCRIBING

rgristroph’s picture

Update:

I am working on this here at the code sprint for DrupalCon Chicago 2011.

The change in #34 does seem to fix the problem of the nodequeues not appearing after the feature is enabled, but I still have a problem from the type of the node not being stored, as noted by mrconnerton .

rgristroph’s picture

More update: the nodequeue_types and nodequeue_roles tables have to made machine-name compatible, and exported along with the nodequeue itself. This means adding an update hook that makes the schema have the machine name of the nodequeue along with the qid, and then making those be exported along with the nodequeue itself.

I'm continuing to work on this and will post an updated patch soon.

mrconnerton’s picture

I have this working already without schema changes but Im not in a place to find and post them right this moment. I will try and get to this later this week

joegraduate’s picture

subscribing

joachim’s picture

Status: Active » Needs work

I'm afraid I don't have much time at all so this is a fly-by review...

Might the problem stated at the TODO note in the patch be to do with this mismatch?

+++ nodequeue.install	23 Jan 2011 19:21:10 -0000
@@ -4,6 +4,19 @@
+        'api' => 'default_nodequeue',  // Base name for api include files.

In hook_schema we seem to be giving a filename for include files...

+++ nodequeue.module	23 Jan 2011 19:21:11 -0000
@@ -1770,9 +1823,44 @@ function nodequeue_load_queues($qids = a
+  // ever includes the feature_name.nodequeue_defaults.inc file that is in the generated

The filename feature_name.nodequeue_defaults.inc does not appear to match this pattern...

Powered by Dreditor.

joachim’s picture

> More update: the nodequeue_types and nodequeue_roles tables have to made machine-name compatible, and exported along with the nodequeue itself.

Do you really need that, or does it suffice to have the $nodequeue->roles and $nodequeue->type be arrays, and the API to know how to save those?

rgristroph’s picture

@joachim: good catch on the mismatch. I will also look at if I can use the arrays the API somehow for the other issue.

kenorb’s picture

+1

amateescu’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
StatusFileSize
new3.28 KB

Attaching a patch with some work on this in the 7.x branch.

There's a ctools bug that's preventing me from finishing up this patch (actually two, but the first one just got fixed #1156014: Export error when using table joins). Will investigate further :)

amateescu’s picture

Some more work on this. Earl told me what I was doing wrong, and now we have nodequeue roles and types, still need to do subqueues.

rickvug’s picture

Some initial testing of patch #48. Creating a node queue and exporting into feature module works. However deleting the queue and enabling the feature has no result. The node queue listing does not show any exported queues or their statues (in code, database, overridden). Beyond this there is the problem of nodequeue_load(), which expects $qid. This means that paths such as admin/structure/nodequeue/%nodequeue/view can not be predicted. If nodequeue_load() could be adapted to support $name and fall back to $qid that would be best (at least based on what I'm seeing here).

admin/structure/nodequeue/%/export works but has the following notices:

Notice: Trying to get property of non-object in nodequeue_export_queue() (line 1054 of path/modules/contrib/nodequeue/includes/nodequeue.admin.inc).
Notice: Trying to get property of non-object in nodequeue_queue_export() (line 1117 of path/modules/contrib/nodequeue/nodequeue.module).
Notice: Trying to get property of non-object in nodequeue_queue_export() (line 1118 of path/modules/contrib/nodequeue/nodequeue.module).

I'd also think that this menu item would be best as MENU_LOCAL_TASK so that users are able to find export as a tab.

amateescu’s picture

@rickvug, sorry for not telling this earlier, but this patch is not in a testable state. However, your findings are correct, all you said that's not working is what should be further worked on :)

rickvug’s picture

@amateescu Not a problem. I did assume the patch was in an early state. Just wanted to see where it was at and hopefully help move things along. FYI I've created a follow up issue to use $name within the default views so that blocks delta are consistent: #1160024: Default views should use $name rather than $qid. I'm not sure if you consider that issue within scope here.

Taking another look at the patch I noticed the new function nodequeue_queue_load($name). The naming is confusing as there already is nodequeue_load($qid). nodequeue_load($qid) is a wrapper around nodequeue_load_queues($qids = array()). Perhaps for consistency the new functions should be nodequeue_load_by_name() and nodequeue_load_queues_by_name()? In either case all of the above will need to be adapted to load from both the db and exports. Finally the output of the functions is different as nodequeue_load returns an array with a queue object while nodequeue_queue_load($name) returns the queue object itself.

jamiecuthill’s picture

Version: 7.x-2.x-dev » 7.x-2.0-alpha2
Status: Needs work » Needs review
StatusFileSize
new4.87 KB

Here is a patch I'm using to enable the export tab on a nodequeue. It's based on patch 48 by amateescu but includes the add_subqueue property so that they are automatically create in nodequeue_save. I've also excluded qid from the export. I've also had to change the schema of i18n as ctools will assume tinyint is a boolean and nodequeue_save will fail if i18n is a boolean.

You can import the export with a call to nodequeue_save($nodequeue).

rickvug’s picture

Version: 7.x-2.0-alpha2 » 7.x-2.x-dev

Moving this ticket back to 7.x-2.x-dev as all patches should be against dev. The updated patch is a step forward in that it includes parts of a UI but further work is still needed:

  • The update function is modifying a d6 function. A D7 specific update function should be created.
    - On the main nodequeue page "Export" should be one of the Operations (ie. View | Edit | Export | Delete).
  • git patch complained about trailing white space on line 46.
  • The export page didn't work for me. I get the following warning:
    Warning: Parameter 1 to nodequeue_export_queue_form() expected to be a reference, value given in drupal_retrieve_form() (line 770 of /vagrant/document_root/includes/form.inc).
    
  • An import form is still needed. Could be a local task beside "Add nodequeue".
  • Exporting to a feature works. I can modify the export and it will show the difference. Reverting causes many errors however. It essentially wipes out the queue.
  • On a fresh Drupal install enabling a feature with the nodequeue export doesn't work.

@jamiecuthill I hope this testing helps you drive this further.

mrfelton’s picture

I'm only been following this thread at an arms length, but from a brief look at the latest patch it look as if you are creating your own export ui? Why not use the ctools export ui?

rickvug’s picture

@mrfelton You're right that there's little consistency in the current patch. The focus of #52 was getting to the point where you can export and import nodequeue objects via the UI without ctools exportability (@jamiecuthill, correct me if I'm wrong). The ctools export work was there from work done in the previous patch but isn't there yet. Ultimately ctools exportables and the use of export ui is what we want but that requires deeper reworking of nodequeue loading. Beyond this $qid is still commonly used which may may need reworking of its own to ensure exported queues do not conflict with one another (see #1160068: Drop all use of $qid).

jamiecuthill’s picture

New patch following feedback rickvug.

@mrfelton As rickvug has responded, the ctools export ui is the ultimate goal but to get this up and running it is easier to do it this way until we can address some other issues.

carn1x’s picture

subscribe

mlncn’s picture

Status: Needs review » Reviewed & tested by the community

Tried this out, and it works just fine! Code looks good too. And i like the clean export it produces.

(Can use this to help create a nodequeue programatically, too.)

benjamin, agaric

dww’s picture

Anyone planning to backport #56 to 6.x-2.x? Are the nodequeue maintainers going to accept this change in D6?

ezra-g’s picture

I'm not planning to backport this, but would accept patches that backport it to 6.x-2.x.

amateescu’s picture

Status: Reviewed & tested by the community » Postponed

I thought we agreed that the export/import patch will wait for #1160068: Drop all use of $qid in the 7.x-1.x branch.

@dww, I'll try to backport this patch to 6.x-2.x, but only after a 7.x-1.0 *stable* release :)

dww’s picture

@amateescu: That seems like it's a counter-productive strategy. Isn't that just going to make even more work for getting this done? Also, #1160068 is currently vaporware, and this is RTBC. Let's not let the perfect be the enemy of progress. Seems like your approach requires:

- Write #1160068 from scratch, review/test, commit
- Re-write this patch as needed to deal with changes from #1160068
- Re-write this patch again mostly back to where it is now and then backport it to D6

vs.

- Commit this one right now
- Relatively simple backport to D6
- Write #1160068 from the first place to take $qid out of the picture

That's a lot less re-rolling and delays to my eyes. Then again, I'm not able to do the work myself in the near term, but I'd *love* to see this getting resolved and #61 seems to delay and extra-complicate it for limited (if any) benefit.

Thanks,
-Derek

amateescu’s picture

Status: Postponed » Needs work

@dww, I guess you're right :) The only problem is that this patch is not RTBC. I tested it and importing a queue into a site that has a queue with the same name fails.

dww’s picture

Excellent, glad you're letting this move forward on its own. I grabbed #56 to try out, review, and perhaps fix on the plane. We'll see if I can get to it.

Thanks for being flexible on the strategy. ;)

Cheers,
-Derek

bleen’s picture

sub

josh waihi’s picture

Reroll or #56 so that drush_make will accept the patch. #56 requires `git apply` rather than `patch -p0`.

Also, subscribing.

mlncn’s picture

Status: Needs work » Reviewed & tested by the community

Works great! Again using it in a production workflow.

mlncn’s picture

Status: Reviewed & tested by the community » Needs work

No, it doesn't work, i got too excited:
* Does not export anything in additional tables: types, roles.
* Does not Features-style export at all.

recidive’s picture

StatusFileSize
new13.71 KB

I applied the patch in #66 and could export a nodequeue using features, but can't get it to import when enabling the feature module. When accessing the feature view page, I get the following notices:

Notice: Undefined index: nodequeue_queue in features_admin_components() (line 501 of /Users/henrique/Webroot/martplug-1.0-beta1/profiles/martplug/modules/contrib/features/features.admin.inc).
Notice: Undefined index: nodequeue_queue in features_admin_components() (line 523 of /Users/henrique/Webroot/martplug-1.0-beta1/profiles/martplug/modules/contrib/features/features.admin.inc).
Notice: Undefined index: in theme_features_storage_link() (line 131 of /Users/henrique/Webroot/martplug-1.0-beta1/profiles/martplug/modules/contrib/features/theme/theme.inc).
Notice: Undefined index: in theme_features_storage_link() (line 138 of /Users/henrique/Webroot/martplug-1.0-beta1/profiles/martplug/modules/contrib/features/theme/theme.inc).

and the status for the nodequeue component appear as in the attached screenshot.

recidive’s picture

My apologies, the errors above were due to updating to latest dev and overwriting the patched files.

It now displays as Default but the queue was not created.

recidive’s picture

Manual export/import seems to be working, here's the exported code, at least types are being exported and imported fine. Only importing through features seems to be broken.

$nodequeue = new stdClass;
$nodequeue->api_version = 2;
$nodequeue->name = 'featured_products';
$nodequeue->title = 'Featured products';
$nodequeue->subqueue_title = '';
$nodequeue->size = 0;
$nodequeue->link = '';
$nodequeue->link_remove = '';
$nodequeue->owner = 'nodequeue';
$nodequeue->show_in_ui = 1;
$nodequeue->show_in_tab = 1;
$nodequeue->show_in_links = 0;
$nodequeue->reference = '0';
$nodequeue->reverse = 0;
$nodequeue->i18n = 0;
$nodequeue->roles = array();
$nodequeue->types = array(
  0 => 'product',
);
$nodequeue->add_subqueue = array(
  2 => 'Featured products',
);
mlncn’s picture

Yes, importing through features actually wipes out existing correctly imported nodequeues. Wasted some hours by failing to remove the feature code before moving to the manual method. Here's code that works in a .install file to create a new nodequeue (some default values abstracted; that's not necessary). Call the function from an install or update hook.


function sdl_create_nodequeues() {
  $default_nodqueue = new stdClass; 

  $export = array();

  $nodequeue = new stdClass;
  $nodequeue->subqueue_title = '';
  $nodequeue->size = 20;
  $nodequeue->owner = 'nodequeue';
  $nodequeue->name = 'front_page_continuum';
  $nodequeue->title = 'Front page continuum';
  $nodequeue->link = 'Add to Top Front';
  $nodequeue->link_remove = 'Pop off Top Front';
  $nodequeue->types = array('event', 'in_the_news', 'page', 'person', 'project');
  $export['front_page_continuum'] = $nodequeue;

  $nodequeue = new stdClass;
  $nodequeue->subqueue_title = '';
  $nodequeue->size = 20;
  $nodequeue->owner = 'nodequeue';
  $nodequeue->name = 'projects_continuum';
  $nodequeue->title = 'Projects continuum';
  $nodequeue->link = 'Add to Top Projects';
  $nodequeue->link_remove = 'Pop off Top Projects';
  $nodequeue->types = array('project');
  $export['projects_continuum'] = $nodequeue;

  foreach ($export as $name => $nodequeue) {
    // $nodequeue->name = $name;
    $nodequeue->api_version = 2;
    $nodequeue->show_in_ui = 1;
    $nodequeue->show_in_tab = 1;
    $nodequeue->show_in_links = 1;
    $nodequeue->reference = '0';
    $nodequeue->i18n = '0';
    $nodequeue->roles = array(3, 4);
    $nodequeue->reverse = 1;
    if ($nodequeue->subqueue_title != '%subqueue') {
      $nodequeue->add_subqueue = array(
        0 => $nodequeue->title,
      );
    }
    nodequeue_save($nodequeue);
  }
}
davidwhthomas’s picture

Had the same issue, the features export/import was working but now on features revert, all existing exported nodequeues are deleted :-/

davidwhthomas’s picture

A workaround for me to restore the deleted nodequeues was to dump the nodequeue_queue table from a backup and restore on the env.

All the other nodequeue tables were still in place, so just needed the nodequeue_queue definitions.

mysqldump -u USERNAME -p --database DATABASENAME --table nodequeue_queue > nodequeue_queue.sql

And then on the environment to restore nodequeues:

drush sqlc < nodequeue_queue.sql

I used drush but you could use regular mysql < as well.

josh waihi’s picture

This issues seems to be related to the fact that features' ctools implementation of hook_features_revert deletes the nodequeue but doesn't recreated it.

I made my function look like this in features and it all worked:

/**
 * Master implementation of hook_features_revert() for all ctools components.
 */
function ctools_component_features_revert($component, $module) {
  if ($objects = features_get_default($component, $module)) {
    foreach ($objects as $name => $object) {
      // Some things (like views) do not use the machine name as key
      // and need to be loaded explicitly in order to be deleted.
      $object = ctools_export_crud_load($component, $name);
      _ctools_features_export_crud_delete($component, $object);
      ctools_export_crud_save($component, $object);
    }
  }
}

Will chase the issue up in features and post back here with the issue number.

josh waihi’s picture

I've posted a comment and patch on the features issue: #1211272: Reverting features deletes ctools components

josh waihi’s picture

Also, as the nodequeue_types and other tables don't get exported, the qid must also remain the same. This is crucial, otherwise the queues get recreated but the foreign key integrity is broken.

So new patch required.

josh waihi’s picture

Status: Needs work » Needs review

Updating status

sutharsan’s picture

Status: Needs review » Needs work

Great that you're working on this functionality!
With the #77 patch Nodequeue fails to create a Nodequeue. Also when the feature is enabled in a vanilla Drupal site. The content of the .default_nodequeue_queues.inc file looks Ok. I can provide the feature upon request.

sutharsan’s picture

An other observation: Export a nodequeue on one site and import it into another site fails. When removing the gid definition ($nodequeue->qid = '3';) form the exported code the import works. Perhaps this is related to the above.

jamiecuthill’s picture

The #77 patch is missing the following setting on the qid section of the schema

'no export' => TRUE,

josh waihi’s picture

Thats intentional, otherwise, referential integrity is lost.

jamiecuthill’s picture

Ah I missed that requirement whilst I was skimming the last few posts.

We really need sub-queues to use machine name in the db to reference the queue they belong to so we can move away from qid.

dixon_’s picture

subscribe

mrryanjohnston’s picture

subscribing

recidive’s picture

@mrryanjohnston Please stop this noise.

mrconnerton’s picture

I just wanted to post an update here to give some progress.

@rgristroph and I met up at Drupal camp Austin to sprint on this. We last worked on this around comment #40 back in chicago but busy lives had led us apart. After catching up on the cue it seems a few more group of people stepped in and also migrated this to a D7 issue.

Rob and I decided to just pick up where we left off with the D6 work since none of the patches above seemed to actually be working and our focus was D6 anyway. What we accomplished was using ctools to successfully export a nodequeue, including roles and types data. We ran out of time and are missing just a bit of code for the import part when the feature is turned on and also subqueue support. I hope to complete that and post the patch this week while rob is looking into subqueues.

I will have a D6 patch up at the end of the week. As far as D7 support, after digging into ctools for this issue and talking to some smarter folk at dcatx, I wanted to through out the idea of having a hard dependency in a new d7 branch of nodequeue. Using ctools will greatly simplify the export/import of features, the ui part, and we could probably wipe out a significant amount of general nodequeue code and let ctools manage it. I will be creating another issue on this topic once I get this d6 patch done, just tossing the idea out there.

joachim’s picture

> having a hard dependency in a new d7 branch of nodequeue

Views depends on CTools on D7, and the main use of nodequeue is with Views -- so adding a dependency on CTools here is no big deal at all :)

amateescu’s picture

I'm really looking forward to the 6.x patch. For now, as the codebase is pretty much the same in the 6.x-2.x and 7.x-2.x branches, I'd go with porting that patch to D7.

As for the future of Nodequeue, I'd prefer to concentrate the efforts on Entityqueue (actually, I'd like to call it 'Orderable Content List'). I'm going to post a message about that in #461826: Generalize to entities.

recidive’s picture

Hello, just FYI, I've posted a patch to #1160068: Drop all use of $qid. I think this is a good cleanup for easing import/export.

mrconnerton’s picture

mrconnerton notes 1 of 3

So I have been working on my d6 ctools method patch for this, but I had to take a step back and think about it because I have been exporting nodequeues for over a year now using this module: http://github.com/blakehall/nodequeue_export and I don't understand why we never pursued that? It only got one or two comments around ~#22 worth of attention but frankly it works with d6 implementing the features api. I did recently have to patch it as $queue->owner was not being exported, but either way, I think those of us interested in the d6 version need to take a look at it. And I'm sure its an "easy" port to D7

I've been super busy like most of us are, but I will see about turning the module on github into a patch for nodequeue.

mrconnerton’s picture

StatusFileSize
new8.12 KB

mrconnerton notes 2 of 3

Here is my patch so far for D6 support implementing ctools that @rgristroph and I worked on at Drupal Camp Austin. Currently it exports the nodequeue including types and roles, implements some schema changes, and an update script for those schema changes. It does not support any type of advanced subqueues.

There are a few more places that will need a "if ctools exists" and where ever nodequeues are loaded, like viewing the nodequeue list screen, we need to add the ctools code to load the queues that are in code and not the DB.

I will try and finish this up as soon as possible. I'm also going to attach a d6 features api implementation next.

merlinofchaos’s picture

In D7, shouldn't you just add the dependency on CTools and be done with it? Views depends on CTools -- it's going to be on most sites, especially if they use nodequeue.

mrconnerton’s picture

mrconnerton notes 3 of 3

I quickly rolled the @blakehall nodequeue_export from here module http://github.com/blakehall/nodequeue_export into a patched integrated with nodequeue. This is the features api method of import/export (the one that has been working for me for at least a year). Might still need tweaking but its a fantastic start.

I'm not fairly sure which we should use: ctools vs features api for import/export. Let's go ahead and toss that up for discussion. Also I think half of the people on this issue are looking for a D6 patch while the other half are looking for a D7 patch. Do we want to create a separate issue for one version or the other so we can tests patches at the same time? Or when we upload patches, change the version number and slap the ticket on needs review?

mrconnerton’s picture

heres the features api patch

mrconnerton’s picture

@merlinofchaos - Yes I completely agree 100%. However I don't think that is a possibly for D6 which is what I'm trying to patch right now. At least not unless we branch it.

recidive’s picture

@mrconnerton: We really need ctools, so we get features integration for free.

I was going to work on a patch for Drupal 7. Maybe I can use some code from patch in #77 and #92. Then once this get commited we backport to D6, if at all needed.

recidive’s picture

Now that #1160068: Drop all use of $qid is committed to 7.x-3.x, I think we should get a patch committed there first, then backport to other branches.

PS: I guess a release node should be created, so we can have an option for 7.x-3.x in Issue Version.

recidive’s picture

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

Attached is an improved patch for 7.x-3.x, based on the one in #77. Improved the code a little bit and made the import part work.

PS: A release node should be created for 7.x-3.x, so we can move this issue to this version.

Status: Needs review » Needs work

The last submitted patch, 373174.patch, failed testing.

amateescu’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev
Status: Needs work » Needs review

Created a 7.x-3.x release node, let the fun begin :)

recidive’s picture

#99: 373174.patch queued for re-testing.

recidive’s picture

StatusFileSize
new18.14 KB

Improved patches with some cleanup and changes on the APIs to make use of CTools CRUD and export APIs.

Here's a detailed list of changes:

  • Implemented a nodeque_load_multiple function using ctools crud that replaces the nodequeue_load_queues function.
  • Changed nodequeue_load function to use CTools APIs.
  • Added a 'subrecords callback' and changed some export metadata/schema settings.
recidive’s picture

StatusFileSize
new18.85 KB

New patch adding CTools CRUD callbacks and making features export subrecords just like manual export.

Figured out objects loaded from the code don't show up in the UI due to how they are loaded in nodequeue_view_queues():

$queues = nodequeue_load_multiple(nodequeue_get_all_queues(25));

nodequeue_get_all_queues() only return items from database, so I wonder if we should change the admin UI to use the Export UI API from CTools, or get rid of the pagination feature from nodequeue_get_all_queues() or nodequeue_view_queues(). Either way, will not allow pagination, do we have setups with too many nodequeues? Chatting with Earl he said there's no easy way for pagination with CTools hybrid object loading stuff.

Update: There's this comment at the top of nodequeue.module file "None of the PagerQueries are working, they're all just normal queries for the time being.". though I saw the pager implementation in nodequeue_get_all_queues() and nodequeue_view_queues() seems complete.

recidive’s picture

StatusFileSize
new54.13 KB
new55.36 KB

New patch implementing CTools Export UI plugin and replacing good part of UI with it. I got this route since this is a lot of code cleanup, instead of having to add more code to handle the override, reset, etc stuff. Attached is a screenshot of the new UI. Needs testing.

Here are some stats on the patch:

$ diffstat 373174.patch 
 addons/nodequeue_service/nodequeue_service.inc              |    2 
 includes/nodequeue.actions.inc                              |   12 
 includes/nodequeue.admin.inc                                |  437 ------------
 includes/views/nodequeue.views_default.inc                  |    2 
 includes/views/nodequeue_handler_field_all_queues.inc       |    2 
 includes/views/nodequeue_handler_relationship_nodequeue.inc |    2 
 nodequeue.info                                              |    3 
 nodequeue.install                                           |   18 
 nodequeue.module                                            |  290 +++----
 nodequeue_generate.module                                   |    4 
 plugins/nodequeue_ctools_export_ui.class.php                |  124 +++
 plugins/nodequeue_ctools_export_ui.inc                      |  307 ++++++++
 12 files changed, 606 insertions(+), 597 deletions(-)
recidive’s picture

StatusFileSize
new54.48 KB

Updated patch:

- Improving access control and removing some duplicated messages.
- Fixing cloning queues.

Known issues:

- Overriding a queue generated by code doesn't seem to be working.
- Cloning queues doesn't clone nodes in queues.

amateescu’s picture

Sorry that I haven't found the time to review this, but I just want to say for now that I'm happy with the direction this patch is taking.

amateescu’s picture

Status: Needs review » Needs work
StatusFileSize
new54.89 KB

Ok, finally tested the patch. Besides the known issues from #106, I found another one: queue node types and roles are not saved when creating or updating a queue.

Re-rolled the patch after #1388796: PDOException: SQLSTATE[HY000]: General error: 1093 and fixed a todo. The issues above still need to be fixed..

recidive’s picture

Assigned: Unassigned » recidive

Ok, so the list of issues we have so far is:

  1. Overriding a queue generated by code doesn't seem to be working.
  2. Queue node types and roles are not saved when creating or updating a queue.
  3. Cloning queues doesn't clone nodes in queues.

@amateescu: is #3 a requirement?

I'm gonna have another crack at this in the next few days.

amateescu’s picture

Nope, I don't think #3 is even recommended, unless we integrate somehow with UUID :)

Rob_Feature’s picture

This looks like it got dropped in February...is there a reason for that? (did it get rolled into dev? did people give up?)

Just wondering why it stopped abruptly.

recidive’s picture

Hi, we decided to use a reference field instead of nodequeue for our use case. I have an updated patch if someone wants to continue the work we started here. I will upload it soon (I tried to do that now, but got a conflict with last commit, and I don't have time to deal with it right now).

damienmckenna’s picture

Status: Needs work » Needs review
damienmckenna’s picture

Has anyone re-rolled the last patch for the current codebase? It currently blows up rather spectacularly when you try to apply the current patch =)

Status: Needs review » Needs work

The last submitted patch, 373174-108.patch, failed testing.

damienmckenna’s picture

I think the patch should be split into two:

  • Restructure the API to use common naming conventions, e.g. "nodequeue_load_multiple" instead of "nodequeue_load_queues".
  • Support for exportables.

One does not depend upon the other, so it'll be easier to test each one individually.

Scrap that, I hadn't finished reading through the patch and I see the point - it's rewriting the module to use CTools-compatible data structures, not just the basic exportables.

damienmckenna’s picture

Check ctools/help/export.html for instructions for adding the join definitions for the additional tables.

yvmarques’s picture

Status: Needs work » Needs review
Issue tags: -CTools exportables

#108: 373174-108.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +CTools exportables

The last submitted patch, 373174-108.patch, failed testing.

a.mikheychik’s picture

Subscribe

areynolds’s picture

When you get a chance @recidive, posting your latest work (even if it is incompatibile with the newest dev commits) might help move things along. Thanks!

jpstrikesback’s picture

Hey, check out Features Extra, it enables some Nodequeue exportables

http://drupal.org/project/features_extra

mrfelton’s picture

Yeah - or better still there is EntityQueue (the 7.x-1.x-ctools is the one you want).

darol100’s picture

Issue summary: View changes

@jpstrikesback, Features Extra have a bug that does not allow export nodequeues.

Here are the issues:

Status: Needs work » Needs review

hswong3i queued 108: 373174-108.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 108: 373174-108.patch, failed testing.

jenlampton’s picture

Version: 7.x-3.x-dev » 7.x-2.x-dev

This would be a great new feature, moving to 2.x since the 3.x branch is nolonger supported.

avpaderno’s picture

Assigned: recidive » Unassigned
Issue tags: -CTools exportables +Needs reroll