PR feedback from app execution changes

corehq/apps/app_execution/api.py

@@ -13,18 +13,14 @@
 from django.http import HttpRequest
 
 from corehq.apps.app_execution import const, data_model
-from corehq.apps.app_execution.exceptions import AppExecutionError
+from corehq.apps.app_execution.exceptions import AppExecutionError, FormplayerException
 from corehq.apps.app_manager.dbaccessors import get_app
 from corehq.apps.formplayer_api.sync_db import sync_db
 from corehq.apps.formplayer_api.utils import get_formplayer_url
 from corehq.util.hmac_request import get_hmac_digest
 from dimagi.utils.web import get_url_base
 
 
-class FormplayerException(Exception):
-    pass
-
-
 class BaseFormplayerClient:
     """Client class used to make requests to Formplayer"""
 
@@ -199,7 +195,7 @@ def app_build_id(self):
         build_on = "Latest Version"
         if app.built_on:
             build_on = app.built_on.strftime("%B %d, %Y")
-        print(f"Using app '{app.name}' ({app._id} - {build_on})", file=self.log)
+        print(f"Using app '{app.name}' ({app._id} - {app.version} - {build_on})", file=self.log)
         return app._id
 
     def sync(self):
@@ -292,6 +288,12 @@ def _get_base_data(self):
             "username": self.client.username,
         }
 
+    def start_session(self):
+        self.sync()
+        data = self.get_session_start_data()
+        self.data = self.client.make_request(data, "navigate_menu_start")
+        print("Starting app session:\n", file=self.log)
+
     def execute_step(self, step):
         is_form_step = isinstance(step, (data_model.AnswerQuestionStep, data_model.SubmitFormStep))
         if is_form_step and self.form_mode == const.FORM_MODE_IGNORE:
@@ -300,13 +302,18 @@ def execute_step(self, step):
         if self.form_mode == const.FORM_MODE_NO_SUBMIT and isinstance(step, data_model.SubmitFormStep):
             self.log_step(step, skipped=True)
             return
-        data = self.get_request_data(step) if step else self.get_session_start_data()
+        data = self.get_request_data(step)
         self.data = self.client.make_request(data, self.request_url(step))
+        self.augment_data_from_request(data, ["query_data", "session_id"])
         self.log_step(step)
 
+    def augment_data_from_request(self, request_data, fields):
+        """Some fields aren't returned by Formplayer, so we need to maintain their value across requests"""
+        for field in fields:
+            if request_data.get(field) and not self.data.get(field):
+                self.data[field] = request_data[field]
+
     def log_step(self, step, indent="  ", skipped=False):
-        if not step:
-            print("Starting app session:\n", file=self.log)
         skipped_log = " (ignored)" if skipped else ""
         print(f"Execute step: {step or 'START'} {skipped_log}", file=self.log)
         if skipped:
@@ -335,8 +342,7 @@ def log_step(self, step, indent="  ", skipped=False):
 
 def execute_workflow(session: FormplayerSession, workflow):
     with session:
-        session.sync()
-        execute_step(session, None)
+        session.start_session()
         for step in workflow.steps:
             execute_step(session, step)
 

corehq/apps/app_execution/data_model.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import copy
 import dataclasses
 from typing import ClassVar
 
@@ -210,6 +211,7 @@ def _append_selection(data, selection):
 
 
 def _steps_from_json(data):
+    data = copy.deepcopy(data)
     return [STEP_MAP[child.pop("type")].from_json(child) for child in data]
 
 

corehq/apps/app_execution/exceptions.py

@@ -1,2 +1,6 @@
 class AppExecutionError(Exception):
     pass
+
+
+class FormplayerException(Exception):
+    pass

corehq/apps/app_execution/forms.py

@@ -7,18 +7,20 @@
 from corehq.apps.app_manager.dbaccessors import get_brief_app
 from corehq.apps.hqwebapp import crispy as hqcrispy
 from corehq.apps.users.models import CommCareUser
+from corehq.apps.users.util import normalize_username
 
 
 class AppWorkflowConfigForm(forms.ModelForm):
     run_every = forms.IntegerField(min_value=1)
