Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.This issue is the reversal of #1477828: Stop relying on VBO, implement a custom Views Form handler instead....
The motivation for this is #2066023: Doesn't work if the View has a pager. I think for a lot of use cases, that issue is a showstopper -- as soon as your view lists more users than you can comfortably fit on a single page, you can't email them all. It doesn't take long for something like an OG group to get to that size.
There is a feature request to include the 'select all items on all pages' as part of Views: #1836126: Select all items on all pages. But there the verdict seems to be that that's not going to happen.
So to fix #2066023: Doesn't work if the View has a pager there are three options:
a. duplicate even more of VBO in here. Ugly.
b. include the 'select all items on all pages' as part of Views: not going to happen.
c. reverse the consequences of this issue, and reintroduce VBO as a dependency.
Given that option c is by far the most preferable, I've had a bit of a poke around at this, and respectfully, I disagree with what was decided at #1477828: Stop relying on VBO, implement a custom Views Form handler instead.
I think the problem wasn't using VBO, it was using hook_action_info() to declare the operation. Core actions are really not very good for anything remotely complex -- in fact I've found them a pain to work with, what with things like the hardcoded callback built on the action name.
If instead of a core action, we define a VBO Operation plugin, then it all becomes easier. There's no need for hook_form_alter(), as we have methods on the plugin class that let us define all the fields we need -- we just need a minor (and IMO very necessary anyway!) patch to VBO: #2072057: pass the $handler object to adminOptionsForm() so it can access the view.
I'm still working on this, but it's looking very promising. There's a patch of work in progress at #1477828: Stop relying on VBO, implement a custom Views Form handler instead, which I'll update here later today.
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | 2072415-24-views_send.restore-vbo-operation.patch | 20.54 KB | samerali |
| #13 | 2072415-13.views_send.restore-vbo-operation.patch | 19.4 KB | joachim |
Comments
Comment #1
hansfn CreditAttribution: hansfn commentedI look forward to your work. You might be very right that the problem was the usage of hook_action_info. (It was just taken directly from the D6 version when I ported the module to D7. I didn't know better and still don't.)
PS! I'll notify bojanz about this issue, but I doubt if he will join the discussion.
Comment #2
bojanz CreditAttribution: bojanz commentedI skimmed through the operation plugin patch and I'm not convinced.
It's not giving you anything that the existing actions plugin already isn't doing, so you might as well go back to a hook_actions_info() implementation.
I still think that Views Send is an odd match for VBO because it doesn't use entities, and because it implements it's own queueing, which is the entire point of VBO.
That's also why I said "select all items on all pages" shouldn't go into Views, VBO's implementation loads entities and does it's own queueing, all of the things that Views Send doesn't need.
The only downside that I see with Views Send not using VBO is that you can't (or can't easily) offer other actions at the same time as email sending, which was never a big issue since Views Send always dictated a particular view structure anyway, you're creating that view with the intention of using it for mailing.
It takes 10 lines of code to load a View, strip its pager, take its results (and then send emails with them).
See views_bulk_operations_adjust_selection().
So I think it makes the most sense for Views Send to just reimplement that functionality. It could have a "Execute on all results" button next to the "Execute" button that did exactly that.
Comment #3
joachim CreditAttribution: joachim commented> which was never a big issue since Views Send always dictated a particular view structure anyway, you're creating that view with the intention of using it for mailing.
It sometimes dictates a dedicated view, but not always. Eg, OG's list of group members. It would be nicer UX to have just the one view where you can do member admin, and also email members, rather than have to go to a separate view when you want to send emails.
> VBO's implementation loads entities and does it's own queueing, all of the things that Views Send doesn't need.
As remarked elsewhere (https://drupal.org/node/1477828#comment-5718450), Views Send could do to lose its custom queuing system, and use someone else's. As a follow-up, we could force the 'Enqueue the operation' setting in the VBO field and use VBO's system.
> It takes 10 lines of code to load a View, strip its pager, take its results (and then send emails with them).
See views_bulk_operations_adjust_selection().
Fair enough. I was under the impression it was more work than that.
However, I am concerned this won't scale the way the module currently stands.
The work that Views Send does on processing the form data and assembling the mail is done in views_send_queue_mail(), which is called 'live' from the form submit. It's only at the end of that process that data is queued into the Views Send spool, or set as a batch.
If you have a view of say, 50k OG members, then the whole email processing gets done once per row. Surely that's going to keel over.
I guess that's something that could probably be tackled as a separate issue by changing to use DrupalQueue and refactoring where the work takes place -- dump the view result row and the form params into the queue rather than the built message, and build the message in the queue's worker callback. But it is another thing that VBO does for us OOTB.
But in the meantime, I'll make a new branch in which I'll take a look at the code from views_bulk_operations_adjust_selection().
Comment #4
bojanz CreditAttribution: bojanz commentedWell, "select all on all pages" didn't work for Views Send on VBO because VBO didn't support passing views rows for rows from other pages.
This was only just recently fixed (in VBO 3.1) after a lot of unexpected help.
Refactoring the Views Send queueing of course makes the rewrite even bigger (in fact, complete), which could be a basis for a 7.x-2.x branch if hansfn decides he wants to do so.
In the meantime, we can implement our custom "select all on all pages" and call it day, at least in 7.x-1.x.
Comment #5
hansfn CreditAttribution: hansfn commentedJust for the record: I'm aware of the problem, but I never expected Views Send to be used for such (big) tasks. However, I don't object to changes along these lines.
Comment #6
scottsawyerHello Folks, Thanks for this discussion.
To expand on joachim's case for VBO on the UX front, we have a view for Organic Groups that I am already using VBO for other OG related operations on a views page, it's basically a dashboard for managing multiple groups on one screen. As a result of VBO and Views Send in the same view, we now have 2 checkboxes per row with form controls above and below the view ( I'll get this sorted, but seems unnecessary ). As it stands, the UX for this view is very confusing for our users.
Since my view will pull in potentially dozens of groups, with hundreds or thousands of users to email, the ability to select all pages is important to us. I'll look at the code mentioned by bojanz, but again, seems like building something that already exists.
Additionally, the performance issues will probably be an issue for us before long. Actually, I suppose keeping rows per page to a low number will mitigate that to fewer instances, but it's hardly friendly to our group managers.
Sounds like it will be a significant effort to build a 2.x to incorporate these features. Based on this discussion, is the decision to NOT use VBO final?
Comment #7
joachim CreditAttribution: joachim commentedI haven't had time to look at this in the last few weeks.
I did look at this bit a little though. I didn't follow it through all the way before I had to move on to other stuff (project launch!!), but my impression from what I saw was that while it's true that you don't need much code to get the results out of the view, adding a 'select all' or 'email to all' button to allow the user to get to that point was a fair bit of work.
> I'll look at the code mentioned by bojanz, but again, seems like building something that already exists.
That is still my gut feeling.
I think it was on IRC that bojanz said that we'd only be using 5% of VBO's capabilities. And that may be true. But that 5% would save us a TON of code in this module.
Comment #8
hansfn CreditAttribution: hansfn commentedNot at all. I have been in doubt about this many times and I see the UX problem when using Views Send and VBO in the same View.
I have advocated that many of these problems could have been fixed in Views itself for all Views Form implementations, but that doesn't seem to happen so I guess the best move for Views Send is to become a VBO plugin again.
Unfortunately, I'm neither a Views or VBO expert, so I really would appreciate patches. I became a maintainer because I needed a D7 version, not because I had the skills needed ;-)
PS! The switch back to VBO will require some work in the D8 branch too, but that can wait.
Comment #9
joachim CreditAttribution: joachim commentedI got quite far with this in a custom branch. I'll post a patch of how far I got soon, and I'll probably be working on it some more later on.
Comment #10
outlierdavid CreditAttribution: outlierdavid commentedHave you made progress? I've built an email tracking platform based on this module but then later ported to drupal 7 not realizing VBO doesn't work on 7.
It's too late to back out and build email tracking on something else... so I've be happy to share my email tracking module if you can tell me where you are with this.
Comment #11
joachim CreditAttribution: joachim commentedArgh, I've got totally sidetracked on other things.
I'll dig out my branch and post how far I got.
Comment #12
hansfn CreditAttribution: hansfn commentedJust a quick comment: I still don't have any principal objections to this change, but since I don't need it myself I don't work on it. I spend my time working on the D8 version of Views Send which currently is in a working condition.
Comment #13
joachim CreditAttribution: joachim commentedHere's the patch from my branch.
My commit messages seem to indicate it works to some extent ;)
What remains looks like polish and removing the bits of the module code that are now obsolete.
There's also a few enhancements I made on the way, which the maintainer here pointed out should be in follow-ups. Unfortunately that means they need to be picked out of this and put to one side.
Comment #14
pixelsweatshop CreditAttribution: pixelsweatshop commentedPatch in #13 applies cleanly to the latest dev, but I am thinking that since there is a recent release of VBO it's not playing nicely anymore. Can even edit the VBO settings in the view to add the views send, getting An AJAX HTTP error occurred. HTTP Result Code: 500...
Comment #15
pdcrane CreditAttribution: pdcrane commentedI have money to throw at this issue if Joachim or anyone else has time.
Comment #16
joachim CreditAttribution: joachim commented@broxen: send me a message with the contact form.
I will say now though, that what I said in #13 about there being unpicking to do stands, and comment #14 sounds like there's more work to do on top of that. So be warned that I don't think this is a small simple task! :)
Comment #17
djdevin#13 sort of works if you add
to the views_send.info file. not sure why it's necessary since it is defined as a ctools plugin. but this allows you to use the bulk op and send emails.
However none of the tokens replace, including the "to" field. I worked on it a little bit but didn't get anywhere.
Comment #18
pdcrane CreditAttribution: pdcrane commentedJust as a follow up: I implemented an alternate method using Rules and VBO to achieve my desired results.
Comment #19
pixelsweatshop CreditAttribution: pixelsweatshop commented@broxen. Can you share what you did?
Comment #20
pdcrane CreditAttribution: pdcrane commented@nicoz:
My configuration may be unique, as I'm building off CRM Core and the contacts in my View are NOT users as far as drupal is concerned. I originally approached joachim to resolve this issue through funded development, but research led me to an alternative implementation for MY use-case.
I abandoned Views Send and utilized VBO and Rules to email those contacts with CRM Core's email action. VBO provides an option to select All Results on All Pages. My users open the contacts view table, filter, and select the users they'd like to email (selecting on all pages), then click the (CRM Core) email button.
It's worth noting that my brother implemented a similar system (not using CRM Core) on his own site, using a similar configuration.
Send me a PM if my explanation is unclear, as I'm trying not to hijack this issue. But in my own use-case, View Send was superfluous to VBO/Rules.
Comment #21
liquidcms CreditAttribution: liquidcms commentedThe issue with VBO/Rules (in my case anyway) was that as far as i can tell there is no way to send Views field info to a Rule. It can only work with the entity ID and then you need to write code in Rules to pull out what you need, even though it is all readily available in the View result. This was also a limitation in Views Send but it has recently been fixed so that VS can now use the info from the View fields.
Comment #22
nithinkolekar CreditAttribution: nithinkolekar commentedis this patch still in review or is it commited, because with the latest views send , there is a VBO action "Send e-mail".
@hansfn
could you please confirm whether we can/cannot use VBO action and instead use views send's own action?
Comment #23
hansfn CreditAttribution: hansfn as a volunteer commentedThe VBO action "Send e-mail" is not related to Views Send. The current version of Views Send does not use/integrate with VBO.
Comment #24
samerali CreditAttribution: samerali as a volunteer and at Lechleider Mitchell LLC commentedThanks @joachim for your work on this.
I used your patch #13 against the latest version of the module with the latest version from VBO While the patch applies mostly fine (minor chunk for a code comment was not applied) the code was not working and it was not detecting the proper send to/name fields for emails.
I have now created new patch that works for me with additional configuration options to predefine message subject/title @see https://www.screencast.com/t/6745k0hh
I was able to actually send emails through VBO successfully and using the full set of tokens from both views and user objects @see https://www.screencast.com/t/X7sCIVSw
I will keep updating the patch if needed but this works for me so far.
Comment #25
timlie CreditAttribution: timlie as a volunteer commentedPatch #24 works for me, only the, views send tokens, do not get replaced correctly (in a user view with relationship to profile2 fields).
Comment #26
hansfn CreditAttribution: hansfn as a volunteer commentedI appreciate the work that is done in the issue, but I don't have time to do the D7 rewrite properly. People who really needs this change can use the patch provided by samerali in comment 24.
I intend to do the rewrite for the D8 version ;-)
Comment #27
nithinkolekar CreditAttribution: nithinkolekar commentedcould you please commit RTBC pending patches for d7 version?
Comment #28
hansfn CreditAttribution: hansfn as a volunteer commentedYes, as 7.x-2.x. Anyway, none of the current patches has been RTBC or has the need completeness.