fix the nested list bug that makes email not be delivered

[HaidYi]

Fix the bug of nested list for generating the reports. Before calling the function PIPELINE_COMPLETION in main.nf, the pipeline.nf emit the multiqc_report as a list (see below):

tools/nf_core/pipeline-template/workflows/pipeline.nf

Lines 89 to 91 in 930ece5

	emit:
	multiqc_report = MULTIQC.out.report.toList() // channel: /path/to/multiqc_report.html
	versions = ch_versions // channel: [ path(versions.yml) ]

But in the utils_nfcore_pipeline_pipeline, the workflow.onComplete function, the report is double listed:

tools/nf_core/pipeline-template/subworkflows/local/utils_nfcore_pipeline_pipeline/main.nf

Line 137 in 930ece5

completionEmail(summary_params, email, email_on_fail, plaintext_email, outdir, monochrome_logs, multiqc_report.toList())

This nested list make the groovy.text.GStringTemplateEngine cannot correctly render the HTML to send the email (The resulting HTML string is empty):

tools/nf_core/pipeline-template/subworkflows/nf-core/utils_nfcore_pipeline/main.nf

Lines 343 to 344 in 930ece5

	def sendmail_template = engine.createTemplate(sf).make(smail_fields)
	def sendmail_html = sendmail_template.toString()

So, here remove one toList() call to make email back to work. I also test this change and find the email function back to work well after this change.

PR checklist

• This comment contains a description of changes (with reason)
• CHANGELOG.md is updated
• If you've fixed a bug or added code that should be tested, add tests!
• Documentation in docs is updated

[jfy133]

I don't know what the specific intention was with this code so I can't provide a proper review, however I don't think this fix will be accepted in the current state: some pipelines can produce multiple MultiQC files, so I don't think sending the 'first one' is the right behaviour here.

I think it would make more sense that if multiqc_report.toList().size > 1 then zip up all the multiQC files and send that instead... or some similar strategy.

Unfortuntately I have a pressing deadline so I am unable to explore this futher... but maybe someone from @nf-core/infrastructure or @nf-core/maintainers could tackle this?

[HaidYi]

I don't know what the specific intention was with this code so I can't provide a proper review, however I don't think this fix will be accepted in the current state: some pipelines can produce multiple MultiQC files, so I don't think sending the 'first one' is the right behaviour here.

I think it would make more sense that if multiqc_report.toList().size > 1 then zip up all the multiQC files and send that instead... or some similar strategy.

Unfortuntately I have a pressing deadline so I am unable to explore this futher... but maybe someone from @nf-core/infrastructure or @nf-core/maintainers could tackle this?

@jfy133 Its intention is to fix the failure of mailing function. The multiqc_report is performed .toList() two times, which makes the template engine cannot generate the email with multiqc_report as the attachment.

sendmail_template      = engine.createTemplate(sf).make(smail_fields) 
def sendmail_html          = sendmail_template.toString() 

In addition, I agree with that the best way is to zip up all the multiQC reports together and send that instead. But current code in the template workflow only uses the first one.

[jfy133]

Sorry @HaidYi , by the intention I meant the original code not your fix (which I understand the logic!) :)

[HaidYi]

Sorry @HaidYi , by the intention I meant the original code not your fix (which I understand the logic!) :)

@jfy133 Okay, I understand what you mean. But could you test that if the original code in the tools can really send the email? When I test it, I find that it cannot send the email and the tmp email file .sendmail_tmp.html is empty.

[jfy133]

Sorry @HaidYi , by the intention I meant the original code not your fix (which I understand the logic!) :)

@jfy133 Okay, I understand what you mean. But could you test that if the original code in the tools can really send the email? When I test it, I find that it cannot send the email and the tmp email file .sendmail_tmp.html is empty.

Currently not myself as I'm currently part time and have other deadlines.

But maybe someone from @nf-core/infrastructure or @nf-core/maintainers could check this for you?

[HaidYi]

Sorry @HaidYi , by the intention I meant the original code not your fix (which I understand the logic!) :)

@jfy133 Okay, I understand what you mean. But could you test that if the original code in the tools can really send the email? When I test it, I find that it cannot send the email and the tmp email file .sendmail_tmp.html is empty.

Currently not myself as I'm currently part time and have other deadlines.

But maybe someone from @nf-core/infrastructure or @nf-core/maintainers could check this for you?

Sure, that sounds good to me.

[jfy133]

I saw on slack that @mirpedrol was investigating 👍

[jfy133]

@mirpedrol did you make any progress? Or do we need to wait for Phil?

[mirpedrol]

@mirpedrol did you make any progress? Or do we need to wait for Phil?

No, sorry. I couldn't find anyone able to test email sending

[jfy133]

I guess we will need @ewels when he's back... I've not used the email system for a long time now either...

[ewels]

I no longer have access to the system that I used when developing it (which had the email services set up). @mashehu does though (UPPMAX). Maybe @MatthiasZepper might be interested as well?

[MatthiasZepper]

What exactly would you want to test here? And wouldn't Mailcatcher suffice to test that functionality? It is also available as a separate Docker container.