From a350176cf8f02f4a0acec2f324a1922f3ff5c2c3 Mon Sep 17 00:00:00 2001
From: Edwin Flores <Edwin.Flores@man.com>
Date: Tue, 15 Dec 2020 17:24:43 +0000
Subject: [PATCH 1/2] Add option to hide code from emails and PDF reports

---
 notebooker/_entrypoints.py                    |  3 ++
 notebooker/constants.py                       | 14 ++++++-
 notebooker/execute_notebook.py                | 11 +++++-
 .../nbtemplates/notebooker_html_output.tpl    | 18 ++++++++-
 .../nbtemplates/notebooker_pdf_output.tplx    |  7 ++++
 notebooker/serialization/mongo.py             |  6 +++
 notebooker/utils/conversion.py                | 14 +++++--
 notebooker/utils/notebook_execution.py        |  2 +-
 notebooker/web/routes/run_report.py           | 11 +++++-
 notebooker/web/templates/run_report.html      | 10 ++++-
 tests/integration/test_e2e.py                 | 38 ++++++++++++++++++-
 tests/integration/test_report_hunter.py       |  2 +
 .../utils/test_utils_notebook_execution.py    | 30 +++++++++++++++
 13 files changed, 155 insertions(+), 11 deletions(-)
 create mode 100644 notebooker/nbtemplates/notebooker_pdf_output.tplx

diff --git a/notebooker/_entrypoints.py b/notebooker/_entrypoints.py
index 309d471b..93e54b7a 100644
--- a/notebooker/_entrypoints.py
+++ b/notebooker/_entrypoints.py
@@ -125,6 +125,7 @@ def start_webapp(config: BaseConfig, port, logging_level, debug, base_cache_dir)
 )
 @click.option("--mailto", default="", help="A comma-separated list of email addresses which will receive results.")
 @click.option("--pdf-output/--no-pdf-output", default=True, help="Whether we generate PDF output or not.")
+@click.option("--hide-code/--show-code", default=False, help="Hide code from email and PDF output.")
 @click.option(
     "--prepare-notebook-only",
     is_flag=True,
@@ -141,6 +142,7 @@ def execute_notebook(
     job_id,
     mailto,
     pdf_output,
+    hide_code,
     prepare_notebook_only,
 ):
     if report_name is None:
@@ -155,6 +157,7 @@ def execute_notebook(
         job_id,
         mailto,
         pdf_output,
+        hide_code,
         prepare_notebook_only,
     )
 
diff --git a/notebooker/constants.py b/notebooker/constants.py
index 345da3fb..99f76382 100644
--- a/notebooker/constants.py
+++ b/notebooker/constants.py
@@ -75,6 +75,7 @@ class NotebookResultBase(object):
     overrides = attr.ib(default=attr.Factory(dict))
     mailto = attr.ib(default="")
     generate_pdf_output = attr.ib(default=True)
+    hide_code = attr.ib(default=False)
     stdout = attr.ib(default=attr.Factory(list))
 
     def saveable_output(self):
@@ -91,6 +92,7 @@ class NotebookResultPending(NotebookResultBase):
     overrides = attr.ib(default=attr.Factory(dict))
     mailto = attr.ib(default="")
     generate_pdf_output = attr.ib(default=True)
+    hide_code = attr.ib(default=False)
 
 
 @attr.s()
@@ -102,6 +104,7 @@ class NotebookResultError(NotebookResultBase):
     overrides = attr.ib(default=attr.Factory(dict))
     mailto = attr.ib(default="")
     generate_pdf_output = attr.ib(default=True)
+    hide_code = attr.ib(default=False)
 
     @property
     def raw_html(self):
@@ -109,6 +112,10 @@ def raw_html(self):
             self.error_info
         )
 
+    @property
+    def email_html(self):
+        return self.raw_html
+
 
 @attr.s(repr=False)
 class NotebookResultComplete(NotebookResultBase):
@@ -118,12 +125,14 @@ class NotebookResultComplete(NotebookResultBase):
     status = attr.ib(default=JobStatus.DONE)
     raw_ipynb_json = attr.ib(default="")
     raw_html = attr.ib(default="")
+    email_html = attr.ib(default="")
     update_time = attr.ib(default=datetime.datetime.now())
     pdf = attr.ib(default="")
     report_title = attr.ib(default="")
     overrides = attr.ib(default=attr.Factory(dict))
     mailto = attr.ib(default="")
     generate_pdf_output = attr.ib(default=True)
+    hide_code = attr.ib(default=False)
     stdout = attr.ib(default=attr.Factory(list))
 
     def html_resources(self):
@@ -143,6 +152,7 @@ def saveable_output(self):
             "report_name": self.report_name,
             "report_title": self.report_title,
             "raw_html": self.raw_html,
+            "email_html": self.email_html,
             "raw_html_resources": self.html_resources(),
             "job_id": self.job_id,
             "job_start_time": self.job_start_time,
@@ -150,6 +160,7 @@ def saveable_output(self):
             "mailto": self.mailto,
             "overrides": self.overrides,
             "generate_pdf_output": self.generate_pdf_output,
+            "hide_code": self.hide_code,
             "update_time": self.update_time,
         }
 
