Repository: XLSForm/pyxform (python)

Issue:
type=calculate with (ignored) label does not show an error when used as trigger
#### Software and hardware versions 
pyxform v1.2.1

#### Problem description
Users may accidentally add a label to type=calculate. This label is mostly ignored by pyxform and not present in the Pyxform output. That's all good. However, the helpful error message generation logic introduced in https://github.com/XLSForm/pyxform/issues/452 actually gets fooled by this otherwise ignored label.

#### Steps to reproduce the problem

type | name | label | calculation | trigger
-- | -- | -- | -- | --
calculate | calc2 | calculate item label | 1+1 |  
text | text2 | triggered from calc2 | 4+4 | ${calc2}

#### Expected behavior

To output the same pyxform error message as with this form (since they are essentially exactly the same form) :

type | name | label | calculation | trigger
-- | -- | -- | -- | --
calculate | calc2 |  | 1+1 |  
text | text2 | triggered from calc2 | 4+4 | ${calc2}

#### Other info

I also considered an alternative solution where a type=calculate with a label gets a warning. However, since that should probably be a warning and not an error, I think we would still need the above error that prevents the XForm from being created. 

Pull request:
Prevent untyped calculates from being used as calculation triggers
Closes #486

@gushil I took this on to hopefully give you a little extra time for #485. I think you're best positioned to see if there's a way to make it a little easier to work with. I should hopefully also have a bit of time for it tomorrow.

#### Why is this the best possible solution? Were any other approaches considered?
I didn't consider any alternatives. This extends the existing logic to treat any question with a `calculate` type as ineligible to be a calculation trigger.

#### What are the regression risks?
The only change is to the logic that for a given question suppresses body tags and produces an error if it's used as a trigger. The worst case would be if this adds cases that are actually suppose to lead to body elements

#### 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

Patch:
diff --git a/pyxform/question.py b/pyxform/question.py
index ed93215..7d9ab56 100644
--- a/pyxform/question.py
+++ b/pyxform/question.py
@@ -32,7 +32,9 @@ class Question(SurveyElement):
         return node(self.name, **attributes)
 
     def xml_control(self):
-        if ("calculate" in self.bind or self.trigger) and not (self.label or self.hint):
+        if self.type == "calculate" or (
+            ("calculate" in self.bind or self.trigger) and not (self.label or self.hint)
+        ):
             nested_setvalues = self.get_root().get_setvalues_for_question_name(
                 self.name
             )


Test patch:
diff --git a/pyxform/tests_v1/test_trigger.py b/pyxform/tests_v1/test_trigger.py
index d3be5b5..8305126 100644
--- a/pyxform/tests_v1/test_trigger.py
+++ b/pyxform/tests_v1/test_trigger.py
@@ -174,6 +174,21 @@ class TriggerSetvalueTests(PyxformTestCase):
             ],
         )
 
+    def test_when_trigger_refers_to_calculate_with_label_error_is_shown(self):
+        self.assertPyxformXform(
+            name="trigger-invalid-ref",
+            md="""
+            | survey |           |        |                      |         |             |
+            |        | type      | name   | label                | trigger | calculation |
+            |        | calculate | one    | A label              |         | 5 + 4       |
+            |        | calculate | one-ts |                      | ${one}  | now()       |
+            """,
+            errored=True,
+            error__contains=[
+                "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",