+    username = forms.CharField(max_length=255, label="Username",
+                               help_text="Username of the user to run the workflow")
 
     class Meta:
         model = AppWorkflowConfig
         fields = (
             "name",
             "domain",
             "app_id",
-            "user_id",
             "workflow",
             "sync_before_run",
             "form_mode",
@@ -31,26 +33,45 @@ class Meta:
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
+        if self.instance.id:
+            self.fields["username"].initial = self.instance.django_user.username
         self.helper = hqcrispy.HQFormHelper()
 
         self.helper.layout = crispy.Layout(
-            *self.fields.keys(),
+            "name",
+            "domain",
+            "app_id",
+            "username",
+            "workflow",
+            "sync_before_run",
+            "form_mode",
+            "run_every",
+            "notification_emails",
             hqcrispy.FormActions(
                 twbscrispy.StrictButton("Save", type='submit', css_class='btn-primary')
             ),
         )
 
     def clean(self):
         self.final_clean_app_id()
-        self.final_clean_user_id()
+        self.final_clean_username()
         return self.cleaned_data
 
-    def final_clean_user_id(self):
+    def final_clean_username(self):
         domain = self.cleaned_data.get("domain")
+        username = self.cleaned_data.get("username")
+        if username and "@" not in username:
+            try:
+                username = normalize_username(self.cleaned_data.get("username"), domain)
+            except ValueError:
+                self.add_error("username", "Invalid username")
         try:
-            self.commcare_user = CommCareUser.get_by_user_id(self.cleaned_data.get("user_id"), domain)
+            self.commcare_user = CommCareUser.get_by_username(username)
         except ResourceNotFound:
-            raise forms.ValidationError(f"User not found in domain: {domain}:{self.cleaned_data.get('user_id')}")
+            self.add_error("username", "User not found")
+
+        if self.commcare_user.domain != domain:
+            self.add_error("username", f"User not found in domain: {domain}")
 
     def final_clean_app_id(self):
         domain = self.cleaned_data.get("domain")

corehq/apps/app_execution/migrations/0002_alter_appworkflowconfig_unique_together.py

@@ -0,0 +1,17 @@
+# Generated by Django 3.2.25 on 2024-04-30 11:10
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('app_execution', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AlterUniqueTogether(
+            name='appworkflowconfig',
+            unique_together=set(),
+        ),
+    ]

corehq/apps/app_execution/models.py

@@ -41,9 +41,6 @@ class AppWorkflowConfig(models.Model):
 
     objects = AppWorkflowManager()
 
-    class Meta:
-        unique_together = ("domain", "user_id")
-
     @cached_property
     def app_name(self):
         app = get_brief_app(self.domain, self.app_id)

corehq/apps/app_execution/tasks.py

@@ -10,7 +10,7 @@
 from corehq.util.log import send_HTML_email
 
 
-@periodic_task(run_every=crontab(minute=0, hour=0))
+@periodic_task(run_every=crontab())  # run every minute
 def run_app_workflows():
 
     for config in AppWorkflowConfig.objects.get_due():

corehq/apps/app_execution/templates/app_execution/workflow_form.html

@@ -1,7 +1,5 @@
 {% extends "hqwebapp/bootstrap5/base_section.html" %}
 {% load crispy_forms_tags %}
-{% load hq_shared_tags %}
-{% load i18n %}
 {% block page_content %}
   {% crispy form %}
 {% endblock %}

corehq/apps/app_execution/views.py

@@ -4,7 +4,7 @@
 from corehq.apps.app_execution import const
 from corehq.apps.app_execution.api import execute_workflow
 from corehq.apps.app_execution.data_model import EXAMPLE_WORKFLOW
-from corehq.apps.app_execution.exceptions import AppExecutionError
+from corehq.apps.app_execution.exceptions import AppExecutionError, FormplayerException
 from corehq.apps.app_execution.forms import AppWorkflowConfigForm
 from corehq.apps.app_execution.models import AppWorkflowConfig
 from corehq.apps.domain.decorators import require_superuser_or_contractor
@@ -73,7 +73,7 @@ def test_workflow(request, pk):
         session = config.get_formplayer_session()
         try:
             execute_workflow(session, config.workflow)
-        except AppExecutionError as e:
+        except (AppExecutionError, FormplayerException) as e:
             context["error"] = str(e)
 
         context["result"] = True

corehq/util/jsonattrs.py

@@ -95,7 +95,7 @@ class Plane(models.Model):
 
 class JsonAttrsField(JSONField):
     default_error_messages = {
-        'invalid': _("'%(field)s' field value has an invalid format: %(exc)s"),
+        'invalid_attr': _("'%(field)s' field value has an invalid format: %(exc)s"),
     }
 
     def __init__(self, *args, builder, **kw):
@@ -113,8 +113,8 @@ def to_python(self, value):
             return self.builder.attrify(value)
         except Exception as exc:
             raise ValidationError(
-                self.error_messages['invalid'],
-                code='invalid',
+                self.error_messages['invalid_attr'],
+                code='invalid_attr',
                 params={
                     'field': self.name,
                     'exc': BadValue.format(value, exc),
@@ -196,15 +196,15 @@ def __init__(self, item_type, /, **jsonfield_args):
 
 
 @define
-class AttrsListBuilder:
+class BaseBuilder:
     attrs_type = field()
 
-    def attrify(self, items):
+    def attrify(self, value):
         attrs_type = self.attrs_type
-        if items is None:
-            return items
+        if value is None:
+            return value
         from_json = make_from_json(attrs_type)
-        return [from_json(item) for item in items]
+        return self._attrify(value, from_json)
 
     def jsonify(self, value):
         if not value:
@@ -213,48 +213,42 @@ def jsonify(self, value):
             to_json = self.attrs_type.__jsonattrs_to_json__
         else:
             to_json = asdict
+        return self._jsonify(value, to_json)
+
+    def _attrify(self, value, from_json):
+        raise NotImplementedError()
+
+    def _jsonify(self, value, to_json):
+        raise NotImplementedError()
+
+
+@define
+class AttrsListBuilder(BaseBuilder):
+
+    def _attrify(self, value, from_json):
+        return [from_json(item) for item in value]
+
+    def _jsonify(self, value, to_json):
         return [to_json(v) for v in value]
 
 
 @define
-class AttrsDictBuilder:
-    attrs_type = field()
+class AttrsDictBuilder(BaseBuilder):
 
-    def attrify(self, values):
-        attrs_type = self.attrs_type
-        if values is None:
-            return values
-        from_json = make_from_json(attrs_type)
-        return {key: from_json(value) for key, value in values.items()}
+    def _attrify(self, value, from_json):
+        return {key: from_json(value) for key, value in value.items()}
 
-    def jsonify(self, value):
-        if not value:
-            return value
-        if hasattr(self.attrs_type, "__jsonattrs_to_json__"):
-            to_json = self.attrs_type.__jsonattrs_to_json__
-        else:
-            to_json = asdict
+    def _jsonify(self, value, to_json):
         return {k: to_json(v) for k, v in value.items()}
 
 
 @define
-class AttrsObjectBuilder:
-    attrs_type = field()
+class AttrsObjectBuilder(BaseBuilder):
 
-    def attrify(self, value):
-        attrs_type = self.attrs_type
-        if value is None:
-            return value
-        from_json = make_from_json(attrs_type)
+    def _attrify(self, value, from_json):
         return from_json(value)
 
-    def jsonify(self, value):
-        if not value:
-            return value
-        if hasattr(self.attrs_type, "__jsonattrs_to_json__"):
-            to_json = self.attrs_type.__jsonattrs_to_json__
-        else:
-            to_json = asdict
+    def _jsonify(self, value, to_json):
         return to_json(value)
 
 
@@ -347,13 +341,26 @@ def __name__(self):
 
 class JsonAttrsFormField(forms.JSONField):
     default_error_messages = {
-        'invalid': _("'%(field)s' field value has an invalid format: %(exc)s"),
+        'invalid_attr': _("%(exc)s"),
     }
 
     def __init__(self, builder, encoder=None, decoder=None, **kwargs):
         self.builder = builder
         super().__init__(encoder, decoder, **kwargs)
 
+    def validate(self, value):
+        super().validate(value)
+        try:
+            self.builder.attrify(value)
+        except Exception as exc:
+            raise ValidationError(
+                self.error_messages['invalid_attr'],
+                code='invalid_attr',
+                params={
+                    'exc': BadValue.format(value, exc),
+                },
+            )
+
     def prepare_value(self, value):
         if isinstance(value, str):
             return value

migrations.lock

@@ -128,6 +128,7 @@ api
  0004_rename_apiuser
 app_execution
  0001_initial
+ 0002_alter_appworkflowconfig_unique_together
 app_manager
  0001_linked_app_domain
  0002_latestenabledbuildprofiles