@@ -158,7 +169,7 @@ def __repr__(self):
             "NotebookResultComplete(job_id={job_id}, status={status}, report_name={report_name}, "
             "job_start_time={job_start_time}, job_finish_time={job_finish_time}, update_time={update_time}, "
             "report_title={report_title}, overrides={overrides}, mailto={mailto}, "
-            "generate_pdf_output={generate_pdf_output})".format(
+            "generate_pdf_output={generate_pdf_output}, hide_code={hide_code})".format(
                 job_id=self.job_id,
                 status=self.status,
                 report_name=self.report_name,
@@ -169,5 +180,6 @@ def __repr__(self):
                 overrides=self.overrides,
                 mailto=self.mailto,
                 generate_pdf_output=self.generate_pdf_output,
+                hide_code=self.hide_code,
             )
         )
diff --git a/notebooker/execute_notebook.py b/notebooker/execute_notebook.py
index aa09d41b..f596e33e 100644
--- a/notebooker/execute_notebook.py
+++ b/notebooker/execute_notebook.py
@@ -37,6 +37,7 @@ def _run_checks(
     template_base_dir: str,
     overrides: Dict[AnyStr, Any],
     generate_pdf_output: Optional[bool] = True,
+    hide_code: Optional[bool] = False,
     mailto: Optional[str] = "",
     prepare_only: Optional[bool] = False,
     notebooker_disable_git: bool = False,
@@ -101,7 +102,8 @@ def _run_checks(
 
     logger.info("Saving output notebook as HTML from {}".format(ipynb_executed_path))
     html, resources = ipython_to_html(ipynb_executed_path, job_id)
-    pdf = ipython_to_pdf(raw_executed_ipynb, report_title) if generate_pdf_output else ""
+    email_html, resources = ipython_to_html(ipynb_executed_path, job_id, hide_code=hide_code)
+    pdf = ipython_to_pdf(raw_executed_ipynb, report_title, hide_code=hide_code) if generate_pdf_output else ""
 
     notebook_result = NotebookResultComplete(
         job_id=job_id,
@@ -110,6 +112,7 @@ def _run_checks(
         raw_html_resources=resources,
         raw_ipynb_json=raw_executed_ipynb,
         raw_html=html,
+        email_html=email_html,
         mailto=mailto,
         pdf=pdf,
         generate_pdf_output=generate_pdf_output,
@@ -132,6 +135,7 @@ def run_report(
     attempts_remaining=2,
     mailto="",
     generate_pdf_output=True,
+    hide_code=False,
     prepare_only=False,
     notebooker_disable_git=False,
     py_template_base_dir="",
@@ -164,6 +168,7 @@ def run_report(
             overrides,
             mailto=mailto,
             generate_pdf_output=generate_pdf_output,
+            hide_code=hide_code,
             prepare_only=prepare_only,
             notebooker_disable_git=notebooker_disable_git,
             py_template_base_dir=py_template_base_dir,
@@ -206,6 +211,7 @@ def run_report(
                 attempts_remaining=attempts_remaining - 1,
                 mailto=mailto,
                 generate_pdf_output=generate_pdf_output,
+                hide_code=hide_code,
                 prepare_only=prepare_only,
                 notebooker_disable_git=notebooker_disable_git,
                 py_template_base_dir=py_template_base_dir,
@@ -292,6 +298,7 @@ def execute_notebook_entrypoint(
     job_id: str,
     mailto: str,
     pdf_output: bool,
+    hide_code: bool,
     prepare_notebook_only: bool,
 ):
     report_title = report_title or report_name
@@ -313,6 +320,7 @@ def execute_notebook_entrypoint(
     logger.info("template_dir = %s", template_dir)
     logger.info("mailto = %s", mailto)
     logger.info("pdf_output = %s", pdf_output)
+    logger.info("hide_code = %s", hide_code)
     logger.info("prepare_notebook_only = %s", prepare_notebook_only)
     logger.info("notebooker_disable_git = %s", notebooker_disable_git)
     logger.info("py_template_base_dir = %s", py_template_base_dir)
@@ -336,6 +344,7 @@ def execute_notebook_entrypoint(
             attempts_remaining=n_retries - 1,
             mailto=mailto,
             generate_pdf_output=pdf_output,
+            hide_code=hide_code,
             prepare_only=prepare_notebook_only,
             notebooker_disable_git=notebooker_disable_git,
             py_template_base_dir=py_template_base_dir,
diff --git a/notebooker/nbtemplates/notebooker_html_output.tpl b/notebooker/nbtemplates/notebooker_html_output.tpl
index a662ed80..f868ca65 100644
--- a/notebooker/nbtemplates/notebooker_html_output.tpl
+++ b/notebooker/nbtemplates/notebooker_html_output.tpl
@@ -16,6 +16,16 @@
 }
 
 </style>
+
+<!--[if mso]>
+<style type="text/css">
+div div.cell {
+  border-style: none;
+  margin-top: 4;
+}
+</style>
+<![endif]-->
+
 <script>
 (function() {
   function addToggleCodeButton() {
@@ -63,4 +73,10 @@
   document.addEventListener('DOMContentLoaded', addToggleCodeButton);
 }());
 </script>
-{%- endblock html_head -%}
\ No newline at end of file
+{%- endblock html_head -%}
+
+{% block stream %}
+  {%- if resources.global_content_filter.include_output_prompt -%}
+    {{ super() }}
+  {%- endif -%}
+{%- endblock stream %}
diff --git a/notebooker/nbtemplates/notebooker_pdf_output.tplx b/notebooker/nbtemplates/notebooker_pdf_output.tplx
new file mode 100644
index 00000000..f40fe967
--- /dev/null
+++ b/notebooker/nbtemplates/notebooker_pdf_output.tplx
@@ -0,0 +1,7 @@
+((* extends 'article.tplx' *))
+
+((*- block stream -*))
+  ((*- if resources.global_content_filter.include_output_prompt -*))
+    ((( super() )))
+  ((*- endif -*))
+((*- endblock stream -*))
diff --git a/notebooker/serialization/mongo.py b/notebooker/serialization/mongo.py
index 0a181f93..e6babfa0 100644
--- a/notebooker/serialization/mongo.py
+++ b/notebooker/serialization/mongo.py
@@ -97,6 +97,7 @@ def save_check_stub(
         overrides: Optional[Dict] = None,
         mailto: str = "",
         generate_pdf_output: bool = True,
+        hide_code: bool = False,
     ) -> None:
         """ Call this when we are just starting a check. Saves a "pending" job into storage. """
         job_start_time = job_start_time or datetime.datetime.now()
@@ -110,6 +111,7 @@ def save_check_stub(
             mailto=mailto,
             generate_pdf_output=generate_pdf_output,
             overrides=overrides or {},
+            hide_code=hide_code,
         )
         self._save_to_db(pending_result)
 
@@ -176,11 +178,13 @@ def read_file(path):
                 raw_html_resources=result.get("raw_html_resources", {}),
                 raw_ipynb_json=result.get("raw_ipynb_json"),
                 raw_html=result.get("raw_html"),
+                email_html=result.get("email_html"),
                 pdf=result.get("pdf", ""),
                 overrides=result.get("overrides", {}),
                 generate_pdf_output=result.get("generate_pdf_output", True),
                 report_title=result.get("report_title", result["report_name"]),
                 mailto=result.get("mailto", ""),
+                hide_code=result.get("hide_code", False),
                 stdout=result.get("stdout", []),
             )
         elif cls == NotebookResultPending:
@@ -194,6 +198,7 @@ def read_file(path):
                 generate_pdf_output=result.get("generate_pdf_output", True),
                 report_title=result.get("report_title", result["report_name"]),
                 mailto=result.get("mailto", ""),
+                hide_code=result.get("hide_code", False),
                 stdout=result.get("stdout", []),
             )
 
@@ -209,6 +214,7 @@ def read_file(path):
                 generate_pdf_output=result.get("generate_pdf_output", True),
                 report_title=result.get("report_title", result["report_name"]),
                 mailto=result.get("mailto", ""),
+                hide_code=result.get("hide_code", False),
                 stdout=result.get("stdout", []),
             )
         else:
diff --git a/notebooker/utils/conversion.py b/notebooker/utils/conversion.py
index 9ec79831..1cc57518 100644
--- a/notebooker/utils/conversion.py
+++ b/notebooker/utils/conversion.py
@@ -19,12 +19,14 @@ def get_resources_dir(job_id):
     return "{}/resources".format(job_id)
 
 
-def ipython_to_html(ipynb_path: str, job_id: str) -> (nbformat.NotebookNode, Dict[str, Any]):
+def ipython_to_html(ipynb_path: str, job_id: str, hide_code: bool = False) -> (nbformat.NotebookNode, Dict[str, Any]):
     c = Config()
     c.HTMLExporter.preprocessors = ["nbconvert.preprocessors.ExtractOutputPreprocessor"]
     c.HTMLExporter.template_file = pkg_resources.resource_filename(
         __name__, "../nbtemplates/notebooker_html_output.tpl"
     )
+    c.HTMLExporter.exclude_input = hide_code
+    c.HTMLExporter.exclude_output_prompt = hide_code
     html_exporter_with_figs = HTMLExporter(config=c)
 
     with open(ipynb_path, "r") as nb_file:
@@ -34,8 +36,14 @@ def ipython_to_html(ipynb_path: str, job_id: str) -> (nbformat.NotebookNode, Dic
     return html, resources
 
 
-def ipython_to_pdf(raw_executed_ipynb: str, report_title: str) -> AnyStr:
-    pdf_exporter = PDFExporter(Config())
+def ipython_to_pdf(raw_executed_ipynb: str, report_title: str, hide_code: bool = False) -> AnyStr:
+    c = Config()
+    c.PDFExporter.exclude_input = hide_code
+    c.PDFExporter.exclude_output_prompt = hide_code
+    c.HTMLExporter.template_file = pkg_resources.resource_filename(
+        __name__, "../nbtemplates/notebooker_pdf_output.tplx"
+    )
+    pdf_exporter = PDFExporter(c)
     resources = ResourcesDict()
     resources["metadata"] = ResourcesDict()
     resources["metadata"]["name"] = report_title
diff --git a/notebooker/utils/notebook_execution.py b/notebooker/utils/notebook_execution.py
index ab1303e8..12636ae4 100644
--- a/notebooker/utils/notebook_execution.py
+++ b/notebooker/utils/notebook_execution.py
@@ -22,7 +22,7 @@ def send_result_email(result: Union[NotebookResultComplete, NotebookResultError]
         result.report_title.decode("utf-8") if isinstance(result.report_title, bytes) else result.report_title
     )
     subject = "Notebooker: {} report completed with status: {}".format(report_title, result.status.value)
-    body = result.raw_html
+    body = result.email_html or result.raw_html
     attachments = []
     tmp_dir = None
     try:
diff --git a/notebooker/web/routes/run_report.py b/notebooker/web/routes/run_report.py
index 77ff09b3..4eedf3c6 100644
--- a/notebooker/web/routes/run_report.py
+++ b/notebooker/web/routes/run_report.py
@@ -116,7 +116,9 @@ def _monitor_stderr(process, job_id, serializer_cls, serializer_args):
     return "".join(stderr)
 
 
-def run_report(report_name, report_title, mailto, overrides, generate_pdf_output=False, prepare_only=False):
+def run_report(
+    report_name, report_title, mailto, overrides, hide_code=False, generate_pdf_output=False, prepare_only=False
+):
     """
     Actually run the report in earnest.
     Uses a subprocess to execute the report asynchronously, which is identical to the non-webapp entrypoint.
@@ -140,6 +142,7 @@ def run_report(report_name, report_title, mailto, overrides, generate_pdf_output
         overrides=overrides,
         mailto=mailto,
         generate_pdf_output=generate_pdf_output,
+        hide_code=hide_code,
     )
     app_config = current_app.config
     p = subprocess.Popen(
@@ -170,6 +173,7 @@ def run_report(report_name, report_title, mailto, overrides, generate_pdf_output
             "--overrides-as-json",
             json.dumps(overrides),
             "--pdf-output" if generate_pdf_output else "--no-pdf-output",
+            "--hide-code" if hide_code else "--show-code",
         ]
         + (["--prepare-notebook-only"] if prepare_only else []),
         stderr=subprocess.PIPE,
@@ -192,10 +196,13 @@ def _handle_run_report(
     mailto = validate_mailto(request.values.get("mailto"), issues)
     # Find whether to generate PDF output
     generate_pdf_output = validate_generate_pdf_output(request.values.get("generatepdf"), issues)
+    hide_code = request.values.get("hide_code") == "on"
     if issues:
         return jsonify({"status": "Failed", "content": ("\n".join(issues))})
     report_name = convert_report_name_url_to_path(report_name)
-    job_id = run_report(report_name, report_title, mailto, overrides_dict, generate_pdf_output=generate_pdf_output)
+    job_id = run_report(
+        report_name, report_title, mailto, overrides_dict, generate_pdf_output=generate_pdf_output, hide_code=hide_code
+    )
     return (
         jsonify({"id": job_id}),
         202,  # HTTP Accepted code
diff --git a/notebooker/web/templates/run_report.html b/notebooker/web/templates/run_report.html
index 88284ca7..a91d874b 100644
--- a/notebooker/web/templates/run_report.html
+++ b/notebooker/web/templates/run_report.html
@@ -46,7 +46,15 @@ <h3>Email to:</h3>
                     <label>Generate PDF output</label>
                 </div>
             </div>
-            <button class="positive ui button" id="runReportButton">Run Report</button>
+            <div class="ui checkbox container">
+                <input type="checkbox"
+                       name="hide_code"
+                       id="hide_code"/>
+                <label for="hide_code">Hide code from email and PDF output</label>
+            </div>
+            <div class="ui">
+                <button class="positive ui button" id="runReportButton">Run Report</button>
+            </div>
             <div class="ui right aligned container" id="parametersDimmer" style="display:none">
                 <div class="ui active inline loader"></div>
                 <span>Converting your parameters to JSON...</span>
diff --git a/tests/integration/test_e2e.py b/tests/integration/test_e2e.py
index 8d8037ca..655d1d4b 100644
--- a/tests/integration/test_e2e.py
+++ b/tests/integration/test_e2e.py
@@ -47,14 +47,20 @@ def setup_workspace(workspace):
     report_to_run.write_lines(DUMMY_REPORT.split("\n"))
 
 
-def _check_report_output(job_id, serialiser, **kwargs):
+def _get_report_output(job_id, serialiser):
     while True:
         result = serialiser.get_check_result(job_id)
         if result.status not in (JobStatus.PENDING, JobStatus.SUBMITTED):
             break
+    return result
+
+
+def _check_report_output(job_id, serialiser, **kwargs):
+    result = _get_report_output(job_id, serialiser)
     assert result.status == JobStatus.DONE, result.error_info
     assert result.stdout != ""
     assert result.raw_html
+    assert result.email_html
     assert result.raw_ipynb_json
     assert result.pdf == ""
     assert result.job_start_time < result.job_finish_time
@@ -74,6 +80,11 @@ def test_run_report(bson_library, flask_app, setup_and_cleanup_notebooker_filesy
         _check_report_output(
             job_id, serialiser, overrides=overrides, report_name=report_name, report_title=report_title, mailto=mailto
         )
+
+        result = _get_report_output(job_id, serialiser)
+        assert 'DataFrame' in result.email_html
+        assert 'DataFrame' in result.raw_html
+
         assert job_id == serialiser.get_latest_job_id_for_name_and_params(report_name, overrides)
         assert job_id == serialiser.get_latest_job_id_for_name_and_params(report_name, None)
         assert job_id == serialiser.get_latest_successful_job_id_for_name_and_params(report_name, overrides)
@@ -113,3 +124,28 @@ def test_run_report_and_rerun(bson_library, flask_app, setup_and_cleanup_noteboo
         assert not {job_id, new_job_id} - set(serialiser.get_all_job_ids_for_name_and_params(report_name, overrides))
         assert new_job_id == serialiser.get_latest_successful_job_id_for_name_and_params(report_name, overrides)
         assert job_id != serialiser.get_latest_successful_job_id_for_name_and_params(report_name, overrides)
+
+
+@freezegun.freeze_time(datetime.datetime(2018, 1, 12))
+def test_run_report(bson_library, flask_app, setup_and_cleanup_notebooker_filesystem, setup_workspace):
+    with flask_app.app_context():
+        serialiser = get_serializer()
+        overrides = {"n_points": 5}
+        report_name = "fake/report"
+        report_title = "my report title"
+        mailto = "jon@fakeemail.com"
+        job_id = run_report(
+            report_name, report_title, mailto, overrides, hide_code=True, generate_pdf_output=False, prepare_only=True
+        )
+        _check_report_output(
+            job_id, serialiser, overrides=overrides, report_name=report_name, report_title=report_title, mailto=mailto
+        )
+
+        result = _get_report_output(job_id, serialiser)
+        assert 'DataFrame' not in result.email_html
+        assert 'DataFrame' in result.raw_html
+
+        assert job_id == serialiser.get_latest_job_id_for_name_and_params(report_name, overrides)
+        assert job_id == serialiser.get_latest_job_id_for_name_and_params(report_name, None)
+        assert job_id == serialiser.get_latest_successful_job_id_for_name_and_params(report_name, overrides)
+        assert job_id == serialiser.get_latest_successful_job_id_for_name_and_params(report_name, None)
diff --git a/tests/integration/test_report_hunter.py b/tests/integration/test_report_hunter.py
index 1489f955..9ce3e065 100644
--- a/tests/integration/test_report_hunter.py
+++ b/tests/integration/test_report_hunter.py
@@ -164,6 +164,7 @@ def test_report_hunter_pending_to_done(bson_library, webapp_config):
             pdf="",
             raw_ipynb_json="[]",
             raw_html="",
+            email_html="",
         )
         _report_hunter(webapp_config=webapp_config, run_once=True)
 
@@ -176,6 +177,7 @@ def test_report_hunter_pending_to_done(bson_library, webapp_config):
             job_start_time=datetime.datetime(2018, 1, 12, 2, 30),
             job_finish_time=datetime.datetime(2018, 1, 12, 2, 37),
             raw_html="",
+            email_html="",
             raw_html_resources={"outputs": {}},
             raw_ipynb_json="[]",
         )
diff --git a/tests/unit/utils/test_utils_notebook_execution.py b/tests/unit/utils/test_utils_notebook_execution.py
index eaa6b447..109036ab 100644
--- a/tests/unit/utils/test_utils_notebook_execution.py
+++ b/tests/unit/utils/test_utils_notebook_execution.py
@@ -38,6 +38,7 @@ def test_send_result_email(test_name, job_id, report_name, report_title, expecte
         raw_html_resources={},
         raw_ipynb_json={},
         raw_html=body_in,
+        email_html=body_in,
         pdf="",
         report_name=report_name,
         report_title=report_title,
@@ -56,3 +57,32 @@ def test_send_result_email(test_name, job_id, report_name, report_title, expecte
     assert to_address == to_email
     assert title == expected_title
     assert body == ["Please either activate HTML emails, or see the PDF attachment.", body_in]
+
+
+def test_send_result_email_fallback_to_raw_html():
+    body_in = "<body><h1>hello  😆 😉 😊 😋 😎</h1></body>"
+    result = NotebookResultComplete(
+        job_id="fallback",
+        job_start_time=datetime.datetime.now(),
+        job_finish_time=datetime.datetime.now(),
+        raw_html_resources={},
+        raw_ipynb_json={},
+        raw_html=body_in,
+        pdf="",
+        report_name="fallback",
+        report_title="fallback",
+    )
+    to_email = "∫åññîsté®@test-email.com"
+    with mock.patch("notebooker.utils.notebook_execution.mail") as mail:
+        notebook_execution.send_result_email(result, to_email)
+    email_sent = mail.mock_calls[0][1]
+    assert len(email_sent) == 4, "mail() was not called with the correct number of args"
+    from_address = email_sent[0]
+    to_address = email_sent[1]
+    title = email_sent[2]
+    body = email_sent[3]
+
+    assert from_address == "notebooker@notebooker.io"
+    assert to_address == to_email
+    assert title == "Notebooker: fallback report completed with status: Checks done!"
+    assert body == ["Please either activate HTML emails, or see the PDF attachment.", body_in]

From 266e40c0a8b636b75135d2e26ad85e53c15da0ad Mon Sep 17 00:00:00 2001
From: Edwin Flores <Edwin.Flores@man.com>
Date: Tue, 15 Dec 2020 18:06:27 +0000
Subject: [PATCH 2/2] Add options to specify subject and a separate list of
 email addresses for errors

---
 notebooker/_entrypoints.py                    | 10 ++++
 notebooker/constants.py                       |  9 +++-
 notebooker/execute_notebook.py                | 18 ++++++-
 notebooker/utils/notebook_execution.py        |  8 +--
 tests/integration/test_execute_notebook.py    | 44 +++++++++++++++++
 .../utils/test_utils_notebook_execution.py    | 49 +++++++++++++++++--
 6 files changed, 127 insertions(+), 11 deletions(-)

diff --git a/notebooker/_entrypoints.py b/notebooker/_entrypoints.py
index 93e54b7a..b73e8eac 100644
--- a/notebooker/_entrypoints.py
+++ b/notebooker/_entrypoints.py
@@ -124,6 +124,12 @@ def start_webapp(config: BaseConfig, port, logging_level, debug, base_cache_dir)
     help="The unique job ID for this notebook. Can be non-unique, but note that you will overwrite history.",
 )
 @click.option("--mailto", default="", help="A comma-separated list of email addresses which will receive results.")
+@click.option(
+    "--error-mailto",
+    default="",
+    help="A comma-separated list of email addresses which will receive errors. Deafults to --mailto argument."
+)
+@click.option("--email-subject", default="", help="The subject of the email sent on a successful result.")
 @click.option("--pdf-output/--no-pdf-output", default=True, help="Whether we generate PDF output or not.")
 @click.option("--hide-code/--show-code", default=False, help="Hide code from email and PDF output.")
 @click.option(
@@ -141,6 +147,8 @@ def execute_notebook(
     n_retries,
     job_id,
     mailto,
+    error_mailto,
+    email_subject,
     pdf_output,
     hide_code,
     prepare_notebook_only,
@@ -156,6 +164,8 @@ def execute_notebook(
         n_retries,
         job_id,
         mailto,
+        error_mailto,
+        email_subject,
         pdf_output,
         hide_code,
         prepare_notebook_only,
diff --git a/notebooker/constants.py b/notebooker/constants.py
index 99f76382..3c75fb7d 100644
--- a/notebooker/constants.py
+++ b/notebooker/constants.py
@@ -106,6 +106,10 @@ class NotebookResultError(NotebookResultBase):
     generate_pdf_output = attr.ib(default=True)
     hide_code = attr.ib(default=False)
 
+    @property
+    def email_subject(self):
+        return ""
+
     @property
     def raw_html(self):
         return """<p>This job resulted in an error: <br/><code style="white-space: pre-wrap;">{}</code></p>""".format(
@@ -131,6 +135,7 @@ class NotebookResultComplete(NotebookResultBase):
     report_title = attr.ib(default="")
     overrides = attr.ib(default=attr.Factory(dict))
     mailto = attr.ib(default="")
+    email_subject = attr.ib(default="")
     generate_pdf_output = attr.ib(default=True)
     hide_code = attr.ib(default=False)
     stdout = attr.ib(default=attr.Factory(list))
@@ -158,6 +163,7 @@ def saveable_output(self):
             "job_start_time": self.job_start_time,
             "job_finish_time": self.job_finish_time,
             "mailto": self.mailto,
+            "email_subject": self.email_subject,
             "overrides": self.overrides,
             "generate_pdf_output": self.generate_pdf_output,
             "hide_code": self.hide_code,
@@ -169,7 +175,7 @@ def __repr__(self):
             "NotebookResultComplete(job_id={job_id}, status={status}, report_name={report_name}, "
             "job_start_time={job_start_time}, job_finish_time={job_finish_time}, update_time={update_time}, "
             "report_title={report_title}, overrides={overrides}, mailto={mailto}, "
-            "generate_pdf_output={generate_pdf_output}, hide_code={hide_code})".format(
+            "email_subject={email_subject}, generate_pdf_output={generate_pdf_output}, hide_code={hide_code})".format(
                 job_id=self.job_id,
                 status=self.status,
                 report_name=self.report_name,
@@ -179,6 +185,7 @@ def __repr__(self):
                 report_title=self.report_title,
                 overrides=self.overrides,
                 mailto=self.mailto,
+                email_subject=self.email_subject,
                 generate_pdf_output=self.generate_pdf_output,
                 hide_code=self.hide_code,
             )
diff --git a/notebooker/execute_notebook.py b/notebooker/execute_notebook.py
index f596e33e..2ccd8b93 100644
--- a/notebooker/execute_notebook.py
+++ b/notebooker/execute_notebook.py
@@ -39,6 +39,8 @@ def _run_checks(
     generate_pdf_output: Optional[bool] = True,
     hide_code: Optional[bool] = False,
     mailto: Optional[str] = "",
+    error_mailto: Optional[str] = "",
+    email_subject: Optional[str] = "",
     prepare_only: Optional[bool] = False,
     notebooker_disable_git: bool = False,
     py_template_base_dir: str = "",
@@ -114,6 +116,7 @@ def _run_checks(
         raw_html=html,
         email_html=email_html,
         mailto=mailto,
+        email_subject=email_subject,
         pdf=pdf,
         generate_pdf_output=generate_pdf_output,
         report_name=template_name,
@@ -134,6 +137,8 @@ def run_report(
     template_base_dir=None,
     attempts_remaining=2,
     mailto="",
+    error_mailto="",
+    email_subject="",
     generate_pdf_output=True,
     hide_code=False,
     prepare_only=False,
@@ -167,6 +172,7 @@ def run_report(
             template_base_dir,
             overrides,
             mailto=mailto,
+            email_subject=email_subject,
             generate_pdf_output=generate_pdf_output,
             hide_code=hide_code,
             prepare_only=prepare_only,
@@ -187,7 +193,7 @@ def run_report(
             report_title=report_title,
             error_info=error_info,
             overrides=overrides,
-            mailto=mailto,
+            mailto=error_mailto or mailto,
             generate_pdf_output=generate_pdf_output,
         )
         logger.error(
@@ -210,6 +216,8 @@ def run_report(
                 template_base_dir=template_base_dir,
                 attempts_remaining=attempts_remaining - 1,
                 mailto=mailto,
+                error_mailto=error_mailto,
+                email_subject=email_subject,
                 generate_pdf_output=generate_pdf_output,
                 hide_code=hide_code,
                 prepare_only=prepare_only,
@@ -297,6 +305,8 @@ def execute_notebook_entrypoint(
     n_retries: int,
     job_id: str,
     mailto: str,
+    error_mailto: str,
+    email_subject: str,
     pdf_output: bool,
     hide_code: bool,
     prepare_notebook_only: bool,
@@ -319,6 +329,8 @@ def execute_notebook_entrypoint(
     logger.info("output_dir = %s", output_dir)
     logger.info("template_dir = %s", template_dir)
     logger.info("mailto = %s", mailto)
+    logger.info("error_mailto = %s", error_mailto)
+    logger.info("email_subject = %s", email_subject)
     logger.info("pdf_output = %s", pdf_output)
     logger.info("hide_code = %s", hide_code)
     logger.info("prepare_notebook_only = %s", prepare_notebook_only)
@@ -343,6 +355,8 @@ def execute_notebook_entrypoint(
             template_base_dir=template_dir,
             attempts_remaining=n_retries - 1,
             mailto=mailto,
+            error_mailto=error_mailto,
+            email_subject=email_subject,
             generate_pdf_output=pdf_output,
             hide_code=hide_code,
             prepare_only=prepare_notebook_only,
@@ -350,7 +364,7 @@ def execute_notebook_entrypoint(
             py_template_base_dir=py_template_base_dir,
             py_template_subdir=py_template_subdir,
         )
-        if mailto:
+        if result.mailto:
             send_result_email(result, mailto)
         if isinstance(result, NotebookResultError):
             logger.warning("Notebook execution failed! Output was:")
diff --git a/notebooker/utils/notebook_execution.py b/notebooker/utils/notebook_execution.py
index 12636ae4..f805df66 100644
--- a/notebooker/utils/notebook_execution.py
+++ b/notebooker/utils/notebook_execution.py
@@ -15,13 +15,13 @@ def _output_dir(output_base_dir, report_name, job_id):
     return os.path.join(output_base_dir, report_name, job_id)
 
 
-def send_result_email(result: Union[NotebookResultComplete, NotebookResultError], mailto: AnyStr) -> None:
+def send_result_email(result: Union[NotebookResultComplete, NotebookResultError]) -> None:
     from_email = "notebooker@notebooker.io"
-    to_email = mailto
+    to_email = result.mailto
     report_title = (
         result.report_title.decode("utf-8") if isinstance(result.report_title, bytes) else result.report_title
     )
-    subject = "Notebooker: {} report completed with status: {}".format(report_title, result.status.value)
+    subject = result.email_subject or f"Notebooker: {report_title} report completed with status: {result.status.value}"
     body = result.email_html or result.raw_html
     attachments = []
     tmp_dir = None
@@ -53,7 +53,7 @@ def send_result_email(result: Union[NotebookResultComplete, NotebookResultError]
 
         msg = ["Please either activate HTML emails, or see the PDF attachment.", body]
 
-        logger.info("Sending email to %s with %d attachments", mailto, len(attachments))
+        logger.info("Sending email to %s with %d attachments", to_email, len(attachments))
         mail(from_email, to_email, subject, msg, attachments=attachments)
     finally:
         if tmp_dir:
diff --git a/tests/integration/test_execute_notebook.py b/tests/integration/test_execute_notebook.py
index a588db4f..21e0e230 100644
--- a/tests/integration/test_execute_notebook.py
+++ b/tests/integration/test_execute_notebook.py
@@ -1,6 +1,7 @@
 from __future__ import unicode_literals
 
 import mock
+import pytest
 from click.testing import CliRunner
 from nbformat import NotebookNode
 from nbformat import __version__ as nbv
@@ -54,3 +55,46 @@ def test_main(mongo_host):
         )
         assert result.raw_ipynb_json
         assert result.pdf == pdf_contents
+
+@pytest.mark.parametrize(
+    ('cli_args', 'expected_mailto'),
+    [
+        (
+            ['--report-name', 'crashyreport', '--mailto', 'happy@email'],
+            'happy@email',
+        ), (
+            ['--report-name', 'crashyreport', '--mailto', 'happy@email', '--error-mailto', 'sad@email'],
+            'sad@email',
+        ), (
+            ['--report-name', 'crashyreport', '--error-mailto', 'sad@email'],
+            'sad@email',
+        )
+    ]
+)
+def test_main(mongo_host, cli_args, expected_mailto):
+    with mock.patch("notebooker.execute_notebook.pm.execute_notebook") as exec_nb, mock.patch(
+        "notebooker.utils.conversion.jupytext.read"
+    ) as read_nb, mock.patch("notebooker.execute_notebook.send_result_email") as send_email:
+        exec_nb.side_effect = Exception()
+        versions = nbv.split(".")
+        major, minor = int(versions[0]), int(versions[1])
+        if major >= 5:
+            major, minor = 4, 4
+        read_nb.return_value = NotebookNode({"cells": [], "metadata": {}, "nbformat": major, "nbformat_minor": minor})
+        job_id = "ttttteeeesssstttt"
+        runner = CliRunner()
+        cli_result = runner.invoke(
+            base_notebooker,
+            [
+                "--serializer-cls",
+                DEFAULT_SERIALIZER,
+                "--mongo-host",
+                mongo_host,
+                "execute-notebook",
+                "--job-id",
+                job_id,
+            ] + cli_args,
+        )
+
+        mailto = send_email.call_args_list[0][0][0].mailto
+        assert mailto == expected_mailto
diff --git a/tests/unit/utils/test_utils_notebook_execution.py b/tests/unit/utils/test_utils_notebook_execution.py
index 109036ab..59f4468b 100644
--- a/tests/unit/utils/test_utils_notebook_execution.py
+++ b/tests/unit/utils/test_utils_notebook_execution.py
@@ -30,6 +30,7 @@
     ],
 )
 def test_send_result_email(test_name, job_id, report_name, report_title, expected_title):
+    to_email = "∫åññîsté®@test-email.com"
     body_in = "<body><h1>hello  😆 😉 😊 😋 😎</h1></body>"
     result = NotebookResultComplete(
         job_id=job_id,
@@ -39,13 +40,13 @@ def test_send_result_email(test_name, job_id, report_name, report_title, expecte
         raw_ipynb_json={},
         raw_html=body_in,
         email_html=body_in,
+        mailto=to_email,
         pdf="",
         report_name=report_name,
         report_title=report_title,
     )
-    to_email = "∫åññîsté®@test-email.com"
     with mock.patch("notebooker.utils.notebook_execution.mail") as mail:
-        notebook_execution.send_result_email(result, to_email)
+        notebook_execution.send_result_email(result)
     email_sent = mail.mock_calls[0][1]
     assert len(email_sent) == 4, "mail() was not called with the correct number of args"
     from_address = email_sent[0]
@@ -61,10 +62,12 @@ def test_send_result_email(test_name, job_id, report_name, report_title, expecte
 
 def test_send_result_email_fallback_to_raw_html():
     body_in = "<body><h1>hello  😆 😉 😊 😋 😎</h1></body>"
+    to_email = "hello@you"
     result = NotebookResultComplete(
         job_id="fallback",
         job_start_time=datetime.datetime.now(),
         job_finish_time=datetime.datetime.now(),
+        mailto=to_email,
         raw_html_resources={},
         raw_ipynb_json={},
         raw_html=body_in,
@@ -72,9 +75,8 @@ def test_send_result_email_fallback_to_raw_html():
         report_name="fallback",
         report_title="fallback",
     )
-    to_email = "∫åññîsté®@test-email.com"
     with mock.patch("notebooker.utils.notebook_execution.mail") as mail:
-        notebook_execution.send_result_email(result, to_email)
+        notebook_execution.send_result_email(result)
     email_sent = mail.mock_calls[0][1]
     assert len(email_sent) == 4, "mail() was not called with the correct number of args"
     from_address = email_sent[0]
@@ -86,3 +88,42 @@ def test_send_result_email_fallback_to_raw_html():
     assert to_address == to_email
     assert title == "Notebooker: fallback report completed with status: Checks done!"
     assert body == ["Please either activate HTML emails, or see the PDF attachment.", body_in]
+
+
+@pytest.mark.parametrize(
+    ("subject", "expected"),
+    [
+        ("", "Notebooker: subjecttest report completed with status: Checks done!"),
+        (None, "Notebooker: subjecttest report completed with status: Checks done!"),
+        ("my super cool report title", "my super cool report title"),
+    ]
+)
+def test_send_result_email_subject(subject, expected):
+    body_in = "<body><h1>hello  😆 😉 😊 😋 😎</h1></body>"
+    to_email = "∫åññîsté®@ahl.com"
+    result = NotebookResultComplete(
+        job_id="subjecttest",
+        job_start_time=datetime.datetime.now(),
+        job_finish_time=datetime.datetime.now(),
+        raw_html_resources={},
+        raw_ipynb_json={},
+        raw_html=body_in,
+        mailto=to_email,
+        email_subject=subject,
+        pdf="",
+        report_name="subjecttest",
+        report_title="subjecttest",
+    )
+    with mock.patch("notebooker.utils.notebook_execution.mail") as mail:
+        notebook_execution.send_result_email(result)
+    email_sent = mail.mock_calls[0][1]
+    assert len(email_sent) == 4, "mail() was not called with the correct number of args"
+    from_address = email_sent[0]
+    to_address = email_sent[1]
+    title = email_sent[2]
+    body = email_sent[3]
+
+    assert from_address == "notebooker@notebooker.io"
+    assert to_address == to_email
+    assert title == expected
+    assert body == ["Please either activate HTML emails, or see the PDF attachment.", body_in]