Sending way too many emails!

I'm seeing this as well, though luckily on my dev server using Devel module as the mail gateway, so we're not actively ticking anyone off, yet. ;)

I would've expected that the module would only send off mails for *new* orders since turning it on, but it seems to do them for all of them, each time cron runs.

In uc_followup_cron(), there is a query aptly named $mega_query:

 $mega_query = '
      SELECT o.order_id, fu.* , fus.date, fus.manual, fus.email
      FROM uc_followup fu
      LEFT JOIN uc_orders o ON fu.order_status = o.order_status
      LEFT JOIN (
        SELECT *, COUNT(order_id) as "sent_count"
          FROM uc_followup_sent
          GROUP BY followup_id, order_id
          ORDER BY date DESC 
        ) fus ON fu.followup_id = fus.followup_id AND fus.order_id = o.order_id
      WHERE
      ( 
        (fus.date IS NULL AND (o.modified + (fu.hours_past *3600 )) < %d) OR
        (
          (fu.repeat_after > 0 AND (fus.date + ( fu.repeat_after *3600 )) < %d) AND
          (fu.repeat_max = 0 OR fu.repeat_max < fus.sent_count)
        )
      )
      AND fu.status = 1
    ';
    $result = db_query($mega_query, time(), time());

The issue is in the where clause:

(fus.date IS NULL AND (o.modified + (fu.hours_past *3600 )) < %d) OR

For most orders, o.modified will be a date long in the past. Therefore, it's always going to be less than the current date, and the code will fire for every order assigned to the proper status. :\

This should be switched to BETWEEN logic, ranging from the current time to X hours previous.

I think this will work. Though note that I didn't test the branch utilizing repeat_after, which I think was the original poster's issue. I also put in a minor performance enhancement of only calculating time() once.

With this patch applied, the query returns 3 records for me rather than 231 on the first run. Then on the second run, it returns 0 records, since these records now exist in the uc_followup_sent table.

Commited. Thanks for figuring this out.

Oh, awesome! Thanks for the speedy commits, neochief! :)

I'm super-fast if someone provides patches (rare thing). Thanks anyways :)

I think that the BETWEEN date logic here is flawed. I made a full review of the $mega_query, to get a better understanding of what it's doing. It is the third comment in this issue: #1477036: Reworking uc_followup_cron() and the $mega_query

I came up with a better possible solution to the issue of past orders sending out unwanted followups, which seems to be the real problem in THIS issue. See my other comment on the thread mentioned above here: http://drupal.org/node/1477036#comment-5751218