Send PDF via e-mail in StormInvoice fails to attach invoice

Just a few queries:
- From the problematic installation, does it work if you send to a different email address?
- Does it work if you send a different invoice?
- Are there any log entries, error messages, etc etc etc?

Changing status.

OK, a few more questions...
- Have you had this working on your installation before?
- Which other modules do you have enabled?
- Are you using the default drupal mail handlers, i.e. are you using SMTP module, or anything like that?
- If you use the demo site (which works for me), at the same email address - does that work?
- The headers above indicate a multipart message - of which the first is the plaintext covernote - is there anything in the second part?

Also - has anyone else had this problem?

Lets wait to see if anyone else is having this issue too then - I can't see from your post any reason why it should go wrong in this way.

The code is mainly in storminvoice.admin.inc (from memory - hope that's correct).

I've tested this on latest dev using Drupal 6.13 and get the same issue.

1) I've never used the feature before so cannot say if it's ever worked before
2) Administration Menu, Advanced Help, CCK, Date, Devel, Node Import, Popups API, Storm, Storm QT, Views
3) Default mail handlers (it doesn't seem to like SMTP Auth module)
4) Not tired demo site
5) Same as above no second part
6) Yes me :)

I'll have a play at some point and see if I can narrow it down.

Cheers
Mark

Okay, I think I see where this is stumbling up....

In storminvoice.admin.inc the function that is called when submitting the send is storminvoice_send_form_submit.

This function gets a variable for the PDF File Name $pdffile = variable_get('storminvoice_email_pdf_file_path', ''); and checks it before attaching the file if($pdffile){. However this never gets set until the function _storminvoice_report_file_pdf is called!

Quick work-around rem out the check and send two test PDFs the second should come through okay

  //if($pdffile){
    $params['body']  .= "--$trenner"."\n";
    $params['body']  .= "Content-Type:$pdffile_mime; name='$pdffile_name'\n";
    $params['body']  .= "Content-Disposition: attachment; filename='$pdffile_name'\n";
    $params['body']  .= "Content-Transfer-Encoding: base64\n\n";

    $node =  $form['#parameters'][3];
    $language = $form['#parameters'][4];
    $filedata = _storminvoice_report_file_pdf($node, $language);

    $params['body']  .= chunk_split(base64_encode($filedata));
  //}

After removing the check, the first time you send the PDF the variable will get set. The second time you send it it's correctly there to attach!

I haven't got time to write the patch at the mo and have no idea how this is working on a fresh system as I can't spot where those variables are being set without calling that function.

Thanks
Mark

those variables are set in function _storminvoice_report_file_pdf at the end of storminvoice.admin.inc file.
i will, also, search for solution. i will, probable, need some time to catch up with other things, but this is high on mine todo list.

OK - it looks like I had the variables left in the system. On manually removing one of the variables, I get back to the situation that everyone else is having.

I wonder if this is due to the changes we made to the function prior to putting it in regarding not saving the invoice in the filesystem prior to emailing?

@homoludens, yeah kinda figured that with my posting in #8 :)

Seriously though I don't think this will take much getting around, it's just wrong logic at the moment.

However it did point out to me that we're not checking the filename that's constructed. It should really be checked for invalid characters etc as per transliteration (http://drupal.org/project/transliteration)

@magnity: what were the changes, at first glance I'd say that we'd just need to check that we can PDF, but would need to take more time with it before confirming.

Cheers
Mark

See #450360: Automatic E-Mail Invoicing comments #19 - #21.

Changing the function to:

function storminvoice_send_form_submit($form, &$form_state) {
  $form_filled = $form_state['clicked_button']['#post'];

  $to = $form_filled['recipients'];
  $language = null;
  $from = $form_state['values']['yname'].'<'.$form_state['values']['yemail'].'>';

  $params = array();

  $params['subject'] = $form_filled['subject'];
  $params['bcc'] = $form_state['values']['bcc'];

  $trenner  = md5(uniqid(time()));
  variable_set('storminvoice_email_pdf_trenner', $trenner);

  $headers['Content-Type'] = "multipart/mixed; boundary=$trenner";
  $params['body']  = "\n--$trenner\n";
  $params['body'] .= "Content-Type: text/plain; charset=UTF-8; format=flowed;"."\n\n"; // sets the mime type
  $params['body'] .= $form_filled['body']."\n";
  $params['body'] .= "\n\n";
  $params['body'] .= "\n\n";

  $node =  $form['#parameters'][3];
  $language = $form['#parameters'][4];
  $filedata = _storminvoice_report_file_pdf($node, $language);

  $pdffile = variable_get('storminvoice_email_pdf_file_path', '');
  $pdffile_name = variable_get('storminvoice_email_pdf_file_name', '');
  $pdffile_mime = file_get_mimetype($pdffile);
  $params['body']  .= "--$trenner"."\n";
  $params['body']  .= "Content-Type:$pdffile_mime; name='$pdffile_name'\n";
  $params['body']  .= "Content-Disposition: attachment; filename='$pdffile_name'\n";
  $params['body']  .= "Content-Transfer-Encoding: base64\n\n";
  $params['body']  .= chunk_split(base64_encode($filedata));
  $params['body'] .= "--$trenner--";

  drupal_mail('storminvoice', 'invoice', $to, $language, $params, $from , True);

  drupal_set_message(t(variable_get('storminvoice_sent', 'You have sent invoice to !email.'),array('!email' => $to )));
}

Seems to have done it for me, i.e. getting rid of the if and moving the PDF generation to the top.

I'm going to work on abstracting the PDF transformation so we're not duplicating code.

I think @homoludens knows this bit of the code best - so i'd like to get a review from him. I'll be testing the code changes myself later on today or tomorrow.

Committed that fix.

A couple of issue sprouting from here though:
- #553556: Combine PDF invoice generation functions
- #553560: Tax fields blank in PDF invoice sent by email