For reference see:

https://github.com/phase2/oa_core/pull/56 and https://drupal.org/node/2273515

oa_worktracker.module calls _oa_buttons_get_node_command_buttons() and if the previous changes are committed then this breaks the select list.

CommentFileSizeAuthor
#5 oa_worktracker_section-options_2273639_5.patch1.37 KBscottalan
#1 oa_worktracker-multiple-command-buttons-same-section_2273639_1.patch594 bytesscottalan

Comments

scottalan’s picture

scottalan commented
StatusFileSize
new oa_worktracker-multiple-command-buttons-same-section_2273639_1.patch594 bytes
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Active » Fixed

Works with or without the linked PR, so I went ahead and committed it right away. :-)

  • Commit abf44a5 on 7.x-2.x authored by scottalan, committed by dsnopek: 
    Issue #2273639 by scottalan: Added Multiple command buttons same section...
scottalan’s picture

scottalan commented

Thanks dsnopek! I'm actually going to re-roll this per some comments from mpotter here: https://github.com/phase2/oa_core/pull/56#issuecomment-44280400

This will get rid of strpos() and I also went ahead and added some caching. Granted, this is just in the default cache table. I guess we could create our own cache table oa_worktracker_cache if you wanted to. If you think we should do that let me know and I can add it. For now, without an update, this will work. Let me know.

Patch to follow...

scottalan’s picture

scottalan commented
StatusFileSize
new oa_worktracker_section-options_2273639_5.patch1.37 KB

Here's the patch.

mpotter’s picture

mpotter commented

Think there is a bug in this one. It loops through the command buttons looking for a worktracker button and unsets if any command button other than worktracker is found. So if you have a section that allows both worktracker and some other content type, the other content type will cause the worktracker to be removed. So the logic in this loop needs some work.

mpotter’s picture

mpotter commented
Status: Fixed » Needs work
scottalan’s picture

scottalan commented

@mpotter

I'm pretty sure that's what it was doing before. This is for a select list that allows a selection of a 'Task Section'. If it's not a 'Task Section' then it is removed from the select list. Let me know if that's not right.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

@scottalan: No, Mike is right. The previous code removed sections that didn't allow creating 'oa_worktracker_task' nodes. This code will remove any section where the first content type on the list isn't 'oa_worktracker_task'.

Also, as I mentioned on IRC, the caching here isn't quite right. It will cache one global result, but the list of available Sections changes depending on what Space you are in. And the cache is never cleared - so, if a Section is added or removed, it'll never appear. Personally, I'd say just remove the caching from Work Tracker. If it's going to be cached anywhere, it should probably be inside _oa_buttons_get_node_command_buttons() or higher up.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs work » Fixed

Since OA2 has a release comming up, I've implemented a simplified version of the patch in #5, which doesn't have the logic bug or the caching. I'm going to close this, but if we want to implement caching somehow, let's do that on a seperate issue!

  • Commit c3f1180 on 7.x-2.x by dsnopek: 
    Issue #2273639 by scottalan: Added Multiple command buttons same section...
scottalan’s picture

scottalan commented
Assigned: scottalan » Unassigned
scottalan’s picture

scottalan commented
Assigned: scottalan » Unassigned
scottalan’s picture

scottalan commented

@dsnopek Sorry, had a fix but didn't get it up quick enough. Looked back through the code an saw the bug you were talking about. On the caching...I agree it should be higher up. Thanks for the fix!

Status: Fixed » Closed (fixed)

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