Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
We added $form and $form_state to the action form in order to support rules actions.
Of course, that killed the possibility to use VBO actions outside of VBO. Not a big waste, considering it isn't exactly pretty anyway.
I'm guessing there should be a drupal.action.inc same as there is rules.action.inc, as well as a generic way of getting the available actions from the individual sources.
Comment | File | Size | Author |
---|---|---|---|
#9 | 1188400-refactor-part2.patch | 39.63 KB | bojanz |
#5 | 1188400-refactor.patch | 54.14 KB | bojanz |
#4 | refactor.patch | 56.86 KB | bojanz |
#3 | refactor.patch | 46.15 KB | bojanz |
Comments
Comment #1
bojanz CreditAttribution: bojanz commentedWe need this before alpha3, it's a BC break so better to do it as early as possible.
And actually, the real problem here was trying to force the Rules integration into the same core-actions mold.
So we could continue to declare VBO actions as core actions (just in case someone finds that useful), but handle the separation between core action / rules action better in code.
Comment #2
bojanz CreditAttribution: bojanz commentedI've committed a quickfix: http://drupalcode.org/project/views_bulk_operations.git/commitdiff/cdd6d...
It makes VBO actions work correctly outside VBO again. Also removes the get_custom_actions() code that's no longer needed in D7 (since actions_list() invokes hook_actions_info() without touching the DB).
So, now it's no longer broken (which makes it less of a priority), just ugly.
Comment #3
bojanz CreditAttribution: bojanz commentedThis is broken, incomplete, and wrong at places, but it shows where I'm going.
Comment #4
bojanz CreditAttribution: bojanz commentedIf anyone is following, here is an updated (still broken & untested) version. Makes the codebase much cleaner and easier to understand.
With the rate this is growing, I will have to split it into multiple commits, otherwise the diff will be useless.
Comment #5
bojanz CreditAttribution: bojanz commentedOkay, committed some of the cleanups:
http://drupalcode.org/project/views_bulk_operations.git/commitdiff/1125d...
This removed 300 lines from the diff.
Here's the remaining part. Works in my limited testing.
Summary:
1) Operation types are ctools plugins.
2) An operation is now an object. All related code is in one class, instead of being all over the place.
3) Much less throwing of data around, and confusion ($params VS $options VS $callback_arguments VS whatever)
Comment #6
infojunkie CreditAttribution: infojunkie commentedGreat stuff - I'd been wanting to do that for some time!
Comment #7
bojanz CreditAttribution: bojanz commentedDid some more work and committed:
http://drupalcode.org/project/views_bulk_operations.git/commitdiff/5728c...
Giant step forward!
Comment #9
bojanz CreditAttribution: bojanz commentedTime for a sequel!
Last time I did this I forgot to abstract out the admin options form, so we had actions api specific code in the VBO views field handler, which is bad since we might want to embed some rules settings there at some point as well.
Now fixed. Basically all operation-related options are now handled by the operation itself, leaving the VBO views field handler very thin, as it should be.
This change takes up most of the patch, and is pretty straightforward.
Also, performance improvements! As das-peter pointed out in #1319504: Populate operations only if really needed, VBO currently loads all the operations all the time. Bad if you have a lot of Rules (you selected 4 operations out of 200 for the VBO form, but once the user visits the form, VBO loads all 200 of them, for no good reason).
Changed that so we only load all operations when showing the VBO views field settings (where the operations are selected).
For everything else, only the needed operations are actually loaded.
Due to limitations of the core Action API, we still need to load all advanced actions from the DB and invoke hook_action_info() if there is an action used,
but for Rules the situation is much better since each operation (rules_component) can be loaded separately.
Also, all operation_ids are now prefixed with the operation name (action::system_send_mail_action and rules_component::commerce_calculate_tax for example), to prevent possible collisions. There is update code in the VBO view field handler that prefixes all operations, but that requires loading all operations, so if you want the performance boost described above, you will need to resave your VBO views (I will re-export the views shipped with VBO prior to commit).
Also, this patch fixes actions_permissions for advanced actions, that was broken.
TODO:
1) Need to test the Drush integration, might be broken now, I just updated the code blindly.
2) This code
is a bit of a hack, needed because we save the operation in form_state between form steps (config, confirmation / validate, submit...).
Might be a better idea to forget about keeping the operation in form state, and explicitly loading it instead when needed.
Not sure right now which way is better.
EDIT: Okay, yeah, I'm pretty sure the current way is the way to go. We should keep the operation in form state so that it's the same object, persisted through the various validate / submit stages until execution.
EDIT2: Drush works.
Comment #10
bojanz CreditAttribution: bojanz commentedAnd committed: http://drupalcode.org/project/views_bulk_operations.git/commitdiff/db38f....
It's a big patch, and I've gone many times through it, so I thought better to get it in, and address any issues or criticisms through followups.
So, feel free to speak your mind.
Comment #11
das-peter CreditAttribution: das-peter commentedFor now I've just to say awesome! :)
Comment #12
bojanz CreditAttribution: bojanz commentedGreat :)