Repository: XLSForm/pyxform
Pull request #460 (2020-07-24 05:27:10)

Add test for typed calculate, update error message
Addresses the comments I left at https://github.com/XLSForm/pyxform/pull/457 for @gushil and @MartijnR to consider.

#### Why is this the best possible solution? Were any other approaches considered?
I considered not adding the test but it's easy to do and I can imagine a regression sneaking in.

#### What are the regression risks?
None. This changes a string and adds a test.

#### Does this change require updates to documentation? If so, please file an issue [here](https://github.com/XLSForm/xlsform.github.io) and include the link below.
No.

#### Before submitting this PR, please make sure you have:
- [x] included test cases for core behavior and edge cases in `tests_v1`
- [x] run `nosetests` and verified all tests pass
- [x] run `black pyxform` to format code
- [x] verified that any code or assets from external sources are properly credited in comments

codecov-commenter: # [Codecov](https://codecov.io/gh/XLSForm/pyxform/pull/460?src=pr&el=h1) Report
> Merging [#460](https://codecov.io/gh/XLSForm/pyxform/pull/460?src=pr&el=desc) into [master](https://codecov.io/gh/XLSForm/pyxform/commit/b6093a1fea056ac6ed3d4de876980377a08fdf92&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/XLSForm/pyxform/pull/460/graphs/tree.svg?width=650&height=150&src=pr&token=DunISqIokF)](https://codecov.io/gh/XLSForm/pyxform/pull/460?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #460   +/-   ##
=======================================
  Coverage   83.39%   83.39%           
=======================================
  Files          25       25           
  Lines        3601     3601           
  Branches      836      836           
=======================================
  Hits         3003     3003           
  Misses        454      454           
  Partials      144      144           
```


| [Impacted Files](https://codecov.io/gh/XLSForm/pyxform/pull/460?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pyxform/question.py](https://codecov.io/gh/XLSForm/pyxform/pull/460/diff?src=pr&el=tree#diff-cHl4Zm9ybS9xdWVzdGlvbi5weQ==) | `92.85% <ø> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/XLSForm/pyxform/pull/460?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/XLSForm/pyxform/pull/460?src=pr&el=footer). Last update [b6093a1...2b660d3](https://codecov.io/gh/XLSForm/pyxform/pull/460?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

MartijnR: Looks great to me. Thanks!
lognaturel: Another reviewer will need to approve and merge! (@MartijnR?)

Patch:
diff --git a/pyxform/question.py b/pyxform/question.py
index 02e7f19..579b7cf 100644
--- a/pyxform/question.py
+++ b/pyxform/question.py
@@ -38,8 +38,8 @@ class Question(SurveyElement):
             if nested_setvalues:
                 for setvalue in nested_setvalues:
                     msg = (
-                        "Trigger was added for ${%s} that refers to hidden question. This is not allowed."
-                        % setvalue[0]
+                        "The question ${%s} is not user-visible so it can't be used as a calculation trigger for question ${%s}."
+                        % (self.name, setvalue[0])
                     )
                     raise PyXFormError(msg)
             return None


Test patch:
diff --git a/pyxform/tests_v1/test_trigger.py b/pyxform/tests_v1/test_trigger.py
index 6da047f..e2f6ebc 100644
--- a/pyxform/tests_v1/test_trigger.py
+++ b/pyxform/tests_v1/test_trigger.py
@@ -170,7 +170,22 @@ class TriggerSetvalueTests(PyxformTestCase):
             """,
             errored=True,
             error__contains=[
-                "Trigger was added for ${one-ts} that refers to hidden question. This is not allowed."
+                "The question ${one} is not user-visible so it can't be used as a calculation trigger for question ${one-ts}.",
+            ],
+        )
+
+    def test_typed_calculate_cant_be_trigger(self):
+        self.assertPyxformXform(
+            name="trigger-invalid-ref",
+            md="""
+                | survey |           |        |         |             |
+                |        | type      | name   | trigger | calculation |
+                |        | integer   | two    |         | 1 + 1       |
+                |        | integer   | two-ts | ${two}  | now()       | 
+                """,
+            errored=True,
+            error__contains=[
+                "The question ${two} is not user-visible so it can't be used as a calculation trigger for question ${two-ts}."
             ],
         )