From 5c559be5a0762ac015b2c8e08795a8f8f16defb5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Saugat=20Pachhai=20=28=E0=A4=B8=E0=A5=8C=E0=A4=97=E0=A4=BE?=
 =?UTF-8?q?=E0=A4=A4=29?= <suagatchhetri@outlook.com>
Date: Tue, 9 Jun 2020 19:47:09 +0545
Subject: [PATCH] plots modify: complain if template does not exist

---
 dvc/repo/plots/__init__.py     |  9 +++++--
 tests/func/plots/__init__.py   |  4 +--
 tests/func/plots/test_plots.py | 47 ++++++++++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/dvc/repo/plots/__init__.py b/dvc/repo/plots/__init__.py
index fd092bdf1c..a4340859e7 100644
--- a/dvc/repo/plots/__init__.py
+++ b/dvc/repo/plots/__init__.py
@@ -84,6 +84,11 @@ def diff(self, *args, **kwargs):
     def modify(self, path, props=None, unset=None):
         from dvc.dvcfile import Dvcfile
 
+        props = props or {}
+        template = props.get("template")
+        if template:
+            self.repo.plot_templates.get_template(template)
+
         (out,) = self.repo.find_outs_by_path(path)
 
         # This out will become a plot unless it is one already
@@ -92,7 +97,7 @@ def modify(self, path, props=None, unset=None):
 
         for field in unset or ():
             out.plot.pop(field, None)
-        out.plot.update(props or {})
+        out.plot.update(props)
 
         # Empty dict will move it to non-plots
         if not out.plot:
@@ -101,7 +106,7 @@ def modify(self, path, props=None, unset=None):
         out.verify_metric()
 
         dvcfile = Dvcfile(self.repo, out.stage.path)
-        dvcfile.dump(out.stage, update_pipeline=True)
+        dvcfile.dump(out.stage, update_pipeline=True, no_lock=True)
 
 
 def _collect_plots(repo, targets=None, rev=None):
diff --git a/tests/func/plots/__init__.py b/tests/func/plots/__init__.py
index abeba7bda1..41851900f8 100644
--- a/tests/func/plots/__init__.py
+++ b/tests/func/plots/__init__.py
@@ -5,11 +5,11 @@
 
 @pytest.fixture
 def run_copy_metrics(tmp_dir, run_copy):
-    def run(file1, file2, commit=None, tag=None, **kwargs):
+    def run(file1, file2, commit=None, tag=None, single_stage=True, **kwargs):
         stage = tmp_dir.dvc.run(
             cmd=f"python copy.py {file1} {file2}",
             deps=[file1],
-            single_stage=True,
+            single_stage=single_stage,
             **kwargs,
         )
 
diff --git a/tests/func/plots/test_plots.py b/tests/func/plots/test_plots.py
index 46b145deb0..33a33d047e 100644
--- a/tests/func/plots/test_plots.py
+++ b/tests/func/plots/test_plots.py
@@ -9,6 +9,7 @@
 import yaml
 from funcy import first
 
+from dvc.dvcfile import PIPELINE_LOCK
 from dvc.repo.plots.data import (
     JSONPlotData,
     NoMetricInHistoryError,
@@ -17,6 +18,7 @@
     YAMLPlotData,
 )
 from dvc.repo.plots.template import NoFieldInDataError, TemplateNotFoundError
+from dvc.utils import relpath
 
 
 def _write_csv(metric, filename, header=True):
@@ -538,3 +540,48 @@ def test_load_metric_from_dict_yaml(tmp_dir):
         d["rev"] = "revision"
 
     assert list(map(dict, plot_data.to_datapoints())) == expected
+
+
+def test_plots_modify_existing_template(
+    tmp_dir, dvc, run_copy_metrics, custom_template
+):
+    metric = [{"a": 1, "b": 2}, {"a": 2, "b": 3}]
+    _write_json(tmp_dir, metric, "metric_t.json")
+    stage = run_copy_metrics(
+        "metric_t.json",
+        "metric.json",
+        plots_no_cache=["metric.json"],
+        name="copy-metrics",
+        single_stage=False,
+    )
+    dvc.plots.modify(
+        "metric.json", props={"template": relpath(custom_template)}
+    )
+    stage = stage.reload()
+    assert stage.outs[0].plot == {"template": relpath(custom_template)}
+
+
+def test_plots_modify_should_not_change_lockfile(
+    tmp_dir, dvc, run_copy_metrics, custom_template
+):
+    _write_json(tmp_dir, [{"a": 1, "b": 2}], "metric_t.json")
+    run_copy_metrics(
+        "metric_t.json",
+        "metric.json",
+        plots_no_cache=["metric.json"],
+        name="copy-metrics",
+        single_stage=False,
+    )
+
+    (tmp_dir / PIPELINE_LOCK).unlink()
+    dvc.plots.modify(
+        "metric.json", props={"template": relpath(custom_template)}
+    )
+    assert not (tmp_dir / PIPELINE_LOCK).exists()
+
+
+def test_plots_modify_not_existing_template(dvc):
+    with pytest.raises(TemplateNotFoundError):
+        dvc.plots.modify(
+            "metric.json", props={"template": "not-existing-template.json"}
+        )